Skip to content

odb: verify bump pin geometry against region's declared contact layer#10963

Open
osamahammad21 wants to merge 4 commits into
The-OpenROAD-Project:masterfrom
osamahammad21:3dblox-bump-layer-check
Open

odb: verify bump pin geometry against region's declared contact layer#10963
osamahammad21 wants to merge 4 commits into
The-OpenROAD-Project:masterfrom
osamahammad21:3dblox-bump-layer-check

Conversation

@osamahammad21

Copy link
Copy Markdown
Member

Summary

  • 3DBlox linter: verify each bump's pin geometry lands on its region's declared contact layer (dbChipRegion::getLayer())
  • New Checker::checkBumpLayer() flags mismatches with a marker per bump (category "Bump layer mismatch") plus a summary warning; regions with no declared layer are left unverifiable and not flagged
  • Bump masters are also now required to expose exactly one pin, matching the assumption downstream code already makes; violations warn once per master from the bump map ingestion path (ThreeDBlox::warnIfMultiPinBumpMaster)

Type of Change

  • New feature

Impact

  • check_3dblox output can now include a "Bump layer mismatch" marker category for bumps whose master pin geometry doesn't touch their region's declared contact layer, and a one-time warning for multi-pin bump masters
  • No behavior change for regions that don't declare a contact layer, or for existing checks/flows

Verification

  • I have verified that the local build succeeds (./etc/Build.sh).
  • I have run the relevant tests and they pass.
  • My code follows the repository's formatting guidelines.
  • I have included tests to prevent regressions.
  • I have signed my commits (DCO).

Related Issues

None

Signed-off-by: osamahammad21 <osama@precisioninno.com>
@osamahammad21
osamahammad21 requested a review from a team as a code owner July 21, 2026 13:26
@osamahammad21
osamahammad21 requested a review from maliberty July 21, 2026 13:26

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

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.

Code Review

This pull request introduces a validation check to ensure that bump cells expose exactly one pin, warning if a multi-pin bump master is encountered. It also adds a checker to verify that a bump's pin geometry lands on its region's declared contact layer, along with corresponding unit tests. The review feedback highlights several critical areas where missing null pointer checks on instances and masters could lead to application crashes, recommending defensive checks before dereferencing these pointers.

Comment on lines +293 to +296
for (dbChipBump* bump : region->getChipBumps()) {
bump_layer_match_[bump]
= masterPinOnLayer(bump->getInst()->getMaster(), layer);
}

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.

high

If a bump does not have a valid instance or master (e.g., due to database inconsistency or deletion), calling bump->getInst()->getMaster() will result in a null pointer dereference and crash. We should defensively check if the instance and master are valid before calling masterPinOnLayer.

      for (dbChipBump* bump : region->getChipBumps()) {
        dbInst* inst = bump->getInst();
        if (inst && inst->getMaster()) {
          bump_layer_match_[bump]
              = masterPinOnLayer(inst->getMaster(), layer);
        }
      }

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

defensive

Comment on lines +585 to +610
for (dbUnfoldedChipBumpInst* bump : db_->getUnfoldedChipBumpInsts()) {
dbChipBump* chip_bump = bump->getChipBumpInst()->getChipBump();
const auto it = bump_layer_match_.find(chip_bump);
if (it == bump_layer_match_.end() || it->second) {
continue;
}
violation_count++;
if (!cat) {
cat = dbMarkerCategory::createOrReplace(top_cat, "Bump layer mismatch");
}
if (auto* marker = dbMarker::create(cat)) {
const Point3D p = bump->getGlobalPosition();
marker->addSource(bump->getChipBumpInst());
marker->addShape(Rect(p.x() - kBumpMarkerHalfSize,
p.y() - kBumpMarkerHalfSize,
p.x() + kBumpMarkerHalfSize,
p.y() + kBumpMarkerHalfSize));
dbChipRegion* chip_region = chip_bump->getChipRegion();
marker->setComment(fmt::format(
"Bump {} in region {} has no pin geometry on the region's "
"declared contact layer {}",
chip_bump->getInst()->getName(),
chip_region->getName(),
chip_region->getLayer()->getName()));
}
}

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.

high

If chip_bump->getInst() is null, calling chip_bump->getInst()->getName() will cause a null pointer dereference and crash. We should check if the instance is valid before proceeding and using its name.

  for (dbUnfoldedChipBumpInst* bump : db_->getUnfoldedChipBumpInsts()) {
    dbChipBump* chip_bump = bump->getChipBumpInst()->getChipBump();
    const auto it = bump_layer_match_.find(chip_bump);
    if (it == bump_layer_match_.end() || it->second) {
      continue;
    }
    dbInst* inst = chip_bump->getInst();
    if (!inst) {
      continue;
    }
    violation_count++;
    if (!cat) {
      cat = dbMarkerCategory::createOrReplace(top_cat, "Bump layer mismatch");
    }
    if (auto* marker = dbMarker::create(cat)) {
      const Point3D p = bump->getGlobalPosition();
      marker->addSource(bump->getChipBumpInst());
      marker->addShape(Rect(p.x() - kBumpMarkerHalfSize,
                            p.y() - kBumpMarkerHalfSize,
                            p.x() + kBumpMarkerHalfSize,
                            p.y() + kBumpMarkerHalfSize));
      dbChipRegion* chip_region = chip_bump->getChipRegion();
      marker->setComment(fmt::format(
          "Bump {} in region {} has no pin geometry on the region's "
          "declared contact layer {}",
          inst->getName(),
          chip_region->getName(),
          chip_region->getLayer()->getName()));
    }
  }

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ditto

Comment on lines +588 to +601
void ThreeDBlox::warnIfMultiPinBumpMaster(dbMaster* master)
{
// Downstream code (net hookup, bpin creation, face inference) assumes a
// bump cell exposes exactly one pin.
if (master->getMTermCount() != 1
&& multi_pin_warned_masters_.insert(master).second) {
logger_->warn(utl::ODB,
557,
"Bump cell {} has {} pins; bump cells are expected to have "
"exactly one pin",
master->getName(),
master->getMTermCount());
}
}

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.

medium

If warnIfMultiPinBumpMaster is called with a null master pointer, it will cause a null pointer dereference and crash. Adding a defensive null check at the beginning of the function ensures safety.

void ThreeDBlox::warnIfMultiPinBumpMaster(dbMaster* master)
{
  if (master == nullptr) {
    return;
  }
  // Downstream code (net hookup, bpin creation, face inference) assumes a
  // bump cell exposes exactly one pin.
  if (master->getMTermCount() != 1
      && multi_pin_warned_masters_.insert(master).second) {
    logger_->warn(utl::ODB,
                  557,
                  "Bump cell {} has {} pins; bump cells are expected to have "
                  "exactly one pin",
                  master->getName(),
                  master->getMTermCount());
  }
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ditto

osamahammad21 added 2 commits July 21, 2026 16:34
…is the only user

Signed-off-by: osamahammad21 <osama@precisioninno.com>
Signed-off-by: osamahammad21 <osama@precisioninno.com>
@osamahammad21
osamahammad21 requested a review from a team as a code owner July 22, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant