Skip to content

odb: initialize all tables in _dbDatabase::clear() constructor#10637

Open
osamahammad21 wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
osamahammad21:odb-fix-db-clear-constructor
Open

odb: initialize all tables in _dbDatabase::clear() constructor#10637
osamahammad21 wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
osamahammad21:odb-fix-db-clear-constructor

Conversation

@osamahammad21

Copy link
Copy Markdown
Member

Summary

Fixes a latent bug in _dbDatabase::_dbDatabase(_dbDatabase*, int id) — the placement-new constructor used by dbDatabase::clear() — that only allocated a subset of the tables that ~_dbDatabase deletes. After clear(), the remaining table pointers held dangling values from the just-destructed object, and any iterator built against them (chip_inst_itr_, chip_region_inst_itr_, chip_conn_itr_, chip_bump_inst_itr_, chip_net_itr_, unfolded_region_itr_, unfolded_bump_itr_) dereferenced uninitialized memory.

Type of Change

  • Bug fix

Impact

  • dbDatabase::clear() is now safe to call: after the destructor runs and the placement-new constructor reconstructs the object, all tables the destructor expects are allocated, and all iterator pointers reference live tables.

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 signed my commits (DCO).

Signed-off-by: osamahammad21 <osama@precisioninno.com>
@osamahammad21 osamahammad21 requested a review from a team as a code owner June 10, 2026 23:51
@osamahammad21 osamahammad21 requested a review from maliberty June 10, 2026 23:51

@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 initializes several new database tables (such as alignment marker rules, chip instances, and unfolded chip tables) in the dbDatabase constructor, and relocates the initialization of prop_tbl. Feedback on the changes points out a critical issue where logger_ is initialized to nullptr during reconstruction (e.g., in dbDatabase::clear()), which can lead to program termination during subsequent logging operations. It is recommended to initialize logger_ to utl::Logger::defaultLogger() to avoid this.

dbu_per_micron_ = 0;
hierarchy_ = false;

alignment_marker_rule_tbl_ = new dbTable<_dbAlignmentMarkerRule>(

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

In the placement-new constructor _dbDatabase::_dbDatabase(_dbDatabase*, int id), logger_ is initialized to nullptr (at line 612). Since dbDatabase::clear() destructs the database and reconstructs it using this constructor, the database's logger becomes nullptr after clear(). Any subsequent database operation that attempts to log (which calls getLogger()) will find logger_ is nullptr and terminate the program. To prevent this, we should initialize logger_ to utl::Logger::defaultLogger() during reconstruction.

  logger_ = utl::Logger::defaultLogger();
  alignment_marker_rule_tbl_ = new dbTable<_dbAlignmentMarkerRule>(

@maliberty

Copy link
Copy Markdown
Member

Can you just do

_dbDatabase::_dbDatabase(_dbDatabase* db) : _dbDatabase(db, db_unique_id++)
{
}

and avoid having two copies of the same. I haven't fully verified this is actually equivalent but I hope it would be.

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.

2 participants