Merge pull request #10225 from The-OpenROAD-Project-staging/secure-fix-10210-sizeup-stale-prev-pin

rsz: use prev_arc input port to fix repair_timing SIGSEGV (#10210)

Author: maliberty

src/dbSta/test/BUILD

@@ -295,6 +295,7 @@ cc_test(
         "Nangate45/Nangate45.lef",
         "Nangate45/Nangate45_typ.lib",
         "cpp/TestDbSta_0.v",
+        "cpp/TestDbSta_StalePrevPath.v",
     ],
     deps = [
         "//src/dbSta",

src/dbSta/test/cpp/TestDbSta.cc

@@ -8,7 +8,12 @@
 #include "db_sta/dbNetwork.hh"
 #include "gtest/gtest.h"
 #include "odb/db.h"
+#include "odb/dbTypes.h"
+#include "sta/Graph.hh"
 #include "sta/NetworkClass.hh"
+#include "sta/Path.hh"
+#include "sta/SdcClass.hh"
+#include "sta/Sta.hh"
 #include "tst/IntegratedFixture.h"
 
 namespace sta {
@@ -97,4 +102,71 @@ TEST_F(TestDbSta, TestHierarchyConnectivity)
   ASSERT_EQ(bterm_clk->getITerm(), nullptr);
 }
 
+// Regression for #10210 (stale Path* dereference in rsz).
+//
+// Topology (TestDbSta_StalePrevPath.v):
+//   clk -> b1(BUF) -> inv1(INV) -> nd1(NAND2) -> out1
+//                                   nd1/A2 <- in2
+//
+// Flow:
+//   1. Capture drvr_path at nd1/ZN and snapshot prevPath() pointer + pin name
+//   2. Delete upstream b1 + updateTiming -> free
+//   3. Add a fresh BUF + clock + updateTiming -> recycle
+//   4. Assert the captured Path's prev slot has been recycled: pin()
+//      decodes to data that belongs to a different instance than nd1's
+//      real input.
+TEST_F(TestDbSta, StalePrevPath)
+{
+  const auto* test_info = testing::UnitTest::GetInstance()->current_test_info();
+  const std::string test_name
+      = std::string(test_info->test_suite_name()) + "_" + test_info->name();
+  readVerilogAndSetup(test_name + ".v");
+  sta_->updateTiming(true);
+
+  Network* network = sta_->network();
+
+  Instance* nd1 = db_network_->dbToSta(block_->findInst("nd1"));
+  Path* drvr_path = sta_->vertexWorstArrivalPath(
+      sta_->ensureGraph()->pinDrvrVertex(network->findPin(nd1, "ZN")),
+      MinMax::max());
+  ASSERT_NE(drvr_path, nullptr);
+  ASSERT_EQ(network->pathName(drvr_path->pin(sta_.get())), "nd1/ZN");
+  const Path* pre_addr = drvr_path->prevPath();
+  ASSERT_NE(pre_addr, nullptr);
+  const std::string pre_pin_name = network->pathName(pre_addr->pin(sta_.get()));
+
+  // 2. Free upstream Path[] slots.
+  sta_->deleteInstance(db_network_->dbToSta(block_->findInst("b1")));
+  sta_->updateTiming(true);
+
+  // 3. Recycle freed slots via a single fresh BUF driven by a new clock.
+  odb::dbNet* in3_net = odb::dbNet::create(block_, "in3");
+  odb::dbBTerm* new_bt = odb::dbBTerm::create(in3_net, "in3");
+  new_bt->setIoType(odb::dbIoType::INPUT);
+  odb::dbNet* nfan_net = odb::dbNet::create(block_, "nfan");
+  odb::dbInst* bnew
+      = odb::dbInst::create(block_, db_->findMaster("BUF_X1"), "bnew");
+  bnew->findITerm("A")->connect(in3_net);
+  bnew->findITerm("Z")->connect(nfan_net);
+
+  PinSet clk2_pins(db_network_);
+  clk2_pins.insert(db_network_->dbToSta(new_bt));
+  FloatSeq clk2_waveform = {0.0f, 0.1f};
+  sta_->makeClock(
+      "clk2", clk2_pins, false, 0.2f, clk2_waveform, "", sta_->cmdMode());
+  sta_->updateTiming(true);
+
+  // 4. Staleness evidence. Pointer address is same but pin name has changed.
+  const Path* post_addr = drvr_path->prevPath();
+  const std::string post_pin_name
+      = post_addr ? network->pathName(post_addr->pin(sta_.get()))
+                  : std::string("<null>");
+
+  EXPECT_EQ(pre_addr, post_addr)
+      << "stale-pointer signature: prev_path_ address unchanged";
+  EXPECT_NE(pre_pin_name, post_pin_name)
+      << "but slot content should differ after free+reuse. before="
+      << pre_pin_name << " after=" << post_pin_name;
+}
+
 }  // namespace sta

src/dbSta/test/cpp/TestDbSta_StalePrevPath.v

@@ -0,0 +1,18 @@
+module top (clk,
+    in2,
+    out1);
+ input clk;
+ input in2;
+ output out1;
+
+ wire n0;
+ wire n1;
+
+ BUF_X1 b1 (.A(clk),
+    .Z(n0));
+ INV_X1 inv1 (.A(n0),
+    .ZN(n1));
+ NAND2_X1 nd1 (.A1(n1),
+    .A2(in2),
+    .ZN(out1));
+endmodule

src/rsz/src/RecoverPower.cc

@@ -327,10 +327,11 @@ bool RecoverPower::downsizeDrvr(const sta::Path* drvr_path,
   sta::Instance* drvr = network_->instance(drvr_pin);
   const float load_cap = graph_delay_calc_->loadCap(
       drvr_pin, drvr_path->scene(sta_), drvr_path->minMax(sta_));
-  const int in_index = drvr_index - 1;
-  const sta::Path* in_path = expanded->path(in_index);
-  const sta::Pin* in_pin = in_path->pin(sta_);
-  const sta::LibertyPort* in_port = network_->libertyPort(in_pin);
+  const sta::TimingArc* in_arc = drvr_path->prevArc(sta_);
+  const sta::LibertyPort* in_port = in_arc ? in_arc->from() : nullptr;
+  if (in_port == nullptr) {
+    return false;
+  }
   if (!resizer_->dontTouch(drvr)) {
     float prev_drive = 0.0;
     if (drvr_index >= 2) {

src/rsz/src/SizeUpMove.cc

@@ -13,6 +13,7 @@
 #include "sta/Liberty.hh"
 #include "sta/NetworkClass.hh"
 #include "sta/Path.hh"
+#include "sta/TimingArc.hh"
 #include "utl/Logger.h"
 
 namespace rsz {
@@ -48,26 +49,27 @@ bool SizeUpMove::doMove(const sta::Path* drvr_path, float setup_slack_margin)
     return false;
   }
 
-  sta::Pin* drvr_input_pin = drvr_path->prevPath()->pin(sta_);
+  const sta::TimingArc* in_arc = drvr_path->prevArc(sta_);
+  sta::LibertyPort* in_port = in_arc ? in_arc->from() : nullptr;
+  if (in_port == nullptr) {
+    return false;
+  }
+
   sta::Path* prev_drvr_path = drvr_path->prevPath()->prevPath();
   sta::Pin* prev_drvr_pin
       = prev_drvr_path ? prev_drvr_path->pin(sta_) : nullptr;
 
-  float prev_drive;
+  float prev_drive = 0.0;
   if (prev_drvr_pin) {
-    prev_drive = 0.0;
     sta::LibertyPort* prev_drvr_port = network_->libertyPort(prev_drvr_pin);
     if (prev_drvr_port) {
       prev_drive = prev_drvr_port->driveResistance();
     }
-  } else {
-    prev_drive = 0.0;
   }
 
   sta::Scene* scene = drvr_path->scene(sta_);
   const sta::MinMax* min_max = drvr_path->minMax(sta_);
   const float load_cap = graph_delay_calc_->loadCap(drvr_pin, scene, min_max);
-  sta::LibertyPort* in_port = network_->libertyPort(drvr_input_pin);
   sta::LibertyPort* drvr_port = network_->libertyPort(drvr_pin);
   sta::LibertyCell* upsize
       = upsizeCell(in_port, drvr_port, load_cap, prev_drive, scene, min_max);

src/rsz/src/SwapPinsMove.cc

@@ -95,10 +95,12 @@ bool SwapPinsMove::doMove(const sta::Path* drvr_path, float setup_slack_margin)
   sta::Scene* scene = drvr_path->scene(sta_);
   const sta::MinMax* min_max = drvr_path->minMax(sta_);
   const float load_cap = graph_delay_calc_->loadCap(drvr_pin, scene, min_max);
-  sta::Pin* drvr_input_pin = drvr_path->prevPath()->pin(sta_);
 
-  // We get the driver port and the cell for that port.
-  sta::LibertyPort* input_port = network_->libertyPort(drvr_input_pin);
+  const sta::TimingArc* in_arc = drvr_path->prevArc(sta_);
+  sta::LibertyPort* input_port = in_arc ? in_arc->from() : nullptr;
+  if (input_port == nullptr) {
+    return false;
+  }
   sta::LibertyPort* swap_port = input_port;
   LibertyPortVec ports;
 

src/rsz/src/UnbufferMove.cc

@@ -24,6 +24,7 @@
 #include "sta/Mode.hh"
 #include "sta/NetworkClass.hh"
 #include "sta/Path.hh"
+#include "sta/TimingArc.hh"
 #include "sta/Transition.hh"
 #include "utl/Logger.h"
 
@@ -99,7 +100,12 @@ bool UnbufferMove::doMove(const sta::Path* drvr_path, float setup_slack_margin)
 
   // Don't remove buffer if new max fanout violations are created
   sta::Vertex* drvr_vertex = graph_->pinDrvrVertex(drvr_pin);
-  sta::Pin* drvr_input_pin = drvr_path->prevPath()->pin(sta_);
+  const sta::TimingArc* in_arc = drvr_path->prevArc(sta_);
+  const sta::LibertyPort* in_lib_port = in_arc ? in_arc->from() : nullptr;
+  if (in_lib_port == nullptr) {
+    return false;
+  }
+  sta::Pin* drvr_input_pin = network_->findPin(drvr, in_lib_port);
   sta::Path* prev_drvr_path = drvr_path->prevPath()->prevPath();
   sta::Pin* prev_drvr_pin
       = prev_drvr_path ? prev_drvr_path->pin(sta_) : nullptr;