address codex plus one object that could still leak-vertexiterator

Signed-off-by: dsengupta0628 <dsengupta@precisioninno.com>

graph/Graph.i

@@ -166,7 +166,10 @@ VertexPathIterator *
 path_iterator(const RiseFall *rf,
               const MinMax *min_max)
 {
-  return new VertexPathIterator(self, rf, min_max, Sta::sta());
+  VertexPathIterator *iter = new VertexPathIterator(self, rf, min_max, Sta::sta());
+  // The iterator caches a raw Path*; register it for the stale-handle guard.
+  Sta::sta()->search()->registerValidHandle(iter);
+  return iter;
 }
 
 } // Vertex methods

include/sta/Search.hh

@@ -179,15 +179,16 @@ public:
   void deleteFilter();
   void deleteFilteredArrivals();
 
-  // Stale path-handle guard (OpenROAD #10210). PathEnds are freed when path
-  // groups are deleted, so a handle held across a search update dangles. These
-  // track the currently-valid PathEnds so the Tcl accessors report a clean
-  // error instead of crashing (best-effort: a reused address is not detected).
-  // The Tcl-only guard does not cover C++ callers holding a Path*/PathEnd*
-  // directly; the contract is that a path is valid only until the next search
-  // update, so copy stable fields out first (cf. the rsz Path::prevArc() fix).
-  void registerValidPathEnds(const PathEndSeq &ends);
-  bool pathEndValid(const PathEnd *path_end) const;
+  // Stale path-handle guard (OpenROAD #10210). PathEnds and the Paths they
+  // expose are freed/reallocated on a search update, so a handle held across
+  // one dangles. These track the handles currently handed to Tcl so the Tcl
+  // accessors report a clean error instead of crashing (best-effort: a reused
+  // address is not detected). The Tcl-only guard does not cover C++ callers
+  // holding a Path*/PathEnd* directly; the contract is that a path is valid
+  // only until the next search update, so copy stable fields out first (cf. the
+  // rsz Path::prevArc() fix).
+  void registerValidHandle(const void *handle);
+  bool handleValid(const void *handle) const;
 
   VertexSet &endpoints();
   void endpointsInvalid();
@@ -681,8 +682,8 @@ protected:
   bool postpone_latch_outputs_{false};
   std::vector<Path*> enum_paths_;
 
-  // Currently-valid PathEnds returned to external callers (see guard methods).
-  std::unordered_set<const PathEnd*> valid_path_ends_;
+  // PathEnd*/Path* handles currently handed to external callers (guard methods).
+  std::unordered_set<const void*> valid_handles_;
 
   VisitPathEnds *visit_path_ends_;
   GatedClk *gated_clk_;

search/Search.cc

@@ -334,24 +334,24 @@ Search::clear()
 void
 Search::deletePathGroups()
 {
-  // PathEnds (including any handed to external callers) are destroyed here, so
-  // clear the stale-handle guard set at this free site.
-  valid_path_ends_.clear();
+  // Free site for the stale-handle guard set (see Search.hh).
+  valid_handles_.clear();
   for (Mode *mode : modes_)
     mode->deletePathGroups();
 }
 
-// Stale path-handle guard.
+// Stale path-handle guard (see Search.hh).
 void
-Search::registerValidPathEnds(const PathEndSeq &ends)
+Search::registerValidHandle(const void *handle)
 {
-  valid_path_ends_.insert(ends.begin(), ends.end());
+  if (handle)
+    valid_handles_.insert(handle);
 }
 
 bool
-Search::pathEndValid(const PathEnd *path_end) const
+Search::handleValid(const void *handle) const
 {
-  return valid_path_ends_.contains(path_end);
+  return valid_handles_.contains(handle);
 }
 
 bool

search/Search.i

@@ -42,33 +42,53 @@
 
 using namespace sta;
 
-%}
+// Stale path-handle guard (OpenROAD #10210). Error on a dangling handle; throws.
+// Use directly inside %extend bodies (the global %exception catches the throw).
+static void
+reportStaleHandle()
+{
+  Sta::sta()->report()->error(2310,
+                              "stale path handle: a path or path end is only"
+                              " valid until the next timing search update.");
+}
 
-// Stale path-handle guard. One typemap validates every
-// PathEnd* crossing the Tcl boundary -- all %extend accessors (the self arg),
-// report_path_end, and any future ones. Declared before the PathEnd class so it
-// applies to the %extend methods below.
-%typemap(check) PathEnd * {
-  if (!Sta::sta()->search()->pathEndValid($1)) {
-    // A check typemap runs outside the global %exception try/catch
-    // (tcl/Exception.i), so catch report_'s ExceptionMsg locally and convert it
-    // to a Tcl error the same way -- giving the usual "Error: <id> ..." output.
-    try {
-      Sta::sta()->report()->error(2310,
-                                  "path end used after a timing search update;"
-                                  " a path object is only valid until the next"
-                                  " search update.");
-    }
-    catch (ExceptionMsg &excp) {
-      if (!excp.suppressed()) {
-        Tcl_ResetResult(interp);
-        Tcl_AppendResult(interp, "Error: ", excp.what(), nullptr);
-      }
-      SWIG_fail;
+// Same error for check typemaps, which run outside the global %exception: catch
+// and reformat to a Tcl error here. Follow with SWIG_fail at the call site.
+static void
+staleHandleError(Tcl_Interp *interp)
+{
+  try {
+    reportStaleHandle();
+  }
+  catch (ExceptionMsg &excp) {
+    if (!excp.suppressed()) {
+      Tcl_ResetResult(interp);
+      Tcl_AppendResult(interp, "Error: ", excp.what(), nullptr);
     }
   }
 }
 
+%}
+
+// Validate every PathEnd*/Path* entering Tcl (accessor self args + free
+// functions). Declared before the classes so it covers their %extend methods.
+// VertexPathIterator is guarded inline in has_next()/next() instead -- a typemap
+// would also block finish(), leaking a stale iterator that can't be deleted.
+%typemap(check) PathEnd *, Path * {
+  if (!Sta::sta()->search()->handleValid($1)) {
+    staleHandleError(interp);
+    SWIG_fail;
+  }
+}
+
+// Register every Path* returned to Tcl so the check above can detect staleness.
+// (PathEnds are registered in find_path_ends.)
+%typemap(out) Path *, const Path * {
+  Sta::sta()->search()->registerValidHandle($1);
+  Tcl_SetObjResult(interp, SWIG_NewInstanceObj(SWIG_as_voidptr($1),
+                                               SWIGTYPE_p_Path, 0));
+}
+
 ////////////////////////////////////////////////////////////////
 //
 // Empty class definitions to make swig happy.
@@ -408,9 +428,10 @@ find_path_ends(ExceptionFrom *from,
                                       setup, hold,
                                       recovery, removal,
                                       clk_gating_setup, clk_gating_hold);
-  // Register for the Tcl stale-handle guard. Tcl-only:
-  // internal C++ Sta::findPathEnds callers do not register and pay no cost.
-  sta->search()->registerValidPathEnds(ends);
+  // Register for the Tcl stale-handle guard. Tcl-only: internal C++
+  // Sta::findPathEnds callers do not register and pay no cost.
+  for (PathEnd *end : ends)
+    sta->search()->registerValidHandle(end);
   return ends;
 }
 
@@ -1314,10 +1335,20 @@ start_path()
 }
 
 %extend VertexPathIterator {
-bool has_next() { return self->hasNext(); }
+// has_next()/next() deref the iterator's cached Path*, which a search update
+// frees, so validate the handle first (registered in Vertex::path_iterator).
+// finish() only deletes the iterator, so it stays unguarded.
+bool has_next()
+{
+  if (!Sta::sta()->search()->handleValid(self))
+    reportStaleHandle();
+  return self->hasNext();
+}
 Path *
 next()
 {
+  if (!Sta::sta()->search()->handleValid(self))
+    reportStaleHandle();
   return self->next();
 }
 

tcl/StaTclTypes.i

@@ -1149,6 +1149,8 @@ using namespace sta;
   while (path_iter.hasNext()) {
     Path *path = &path_iter.next();
     Path *copy = new Path(path);
+    // Register for the stale-handle guard (see Search guard methods).
+    Sta::sta()->search()->registerValidHandle(copy);
     Tcl_Obj *obj = SWIG_NewInstanceObj(copy, SWIGTYPE_p_Path, false);
     Tcl_ListObjAppendElement(interp, list, obj);
   }
@@ -1399,6 +1401,8 @@ using namespace sta;
   case PropertyValue::Type::paths: {
     Tcl_Obj *list = Tcl_NewListObj(0, nullptr);
     for (const Path *path : *value.paths()) {
+      // Register for the stale-handle guard (see Search guard methods).
+      Sta::sta()->search()->registerValidHandle(path);
       Tcl_Obj *obj = SWIG_NewInstanceObj(const_cast<Path*>(path), SWIGTYPE_p_Path, false);
       Tcl_ListObjAppendElement(interp, list, obj);
     }

test/stale_path_uaf.ok

@@ -1,2 +1,3 @@
-Error: 2310 path end used after a timing search update; a path object is only valid until the next search update.
-Error: 2310 path end used after a timing search update; a path object is only valid until the next search update.
+Error: 2310 stale path handle: a path or path end is only valid until the next timing search update.
+Error: 2310 stale path handle: a path or path end is only valid until the next timing search update.
+Error: 2310 stale path handle: a path or path end is only valid until the next timing search update.

test/stale_path_uaf.tcl

@@ -1,21 +1,37 @@
 # Guard for stale path handles across a search update (OpenROAD #10210).
-# Holding a PathEnd past the next query must error cleanly, not crash.
+# Holding a PathEnd, or a Path saved separately, past the next query must error
+# cleanly, not crash.
 read_liberty ../examples/nangate45_typ.lib.gz
 read_verilog ../examples/example1.v
 link_design top
 create_clock -name clk -period 10 {clk1 clk2 clk3}
 set_input_delay -clock clk 0 {in1 in2}
 
-# Second query = find_timing_paths.
-set ends [find_timing_paths -through u1/Z]
-set pe [lindex $ends 0]
-set junk [find_timing_paths -through u2/ZN]
-catch { puts "stale : slack=[$pe slack] arrival=[[$pe path] arrival]" } msg
+report_checks
+
+# Stale PathEnd, second query = find_timing_paths.
+set pe [lindex [find_timing_paths -through u1/Z] 0]
+find_timing_paths -through u2/ZN
+catch { puts "stale PathEnd : slack=[$pe slack]" } msg
 puts $msg
 
-# Second query = report_checks.
-set ends [find_timing_paths -through u1/Z]
-set pe [lindex $ends 0]
+# Stale PathEnd, second query = report_checks.
+set pe [lindex [find_timing_paths -through u1/Z] 0]
 report_checks -path_delay max
-catch { puts "stale : slack=[$pe slack] arrival=[[$pe path] arrival]" } msg
+catch { puts "stale PathEnd : slack=[$pe slack]" } msg
+puts $msg
+
+# Stale Path saved separately (set p [$pe path]) across the next query.
+set pe [lindex [find_timing_paths -through u1/Z] 0]
+set p [$pe path]
+find_timing_paths -through u2/ZN
+catch { puts "stale Path : arrival=[$p arrival]" } msg
+puts $msg
+
+# Stale VertexPathIterator held across the next query. Iterate the filtered
+# endpoint vertex, whose paths_ array the next query frees/reallocates.
+set pe [lindex [find_timing_paths -through u1/Z] 0]
+set it [[$pe vertex] path_iterator rise max]
+find_timing_paths -through u2/ZN
+catch { while {[$it has_next]} { set p [$it next]; puts "stale PathIter : arrival=[$p arrival]" } } msg
 puts $msg