Fix AddressSanitizer issues (#6406)

These are all genuine bugs, brief descriptions.

1. V3OrderCFuncEmitter.h used to delete a node early that was still
   reference in a graph dump later. Not a big deal, it can be deleted
   later at the end of V3Order.

2. V3Param.cpp: this one is tricky. The variable referenced by
   AstVarXRef was deleted at the end of `visit(AstGenCase*)`, but then
   `visit(AstVarXRef*)` checks `nodep->varp()` (already deleted) to see
   if it's in an interface.

3. V3String::wildMatch is sometimes called with an empty 's' (the string
   we are matching against tha pattern 'p'), in which case it used to go
   off into the woods. Added check on call. An arbitrary number of `*`
   will still match the empty string.

4. V3Task.cpp: There was an error reported for an unsupported construct,
   then a subsequent SEGV. Just signal the error upward so we bail on an
   error in a more graceful way.

5. verylog.y: Some unsupported constructs failed to set the parsed node,
   so some memory thrash made it into some code downstream. Just parse
   these into nullptr.

Also increased the timeout on one test, which sometimes tripped with
asan on GCC during heavy host load.

src/V3OrderCFuncEmitter.h

@@ -91,8 +91,8 @@ public:
     void emitLogic(const OrderLogicVertex* lVtxp) {
         // Sensitivity domain of logic we are emitting
         AstSenTree* const domainp = lVtxp->domainp();
-        // We are move the logic into a CFunc, so unlink it from the input AstActive
+        // We are move the logic into a CFunc
-        AstNode* const logicp = lVtxp->nodep()->unlinkFrBack();
+        AstNode* const logicp = lVtxp->nodep();
         // If the logic is a procedure, we need to do a few special things
         AstNodeProcedure* const procp = VN_CAST(logicp, NodeProcedure);
 
@@ -114,12 +114,13 @@ public:
         // Process procedures per statement, so we can split CFuncs within procedures.
         // Everything else is handled as a unit.
         AstNode* const headp = [&]() -> AstNode* {
-            if (!procp) return logicp;  // Not a procedure, handle as a unit
+            if (!procp) return logicp->unlinkFrBack();  // Not a procedure, handle as a unit
             AstNode* const stmtsp = procp->stmtsp();
             UASSERT_OBJ(stmtsp, procp, "Empty process should have been deleted earlier");
             stmtsp->unlinkFrBackWithNext();
-            // Procedure is no longer needed and can be deleted right now
+            // 'procp' is no longer needed but the OrderGraph still references
-            VL_DO_DANGLING(procp->deleteTree(), procp);
+            // it (for debug dump). It will get deleted when deleting the
+            // AstActives in V3Order.
             return stmtsp;
         }();
         // Process each statement in the list starting at headp

src/V3Param.cpp

@@ -1302,6 +1302,11 @@ class ParamVisitor final : public VNVisitor {
         return false;
     }
     void visit(AstVarXRef* nodep) override {
+        if (nodep->containsGenBlock()) {
+            // Needs relink, as may remove pointed-to var
+            nodep->varp(nullptr);
+            return;
+        }
         // Check to see if the scope is just an interface because interfaces are special
         const string dotted = nodep->dotted();
         if (!dotted.empty() && nodep->varp() && nodep->varp()->isParam()) {
@@ -1349,10 +1354,6 @@ class ParamVisitor final : public VNVisitor {
                 }
             }
         }
-        if (nodep->containsGenBlock()) {
-            // Needs relink, as may remove pointed-to var
-            nodep->varp(nullptr);
-        }
     }
 
     void visit(AstDot* nodep) override {

src/V3Sched.h

@@ -58,7 +58,12 @@ struct LogicByScope final : public std::vector<std::pair<AstScope*, AstActive*>>
     void deleteActives() {
         for (const auto& pair : *this) {
             AstActive* const activep = pair.second;
-            UASSERT_OBJ(!activep->stmtsp(), activep, "Leftover logic");
+            if (v3Global.opt.debugCheck()) {
+                for (AstNode* nodep = activep->stmtsp(); nodep; nodep = nodep->nextp()) {
+                    AstNodeProcedure* const procp = VN_CAST(nodep, NodeProcedure);
+                    UASSERT_OBJ(procp && !procp->stmtsp(), activep, "Leftover logic");
+                }
+            }
             if (activep->backp()) activep->unlinkFrBack();
             activep->deleteTree();
         }

src/V3String.cpp

@@ -41,14 +41,15 @@ std::map<string, string> VName::s_dehashMap;
 // Wildcard
 
 // Double procedures, inlined, unrolls loop much better
-bool VString::wildmatchi(const char* s, const char* p) VL_PURE {
+template <bool Even = true>
+static bool wildMatchImpl(const char* s, const char* p) VL_PURE {
     for (; *p; s++, p++) {
         if (*p != '*') {
             if (((*s) != (*p)) && *p != '?') return false;
         } else {
             // Trailing star matches everything.
             if (!*++p) return true;
-            while (!wildmatch(s, p)) {
+            while (!wildMatchImpl<!Even>(s, p)) {
                 if (*++s == '\0') return false;
             }
             return true;
@@ -58,19 +59,11 @@ bool VString::wildmatchi(const char* s, const char* p) VL_PURE {
 }
 
 bool VString::wildmatch(const char* s, const char* p) VL_PURE {
-    for (; *p; s++, p++) {
+    if (*s == '\0') {
-        if (*p != '*') {
+        while (*p == '*') ++p;
-            if (((*s) != (*p)) && *p != '?') return false;
+        return *p == '\0';
-        } else {
-            // Trailing star matches everything.
-            if (!*++p) return true;
-            while (!wildmatchi(s, p)) {
-                if (*++s == '\0') return false;
-            }
-            return true;
-        }
     }
-    return (*s == '\0');
+    return wildMatchImpl(s, p);
 }
 
 bool VString::wildmatch(const string& s, const string& p) VL_PURE {

src/V3Task.cpp

@@ -976,7 +976,7 @@ class TaskVisitor final : public VNVisitor {
             stmt += rtnvarp->basicp()->isDpiPrimitive() ? ";\n" : "[0];\n";
             funcp->addStmtsp(new AstCStmt{nodep->fileline(), stmt});
         }
-        makePortList(nodep, funcp);
+        if (!makePortList(nodep, funcp)) return nullptr;
         return funcp;
     }
 
@@ -1003,7 +1003,7 @@ class TaskVisitor final : public VNVisitor {
 
         // Add DPI Import to top, since it's a global function
         m_topScopep->scopep()->addBlocksp(funcp);
-        makePortList(nodep, funcp);
+        if (!makePortList(nodep, funcp)) return nullptr;
         return funcp;
     }
 
@@ -1047,7 +1047,8 @@ class TaskVisitor final : public VNVisitor {
         }
     }
 
-    static void makePortList(AstNodeFTask* nodep, AstCFunc* dpip) {
+    static bool makePortList(AstNodeFTask* nodep, AstCFunc* dpip) {
+        bool allOk = true;
         // Copy nodep's list of function I/O to the new dpip c function
         for (AstNode* stmtp = nodep->stmtsp(); stmtp; stmtp = stmtp->nextp()) {
             if (AstVar* const portp = VN_CAST(stmtp, Var)) {
@@ -1057,6 +1058,7 @@ class TaskVisitor final : public VNVisitor {
                     newPortp->funcLocal(true);
                     dpip->addArgsp(newPortp);
                     if (!portp->basicp()) {
+                        allOk = false;
                         portp->v3warn(
                             E_UNSUPPORTED,
                             "Unsupported: DPI argument of type "
@@ -1069,6 +1071,7 @@ class TaskVisitor final : public VNVisitor {
                 }
             }
         }
+        return allOk;
     }
 
     void bodyDpiImportFunc(AstNodeFTask* nodep, AstVarScope* rtnvscp, AstCFunc* cfuncp,

src/verilog.y

@@ -5918,10 +5918,10 @@ driveStrengthE<nodep>:
 driveStrength<nodep>:
                 yP_PAR__STRENGTH strength0 ',' strength1 ')' { $$ = new AstStrengthSpec{$1, $2, $4}; }
         |       yP_PAR__STRENGTH strength1 ',' strength0 ')' { $$ = new AstStrengthSpec{$1, $4, $2}; }
-        |       yP_PAR__STRENGTH strength0 ',' yHIGHZ1 ')' { BBUNSUP($<fl>4, "Unsupported: highz strength"); }
+        |       yP_PAR__STRENGTH strength0 ',' yHIGHZ1 ')' { $$ = nullptr; BBUNSUP($<fl>4, "Unsupported: highz strength"); }
-        |       yP_PAR__STRENGTH strength1 ',' yHIGHZ0 ')' { BBUNSUP($<fl>4, "Unsupported: highz strength"); }
+        |       yP_PAR__STRENGTH strength1 ',' yHIGHZ0 ')' { $$ = nullptr; BBUNSUP($<fl>4, "Unsupported: highz strength"); }
-        |       yP_PAR__STRENGTH yHIGHZ0 ',' strength1 ')' { BBUNSUP($<fl>2, "Unsupported: highz strength"); }
+        |       yP_PAR__STRENGTH yHIGHZ0 ',' strength1 ')' { $$ = nullptr; BBUNSUP($<fl>2, "Unsupported: highz strength"); }
-        |       yP_PAR__STRENGTH yHIGHZ1 ',' strength0 ')' { BBUNSUP($<fl>2, "Unsupported: highz strength"); }
+        |       yP_PAR__STRENGTH yHIGHZ1 ',' strength0 ')' { $$ = nullptr; BBUNSUP($<fl>2, "Unsupported: highz strength"); }
         ;
 
 pulldown_strengthE<nodep>:  // IEEE: [ pulldown_strength ]
@@ -5930,9 +5930,9 @@ pulldown_strengthE<nodep>:  // IEEE: [ pulldown_strength ]
         ;
 
 pulldown_strength<nodep>:  // IEEE: pulldown_strength
-                yP_PAR__STRENGTH strength0 ',' strength1 ')'  { BBUNSUP($<fl>2, "Unsupported: pulldown strength"); }
+                yP_PAR__STRENGTH strength0 ',' strength1 ')'  { $$ = nullptr; BBUNSUP($<fl>2, "Unsupported: pulldown strength"); }
-        |       yP_PAR__STRENGTH strength1 ',' strength0 ')'  { BBUNSUP($<fl>2, "Unsupported: pulldown strength"); }
+        |       yP_PAR__STRENGTH strength1 ',' strength0 ')'  { $$ = nullptr; BBUNSUP($<fl>2, "Unsupported: pulldown strength"); }
-        |       yP_PAR__STRENGTH strength0 ')'                { BBUNSUP($<fl>2, "Unsupported: pulldown strength"); }
+        |       yP_PAR__STRENGTH strength0 ')'                { $$ = nullptr; BBUNSUP($<fl>2, "Unsupported: pulldown strength"); }
         ;
 
 pullup_strengthE<nodep>:  // IEEE: [ pullup_strength ]
@@ -5941,9 +5941,9 @@ pullup_strengthE<nodep>:  // IEEE: [ pullup_strength ]
         ;
 
 pullup_strength<nodep>:  // IEEE: pullup_strength
-                yP_PAR__STRENGTH strength0 ',' strength1 ')'  { BBUNSUP($<fl>2, "Unsupported: pullup strength"); }
+                yP_PAR__STRENGTH strength0 ',' strength1 ')'  { $$ = nullptr; BBUNSUP($<fl>2, "Unsupported: pullup strength"); }
-        |       yP_PAR__STRENGTH strength1 ',' strength0 ')'  { BBUNSUP($<fl>2, "Unsupported: pullup strength"); }
+        |       yP_PAR__STRENGTH strength1 ',' strength0 ')'  { $$ = nullptr; BBUNSUP($<fl>2, "Unsupported: pullup strength"); }
-        |       yP_PAR__STRENGTH strength1 ')'                { BBUNSUP($<fl>2, "Unsupported: pullup strength"); }
+        |       yP_PAR__STRENGTH strength1 ')'                { $$ = nullptr; BBUNSUP($<fl>2, "Unsupported: pullup strength"); }
         ;
 
 //************************************************

test_regress/driver.py

@@ -126,6 +126,7 @@ class Capabilities:
     # @lru_cache(maxsize=1024) broken with @staticmethod on older pythons we use
     _cached_cmake_version = None
     _cached_cxx_version = None
+    _cached_have_asan = None
     _cached_have_coroutines = None
     _cached_have_gdb = None
     _cached_have_sc = None
@@ -153,6 +154,12 @@ class Capabilities:
 
         return Capabilities._cached_cxx_version
 
+    @staticproperty
+    def have_asan() -> bool:  # pylint: disable=no-method-argument
+        if Capabilities._cached_have_asan is None:
+            Capabilities._cached_have_asan = bool(Capabilities._verilator_get_supported('ASAN'))
+        return Capabilities._cached_have_asan
+
     @staticproperty
     def have_coroutines() -> bool:  # pylint: disable=no-method-argument
         if Capabilities._cached_have_coroutines is None:
@@ -204,6 +211,7 @@ class Capabilities:
     # Fetch
     @staticmethod
     def warmup_cache() -> None:
+        _ignore = Capabilities.have_asan
         _ignore = Capabilities.have_coroutines
         _ignore = Capabilities.have_gdb
         _ignore = Capabilities.have_sc
@@ -1635,6 +1643,10 @@ class VlTest:
     def cxx_version(self) -> str:
         return Capabilities.cxx_version
 
+    @property
+    def have_asan(self) -> bool:
+        return Capabilities.have_asan
+
     @property
     def have_cmake(self) -> bool:
         ver = Capabilities.cmake_version

test_regress/t/t_const_number_unsized_parse.py

@@ -19,7 +19,7 @@ with open(test.top_filename, "w", encoding="utf8") as f:
         f.write(f"  int x{i} = 'd{i};\n")
     f.write("endmodule\n")
 
-test.timeout(30)
+test.timeout(30 if not test.have_asan else 60)
 
 test.lint(verilator_flags2=[f"--max-num-width {2**29}"])
 

test_regress/t/t_dpi_argtype_bad.out

@@ -3,5 +3,4 @@
    13 |    import "DPI-C" task dpix_twice(foo_t arg);
       |                                         ^~~
                     ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest
-%Error: Verilator internal fault, sorry. Suggest trying --debug --gdbbt
+%Error: Exiting due to
-%Error: Command Failed