slang-netlist: segmentation fault on large designs

[udif]

Describe the bug I'm sorry I can't describe it any better other than the fact that it segfaults.
I've seen this in our proprietary code but I was able to easily recreate it with a simple open source design.

To Reproduce

git clone https://github.com/ultraembedded/core_jpeg.git  
cd core_jpeg/src_v  
slang-netlist jpeg_output*  

You will get a segmentation fault. Compiling with debug info and running this under WSL2 (Ubuntu22.04) in Vscode yields the following stack trace:

slang::ast::Expression::visitExpression<slang::ast::Expression const, netlist::VariableReferenceVisitor&>(const slang::ast::Expression * const this, const slang::ast::Expression * expr, netlist::VariableReferenceVisitor & visitor) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:322)  
slang::ast::Expression::visit<netlist::VariableReferenceVisitor&>(const slang::ast::Expression * const this, netlist::VariableReferenceVisitor & visitor) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:372)  
netlist::NetlistVisitor::handle(netlist::NetlistVisitor * const this, const slang::ast::InstanceSymbol & symbol) (\home\udif\git\slang\tools\netlist\include\NetlistVisitor.h:455)  
slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false>::visit<slang::ast::InstanceSymbol>(slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false> * const this, const slang::ast::InstanceSymbol & t) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:68)
slang::ast::Symbol::visit<netlist::NetlistVisitor&>(const slang::ast::Symbol * const this, netlist::NetlistVisitor & visitor) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:158)  
slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false>::visitDefault<slang::ast::InstanceBodySymbol>(slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false> * const this, const slang::ast::InstanceBodySymbol & t) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:96)  
slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false>::visit<slang::ast::InstanceBodySymbol>(slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false> * const this, const slang::ast::InstanceBodySymbol & t) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:72)  
slang::ast::Symbol::visit<netlist::NetlistVisitor&>(const slang::ast::Symbol * const this, netlist::NetlistVisitor & visitor) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:159)  
netlist::NetlistVisitor::handle(netlist::NetlistVisitor * const this, const slang::ast::InstanceSymbol & symbol) (\home\udif\git\slang\tools\netlist\include\NetlistVisitor.h:488)  
slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false>::visit<slang::ast::InstanceSymbol>(slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false> * const this, const slang::ast::InstanceSymbol & t) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:68)  
slang::ast::Symbol::visit<netlist::NetlistVisitor&>(const slang::ast::Symbol * const this, netlist::NetlistVisitor & visitor) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:158)  
slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false>::visitDefault<slang::ast::InstanceBodySymbol>(slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false> * const this, const slang::ast::InstanceBodySymbol & t) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:96)  
slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false>::visit<slang::ast::InstanceBodySymbol>(slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false> * const this, const slang::ast::InstanceBodySymbol & t) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:72)  
slang::ast::Symbol::visit<netlist::NetlistVisitor&>(const slang::ast::Symbol * const this, netlist::NetlistVisitor & visitor) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:159)  
netlist::NetlistVisitor::handle(netlist::NetlistVisitor * const this, const slang::ast::InstanceSymbol & symbol) (\home\udif\git\slang\tools\netlist\include\NetlistVisitor.h:488)  
slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false>::visit<slang::ast::InstanceSymbol>(slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false> * const this, const slang::ast::InstanceSymbol & t) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:68)  
slang::ast::Symbol::visit<netlist::NetlistVisitor&>(const slang::ast::Symbol * const this, netlist::NetlistVisitor & visitor) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:158)  
slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false>::visitDefault<slang::ast::RootSymbol>(slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false> * const this, const slang::ast::RootSymbol & t) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:96)  
slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false>::visit<slang::ast::RootSymbol>(slang::ast::ASTVisitor<netlist::NetlistVisitor, true, false> * const this, const slang::ast::RootSymbol & t) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:72)  
slang::ast::Symbol::visit<netlist::NetlistVisitor&>(const slang::ast::Symbol * const this, netlist::NetlistVisitor & visitor) (\home\udif\git\slang\include\slang\ast\ASTVisitor.h:145)  
main(int argc, char ** argv) (\home\udif\git\slang\tools\netlist\netlist.cpp:225)  

From the little debugging I've seen, the 2nd top call in AstVisitor.h:372 is calling visitExpression with a null "this" pointer.

template<typename TVisitor, typename... Args>
decltype(auto) Expression::visit(TVisitor&& visitor, Args&&... args) const {
    return visitExpression(this, visitor, std::forward<Args>(args)...);
}

I also did a full stack backtrace from a core file using gdb, with full parameter values: https://gist.github.com/udif/cffecd0a769be9e71deb73c45d5e566f

Running any of the modules separately (other than the top jpeg_output.v) yields no error. It seems that this occurs only when module instances are involved. I've seen #855, but there are no interfaces here.

[MikePopoloski]

@jameshanlon FYI

[jameshanlon]

The segfault was caused by trying to dereference an expression for an empty port hookup. Fix in the above PR.

[udif]

I can confirm that my proprietary design no longer core dumps and produces a DOT file. Thanks!

[jameshanlon]

Great!

If you are using the netlist tool for real work, I'd just point out that it's still in an experimental phase and so do proceed with caution.

More details in the README: https://github.com/MikePopoloski/slang/blob/master/tools/netlist/README.md

And TODOs: https://github.com/MikePopoloski/slang/blob/master/tools/netlist/TODO.md

[udif]

My aim is to add combinatorial loop detection. I've started with https://github.com/josch/cycles_johnson_meyer , brough it up to date with modern Java in https://github.com/udif/cycles_johnson_meyer , and then did a C++ port here .

The algorithm takes a graph input (list of edges), builds an adjacency matrix, and calls the subroutine that returns a list of all the elementary cycles.

For combinatorial loop detection, you need to create a graph of only the combinatorial edges. For a simple P.O.C I thought of taking the DOT output, modify it a bit to mark all combinatorial edges, and then feed it to the library. An optimized will probably break this first into several sub graphs (if removing non-combinatorial edges divides the whole graph into several sub-graphs) which will improve memory consumption and speed (a^2+b^2 cells instead of (a+b)^2). It would also be cleaner if the code eventually ends up inside slang-netlist or slang-tidy.

[udif]

I forgot to add that I plan detecting non-combinatorial edges as either being an assignment of a non-combinatorial always block (e.g. an always block that triggers on a single posedge or negedge and not on the signal itself), or if the edge is connected between ports of a known DFF (e.g. take list of DFF from a library file or a simpler text list).