From 396237222c8a92b7d57d555a30fdcbc83f07e21b Mon Sep 17 00:00:00 2001 From: Naveen Venkat Date: Wed, 15 Jul 2026 11:31:15 +0000 Subject: [PATCH 1/2] odb: guard instance module reassignment Signed-off-by: Naveen Venkat --- src/odb/src/db/dbInst.cpp | 26 ++++++++++++++++---- src/odb/src/db/dbInst.h | 18 ++++++++------ src/odb/src/db/dbModule.cpp | 24 +++++++++---------- src/odb/src/db/dbModule.h | 3 +-- src/odb/test/cpp/TestModule.cpp | 42 +++++++++++++++++++++++++++++++++ 5 files changed, 87 insertions(+), 26 deletions(-) diff --git a/src/odb/src/db/dbInst.cpp b/src/odb/src/db/dbInst.cpp index 0aecdd4be95..f6371522919 100644 --- a/src/odb/src/db/dbInst.cpp +++ b/src/odb/src/db/dbInst.cpp @@ -74,6 +74,24 @@ class sortITerm } }; +void _dbInst::setModule(dbId<_dbModule> module) +{ + const dbId<_dbModule> previous = module_; + if (previous == module) { + return; + } + + module_ = module; + if (previous == 0 || module == 0) { + return; + } + + _dbBlock* block = (_dbBlock*) getOwner(); + for (dbBlockCallBackObj* callback : block->callbacks_) { + callback->inDbPostInstParentChange((dbInst*) this); + } +} + void _dbInst::setInstBBox(_dbInst* inst) { _dbBlock* block = (_dbBlock*) inst->getOwner(); @@ -866,12 +884,12 @@ dbModule* dbInst::getModule() { _dbInst* inst = (_dbInst*) this; - if (inst->module_ == 0) { + if (inst->getModuleId() == 0) { return nullptr; } _dbBlock* block = (_dbBlock*) inst->getOwner(); - _dbModule* module = block->module_tbl_->getPtr(inst->module_); + _dbModule* module = block->module_tbl_->getPtr(inst->getModuleId()); return (dbModule*) module; } @@ -1072,7 +1090,7 @@ bool dbInst::isPhysicalOnly() { _dbInst* inst = (_dbInst*) this; - return inst->module_ == 0; + return inst->getModuleId() == 0; } dbInst* dbInst::getParent() @@ -1589,7 +1607,7 @@ void dbInst::destroy(dbInst* inst_) block->journal_->pushParam(inst->x_); block->journal_->pushParam(inst->y_); block->journal_->pushParam(inst->group_); - block->journal_->pushParam(inst->module_); + block->journal_->pushParam(inst->getModuleId()); block->journal_->pushParam(inst->region_); block->journal_->endAction(); } diff --git a/src/odb/src/db/dbInst.h b/src/odb/src/db/dbInst.h index b0e3d5a2920..321fef3c1cf 100644 --- a/src/odb/src/db/dbInst.h +++ b/src/odb/src/db/dbInst.h @@ -14,6 +14,7 @@ namespace odb { class _dbBox; +class _dbBlock; class _dbInstHdr; class _dbHier; class _dbITerm; @@ -72,14 +73,17 @@ class _dbInst : public _dbObject dbId<_dbInstHdr> inst_hdr_; dbId<_dbBox> bbox_; dbId<_dbRegion> region_; - // WARNING: re-parenting an existing instance changes its full SDC - // path. dbModule::addInst() fires inDbPostInstParentChange in that - // case so downstream caches (e.g., dbSdcNetwork's path-to-instance - // map) stay consistent. The callback is suppressed on the initial - // assignment during dbInst::create -- that path's accounting belongs - // to inDbInstCreate. Prefer dbModule::addInst() over assigning this - // field directly. + + dbId<_dbModule> getModuleId() const { return module_; } + void setModule(dbId<_dbModule> module); + + private: dbId<_dbModule> module_; + friend dbIStream& operator>>(dbIStream& stream, _dbBlock& block); + friend dbIStream& operator>>(dbIStream& stream, _dbInst& inst); + friend dbOStream& operator<<(dbOStream& stream, const _dbInst& inst); + + public: dbId<_dbGroup> group_; dbId<_dbInst> region_next_; dbId<_dbInst> module_next_; diff --git a/src/odb/src/db/dbModule.cpp b/src/odb/src/db/dbModule.cpp index 0b12c9ffd22..46b41533ab8 100644 --- a/src/odb/src/db/dbModule.cpp +++ b/src/odb/src/db/dbModule.cpp @@ -224,7 +224,7 @@ void dbModule::addInst(dbInst* inst) getName()); } - if (_inst->module_ == module->getOID()) { + if (_inst->getModuleId() == module->getOID()) { return; // already in this module } @@ -240,13 +240,12 @@ void dbModule::addInst(dbInst* inst) // initial assignment during dbInst::create (module_ is unset). Only // the former should fire inDbPostInstParentChange -- otherwise every // create would falsely trigger downstream subtree-invalidation paths. - const bool is_reparent = (_inst->module_ != 0); + const bool is_reparent = (_inst->getModuleId() != 0); if (is_reparent) { - dbModule* mod = dbModule::getModule((dbBlock*) block, _inst->module_); - ((_dbModule*) mod)->removeInst(inst); + dbModule* mod = dbModule::getModule((dbBlock*) block, _inst->getModuleId()); + ((_dbModule*) mod)->removeInst(inst, false); } - _inst->module_ = module->getOID(); module->dbinst_hash_[inst->getName()] = dbId<_dbInst>(_inst->getOID()); if (module->insts_ == 0) { @@ -260,20 +259,16 @@ void dbModule::addInst(dbInst* inst) cur_head->module_prev_ = _inst->getOID(); } - if (is_reparent) { - for (dbBlockCallBackObj* cb : block->callbacks_) { - cb->inDbPostInstParentChange(inst); - } - } + _inst->setModule(module->getOID()); } -void _dbModule::removeInst(dbInst* inst) +void _dbModule::removeInst(dbInst* inst, bool clear_module) { _dbModule* module = (_dbModule*) this; _dbInst* _inst = (_dbInst*) inst; uint32_t id = _inst->getOID(); - if (_inst->module_ != getOID()) { + if (_inst->getModuleId() != getOID()) { return; } @@ -286,6 +281,7 @@ void _dbModule::removeInst(dbInst* inst) } _dbBlock* block = (_dbBlock*) getOwner(); + module->dbinst_hash_.erase(inst->getName()); if (module->insts_ == id) { module->insts_ = _inst->module_next_; @@ -305,7 +301,9 @@ void _dbModule::removeInst(dbInst* inst) prev->module_next_ = _inst->module_next_; } } - _inst->module_ = 0; + if (clear_module) { + _inst->setModule(dbId<_dbModule>()); + } _inst->module_next_ = 0; _inst->module_prev_ = 0; } diff --git a/src/odb/src/db/dbModule.h b/src/odb/src/db/dbModule.h index f1c06349a24..c9f09963b08 100644 --- a/src/odb/src/db/dbModule.h +++ b/src/odb/src/db/dbModule.h @@ -49,8 +49,7 @@ class _dbModule : public _dbObject void collectMemInfo(MemInfo& info); // User Code Begin Methods - // This is only used when destroying an inst - void removeInst(dbInst* inst); + void removeInst(dbInst* inst, bool clear_module = true); // Copy and uniquify a given module based on current instance using modBTMap = odb::PtrMap; diff --git a/src/odb/test/cpp/TestModule.cpp b/src/odb/test/cpp/TestModule.cpp index b779c87b367..2acd9acbd7b 100644 --- a/src/odb/test/cpp/TestModule.cpp +++ b/src/odb/test/cpp/TestModule.cpp @@ -5,6 +5,7 @@ #include "gtest/gtest.h" #include "helper.h" #include "odb/db.h" +#include "odb/dbBlockCallBackObj.h" #include "odb/dbSet.h" namespace odb { @@ -235,6 +236,47 @@ class ModuleFixture : public SimpleDbFixture dbBlock* block_; }; +class ParentChangeCallback : public dbBlockCallBackObj +{ + public: + void inDbPostInstParentChange(dbInst* inst) override + { + calls++; + observed_module = inst->getModule(); + } + + int calls = 0; + dbModule* observed_module = nullptr; +}; + +TEST_F(ModuleFixture, reparent_updates_index_and_fires_callback) +{ + dbModule* top = block_->getTopModule(); + dbModule* child = dbModule::create(block_, "child"); + dbInst* inst = dbInst::create(block_, lib_->findMaster("and2"), "inst"); + ASSERT_NE(inst, nullptr); + ASSERT_EQ(top->findDbInst("inst"), inst); + + ParentChangeCallback callback; + callback.addOwner(block_); + + child->addInst(inst); + EXPECT_EQ(callback.calls, 1); + EXPECT_EQ(callback.observed_module, child); + EXPECT_EQ(inst->getModule(), child); + EXPECT_EQ(top->findDbInst("inst"), nullptr); + EXPECT_EQ(child->findDbInst("inst"), inst); + + child->addInst(inst); + EXPECT_EQ(callback.calls, 1); + + top->addInst(inst); + EXPECT_EQ(callback.calls, 2); + EXPECT_EQ(callback.observed_module, top); + EXPECT_EQ(child->findDbInst("inst"), nullptr); + EXPECT_EQ(top->findDbInst("inst"), inst); +} + TEST_F(ModuleFixture, test_default) { // dbModule::create() Succeed From 1f4ff83df632beb8ab9b3bf2f9f47c28409e89b1 Mon Sep 17 00:00:00 2001 From: Naveen Venkat Date: Wed, 15 Jul 2026 12:35:28 +0000 Subject: [PATCH 2/2] odb: update module index when renaming instances Signed-off-by: Naveen Venkat --- src/odb/src/db/dbInst.cpp | 11 +++++++++++ src/odb/test/cpp/TestModule.cpp | 16 ++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/odb/src/db/dbInst.cpp b/src/odb/src/db/dbInst.cpp index f6371522919..87570c0fc4a 100644 --- a/src/odb/src/db/dbInst.cpp +++ b/src/odb/src/db/dbInst.cpp @@ -398,9 +398,20 @@ bool dbInst::rename(const char* name) std::string old_name(inst->name_); block->inst_hash_.remove(inst); + + _dbModule* module = nullptr; + if (inst->getModuleId() != 0) { + module = (_dbModule*) dbModule::getModule((dbBlock*) block, + inst->getModuleId()); + module->dbinst_hash_.erase(old_name); + } + free((void*) inst->name_); inst->name_ = safe_strdup(name); block->inst_hash_.insert(inst); + if (module != nullptr) { + module->dbinst_hash_[name] = dbId<_dbInst>(inst->getOID()); + } for (dbBlockCallBackObj* cb : block->callbacks_) { cb->inDbPostInstRename(this, old_name.c_str()); diff --git a/src/odb/test/cpp/TestModule.cpp b/src/odb/test/cpp/TestModule.cpp index 2acd9acbd7b..ba2fb447c67 100644 --- a/src/odb/test/cpp/TestModule.cpp +++ b/src/odb/test/cpp/TestModule.cpp @@ -277,6 +277,22 @@ TEST_F(ModuleFixture, reparent_updates_index_and_fires_callback) EXPECT_EQ(top->findDbInst("inst"), inst); } +TEST_F(ModuleFixture, rename_updates_module_index) +{ + dbModule* top = block_->getTopModule(); + dbInst* inst = dbInst::create(block_, lib_->findMaster("and2"), "old_name"); + ASSERT_NE(inst, nullptr); + ASSERT_EQ(top->findDbInst("old_name"), inst); + + ASSERT_TRUE(inst->rename("new_name")); + EXPECT_EQ(top->findDbInst("old_name"), nullptr); + EXPECT_EQ(top->findDbInst("new_name"), inst); + + dbInst::destroy(inst); + EXPECT_EQ(top->findDbInst("old_name"), nullptr); + EXPECT_EQ(top->findDbInst("new_name"), nullptr); +} + TEST_F(ModuleFixture, test_default) { // dbModule::create() Succeed