Fix GUI crash rendering when Acess Points or Gcell grid is enabled #9684
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves GUI crashes in 3DBX designs by addressing null pointer dereferences. While it introduces necessary nullptr checks, it also introduces a new potential null pointer dereference in the drawChip function when handling child blocks with missing technology information. Additionally, I've provided suggestions for minor performance optimizations by reordering checks to fail-fast when a block is outside the visible area.
| dbTech* child_tech = child->getTech(); | ||
| if (child_tech != tech) { | ||
| child_techs.insert(child_tech); | ||
| } | ||
| } | ||
|
|
||
| for (dbTech* child_tech : child_techs) { | ||
| for (dbTechLayer* layer : child_tech->getLayers()) { |
There was a problem hiding this comment.
In RenderThread::drawChip, the code iterates over child blocks and collects their technologies into the child_techs set. If a child block does not have an associated technology (child->getTech() returns nullptr), this nullptr is added to the set (since tech is guaranteed to be non-null at this point). Subsequently, the loop at line 1257 iterates over child_techs and dereferences child_tech at line 1258 by calling getLayers(), which will cause a crash if child_tech is nullptr.
| dbTrackGrid* grid = block->findTrackGrid(layer); | ||
| if (!grid) { | ||
| return; | ||
| } | ||
|
|
||
| Rect block_bounds = viewer_->getBlock()->getDieArea(); | ||
| Rect block_bounds = block->getDieArea(); | ||
| if (!block_bounds.intersects(bounds)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
For a minor optimization, you could check if the block's die area intersects with the drawing bounds before searching for the track grid. This avoids the potentially more expensive findTrackGrid call if the block is not in the visible area.
Rect block_bounds = block->getDieArea();
if (!block_bounds.intersects(bounds)) {
return;
}
dbTrackGrid* grid = block->findTrackGrid(layer);
if (!grid) {
return;
}
alokkumardalei-wq
changed the title
…ze drawTracks - Add nullptr guard for child->getTech() before inserting into child_techs set - Reorder bounds check before findTrackGrid lookup in drawTracks for efficiency Addresses review comments from Gemini bot on PR The-OpenROAD-Project#9684 Signed-off-by: alokkumardalei-wq <alokkumardalei2@gmail.com>
… adding nullptr guard Signed-off-by: alokkumardalei-wq <alokkumardalei2@gmail.com>
alokkumardalei-wq
force-pushed
the
fix-gui-chiplets
branch
from
d52c853 to
1191ec8
Compare
|
Did you reproduce the crash and master and verify that this fixes it? |
|
Hi @maliberty, I verified the fix at the code level but not at gui level— I Also you can check the code level logic implemented in this commit Unfortunately, I don't have access to an If you could share a chiplet dataset (or point me to one), I would be happy to do a full GUI-level runtime test to confirm both the crash on master and the fix on this branch. Alternatively, let me know if there's another testing approach you prefer. Thank you so much for time and looking forward for any kind of feedback further. |
|
clang-tidy review says "All clean, LGTM! 👍" |
2 participants
What does this PR resolves?
Fixes #8927
Resolves GUI crash that occurred when enabling Access Points or GCell grid display options after reading a 3DBX chiplet design.
What have caused this issue?
The rendering functions
drawTracks,drawManufacturingGrid, anddrawGCellGridusedviewer_->getBlock(), which returns the top-level HIER chip's block (nullptr in 3DBX designs) instead of the per-chiplet block being rendered. This caused null pointer dereference crashes.Additionally,
drawAccessPointsanddrawChiplacked nullptr guards forblock->getTech()before iterating technology layers.Changes made to resolve this
odb::dbBlock* blockparameter todrawTracks,drawManufacturingGrid, anddrawGCellGridviewer_->getBlock()calls with the passed-inblockparameterblockandblock->getTech()before dereferencingdrawChipwithif (tech != nullptr)check