odb: guard null-pointer deref in getGCellTileSize()'s layer lookup - #11264
Merged
osamahammad21 merged 2 commits intoAug 30, 2026
Merged
Conversation
getAverageTrackSpacing() (a lambda inside dbBlock::getGCellTileSize()) counts frontside (non-backside) ROUTING layers to find the Nth one for a given layer_idx, but the baseline call site unconditionally asks for the 2nd/3rd/4th frontside layer once the raw (backside-inclusive) max routing layer is at least 4. On a technology whose leading routing layers are backside (e.g. a BSPDN stack), that lookup can fail to find a layer at all, leaving the local tech_layer pointer null. The existing null check on the resulting track_grid then logged an error message that unconditionally dereferenced tech_layer to build it (tech_layer->getName()), crashing on the error-reporting path itself instead of raising the intended error. Guard the error message against a null tech_layer, reporting which frontside layer ordinal was requested and how many actually exist instead of dereferencing a null pointer. Related to The-OpenROAD-Project#10547 (adds is_backside tracking and a similar null-guard pattern for backside layers elsewhere in DRT's track assignment) but a distinct bug, in ODB's GCell tile sizing rather than DRT. Added a regression test (src/odb/test/cpp/TestGCellTileSize.cpp) that reproduces the crash directly: a tech with 4 backside layers ahead of a single frontside layer, with max routing layer set past the early-return threshold. Confirmed this segfaults without the fix and passes (raising the intended ODB-0358 error) with it. Signed-off-by: dgaddy <dgaddy@ucsc.edu>
Contributor
There was a problem hiding this comment.
Code Review
This pull request fixes a potential null-pointer dereference in dbBlock::getGCellTileSize() when a technology contains fewer frontside routing layers than expected (for example, when backside routing layers precede frontside layers). It safely handles a null tech_layer by formatting an informative error message instead of crashing. Additionally, a new unit test TestGCellTileSize has been added to verify this behavior and prevent regressions. I have no further feedback to provide on these changes.
QuantamHD
approved these changes
Aug 29, 2026
| tech_layer = layer; | ||
| break; | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
If the tech layer isn't found we should error out before we call findTrackGrid. Please break up the error message into two separate errors
Per review feedback (The-OpenROAD-Project#11264): the null-tech_layer and null-track_grid cases were folded into one error message via a ternary, calling findTrackGrid() even when tech_layer was still null. Split into two distinct errors instead: error immediately if the requested frontside routing layer doesn't exist at all (ODB-1219, before ever calling findTrackGrid()), then check for a missing track grid only once tech_layer is guaranteed non-null (ODB-0358, now unconditional on tech_layer since it can no longer be null at that point). Tightened the existing regression test to assert on the specific error code raised (ODB-1219) rather than just std::runtime_error, and added a second test covering the other split path (layer found, no track grid yet -> ODB-0358). Signed-off-by: dgaddy <dgaddy@ucsc.edu>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Symptom
dbBlock::getGCellTileSize()segfaults instead of raising the intendedODB-0358error, on a technology whose leading routing layers are backside metals (e.g. a BSPDN stack):Root cause
getAverageTrackSpacing(), a lambda insidegetGCellTileSize(), finds the Nth frontside (non-backside) ROUTING layer by counting layers while skipping any withdbTechLayer::isBackside()set. The baseline call site always requests the 2nd, 3rd, and 4th frontside layer once the block's max routing layer (raw, backside-inclusive numbering) is >= 4, regardless of how many frontside layers actually exist below it. When a technology has fewer than N frontside layers before that point, the search loop never assignstech_layer, leaving it null.The existing null check on the resulting
track_gridthen built its error message by unconditionally callingtech_layer->getName()-- dereferencing the same pointer the check just proved could be null, crashing on the error-reporting path itself instead of raisingODB-0358.Fix
Guard the error message against a null
tech_layer, reporting which frontside layer ordinal was requested and how many actually exist instead of dereferencing it.Testing
New regression
src/odb/test/cpp/TestGCellTileSize.cpp, registered in both CMake and Bazel. It constructs a tech with 4 backside ROUTING layers ahead of a single frontside layer (M1) and a max routing layer past the early-return threshold, then assertsgetGCellTileSize()throwsstd::runtime_error(the documented behavior ofLogger::error()) instead of crashing. Confirmed by reverting just the fix hunk: the test segfaults againstmaster, passes with this change.bazel test //src/odb/test/cpp:TestGCellTileSizeandbazel test //:dup_id_testboth pass.clang-format --dry-run -Werroris clean on both changed files.Type of Change
Impact
No crash (
ODB-0358error instead) whendbBlock::getGCellTileSize()is called on a technology with fewer frontside routing layers than the lookup expects.Verification
./etc/Build.sh).Related Issues
#11263