More fixes for infinitely recursive checkers

MikePopoloski · Mar 14, 2024 · 21f2dfb · 21f2dfb
1 parent ef21b23
commit 21f2dfb
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 11 deletions.
diff --git a/include/slang/ast/symbols/InstanceSymbols.h b/include/slang/ast/symbols/InstanceSymbols.h
@@ -353,7 +353,7 @@ class SLANG_EXPORT CheckerInstanceSymbol : public InstanceSymbolBase {
     /// Creates an intentionally invalid instance by forcing all port connections to
     /// null values. This allows type checking instance members as long as they don't
     /// depend on any port expansion.
-    static CheckerInstanceSymbol& createInvalid(const CheckerSymbol& checker);
+    static CheckerInstanceSymbol& createInvalid(const CheckerSymbol& checker, uint32_t depth);
 
     static CheckerInstanceSymbol& fromSyntax(
         Compilation& compilation, const ASTContext& context, const CheckerSymbol& checker,

diff --git a/source/ast/ElabVisitors.h b/source/ast/ElabVisitors.h
@@ -248,18 +248,25 @@ struct DiagnosticVisitor : public ASTVisitor<DiagnosticVisitor, false, false> {
         visit(symbol.body);
 
         if (!finishedEarly()) {
-            if (symbol.body.instanceDepth >= compilation.getOptions().maxCheckerInstanceDepth) {
-                hierarchyProblem = true;
-                return;
-            }
-
             for (auto& conn : symbol.getPortConnections())
                 conn.getOutputInitialExpr();
 
             symbol.verifyMembers();
         }
     }
 
+    void handle(const CheckerInstanceBodySymbol& symbol) {
+        if (finishedEarly())
+            return;
+
+        if (symbol.instanceDepth > compilation.getOptions().maxCheckerInstanceDepth) {
+            hierarchyProblem = true;
+            return;
+        }
+
+        visitDefault(symbol);
+    }
+
     void handle(const ClockingBlockSymbol& symbol) {
         if (!handleDefault(symbol))
             return;
@@ -357,7 +364,7 @@ struct DiagnosticVisitor : public ASTVisitor<DiagnosticVisitor, false, false> {
         if (!handleDefault(symbol))
             return;
 
-        auto& result = CheckerInstanceSymbol::createInvalid(symbol);
+        auto& result = CheckerInstanceSymbol::createInvalid(symbol, 0);
         result.visit(*this);
     }
 

diff --git a/source/ast/symbols/InstanceSymbols.cpp b/source/ast/symbols/InstanceSymbols.cpp
@@ -1896,7 +1896,7 @@ CheckerInstanceSymbol& CheckerInstanceSymbol::fromSyntax(
                 auto& diag = context.addDiag(diag::MaxInstanceDepthExceeded, loc);
                 diag << "checker"sv;
                 diag << comp.getOptions().maxCheckerInstanceDepth;
-                return createInvalid(checker);
+                return createInvalid(checker, depth);
             }
 
             parentIsFromBind = checkerBody.isFromBind;
@@ -1909,7 +1909,7 @@ CheckerInstanceSymbol& CheckerInstanceSymbol::fromSyntax(
     if (parentIsFromBind) {
         if (isFromBind) {
             context.addDiag(diag::BindUnderBind, syntax.sourceRange());
-            return createInvalid(checker);
+            return createInvalid(checker, depth);
         }
 
         // If our parent is from a bind statement, pass down the flag
@@ -2090,7 +2090,8 @@ CheckerInstanceSymbol& CheckerInstanceSymbol::fromSyntax(
     return *instance;
 }
 
-CheckerInstanceSymbol& CheckerInstanceSymbol::createInvalid(const CheckerSymbol& checker) {
+CheckerInstanceSymbol& CheckerInstanceSymbol::createInvalid(const CheckerSymbol& checker,
+                                                            uint32_t depth) {
     auto scope = checker.getParentScope();
     SLANG_ASSERT(scope);
 
@@ -2101,7 +2102,7 @@ CheckerInstanceSymbol& CheckerInstanceSymbol::createInvalid(const CheckerSymbol&
 
     ASTContext context(*scope, LookupLocation::after(checker));
     auto body = comp.emplace<CheckerInstanceBodySymbol>(comp, checker, *assertionDetails, context,
-                                                        0u,
+                                                        depth,
                                                         /* isProcedural */ false,
                                                         /* isFromBind */ false,
                                                         /* isUninstantiated */ true);

diff --git a/tests/unittests/ast/AssertionTests.cpp b/tests/unittests/ast/AssertionTests.cpp
@@ -2281,3 +2281,21 @@ TEST_CASE("Checker instantiation infinite loop regress 2") {
     // Just check no crashes.
     compilation.getAllDiagnostics();
 }
+
+TEST_CASE("Checker instantiation infinite loop regress 3") {
+    auto tree = SyntaxTree::fromText(R"(
+checker a waty (p_window) else $error(error_msg);
+
+module m5;
+    a aw1(1ss
+endmodule
+
+a aw1(1ss
+)");
+
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+
+    // Just check no crashes.
+    compilation.getAllDiagnostics();
+}