The-OpenROAD-Project / OpenROAD

OpenROAD's unified application implementing an RTL-to-GDS Flow. Documentation at https://openroad.readthedocs.io/en/latest/
https://theopenroadproject.org/
BSD 3-Clause "New" or "Revised" License
1.65k stars 567 forks source link

segfault in report_checks #6025

Closed gadfort closed 4 weeks ago

gadfort commented 1 month ago

Describe the bug

During report_checks OpenROAD segfaults.

Expected Behavior

No segfault

Environment

OpenROAD v2.0-16753-g1230b5e23 
Features included (+) or not (-): +Charts +GPU +GUI +Python
This program is licensed under the BSD-3 license. See the LICENSE file for details.
Components of this program may be licensed under more restrictive licenses which must be honored.

To Reproduce

See updated test case below (previously https://drive.google.com/file/d/1M11jBLsGJc24GXY1dFDw3XNuZuDqXNYc/view?usp=sharing)

tar xvf sc_issue_zerosoc_flat_job0_skywater130_sky130hd_floorplan0_20241025-022636.tar.gz
cd sc_issue_zerosoc_flat_job0_skywater130_sky130hd_floorplan0_20241025-022636
./run.sh

Relevant log output

[INFO RSZ-0026] Removed 859 buffers.
SC_METRIC: report_checks -path_delay max
Signal 11 received
Stack trace:
 0# 0x000055D20522F8C8 in /sc_tools/bin/openroad
 1# 0x00007FFBB4D08090 in /lib/x86_64-linux-gnu/libc.so.6
 2# sta::ArrayTable<float>::destroy(unsigned int, unsigned int) in /sc_tools/bin/openroad
 3# sta::Graph::deletePaths(sta::Vertex*, unsigned int) in /sc_tools/bin/openroad
 4# sta::Search::seedArrival(sta::Vertex*) in /sc_tools/bin/openroad
 5# sta::Search::seedArrivals() in /sc_tools/bin/openroad
 6# sta::Search::findArrivalsSeed() in /sc_tools/bin/openroad
 7# sta::Search::findArrivals1(int) in /sc_tools/bin/openroad
 8# sta::Search::findAllArrivals(bool) in /sc_tools/bin/openroad
 9# sta::Search::findPathEnds(sta::ExceptionFrom*, sta::Vector<sta::ExceptionThru*>*, sta::ExceptionTo*, bool, sta::Corner const*, sta::MinMaxAll const*, int, int, bool, float, float, bool, sta::Set<char const*, sta::CharPtrLess>*, bool, bool, bool, bool, bool, bool) in /sc_tools/bin/openroad
10# sta::Sta::findPathEnds(sta::ExceptionFrom*, sta::Vector<sta::ExceptionThru*>*, sta::ExceptionTo*, bool, sta::Corner const*, sta::MinMaxAll const*, int, int, bool, float, float, bool, sta::Set<char const*, sta::CharPtrLess>*, bool, bool, bool, bool, bool, bool) in /sc_tools/bin/openroad
11# find_path_ends(sta::ExceptionFrom*, sta::Vector<sta::ExceptionThru*>*, sta::ExceptionTo*, bool, sta::Corner*, sta::MinMaxAll const*, int, int, bool, float, float, bool, sta::Set<char const*, sta::CharPtrLess>*, bool, bool, bool, bool, bool, bool) in /sc_tools/bin/openroad
12# 0x000055D20548B0D1 in /sc_tools/bin/openroad
13# TclNRRunCallbacks in /lib/x86_64-linux-gnu/libtcl8.6.so
14# 0x00007FFBB90B9924 in /lib/x86_64-linux-gnu/libtcl8.6.so
15# Tcl_EvalEx in /lib/x86_64-linux-gnu/libtcl8.6.so
16# Tcl_Eval in /lib/x86_64-linux-gnu/libtcl8.6.so
17# sta::sourceTclFile(char const*, bool, bool, Tcl_Interp*) in /sc_tools/bin/openroad
18# 0x000055D20522F1A9 in /sc_tools/bin/openroad
19# Tcl_MainEx in /lib/x86_64-linux-gnu/libtcl8.6.so
20# main in /sc_tools/bin/openroad
21# __libc_start_main in /lib/x86_64-linux-gnu/libc.so.6
22# _start in /sc_tools/bin/openroad

Screenshots

No response

Additional Context

No response

maliberty commented 1 month ago

A more reduced version of the test case is at https://drive.google.com/file/d/1uqTUyRmjeM3Qiv1uoAYdkEIKdR9ZW2bA/view?usp=sharing

maliberty commented 1 month ago

I believe the problem originates with

void Resizer::removeBuffers(sta::InstanceSeq insts, bool recordJournal)
{
  initBlock();
  // Disable incremental timing.
  graph_delay_calc_->delaysInvalid();
  search_->arrivalsInvalid();

When the last line is commented out the problem disappears. I see the arrivals_ in the Graph are cleared:

Search::arrivalsInvalid() -> Search::deletePaths() -> graph->clearArrivals(); -> arrivals.clear();

but I can find no code that clears the Vertex's arrivals. This leads to an invalid access of the arrivals table.

@jjcherry56 please confirm if this is a correct analysis of the problem. The OR test case is provided above.

tspyrou commented 1 month ago

@maliberty @gadfort is there a way to make this testcase read a netlist and then fail versus running the flow?

gadfort commented 1 month ago

@tspyrou the testcase @maliberty posted has already been pruned down. Matt tried to do a deltaDebug thing, but I don't know if that yielded anything. If i revert https://github.com/The-OpenROAD-Project/OpenSTA/commit/be563b67097c5d3f5b7bda7f63a0e2926424355d out of the STA build, the segfault goes away.

tspyrou commented 1 month ago

@jjcherry56 fyi above

maliberty commented 1 month ago

delta debug didn't make it much smaller

parallaxsw commented 1 month ago

The issue for debugging it is that in order to build openroad on macos I have to comment out the rtl macro placer and this test case uses it. What would help is to have a db+command file that starts after the rtl placer.

maliberty commented 1 month ago

@parallaxsw Are you using the test case from "A more reduced version of the test case is at" above? I don't think it uses mpl2

maliberty commented 1 month ago

@gadfort would confirm? The sc scripts are hard to read.

gadfort commented 1 month ago

@maliberty yes, the reduced case, has the macro placement removed.

parallaxsw commented 1 month ago

I can run the reduced case.

maliberty commented 1 month ago

@parallaxsw ETA to fix this? I'm wondering if I need to revert the sta update in OR to unblock @gadfort or if this can be fixed quickly.

parallaxsw commented 4 weeks ago

The failure is because the vss/vssio/vdd/vddio ports are bidirectional when check_setup is called and then change to gnd/power when report_checks "necessary to trigger the failure". The distinction is important to opensta (2 vertices versus no vertices). It is possible to compensate in opensta but I think openroad should be able to correctly identify power and ground ports or delete/add them when changing the direction.

maliberty commented 4 weeks ago

@parallaxsw Please give an example of such a port that changes type. dbNetwork::direction either uses Liberty or LEF in

PortDirection* dbNetwork::dbToSta(const dbSigType& sig_type,
                                  const dbIoType& io_type) const

so its hard to see how the type changes.

maliberty commented 4 weeks ago

There are some pad cells that have:

MACRO sky130_ef_io__com_bus_slice_1um
  PIN VSSA
    DIRECTION INOUT ;
    USE GROUND ;

But ground should take precedence over inout.

maliberty commented 4 weeks ago

nvm - I see it is a top level bterm not an iterm that is bidir

maliberty commented 4 weeks ago

There is no way to know from the Verilog. OR only learns about this during pad frame generation. If I add to the sdc:

set_logic_one vdd
set_logic_one vddio
set_logic_zero vss
set_logic_zero vssio

the problem goes away.

maliberty commented 4 weeks ago

To prevent crashes OR will reset the graph on any sigtype change as sta can't handle it.