rsz: Delay buffer removal to prevent use-after-free

Signed-off-by: Krzysztof Bieganski <kbieganski@antmicro.com>

Author: kbieganski

src/rsz/include/rsz/Resizer.hh

@@ -622,6 +622,8 @@ class Resizer : public dbStaState
   bool removeBuffer(Instance* buffer,
                     bool honorDontTouchFixed = true,
                     bool recordJournal = false);
+  bool canRemoveBuffer(Instance* buffer, bool honorDontTouchFixed = true);
+  void removeBuffer2(Instance* buffer, bool recordJournal = false);
   Instance* makeInstance(LibertyCell* cell,
                          const char* name,
                          Instance* parent,

src/rsz/src/RepairSetup.cc

@@ -561,6 +561,9 @@ bool RepairSetup::repairPath(PathRef& path,
                        drvr_index,
                        &expanded,
                        setup_slack_margin)) {
+          Pin* drvr_pin = drvr_path->pin(this);
+          Instance* drvr = network_->instance(drvr_pin);
+          buf_to_remove_.push_back(drvr);
           changed = true;
           break;
         }
@@ -625,6 +628,10 @@ bool RepairSetup::repairPath(PathRef& path,
         }
       }
     }
+    for (auto inst : buf_to_remove_) {
+      resizer_->removeBuffer2(inst, /* recordJournal */ true);
+    }
+    buf_to_remove_.clear();
   }
   return changed;
 }
@@ -858,8 +865,7 @@ bool RepairSetup::removeDrvr(const PathRef* drvr_path,
       return false;
     }
 
-    if (resizer_->removeBuffer(
-            drvr, /* honorDontTouch */ true, /* recordJournal */ true)) {
+    if (resizer_->canRemoveBuffer(drvr, /* honorDontTouch */ true)) {
       removed_buffer_count_++;
       return true;
     }
@@ -977,7 +983,7 @@ bool RepairSetup::estimatedSlackOK(const SlackEstimatorParams& params)
                  "because side input pin {} will have a violating slack of {}:"
                  " old slack={}, slack margin={}, delay_degrad={}",
                  db_network_->name(params.driver),
-                 db_network_->name(side_input_pin), new_slack, old_slack, 
+                 db_network_->name(side_input_pin), new_slack, old_slack,
                  params.setup_slack_margin, delay_degrad);
       // clang-format on
       return false;

src/rsz/src/RepairSetup.hh

@@ -235,6 +235,7 @@ class RepairSetup : public sta::dbStaState
                          int num_endpts);
   void repairSetupLastGasp(const OptoParams& params, int& num_viols);
 
+  std::vector<Instance*> buf_to_remove_;
   Logger* logger_ = nullptr;
   dbNetwork* db_network_ = nullptr;
   Resizer* resizer_;

src/rsz/src/Resizer.cc

@@ -399,6 +399,116 @@ bool Resizer::removeBuffer(Instance* buffer,
   return buffer_removed;
 }
 
+bool Resizer::canRemoveBuffer(Instance* buffer, bool honorDontTouchFixed)
+{
+  LibertyCell* lib_cell = network_->libertyCell(buffer);
+  if (!lib_cell || !lib_cell->isBuffer()) {
+    return false;
+  }
+  // Do not remove buffers connected to input/output ports
+  // because verilog netlists use the net name for the port.
+  if (bufferBetweenPorts(buffer)) {
+    return false;
+  }
+  dbInst* db_inst = db_network_->staToDb(buffer);
+  if (honorDontTouchFixed) {
+    if (db_inst->isDoNotTouch() || db_inst->isFixed()) {
+      return false;
+    }
+  }
+  LibertyPort *in_port, *out_port;
+  lib_cell->bufferPorts(in_port, out_port);
+  Pin* in_pin = db_network_->findPin(buffer, in_port);
+  Pin* out_pin = db_network_->findPin(buffer, out_port);
+  Net* in_net = db_network_->net(in_pin);
+  Net* out_net = db_network_->net(out_pin);
+  dbNet* in_db_net = db_network_->staToDb(in_net);
+  dbNet* out_db_net = db_network_->staToDb(out_net);
+  // honor net dont-touch on input net or output net
+  if (honorDontTouchFixed) {
+    if (in_db_net->isDoNotTouch() || out_db_net->isDoNotTouch()) {
+      return false;
+    }
+  }
+  bool out_net_ports = hasPort(out_net);
+  Net *removed;
+  if (out_net_ports) {
+    if (hasPort(in_net)) {
+      return false;
+    }
+    removed = in_net;
+  } else {
+    // default or out_net_ports
+    // Default to in_net surviving so drivers (cached in dbNetwork)
+    // do not change.
+    removed = out_net;
+  }
+  return (!sdc_->isConstrained(in_pin) && !sdc_->isConstrained(out_pin)
+          && !sdc_->isConstrained(removed) && !sdc_->isConstrained(buffer));
+}
+
+void Resizer::removeBuffer2(Instance* buffer, bool recordJournal)
+{
+  LibertyCell* lib_cell = network_->libertyCell(buffer);
+  // Do not remove buffers connected to input/output ports
+  // because verilog netlists use the net name for the port.
+  dbInst* db_inst = db_network_->staToDb(buffer);
+  if (db_inst->isDoNotTouch()) {
+    //  remove instance dont touch
+    db_inst->setDoNotTouch(false);
+  }
+  if (db_inst->isFixed()) {
+    // change FIXED to PLACED just in case
+    db_inst->setPlacementStatus(odb::dbPlacementStatus::PLACED);
+  }
+  LibertyPort *in_port, *out_port;
+  lib_cell->bufferPorts(in_port, out_port);
+  Pin* in_pin = db_network_->findPin(buffer, in_port);
+  Pin* out_pin = db_network_->findPin(buffer, out_port);
+  Net* in_net = db_network_->net(in_pin);
+  Net* out_net = db_network_->net(out_pin);
+  dbNet* in_db_net = db_network_->staToDb(in_net);
+  dbNet* out_db_net = db_network_->staToDb(out_net);
+  if (in_db_net->isDoNotTouch() || out_db_net->isDoNotTouch()) {
+    // remove net dont touch for manual ECO
+    in_db_net->setDoNotTouch(false);
+    out_db_net->setDoNotTouch(false);
+  }
+  bool out_net_ports = hasPort(out_net);
+  Net *survivor, *removed;
+  if (out_net_ports) {
+    survivor = out_net;
+    removed = in_net;
+  } else {
+    // default or out_net_ports
+    // Default to in_net surviving so drivers (cached in dbNetwork)
+    // do not change.
+    survivor = in_net;
+    removed = out_net;
+  }
+
+  if (recordJournal) {
+    journalRemoveBuffer(buffer);
+  }
+  debugPrint(
+      logger_, RSZ, "remove_buffer", 1, "remove {}", db_network_->name(buffer));
+
+  if (removed) {
+    odb::dbNet* db_survivor = db_network_->staToDb(survivor);
+    odb::dbNet* db_removed = db_network_->staToDb(removed);
+    db_survivor->mergeNet(db_removed);
+
+    sta_->disconnectPin(in_pin);
+    sta_->disconnectPin(out_pin);
+    sta_->deleteInstance(buffer);
+
+    sta_->deleteNet(removed);
+    parasitics_invalid_.erase(removed);
+  }
+  parasiticsInvalid(survivor);
+  updateParasitics();
+}
+
 void Resizer::ensureLevelDrvrVertices()
 {
   if (!level_drvr_vertices_valid_) {