update sta and add new regression to catch read_def with funny DEFs#10065
update sta and add new regression to catch read_def with funny DEFs#10065openroad-ci wants to merge 2 commits into
Conversation
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
![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 introduces a new integration test, read_def_bus_port, to verify that read_def correctly handles port names with non-numeric bus subscripts and properly groups numeric subscripts into buses. The changes include a new DEF file, a TCL test script, and a subproject commit update for sta. Feedback was provided to move the new test entry in CMakeLists.txt to maintain the alphabetical ordering of the integration tests.
| read_liberty1 | ||
| read_vcd | ||
| read_def_bus_port |
There was a problem hiding this comment.
The integration tests in or_integration_tests are maintained in alphabetical order. read_def_bus_port should be moved before read_liberty1 to maintain this consistency.
read_def_bus_port
read_liberty1
read_vcd
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Secure-CI ongoing with this change: http://secure-ci:8080/job/SB/job/secure-update_sta_handle_weird_DEFs_latest_sta/1/ |
|
Update after new set of STA changes |
2 participants
Summary
Reading a DEF with port names containing non-numeric bracket subscripts (e.g., kv_write[3][write_en]) crashes with a segfault. parseBusName uses rfind('[') to find the last left bracket, then calls std::stoi on the content. For kv_write[3][write_en], rfind finds the [ before write_en, and stoi("write_en]") throws an unhandled std::invalid_argument which terminates the process.
This occurs when externally-produced DEF files use multi-level bracket names where the innermost subscript is a field name rather than a numeric bus index.
Add an isdigit check on the first character after [ before calling stoi in both parseBusName overloads. Non-numeric subscripts like [write_en] are treated as non-bus names (is_bus = false), while ports with numeric trailing subscripts like kv_write[3][ write_entry][4] continue to parse and group correctly as bus kv_write[3][write_entry] with index 4.
This change is done inside src/sta/ParseBus.cc so OpenROAD needs to be updated to that STA. For OpenROAD itself, I added a regression to reproduce the issue and verify the STA fix
Type of Change
Impact
Fixes error when externally generated DEF files are read
Verification
./etc/Build.sh).Related Issues
[Link issues here]