Merge pull request #9142 from The-OpenROAD-Project-staging/secure-fix-restore-odb-hash-in-module

odb: Fixed a bug where `_dbModule::modbterm_hash_` is emtpy after `read_db` is executed

Author: maliberty

src/odb/src/db/dbBlock.cpp

@@ -25,6 +25,7 @@
 #include <set>
 #include <sstream>
 #include <string>
+#include <unordered_map>
 #include <utility>
 #include <vector>
 
@@ -859,6 +860,43 @@ dbOStream& operator<<(dbOStream& stream, const _dbBlock& block)
   return stream;
 }
 
+/**
+ * @brief Rebuilds the name-to-ID hash maps for hierarchical objects within
+ * their respective modules.
+ *
+ * This function is used during database loading (operator>>) to restore the
+ * fast-lookup hashes that were stored in the ODB.
+ *
+ * @tparam T The public ODB object type (e.g., dbInst, dbModNet).
+ * @tparam T_impl The internal implementation type (e.g., _dbInst, _dbModNet).
+ * @param block The database block containing the objects.
+ * @param table The internal database table storing the object implementations.
+ * @param module_field Member pointer to the field storing the parent module ID
+ *                     (e.g., &_dbInst::module_ or &_dbModInst::parent_).
+ * @param hash_field Member pointer to the hash map member in _dbModule where
+ *                   the object ID should be stored.
+ */
+template <typename T, typename T_impl>
+static void rebuildModuleHash(
+    _dbBlock& block,
+    dbTable<T_impl>* table,
+    dbId<_dbModule> T_impl::*module_field,
+    std::unordered_map<std::string, dbId<T_impl>> _dbModule::*hash_field)
+{
+  dbSet<T> items((dbBlock*) &block, table);
+  for (T* obj : items) {
+    T_impl* _obj = (T_impl*) obj;
+    dbId<_dbModule> mid = _obj->*module_field;
+    if (mid == 0) {
+      mid = block.top_module_;
+    }
+    _dbModule* module = block.module_tbl_->getPtr(mid);
+    if (module && _obj->name_) {
+      (module->*hash_field)[_obj->name_] = _obj->getId();
+    }
+  }
+}
+
 dbIStream& operator>>(dbIStream& stream, _dbBlock& block)
 {
   _dbDatabase* db = block.getImpl()->getDatabase();
@@ -950,17 +988,43 @@ dbIStream& operator>>(dbIStream& stream, _dbBlock& block)
     stream >> *block.inst_tbl_;
     stream >> *block.module_tbl_;
   }
+  if (db->isSchema(kSchemaDbRemoveHash)) {
+    // Construct dbinst_hash_
+    rebuildModuleHash<dbInst>(
+        block, block.inst_tbl_, &_dbInst::module_, &_dbModule::dbinst_hash_);
+  }
   if (db->isSchema(kSchemaBlockOwnsScanInsts)) {
     stream >> *block.scan_inst_tbl_;
   }
   stream >> *block.modinst_tbl_;
+  if (db->isSchema(kSchemaDbRemoveHash)) {
+    // Construct modinst_hash_
+    rebuildModuleHash<dbModInst>(block,
+                                 block.modinst_tbl_,
+                                 &_dbModInst::parent_,
+                                 &_dbModule::modinst_hash_);
+  }
   if (db->isSchema(kSchemaUpdateHierarchy)) {
     stream >> *block.modbterm_tbl_;
+    if (db->isSchema(kSchemaDbRemoveHash)) {
+      // Construct modbterm_hash_
+      rebuildModuleHash<dbModBTerm>(block,
+                                    block.modbterm_tbl_,
+                                    &_dbModBTerm::parent_,
+                                    &_dbModule::modbterm_hash_);
+    }
     if (db->isSchema(kSchemaDbRemoveHash)) {
       stream >> *block.busport_tbl_;
     }
     stream >> *block.moditerm_tbl_;
     stream >> *block.modnet_tbl_;
+    if (db->isSchema(kSchemaDbRemoveHash)) {
+      // Construct modnet_hash_
+      rebuildModuleHash<dbModNet>(block,
+                                  block.modnet_tbl_,
+                                  &_dbModNet::parent_,
+                                  &_dbModule::modnet_hash_);
+    }
   }
   stream >> *block.powerdomain_tbl_;
   stream >> *block.logicport_tbl_;

src/odb/src/db/dbInst.cpp

@@ -191,24 +191,6 @@ dbIStream& operator>>(dbIStream& stream, _dbInst& inst)
   stream >> inst.iterms_;
   stream >> inst.halo_;
   stream >> inst.pin_access_idx_;
-
-  dbDatabase* db = (dbDatabase*) (inst.getDatabase());
-  if (((_dbDatabase*) db)->isSchema(kSchemaDbRemoveHash)) {
-    _dbBlock* block = (_dbBlock*) (db->getChip()->getBlock());
-    _dbModule* module = nullptr;
-    // if the instance has no module parent put in the top module
-    // We sometimes see instances with _module set to 0 (possibly
-    // introduced downstream) so we stick them in the hash for the
-    // top module.
-    if (inst.module_ == 0) {
-      module = (_dbModule*) (((dbBlock*) block)->getTopModule());
-    } else {
-      module = block->module_tbl_->getPtr(inst.module_);
-    }
-    if (inst.name_) {
-      module->dbinst_hash_[inst.name_] = dbId<_dbInst>(inst.getId());
-    }
-  }
   return stream;
 }
 

