Fix infinite recursion due to recursive functions/tasks

Signed-off-by: Krzysztof Bieganski <kbieganski@antmicro.com>

Author: kbieganski

src/V3AstNodes.cpp

@@ -76,7 +76,12 @@ bool AstNodeFTaskRef::isPure() {
         // cached.
         return false;
     } else {
-        if (!m_purity.isCached()) m_purity.set(this->getPurityRecurse());
+        if (!m_purity.isCached()) {
+            m_purity.set(true);  // To prevent infinite recursion, set to true before getting
+                                 // the actual purity. If there are impure statements in the
+                                 // task/function, they'll taint this call anyway.
+            m_purity.set(this->getPurityRecurse());
+        }
         return m_purity.get();
     }
 }

src/V3Hasher.cpp

@@ -33,9 +33,25 @@ class HasherVisitor final : public VNVisitorConst {
     // STATE
     V3Hash m_hash;  // Hash value accumulator
     const bool m_cacheInUser4;  // Use user4 to cache each V3Hash?
+    std::set<AstNode*> m_visited;  // Keeps track of some visited nodes to prevent
+                                   // infinite recursion
 
     // METHODS
 
+    void guardRecursion(AstNode* const nodep, std::function<void()>&& f) {
+        // Guard against infinite recursion if there's no caching
+        // Otherwise caching does the same but faster
+        if (!m_cacheInUser4) {
+            bool isOnStack = false;
+            if (m_visited.find(nodep) != m_visited.end()) {
+                m_hash += V3Hash{nodep->name()};
+                return;
+            }
+            m_visited.insert(nodep);
+        }
+        f();
+    }
+
     V3Hash hashNodeAndIterate(AstNode* nodep, bool hashDType, bool hashChildren,
                               std::function<void()>&& f) {
         // See comments in visit(AstCFunc) about this breaking recursion
@@ -410,14 +426,16 @@ class HasherVisitor final : public VNVisitorConst {
         });
     }
     void visit(AstCFunc* nodep) override {
-        m_hash += hashNodeAndIterate(nodep, HASH_DTYPE, HASH_CHILDREN, [this, nodep]() {  //
-            // We might be in a recursive function, if so on *second* call
-            // here we need to break what would be an infinite loop.
-            nodep->user4(V3Hash{1}.value());  // Set this "first" call
-            // So that a second call will then exit hashNodeAndIterate
-            // Having a constant in the hash just means the recursion will
-            // end, it shouldn't change the CFunc having a unique hash itself.
-            m_hash += nodep->isLoose();
+        guardRecursion(nodep, [this, nodep]() {  //
+            m_hash += hashNodeAndIterate(nodep, HASH_DTYPE, HASH_CHILDREN, [this, nodep]() {  //
+                // We might be in a recursive function, if so on *second* call
+                // here we need to break what would be an infinite loop.
+                if (m_cacheInUser4) nodep->user4(V3Hash{1}.value());  // Set this "first" call
+                // So that a second call will then exit hashNodeAndIterate
+                // Having a constant in the hash just means the recursion will
+                // end, it shouldn't change the CFunc having a unique hash itself.
+                m_hash += nodep->isLoose();
+            });
         });
     }
     void visit(AstVar* nodep) override {
@@ -469,8 +487,12 @@ class HasherVisitor final : public VNVisitorConst {
                                      [this, nodep]() { m_hash += nodep->name(); });
     }
     void visit(AstNodeFTask* nodep) override {
-        m_hash += hashNodeAndIterate(nodep, HASH_DTYPE, HASH_CHILDREN, [this, nodep]() {  //
-            m_hash += nodep->name();
+        guardRecursion(nodep, [this, nodep]() {  //
+            m_hash += hashNodeAndIterate(nodep, HASH_DTYPE, HASH_CHILDREN, [this, nodep]() {  //
+                m_hash += nodep->name();
+                // See comments in AstCFunc
+                if (m_cacheInUser4) nodep->user4(V3Hash{1}.value());
+            });
         });
     }
     void visit(AstModport* nodep) override {

test_regress/t/t_infinite_recursion.pl

@@ -0,0 +1,18 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2023 by Wilson Snyder. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+scenarios(vlt => 1);
+
+lint(
+    verilator_flags2 => ["--no-unlimited-stack -Wno-lint"],
+    );
+
+ok(1);
+1;

test_regress/t/t_infinite_recursion.v

@@ -0,0 +1,18 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2024 by Antmicro.
+// SPDX-License-Identifier: CC0-1.0
+
+class cls;
+   task t; t; endtask
+   task pre_randomize;
+      t;
+   endtask
+endclass
+module t;
+   cls obj;
+   task t;
+      int _ = obj.randomize() with {1 == 1;};
+   endtask
+endmodule