Skip to content

Comments

gui: Set default scene to prevent null deref#9531

Merged
maliberty merged 1 commit intoThe-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:gui-mm-crash
Feb 24, 2026
Merged

gui: Set default scene to prevent null deref#9531
maliberty merged 1 commit intoThe-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:gui-mm-crash

Conversation

@openroad-ci
Copy link
Collaborator

Should be fixing #9526

Signed-off-by: Martin Povišer <povik@cutebit.org>
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The review correctly identifies a critical null pointer dereference in the setSTA method, where sta->cmdScene() is called without a null check for sta. The suggested fix, which mirrors the null check in the constructor, is appropriate to prevent a crash.

Comment on lines +329 to +333
void setSTA(sta::dbSta* sta)
{
sta_ = sta;
scene_ = sta->cmdScene();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The sta pointer can be null, which will cause a null pointer dereference when sta->cmdScene() is called. A null check should be added, similar to the one in the constructor, to prevent a crash.

Suggested change
void setSTA(sta::dbSta* sta)
{
sta_ = sta;
scene_ = sta->cmdScene();
}
void setSTA(sta::dbSta* sta)
{
sta_ = sta;
scene_ = sta ? sta->cmdScene() : nullptr;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure there's a code path which would do setSTA(nullptr);

@povik povik assigned maliberty and unassigned maliberty Feb 24, 2026
@povik povik requested a review from maliberty February 24, 2026 00:28
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@maliberty maliberty enabled auto-merge February 24, 2026 00:48
@maliberty maliberty merged commit 446934a into The-OpenROAD-Project:master Feb 24, 2026
12 of 13 checks passed
@maliberty maliberty deleted the gui-mm-crash branch February 24, 2026 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants