From 452f0bf772eda0f4a983d8154e28c921274fa252 Mon Sep 17 00:00:00 2001 From: dsengupta0628 Date: Tue, 2 Jun 2026 15:21:11 +0000 Subject: [PATCH] enforce sta assump by error instead of seg fault Signed-off-by: dsengupta0628 --- include/sta/Search.hh | 13 +++++++++++++ search/Search.cc | 16 ++++++++++++++++ search/Search.i | 28 ++++++++++++++++++++++++++++ test/stale_path_uaf.ok | 2 ++ test/stale_path_uaf.tcl | 21 +++++++++++++++++++++ 5 files changed, 80 insertions(+) create mode 100644 test/stale_path_uaf.ok create mode 100644 test/stale_path_uaf.tcl diff --git a/include/sta/Search.hh b/include/sta/Search.hh index 841dbe6b..13352846 100644 --- a/include/sta/Search.hh +++ b/include/sta/Search.hh @@ -179,6 +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; + VertexSet &endpoints(); void endpointsInvalid(); @@ -671,6 +681,9 @@ protected: bool postpone_latch_outputs_{false}; std::vector enum_paths_; + // Currently-valid PathEnds returned to external callers (see guard methods). + std::unordered_set valid_path_ends_; + VisitPathEnds *visit_path_ends_; GatedClk *gated_clk_; CheckCrpr *check_crpr_; diff --git a/search/Search.cc b/search/Search.cc index ee735a0c..cd5e0d70 100644 --- a/search/Search.cc +++ b/search/Search.cc @@ -334,10 +334,26 @@ 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(); for (Mode *mode : modes_) mode->deletePathGroups(); } +// Stale path-handle guard. +void +Search::registerValidPathEnds(const PathEndSeq &ends) +{ + valid_path_ends_.insert(ends.begin(), ends.end()); +} + +bool +Search::pathEndValid(const PathEnd *path_end) const +{ + return valid_path_ends_.contains(path_end); +} + bool Search::crprPathPruningEnabled() const { diff --git a/search/Search.i b/search/Search.i index 538b320b..fd4ba946 100644 --- a/search/Search.i +++ b/search/Search.i @@ -44,6 +44,31 @@ using namespace sta; %} +// 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: ..." 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; + } + } +} + //////////////////////////////////////////////////////////////// // // Empty class definitions to make swig happy. @@ -383,6 +408,9 @@ 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); return ends; } diff --git a/test/stale_path_uaf.ok b/test/stale_path_uaf.ok new file mode 100644 index 00000000..2c6296da --- /dev/null +++ b/test/stale_path_uaf.ok @@ -0,0 +1,2 @@ +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. diff --git a/test/stale_path_uaf.tcl b/test/stale_path_uaf.tcl new file mode 100644 index 00000000..6fa9083a --- /dev/null +++ b/test/stale_path_uaf.tcl @@ -0,0 +1,21 @@ +# Guard for stale path handles across a search update (OpenROAD #10210). +# Holding a PathEnd 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 +puts $msg + +# Second query = report_checks. +set ends [find_timing_paths -through u1/Z] +set pe [lindex $ends 0] +report_checks -path_delay max +catch { puts "stale : slack=[$pe slack] arrival=[[$pe path] arrival]" } msg +puts $msg