VeniVidiVici
/
verilator
mirror of https://github.com/verilator/verilator.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix checking built-in method arguments (#5839)

This commit is contained in:
Wilson Snyder 2025-03-08 14:11:12 -05:00
parent 24dbb0bc5c
commit 8026b2a7f3
7 changed files with 129 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Changes
View File

@ -33,6 +33,8 @@ Verilator 5.035 devel
* Fix type_id package scope resolution (#5826). [Krzysztof Bieganski, Antmicro Ltd.]
* Fix `rand_mode` method with cast (#5831).
* Fix invalidating variable caches in SenExprBulider (#5834) (#5835). [Geza Lore]
* Fix assignment pattern as function argument (#5839)
* Fix checking built-in method arguments (#5839)
Verilator 5.034 2025-02-24

1
src/V3Ast.h
View File

@ -2359,6 +2359,7 @@ public:
AstNodeDType* findBitDType() const { return findBasicDType(VBasicDTypeKwd::LOGIC); }
AstNodeDType* findDoubleDType() const { return findBasicDType(VBasicDTypeKwd::DOUBLE); }
AstNodeDType* findStringDType() const { return findBasicDType(VBasicDTypeKwd::STRING); }
AstNodeDType* findSigned8DType() const { return findBasicDType(VBasicDTypeKwd::BYTE); }
AstNodeDType* findSigned32DType() const { return findBasicDType(VBasicDTypeKwd::INTEGER); }
AstNodeDType* findUInt32DType() const { return findBasicDType(VBasicDTypeKwd::UINT32); }
AstNodeDType* findUInt64DType() const { return findBasicDType(VBasicDTypeKwd::UINT64); }

1
src/V3Randomize.cpp
View File

@ -392,6 +392,7 @@ class RandomizeMarkVisitor final : public VNVisitor {
exprp = nullptr;
}
if (randVarp == fromVarp) break;
UASSERT_OBJ(randVarp, nodep, "No rand variable found");
AstNode* backp = randVarp;
while (backp && !VN_IS(backp, Class)) backp = backp->backp();
RandomizeMode randMode = {};

73
src/V3Width.cpp
View File

@ -392,7 +392,7 @@ class WidthVisitor final : public VNVisitor {
// See similar handling in visit_cmp_eq_gt where created
iterateCheckString(nodep, "LHS", nodep->lhsp(), BOTH);
iterateCheckSigned32(nodep, "RHS", nodep->rhsp(), BOTH);
iterateCheckSigned32(nodep, "THS", nodep->thsp(), BOTH);
iterateCheckSigned8(nodep, "THS", nodep->thsp(), BOTH);
nodep->dtypeSetString(); // AstPutcN returns the new string to be assigned by
// AstAssign
}
@ -3153,14 +3153,8 @@ class WidthVisitor final : public VNVisitor {
if (debug() >= 9) nodep->dumpTree("- mts-in: ");
// Should check types the method requires, but at present we don't do much
userIterate(nodep->fromp(), WidthVP{SELF, BOTH}.p());
// Args are checked within each particular method's decode
// Any AstWith is checked later when know types, in methodWithArgument
for (AstNode* pinp = nodep->pinsp(); pinp; pinp = pinp->nextp()) {
if (AstArg* const argp = VN_CAST(pinp, Arg)) {
if (argp->exprp())
userIterate(argp->exprp(),
WidthVP{nodep->fromp()->dtypep()->subDTypep(), BOTH}.p());
}
}
// Find the fromp dtype - should be a class
UASSERT_OBJ(nodep->fromp() && nodep->fromp()->dtypep(), nodep, "Unsized expression");
AstNodeDType* const fromDtp = nodep->fromp()->dtypep()->skipRefToEnump();
@ -3252,6 +3246,13 @@ class WidthVisitor final : public VNVisitor {
}
}
AstNodeExpr* methodArg(AstMethodCall* nodep, int arg) {
AstNode* argp = nodep->pinsp();
for (int narg = 0; narg < arg; ++narg) argp = argp->nextp();
UASSERT_OBJ(argp, nodep, "methodOkArguments() should have detected arg count error");
return VN_AS(argp, Arg)->exprp();
}
void methodCallEnum(AstMethodCall* nodep, AstEnumDType* adtypep) {
// Method call on enum without following parenthesis, e.g. "ENUM.next"
// Convert this into a method call, and let that visitor figure out what to do next
@ -3306,12 +3307,13 @@ class WidthVisitor final : public VNVisitor {
}
if (nodep->name() != "name" && nodep->pinsp()) {
if (!VN_IS(VN_AS(nodep->pinsp(), Arg)->exprp(), Const)) {
nodep->pinsp()->v3fatalSrc(
"Unsupported: enum next/prev with non-const argument");
AstNodeExpr* const stepp = methodArg(nodep, 0);
nodep->fileline()->modifyWarnOff(V3ErrorCode::WIDTHEXPAND, true);
iterateCheckUInt32(nodep, "argument", stepp, BOTH);
if (!VN_IS(stepp, Const)) {
stepp->v3fatalSrc("Unsupported: enum next/prev with non-const argument");
} else {
const uint32_t stepWidth
= VN_AS(VN_AS(nodep->pinsp(), Arg)->exprp(), Const)->toUInt();
const uint32_t stepWidth = VN_AS(stepp, Const)->toUInt();
if (stepWidth == 0) {
// Step of 0 "legalizes" like $cast, use .next.prev
AstMethodCall* const newp = new AstMethodCall{
@ -3319,17 +3321,20 @@ class WidthVisitor final : public VNVisitor {
new AstMethodCall{nodep->fileline(), nodep->fromp()->unlinkFrBack(),
"next", nullptr},
"prev", nullptr};
// No dtype assigned, we will recurse the new method and replace
nodep->replaceWith(newp);
VL_DO_DANGLING(nodep->deleteTree(), nodep);
return;
} else if (stepWidth != 1) {
// Unroll of enumVar.next(k) to enumVar.next(1).next(k - 1)
nodep->pinsp()->unlinkFrBack();
AstMethodCall* const clonep = nodep->cloneTree(false);
VN_AS(VN_AS(clonep->pinsp(), Arg)->exprp(), Const)->num().setLong(1);
VN_AS(stepp, Const)->num().setLong(1);
AstConst* const constp = new AstConst(nodep->fileline(), stepWidth - 1);
AstArg* const argp = new AstArg{nodep->fileline(), "", constp};
AstMethodCall* const newp
= new AstMethodCall{nodep->fileline(), clonep, nodep->name(), argp};
// No dtype assigned, we will recurse the new method and replace
nodep->replaceWith(newp);
VL_DO_DANGLING(nodep->deleteTree(), nodep);
return;
@ -3364,6 +3369,7 @@ class WidthVisitor final : public VNVisitor {
} else if (nodep->name() == "exists") { // function int exists(input index)
// IEEE really should have made this a "bit" return
methodOkArguments(nodep, 1, 1);
iterateCheckSizedSelf(nodep, "argument", methodArg(nodep, 0), SELF, BOTH);
AstNodeExpr* const index_exprp = methodCallWildcardIndexExpr(nodep, adtypep);
newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(), "exists",
index_exprp->unlinkFrBack()};
@ -3376,6 +3382,7 @@ class WidthVisitor final : public VNVisitor {
"clear"};
newp->dtypeSetVoid();
} else {
iterateCheckSizedSelf(nodep, "argument", methodArg(nodep, 0), SELF, BOTH);
AstNodeExpr* const index_exprp = methodCallWildcardIndexExpr(nodep, adtypep);
newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(),
"erase", index_exprp->unlinkFrBack()};
@ -3443,6 +3450,7 @@ class WidthVisitor final : public VNVisitor {
|| nodep->name() == "next" //
|| nodep->name() == "prev") {
methodOkArguments(nodep, 1, 1);
iterateCheckTyped(nodep, "argument", methodArg(nodep, 0), adtypep->keyDTypep(), BOTH);
AstNodeExpr* const index_exprp = methodCallAssocIndexExpr(nodep, adtypep);
methodCallLValueRecurse(nodep, index_exprp, VAccess::READWRITE);
newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(),
@ -3453,6 +3461,7 @@ class WidthVisitor final : public VNVisitor {
} else if (nodep->name() == "exists") { // function int exists(input index)
// IEEE really should have made this a "bit" return
methodOkArguments(nodep, 1, 1);
iterateCheckTyped(nodep, "argument", methodArg(nodep, 0), adtypep->keyDTypep(), BOTH);
AstNodeExpr* const index_exprp = methodCallAssocIndexExpr(nodep, adtypep);
newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(), "exists",
index_exprp->unlinkFrBack()};
@ -3465,6 +3474,8 @@ class WidthVisitor final : public VNVisitor {
"clear"};
newp->dtypeSetVoid();
} else {
iterateCheckTyped(nodep, "argument", methodArg(nodep, 0), adtypep->keyDTypep(),
BOTH);
AstNodeExpr* const index_exprp = methodCallAssocIndexExpr(nodep, adtypep);
newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(),
"erase", index_exprp->unlinkFrBack()};
@ -3643,10 +3654,12 @@ class WidthVisitor final : public VNVisitor {
AstCMethodHard* newp = nullptr;
if (nodep->name() == "at") { // Created internally for []
methodOkArguments(nodep, 1, 1);
iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH);
newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(), "at"};
newp->dtypeFrom(adtypep->subDTypep());
} else if (nodep->name() == "atWrite") { // Created internally for []
methodOkArguments(nodep, 1, 1);
iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH);
methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE);
newp
= new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(), "atWrite"};
@ -3689,12 +3702,14 @@ class WidthVisitor final : public VNVisitor {
AstCMethodHard* newp = nullptr;
if (nodep->name() == "at" || nodep->name() == "atBack") { // Created internally for []
methodOkArguments(nodep, 1, 1);
iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH);
newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(),
nodep->name()};
newp->dtypeFrom(adtypep->subDTypep());
} else if (nodep->name() == "atWriteAppend"
|| nodep->name() == "atWriteAppendBack") { // Created internally for []
methodOkArguments(nodep, 1, 1);
iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH);
methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE);
newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(),
nodep->name()};
@ -3712,6 +3727,7 @@ class WidthVisitor final : public VNVisitor {
"clear"};
newp->dtypeSetVoid();
} else {
iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH);
AstNodeExpr* const index_exprp = methodCallQueueIndexExpr(nodep);
if (index_exprp->isZero()) { // delete(0) is a pop_front
newp = new AstCMethodHard{nodep->fileline(), nodep->fromp()->unlinkFrBack(),
@ -3726,6 +3742,7 @@ class WidthVisitor final : public VNVisitor {
}
} else if (nodep->name() == "insert") {
methodOkArguments(nodep, 2, 2);
iterateCheckSigned32(nodep, "index", methodArg(nodep, 0), BOTH);
methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE);
AstNodeExpr* const index_exprp = methodCallQueueIndexExpr(nodep);
AstArg* const argp = VN_AS(nodep->pinsp()->nextp(), Arg);
@ -3750,6 +3767,7 @@ class WidthVisitor final : public VNVisitor {
if (nodep->isStandaloneBodyStmt()) newp->dtypeSetVoid();
} else if (nodep->name() == "push_back" || nodep->name() == "push_front") {
methodOkArguments(nodep, 1, 1);
iterateCheckTyped(nodep, "argument", methodArg(nodep, 0), adtypep->subDTypep(), BOTH);
methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE);
AstArg* const argp = VN_AS(nodep->pinsp(), Arg);
iterateCheckTyped(nodep, "push value", argp->exprp(), adtypep->subDTypep(), BOTH);
@ -3910,11 +3928,17 @@ class WidthVisitor final : public VNVisitor {
m_randomizeFromp = nodep->fromp();
withp = methodWithArgument(nodep, false, false, adtypep->findVoidDType(),
adtypep->findBitDType(), adtypep);
for (AstNode* pinp = nodep->pinsp(); pinp; pinp = pinp->nextp()) {
if (AstArg* const argp = VN_CAST(pinp, Arg)) {
if (argp->exprp()) userIterate(argp->exprp(), WidthVP{SELF, BOTH}.p());
}
}
methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE);
V3Randomize::newRandomizeFunc(m_memberMap, first_classp);
handleRandomizeArgs(nodep, first_classp);
} else if (nodep->name() == "srandom") {
methodOkArguments(nodep, 1, 1);
iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH);
methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE);
V3Randomize::newSRandomFunc(m_memberMap, first_classp);
}
@ -4010,6 +4034,7 @@ class WidthVisitor final : public VNVisitor {
// IEEE 1800-2023 18.9
methodOkArguments(nodep, 0, 1);
if (nodep->pinsp()) {
iterateCheckBool(nodep, "argument", methodArg(nodep, 0), BOTH);
nodep->dtypep(nodep->findBasicDType(VBasicDTypeKwd::INT));
} else {
nodep->dtypeSetVoid();
@ -4025,6 +4050,7 @@ class WidthVisitor final : public VNVisitor {
methodOkArguments(nodep, 0, 1);
// IEEE 1800-2023 18.8
if (nodep->pinsp()) {
iterateCheckBool(nodep, "argument", methodArg(nodep, 0), BOTH);
nodep->dtypeSetVoid();
} else {
nodep->dtypep(nodep->findBasicDType(VBasicDTypeKwd::INT));
@ -4109,18 +4135,23 @@ class WidthVisitor final : public VNVisitor {
VL_DO_DANGLING(pushDeletep(nodep), nodep);
} else if (nodep->name() == "itoa") {
methodOkArguments(nodep, 1, 1);
iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH);
VL_DO_DANGLING(replaceWithSFormat(nodep, "%0d"), nodep);
} else if (nodep->name() == "hextoa") {
methodOkArguments(nodep, 1, 1);
iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH);
VL_DO_DANGLING(replaceWithSFormat(nodep, "%0x"), nodep);
} else if (nodep->name() == "octtoa") {
methodOkArguments(nodep, 1, 1);
iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH);
VL_DO_DANGLING(replaceWithSFormat(nodep, "%0o"), nodep);
} else if (nodep->name() == "bintoa") {
methodOkArguments(nodep, 1, 1);
iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH);
VL_DO_DANGLING(replaceWithSFormat(nodep, "%0b"), nodep);
} else if (nodep->name() == "realtoa") {
methodOkArguments(nodep, 1, 1);
iterateCheckReal(nodep, "argument", methodArg(nodep, 0), BOTH);
VL_DO_DANGLING(replaceWithSFormat(nodep, "%g"), nodep);
} else if (nodep->name() == "tolower") {
methodOkArguments(nodep, 0, 0);
@ -4137,6 +4168,7 @@ class WidthVisitor final : public VNVisitor {
} else if (nodep->name() == "compare" || nodep->name() == "icompare") {
const bool ignoreCase = nodep->name()[0] == 'i';
methodOkArguments(nodep, 1, 1);
iterateCheckString(nodep, "argument", methodArg(nodep, 0), BOTH);
AstArg* const argp = VN_AS(nodep->pinsp(), Arg);
AstNodeExpr* const lhs = nodep->fromp()->unlinkFrBack();
AstNodeExpr* const rhs = argp->exprp()->unlinkFrBack();
@ -4145,6 +4177,8 @@ class WidthVisitor final : public VNVisitor {
VL_DO_DANGLING(nodep->deleteTree(), nodep);
} else if (nodep->name() == "putc") {
methodOkArguments(nodep, 2, 2);
iterateCheckSigned32(nodep, "argument 0", methodArg(nodep, 0), BOTH);
iterateCheckSigned8(nodep, "argument 1", methodArg(nodep, 1), BOTH);
AstArg* const arg0p = VN_AS(nodep->pinsp(), Arg);
AstArg* const arg1p = VN_AS(arg0p->nextp(), Arg);
AstNodeVarRef* const fromp = VN_AS(nodep->fromp()->unlinkFrBack(), VarRef);
@ -4159,6 +4193,7 @@ class WidthVisitor final : public VNVisitor {
VL_DO_DANGLING(nodep->backp()->replaceWith(newp), nodep);
} else if (nodep->name() == "getc") {
methodOkArguments(nodep, 1, 1);
iterateCheckSigned32(nodep, "argument", methodArg(nodep, 0), BOTH);
AstArg* const arg0p = VN_AS(nodep->pinsp(), Arg);
AstNodeExpr* const lhsp = nodep->fromp()->unlinkFrBack();
AstNodeExpr* const rhsp = arg0p->exprp()->unlinkFrBack();
@ -4167,6 +4202,8 @@ class WidthVisitor final : public VNVisitor {
VL_DO_DANGLING(nodep->deleteTree(), nodep);
} else if (nodep->name() == "substr") {
methodOkArguments(nodep, 2, 2);
iterateCheckSigned32(nodep, "argument 0", methodArg(nodep, 0), BOTH);
iterateCheckSigned32(nodep, "argument 1", methodArg(nodep, 1), BOTH);
AstArg* const arg0p = VN_AS(nodep->pinsp(), Arg);
AstArg* const arg1p = VN_AS(arg0p->nextp(), Arg);
AstNodeExpr* const lhsp = nodep->fromp()->unlinkFrBack();
@ -6942,10 +6979,18 @@ class WidthVisitor final : public VNVisitor {
// otherwise self-determined was correct
iterateCheckTypedSelfPrelim(nodep, side, underp, nodep->findDoubleDType(), stage);
}
void iterateCheckSigned8(AstNode* nodep, const char* side, AstNode* underp, Stage stage) {
// Coerce child to signed8 if not already. Child is self-determined
iterateCheckTypedSelfPrelim(nodep, side, underp, nodep->findSigned8DType(), stage);
}
void iterateCheckSigned32(AstNode* nodep, const char* side, AstNode* underp, Stage stage) {
// Coerce child to signed32 if not already. Child is self-determined
iterateCheckTypedSelfPrelim(nodep, side, underp, nodep->findSigned32DType(), stage);
}
void iterateCheckUInt32(AstNode* nodep, const char* side, AstNode* underp, Stage stage) {
// Coerce child to unsigned32 if not already. Child is self-determined
iterateCheckTypedSelfPrelim(nodep, side, underp, nodep->findUInt32DType(), stage);
}
void iterateCheckDelay(AstNode* nodep, const char* side, AstNode* underp, Stage stage) {
// Coerce child to 64-bit delay if not already. Child is self-determined
// underp may change as a result of replacement

8
test_regress/t/t_process_notiming.out
View File

@ -43,12 +43,12 @@
: ... note: In instance 't'
38 | p.srandom(0);
| ^~~~~~~
%Error-NOTIMING: t/t_process.v:39:25: process::get_randstate() requires --timing
: ... note: In instance 't'
39 | p.set_randstate(p.get_randstate());
| ^~~~~~~~~~~~~
%Error-NOTIMING: t/t_process.v:39:9: process::set_randstate() requires --timing
: ... note: In instance 't'
39 | p.set_randstate(p.get_randstate());
| ^~~~~~~~~~~~~
%Error-NOTIMING: t/t_process.v:39:25: process::get_randstate() requires --timing
: ... note: In instance 't'
39 | p.set_randstate(p.get_randstate());
| ^~~~~~~~~~~~~
%Error: Exiting due to

18
test_regress/t/t_queue_arg.py Executable file
View File

@ -0,0 +1,18 @@
#!/usr/bin/env python3
# DESCRIPTION: Verilator: Verilog Test driver/expect definition
#
# Copyright 2025 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
import vltest_bootstrap
test.scenarios('simulator')
test.compile()
test.execute()
test.passes()

44
test_regress/t/t_queue_arg.v Normal file
View File

@ -0,0 +1,44 @@
// DESCRIPTION: Verilator: Verilog Test module
//
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty, 2025 by Wilson Snyder.
// SPDX-License-Identifier: CC0-1.0
`define stop $stop
`define checks(gotv,expv) do if ((gotv) != (expv)) begin $write("%%Error: %s:%0d: got='%s' exp='%s'\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0);
typedef struct {
string name1;
string name2;
} names_t;
class uvm_queue;
names_t m_queue[$];
virtual function void push_back(names_t item);
m_queue.push_back(item);
endfunction
endclass
module t(/*AUTOARG*/);
uvm_queue q;
initial begin
q = new;
// From uvm_queue#(uvm_acs_name_struct) __local_field_names__;
q.push_back('{"n1", "n2"});
q.push_back('{"m1", "m2"});
q.m_queue.push_back('{"o1", "o2"});
$display("%p", q);
`checks(q.m_queue[0].name1, "n1");
`checks(q.m_queue[0].name2, "n2");
`checks(q.m_queue[1].name1, "m1");
`checks(q.m_queue[1].name2, "m2");
`checks(q.m_queue[2].name1, "o1");
`checks(q.m_queue[2].name2, "o2");
$write("*-* All Finished *-*\n");
$finish;
end
endmodule