Skip to content

odb: Fix group removal in region destroy - #8935

Merged
maliberty merged 2 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:odb-fix-region-rm
Dec 3, 2025
Merged

maliberty merged 2 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:odb-fix-region-rm

Conversation

@openroad-ci

Copy link
Copy Markdown
Member

Since the loop body wasn't changing _region->groups_ the loop got stuck on the first element and never progressed past it. This hang can be reproduced with:

set block [ord::get_db_block]
set foo_region  [odb::dbRegion_create $block foo_region]
set foo_group [odb::dbGroup_create $foo_region foo_group]
odb::dbRegion_destroy $foo_region

Thanks to @dbekatli for help.

Since the loop body wasn't changing `_region->groups_` the loop got
stuck on the first element and never progressed past it. This hang can
be reproduced with:

  set block [ord::get_db_block]
  set foo_region  [odb::dbRegion_create $block foo_region]
  set foo_group [odb::dbGroup_create $foo_region foo_group]
  odb::dbRegion_destroy $foo_region

Signed-off-by: Martin Povišer <povik@cutebit.org>
@github-actions

github-actions Bot commented Dec 2, 2025

Copy link
Copy Markdown
Contributor

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

@maliberty

Copy link
Copy Markdown
Member

Please add a unit test

Signed-off-by: Martin Povišer <povik@cutebit.org>
@github-actions

github-actions Bot commented Dec 2, 2025

Copy link
Copy Markdown
Contributor

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

tearDown $db
}

proc test_region_w_groups_destroy { } {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@maliberty test added

@maliberty

Copy link
Copy Markdown
Member

I suspect the code was wrong before your change and is still so.

_group->region_next_ = 0;

will prevent dbRegionGroupItr::next from advancing to the next group. Does you test work if you have two groups assigned to one region?

@povik

povik commented Dec 2, 2025

Copy link
Copy Markdown
Contributor

I don't think we ever call next from the destroy code. Note the for header

for (gitr = groups.begin(); gitr != groups.end(); gitr = groups.begin()) {

@maliberty

Copy link
Copy Markdown
Member

I see, I didn't notice it wasn't using ++.

@maliberty
maliberty merged commit 90b7e4a into The-OpenROAD-Project:master Dec 3, 2025
13 checks passed
@maliberty
maliberty deleted the odb-fix-region-rm branch December 3, 2025 00:07
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