Skip to content

Commit 1167b66

Browse files
committed
Fix infinite recursion due to recursive functions/tasks
Signed-off-by: Krzysztof Bieganski <kbieganski@antmicro.com>
1 parent 4877627 commit 1167b66

File tree

4 files changed

+78
-11
lines changed

4 files changed

+78
-11
lines changed

src/V3AstNodes.cpp

+6-1
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,12 @@ bool AstNodeFTaskRef::isPure() {
7676
// cached.
7777
return false;
7878
} else {
79-
if (!m_purity.isCached()) m_purity.set(this->getPurityRecurse());
79+
if (!m_purity.isCached()) {
80+
m_purity.set(true); // To prevent infinite recursion, set to true before getting
81+
// the actual purity. If there are impure statements in the
82+
// task/function, they'll taint this call anyway.
83+
m_purity.set(this->getPurityRecurse());
84+
}
8085
return m_purity.get();
8186
}
8287
}

src/V3Hasher.cpp

+36-10
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,29 @@ class HasherVisitor final : public VNVisitorConst {
3333
// STATE
3434
V3Hash m_hash; // Hash value accumulator
3535
const bool m_cacheInUser4; // Use user4 to cache each V3Hash?
36+
std::vector<AstNode*> m_stack; // Keeps track of some visited nodes to prevent
37+
// infinite recursion
3638

3739
// METHODS
3840

41+
void guardRecursion(AstNode* const nodep, std::function<void()>&& f) {
42+
// Guard against infinite recursion if there's no caching
43+
// Otherwise caching does the same but faster
44+
if (!m_cacheInUser4) {
45+
bool isOnStack = false;
46+
for (AstNode* stackNodep : m_stack) {
47+
if (stackNodep == nodep) isOnStack = true;
48+
}
49+
if (isOnStack) {
50+
m_hash += V3Hash{1};
51+
return;
52+
}
53+
m_stack.push_back(nodep);
54+
f();
55+
m_stack.pop_back();
56+
}
57+
}
58+
3959
V3Hash hashNodeAndIterate(AstNode* nodep, bool hashDType, bool hashChildren,
4060
std::function<void()>&& f) {
4161
// See comments in visit(AstCFunc) about this breaking recursion
@@ -410,14 +430,16 @@ class HasherVisitor final : public VNVisitorConst {
410430
});
411431
}
412432
void visit(AstCFunc* nodep) override {
413-
m_hash += hashNodeAndIterate(nodep, HASH_DTYPE, HASH_CHILDREN, [this, nodep]() { //
414-
// We might be in a recursive function, if so on *second* call
415-
// here we need to break what would be an infinite loop.
416-
nodep->user4(V3Hash{1}.value()); // Set this "first" call
417-
// So that a second call will then exit hashNodeAndIterate
418-
// Having a constant in the hash just means the recursion will
419-
// end, it shouldn't change the CFunc having a unique hash itself.
420-
m_hash += nodep->isLoose();
433+
guardRecursion(nodep, [this, nodep]() { //
434+
m_hash += hashNodeAndIterate(nodep, HASH_DTYPE, HASH_CHILDREN, [this, nodep]() { //
435+
// We might be in a recursive function, if so on *second* call
436+
// here we need to break what would be an infinite loop.
437+
if (m_cacheInUser4) nodep->user4(V3Hash{1}.value()); // Set this "first" call
438+
// So that a second call will then exit hashNodeAndIterate
439+
// Having a constant in the hash just means the recursion will
440+
// end, it shouldn't change the CFunc having a unique hash itself.
441+
m_hash += nodep->isLoose();
442+
});
421443
});
422444
}
423445
void visit(AstVar* nodep) override {
@@ -469,8 +491,12 @@ class HasherVisitor final : public VNVisitorConst {
469491
[this, nodep]() { m_hash += nodep->name(); });
470492
}
471493
void visit(AstNodeFTask* nodep) override {
472-
m_hash += hashNodeAndIterate(nodep, HASH_DTYPE, HASH_CHILDREN, [this, nodep]() { //
473-
m_hash += nodep->name();
494+
guardRecursion(nodep, [this, nodep]() { //
495+
m_hash += hashNodeAndIterate(nodep, HASH_DTYPE, HASH_CHILDREN, [this, nodep]() { //
496+
m_hash += nodep->name();
497+
// See comments in AstCFunc
498+
if (m_cacheInUser4) nodep->user4(V3Hash{1}.value());
499+
});
474500
});
475501
}
476502
void visit(AstModport* nodep) override {
+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#!/usr/bin/env perl
2+
if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
3+
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
4+
#
5+
# Copyright 2023 by Wilson Snyder. This program is free software; you
6+
# can redistribute it and/or modify it under the terms of either the GNU
7+
# Lesser General Public License Version 3 or the Perl Artistic License
8+
# Version 2.0.
9+
# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
10+
11+
scenarios(vlt => 1);
12+
13+
lint(
14+
verilator_flags2 => ["--no-unlimited-stack -Wno-lint"],
15+
);
16+
17+
ok(1);
18+
1;

test_regress/t/t_infinite_recursion.v

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// DESCRIPTION: Verilator: Verilog Test module
2+
//
3+
// This file ONLY is placed under the Creative Commons Public Domain, for
4+
// any use, without warranty, 2024 by Antmicro.
5+
// SPDX-License-Identifier: CC0-1.0
6+
7+
class cls;
8+
task t; t; endtask
9+
task pre_randomize;
10+
t;
11+
endtask
12+
endclass
13+
module t;
14+
cls obj;
15+
task t;
16+
int _ = obj.randomize() with {1 == 1;};
17+
endtask
18+
endmodule

0 commit comments

Comments
 (0)