[DO NOT MERGE YET] search: report a clean error for stale PathEnd handles instead of crashing

[dsengupta0628]

PathEnds returned to Tcl by find_timing_paths are owned by the search's PathGroups and freed on the next timing query (searchPreamble → deletePathGroups). Holding a handle across a query and accessing it (e.g. $pe slack) dereferenced freed memory — SIGSEGV or silent corruption (root cause behind OpenROAD #10210).

This adds a lightweight guard: Search tracks the PathEnds it hands to Tcl (registerValidPathEnds) and clears the set at the free site (deletePathGroups). A single %typemap(check) PathEnd* validates the handle before any accessor derefs it and raises a normal STA error (Error: 2310 ...) when stale.

• One typemap covers all current and future PathEnd accessors — no per-accessor code.
• Tcl-only: internal C++ Sta::findPathEnds callers don't register and pay no cost; the per-query clear() is on an empty set for pure-C++ flows.
• Best-effort by design (a reused address isn't detected); C++ callers holding raw Path*/PathEnd* are unguarded — the contract is "valid only until the next search update" (cf. the rsz Path::prevArc() fix).

Test: test/stale_path_uaf.tcl — holds a PathEnd across both a second find_timing_paths and a report_checks, asserts the clean error (previously SIGSEGV).

[gemini-code-assist]

Code Review

This pull request introduces a stale path-handle guard to prevent crashes when Tcl accessors use dangling PathEnd pointers after a search update. It tracks valid PathEnds using an unordered set, registers them during path-end retrieval, and clears them when path groups are deleted. A SWIG typemap is added to validate PathEnd pointers crossing the Tcl boundary and raise a clean Tcl error if they are stale. A test case is also added to verify this behavior. Feedback on the changes suggests casting nullptr to char* when calling Tcl_AppendResult to avoid potential compiler warnings or build failures in variadic contexts.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Passing nullptr directly to a variadic function like Tcl_AppendResult as a sentinel can lead to compiler warnings (e.g., -Wsentinel or -Wnon-pod-varargs in Clang/GCC) or build failures when strict warning settings are enabled. This is because std::nullptr_t is not a pointer type and does not implicitly convert to char* in a variadic context. Casting it to char* or using static_cast<char*>(nullptr) avoids this issue.

        Tcl_AppendResult(interp, "Error: ", excp.what(), static_cast<char*>(nullptr));

[dsengupta0628]

That's the established upstream pattern and it compiles clean under the project's warning flags. So:

• nullptr is correct and consistent with the codebase.
• Adding static_cast<char*>(nullptr) is harmless but diverges from every other Tcl_AppendResult call in OpenSTA- this is unnecessary inconsistency for a non-issue.

[dsengupta0628]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 452f0bf772

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Guard Path handles returned from PathEnd too

When a Tcl script saves the path separately (set p [$pe path]) before running another timing query, this new check never runs on later $p arrival/$p slack calls because it only validates PathEnd *. In the filtered-path case used by find_timing_paths -through, Sta::searchPreamble() still deletes the filtered arrivals on the next query, so the saved Path * can dangle and the original crash remains reachable instead of producing Error 2310. Please register/validate the Path * handles returned by PathEnd::path()/target_clk_path() as well, or avoid exposing them past the search update.

Useful? React with 👍 / 👎.

[dsengupta0628]

Hmm legit finding. The PathEnd-only guard leaves the held-Path* path crashing. Need to guard Path* too. Let me fix

[dsengupta0628]

Now extended the guard to all 3 Tcl path handles:

1. Unified registry — replaced the PathEnd-only set with one valid_handles_ (unordered_set<const void*>) + registerValidHandle/handleValid; cleared at the free site deletePathGroups.
2. Path* now guarded —
   - check: combined %typemap(check) PathEnd , Path .
   - register at every Path→Tcl emission site: %typemap(out) Path (function returns) + points and PathSeq in StaTclTypes.i (those were the missing channels that false-rejected property_inst_cell).
3. VertexPathIterator* now guarded — register at path_iterator(); validate inline in has_next/next (not via typemap, so finish() can still free a stale iterator).
4. Helpers — reportStaleHandle() (throws, for %extend bodies) + staleHandleError() (catches locally, for check typemaps).
5. Test — stale_path_uaf.tcl now exercises 4 variants (stale PathEnd ×2, stale Path, stale iterator).

[dsengupta0628]

Revisit if we hit a crash with

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}

report_checks

# Stale PathEnd, second query = find_timing_paths.
set pe [lindex [find_timing_paths] 0]


puts "1st PathEnd : slack=[$pe slack]" 
report_checks -path_delay max

puts "2nd PathEnd : slack=[$pe slack]"