Merge upstream OpenSTA 08/24

[minjukim55]

Merge parallaxsw/OpenSTA master (737b52f3).

src/sta bump plus the OpenROAD changes this merge needs:
The-OpenROAD-Project/OpenROAD#11226

• #470 moved ownership of the SWIG-allocated Seq containers to a
  %typemap(freearg), so remove_buffers_cmd and write_verilog_cmd had to
  stop deleting them - double free, 13 tests.
• #489 turned the throwing std::stol on an unsized constant into an
  STA-2724/STA-2725 warning, which invalidated five hier_expected_fail.bzl
  entries. Worth a look: input that used to stop the reader now yields a netlist
  with the constant dropped, which kepler-formal will not load.

CI-Public/pr-merge builds OpenROAD master against this branch, so it is red
until #11226 lands.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 5 committers have signed the CLA.

✅ dsengupta0628
✅ minjukim55
❌ James Cherry
❌ dependabot[bot]
❌ akashlevy

---

James Cherry seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive documentation generation system using MkDocs and Read the Docs, adding detailed help text and argument descriptions to numerous Tcl commands and variables. It also adds support for Liberty contamination delay tables (retaining_* and retain_*_slew), fixes SWIG memory leaks and double-free issues, and hardens ConcreteNetwork against memory corruption from bus and bundle ports. The review feedback highlights several critical issues in the new code, including a potential null pointer dereference in ConcreteNetwork.cc, regular expression bugs in tcl/Util.tcl and etc/WriteCmdDocs.tcl that truncate hyphenated options, incorrect anchor generation for positional arguments, variable shadowing in search/Sta.cc, and unsafe /tmp usage in etc/build_sta.sh, alongside various typos in the newly added help texts.

[gemini-code-assist]

If cnet is nullptr, calling connectNetPin(cnet, cpin) will result in a null pointer dereference and crash, as connectNetPin directly calls net->addPin(pin). Since cnet can be nullptr (as evidenced by the if (cnet) guard in the top_instance_ block above), we should also guard the connectNetPin call here.

    else {
      cpin->net_ = cnet;
      if (cnet)
        connectNetPin(cnet, cpin);
    }

[gemini-code-assist]

The regular expression {-[a-zA-Z][a-zA-Z0-9_]*} does not allow hyphens inside option names. For commands with hyphenated options (such as -highest_power_instances, -subtract_pin_load, or -stop_propagation), this regular expression will truncate the option name to its first hyphenated segment (e.g., -highest, -subtract, or -stop). This breaks help generation and documentation lookup for these options. Consider updating the pattern to allow hyphens.

  foreach match [regexp -all -inline -- {-[a-zA-Z][-a-zA-Z0-9_]*} $arglist] {

[gemini-code-assist]

Similar to the issue in tcl/Util.tcl, the regular expression ^(-[a-zA-Z][a-zA-Z0-9_]*) will truncate any option containing a hyphen (e.g., -highest_power_instances -> -highest), causing incorrect anchor links and broken documentation generation. Consider updating the pattern to allow hyphens.

    if { [regexp {^(-[a-zA-Z][-a-zA-Z0-9_]*)} $rest match] } {

[gemini-code-assist]

The loop variable min_max shadows the parameter min_max of the enclosing function. While valid in C++, this shadowing is highly confusing and can trigger compiler warnings (e.g., -Wshadow). Consider renaming the loop variable to mm or m to improve readability and maintainability.

      for (const MinMax *mm : min_max->range())
        scene->addLiberty(lib, mm);

[gemini-code-assist]

The cmd_opt_anchor procedure assumes that all options start with a hyphen - and unconditionally strips the first character using string range $opt 1 end. However, for positional arguments (such as delay or slew), this results in incorrect anchor names like opt-set_assigned_delay-elay or opt-set_assigned_transition-lew. Consider checking if the option starts with a hyphen before stripping it.

proc cmd_opt_anchor { cmd opt } {
  if { [string index $opt 0] == "-" } {
    set opt [string range $opt 1 end]
  }
  return "opt-$cmd-$opt"
}

[gemini-code-assist]

There is a typo in the help text for -source_latency_included: "D no add" instead of "Do not add".

    -source_latency_included {Do not add the clock source latency (insertion delay) to the delay value.}

[gemini-code-assist]

There is a copy-paste error in the description of -clock_gating_hold, which says "hold setup checks" instead of "hold checks" or "enable hold checks".

    -clock_gating_hold {Report gated clock enable hold checks.}

[gemini-code-assist]

There is a grammatical error/typo in the help text: "will be used will improve" instead of "will improve".

Using set_input_transition with the slew from the block context will improve the match between the timing model and the block netlist.

[gemini-code-assist]

There is a grammatical error in the get_fanin help text: "returns traverses" instead of "traverses", and "and return" instead of "and returns".

  -help {The get_fanin command traverses the design from sink_list pins, ports or nets backwards and returns the fanin pins or instances.} \

[gemini-code-assist]

Similar to get_fanin, there is a grammatical error in the get_fanout help text: "returns traverses" instead of "traverses", and "and return" instead of "and returns".

  -help {The get_fanout command traverses the design from source_list pins, ports or nets backwards and returns the fanout pins or instances.} \

[dsengupta0628]

Hi Minju. Please check the reviews from Gemini. Sometimes it is actually able to find some true issues, then those will need to be followed up. I usually create a very minimal test (in parallaxsw/OpenSTA fork not this one) to depict any problematic issue and let James fix it upstream or I propose the fix too and he either takes it or explains the caveats what that situation is not possible. Of course you can ignore the comments about spelling mistakes and what not from Gemini- James typically fixes them in subsequent commits.

Also, you can reference the OpenROAD PR that bumps the src/sta to this version with whatever changes required in OpenROAD for this CI to pass.. sometimes those can expose issues with the OpenSTA changes too- then we again have to follow up- otherwise, for mechanical changes, you can just add them here.

Please note that while a cronjob is good to pull the changes regularly, I think you may need some more manual intervention- this PR's description is unnecessarily large. For example, the "Merge conflict resolved section" is not needed here- the change in .gitignore was rather simple to resolve- this section would be helpful if and when our repo deviates significantly from upstream repo- but not now.
Also, this PR description refers to private repos here which has no place in a public PR.

Finally the PR says "Generated by etc/sta_sync/sta_sync.sh". where is this file? Unless it is part of a public repo, please do not mention here- or maybe consider merging this script to the public repo for us to use - please check with @maliberty for that