src/odb/src/db/dbModBTerm.cpp

@@ -107,16 +107,6 @@ dbIStream& operator>>(dbIStream& stream, _dbModBTerm& obj)
   if (obj.getDatabase()->isSchema(kSchemaHierPortRemoval)) {
     stream >> obj.prev_entry_;
   }
-  // User Code Begin >>
-  if (obj.getDatabase()->isSchema(kSchemaDbRemoveHash)) {
-    dbDatabase* db = (dbDatabase*) (obj.getDatabase());
-    _dbBlock* block = (_dbBlock*) (db->getChip()->getBlock());
-    _dbModule* module = block->module_tbl_->getPtr(obj.parent_);
-    if (obj.name_) {
-      module->modbterm_hash_[obj.name_] = obj.getId();
-    }
-  }
-  // User Code End >>
   return stream;
 }
 

src/odb/src/db/dbModInst.cpp

@@ -106,13 +106,6 @@ dbIStream& operator>>(dbIStream& stream, _dbModInst& obj)
   if (db_->isSchema(kSchemaUpdateHierarchy)) {
     stream >> obj.moditerms_;
   }
-  if (db_->isSchema(kSchemaDbRemoveHash)) {
-    _dbBlock* block = (_dbBlock*) (((dbDatabase*) db_)->getChip()->getBlock());
-    _dbModule* module = block->module_tbl_->getPtr(obj.parent_);
-    if (obj.name_) {
-      module->modinst_hash_[obj.name_] = obj.getId();
-    }
-  }
   // User Code End >>
   return stream;
 }

src/odb/src/db/dbModNet.cpp

@@ -106,16 +106,6 @@ dbIStream& operator>>(dbIStream& stream, _dbModNet& obj)
   if (obj.getDatabase()->isSchema(kSchemaUpdateHierarchy)) {
     stream >> obj.bterms_;
   }
-  // User Code Begin >>
-  if (obj.getDatabase()->isSchema(kSchemaDbRemoveHash)) {
-    dbDatabase* db = (dbDatabase*) (obj.getDatabase());
-    _dbBlock* block = (_dbBlock*) (db->getChip()->getBlock());
-    _dbModule* module = block->module_tbl_->getPtr(obj.parent_);
-    if (obj.name_) {
-      module->modnet_hash_[obj.name_] = dbId<_dbModNet>(obj.getId());
-    }
-  }
-  // User Code End >>
   return stream;
 }
 

src/odb/test/cpp/TestWriteReadDbHier.cpp

@@ -178,5 +178,58 @@ TEST_F(TestWriteReadDbHier, WriteReadOdb)
   ASSERT_NE(db2_mi1, nullptr);
 }
 
+// Construct hierarchical netlist with multiple blocks and write/read it back.
+// The hierarchical netlist should be read back successfully.
+TEST_F(TestWriteReadDbHier, WriteReadOdbMultiBlocks)
+{
+  SetUpTmpPath("WriteReadOdbMultiBlocks");
+  db_->setHierarchy(true);
+
+  // Create masters
+  dbMaster* buf_master = db_->findMaster("BUF_X1");
+  ASSERT_TRUE(buf_master);
+
+  // Create child block
+  dbBlock* child_block = dbBlock::create(block_, "CHILD_BLOCK");
+  ASSERT_TRUE(child_block);
+
+  // Create module in child block
+  dbModule* mod0 = dbModule::create(child_block, "MOD0");
+  ASSERT_TRUE(mod0);
+
+  dbModBTerm* mod0_a = dbModBTerm::create(mod0, "A");
+  ASSERT_TRUE(mod0_a);
+
+  // Create instance in child block
+  dbInst* child_inst
+      = dbInst::create(child_block, buf_master, "child_inst", false, mod0);
+  ASSERT_TRUE(child_inst);
+
+  //--------------------------------------------------------------
+  // Write ODB and read back
+  //--------------------------------------------------------------
+  dbDatabase* db2 = writeReadDb();
+  ASSERT_TRUE(db2->hasHierarchy());
+  dbBlock* top_block2 = db2->getChip()->getBlock();
+
+  // Find child block
+  dbBlock* child_block2 = top_block2->findChild("CHILD_BLOCK");
+  ASSERT_NE(child_block2, nullptr);
+
+  // Find module in child block
+  dbModule* mod0_2 = child_block2->findModule("MOD0");
+  ASSERT_NE(mod0_2, nullptr);
+
+  // Verify ModBTerm hash is populated
+  dbModBTerm* mod0_a_2 = mod0_2->findModBTerm("A");
+  ASSERT_NE(mod0_a_2, nullptr);
+  EXPECT_STREQ(mod0_a_2->getName(), "A");
+
+  // Verify Inst hash is populated
+  dbInst* child_inst_2 = mod0_2->findDbInst("child_inst");
+  ASSERT_NE(child_inst_2, nullptr);
+  EXPECT_EQ(child_inst_2->getName(), "child_inst");
+}
+
 }  // namespace
 }  // namespace odb

src/tst/include/tst/IntegratedFixture.h

@@ -35,9 +35,9 @@ class IntegratedFixture : public tst::Fixture
   void removeFile(const std::string& path);
 
  protected:
-  odb::dbLib* lib_;
+  odb::dbLib* lib_{nullptr};
   odb::dbBlock* block_{nullptr};
-  sta::dbNetwork* db_network_;
+  sta::dbNetwork* db_network_{nullptr};
 
   stt::SteinerTreeBuilder stt_;
   utl::CallBackHandler callback_handler_;