gui: fix two empty-container dereferences in ClockTree

[openroad-ci]

Fixes The-OpenROAD-Project/OpenROAD-flow-scripts#4371, reported by @jeffng-or and investigated by @LucasYuki.

Symptom

An OpenROAD GUI binary built with Bazel aborts in the ORFS finish stage while flow/scripts/save_images.tcl populates the Clock Tree Viewer:

[ERROR ORD-2018] Pin is not ITerm or BTerm or modITerm.
[ERROR GUI-0070] ORD-2018
Error: final_report.tcl, 7 GUI-0070

A CMake-built binary from the same source is fine, which is what made this look like a build-system problem.

Root cause

ClockTree's clock constructor (src/gui/src/staGuiInterface.cpp:552) did:

net_ = getNet(*clock_->pins().begin());

STAGuiInterface::getClockTrees() builds a ClockTree for every clock in the SDC (staGuiInterface.cpp:1522), including virtual clocks. A virtual clock — create_clock with no object list — has an empty pins(), so this dereferences the end iterator of an empty std::set. That is UB.

The consumer already anticipates a null net for that case:

// src/gui/src/clockWidget.cpp:1631
if (!tree->getNet()) {  // skip virtual clocks

and libstdc++ happens to supply one: its _Rb_tree is 48 bytes, the read lands on _Rb_tree_header::_M_node_count (zero), getNet receives nullptr, and dbNetwork::staToDb's null guard (src/dbSta/src/dbNetwork.cc:3141) swallows it. So the CMake builds work by accident.

libc++'s __tree is 32 bytes, so the same read lands on the adjacent leaf_pins_ member and returns a live, 8-byte-aligned end-node pointer. sta::Pin* is pointer-tagged in its low 3 bits (dbNetwork.cc:246-253); an untagged-but-non-null pointer is exactly the kNone case that raises ORD-2018 at dbNetwork.cc:3154.

Bazel builds against libc++ (hermetic-llvm, no cxxstdlib constraint), CMake against system gcc/libstdc++ — hence bazel-only, and fully deterministic rather than a heisenbug.

Blast radius

64 SDC files under flow/designs/ in ORFS declare a bare create_clock, so this breaks save_images.tcl for most designs there, not just asap7/riscv32i.

Fix

Guard the dereference. net_ is already nullptr in the init list, so virtual clocks keep taking the existing skip in ClockWidget::populate — identical to today's libstdc++ behaviour.

Verification

Built --//:platform=gui //:openroad and compared against the CMake binary at the same commit (20d2d5c16e), on Nangate45 gcd plus one bare create_clock:

	before	after
CMake (libstdc++)	passes	passes
Bazel (libc++)	ORD-2018	passes

bazelisk test --//:platform=gui //src/gui/... and bazelisk test //src/gui/... both pass 4/4.

Test

New src/gui/test/clocktree_virtual_clock.tcl, registered in both CMake (GUI_PASSFAIL_TESTS, gated on Qt5_FOUND AND BUILD_GUI) and Bazel (ALL_TESTS + PASSFAIL_TESTS). It opens the Clock Tree Viewer on a design with a virtual clock and asserts a clock tree image is written. Verified to fail on the unfixed code under --//:platform=gui, and to report pass without exercising anything when the binary has no GUI.

There was previously no test that opened the Clock Tree Viewer at all, which is why this survived.

Not fixed here

The same issue log also shows GUI-0010 rewriting final_clocks.webp to final_clocks.webp.png. That is an independent defect: the qt-bazel prebuilts don't ship Qt's qwebp image-format plugin, so QImageWriter::supportedImageFormats() has no webp and Utils::fixImagePath appends .png. Addressed separately in The-OpenROAD-Project/qt_bazel_prebuilts.

Second commit: the sibling site

ClockTree::findPathTo had the same shape and is now fixed in this PR too:

• drivers_.begin()->first on a ClockTree with no driver — identical to the constructor bug.
• search_pin = *connections.begin() — independently reachable. pin_map is a std::map and connections comes from operator[], which default-constructs an empty set for a pin that isn't in the mapping, so a search_pin outside this tree falls through the find(root) check and then dereferences that empty set's end iterator.

The only caller (clockWidget.cpp:97) already handles an empty result by drawing nothing. No test accompanies it: unlike the constructor, this path is reachable only from the interactive "highlight path to" action, which needs GUI mouse interaction to drive.

Correction: no sanitizer or hardening mode catches this

An earlier revision of this description suggested enabling _LIBCPP_HARDENING_MODE / _GLIBCXX_ASSERTIONS as prevention. I measured it, and that's wrong — none of them catch an empty-container dereference of this shape. With the hermetic clang 22 / libc++ 22 toolchain, *set.begin() and map.begin()->first on an empty container trap at none of _LIBCPP_HARDENING_MODE_FAST, _EXTENSIVE, or _DEBUG:

requested FAST       -> hardening mode: 4   set deref -> (nil) (no trap)
requested EXTENSIVE  -> hardening mode: 16  set deref -> (nil) (no trap)
requested DEBUG      -> hardening mode: 8   set deref -> (nil) (no trap)

(The mode value confirms the flag was actually in effect, so this isn't a misconfigured probe. Default in these builds is NONE = 2.)

libstdc++ with _GLIBCXX_ASSERTIONS doesn't trap either. Only _GLIBCXX_DEBUG catches it (state = past-the-end) — and that is ABI-breaking and applies solely to the CMake/libstdc++ builds, i.e. the ones that already happen to work. So it isn't a usable CI lever for the configuration that actually breaks.

ASan can't help either, and not just incidentally: in the real failure the read lands on leaf_pins_, an adjacent member of the same sta::Clock object, which is a perfectly in-bounds read as far as ASan is concerned. (I couldn't execute an ASan build here — the hermetic toolchain in my environment is missing the libclang_rt.asan* archives — but the mechanism doesn't depend on running it.)

The practical prevention is therefore what this PR does: fix both sites, and add a test that actually opens the Clock Tree Viewer. Worth keeping in mind for future reviews that libc++'s __tree iterators are unchecked at every hardening level.

[gemini-code-assist]

Code Review

This pull request fixes undefined behavior in the Clock Tree Viewer when handling virtual clocks (which have no pins) by checking if the clock's pins are empty before dereferencing. It also adds safety checks in ClockTree::findPathTo for empty drivers or connections, and introduces a regression test clocktree_virtual_clock to verify this behavior. Feedback on the changes suggests simplifying the ternary expressions in the Bazel BUILD file to direct, idiomatic boolean expressions using in and not in.

[gemini-code-assist]

These ternary expressions can be simplified to direct boolean expressions using in and not in operators, which is more idiomatic and readable in Starlark/Python.

Suggested change

	check_log = False if test_name in PASSFAIL_TESTS else True,
	check_passfail = True if test_name in PASSFAIL_TESTS else False,
	check_log = test_name not in PASSFAIL_TESTS,
	check_passfail = test_name in PASSFAIL_TESTS,