Make class ref typing stricter (#3671)

Prevents the possibility of assigning an integer to a class reference,
both at the SystemVerilog and the emitted C++ levels.

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

include/verilated_types.h

@@ -1070,6 +1070,13 @@ public:
     virtual ~VlClass() {}
 };
 
+//===================================================================
+// Represents the null pointer. Used for setting VlClassRef to null instead of
+// via nullptr_t, to prevent the implicit conversion of 0 to nullptr.
+struct VlNull {
+    operator bool() const { return false; }
+};
+
 //===================================================================
 // Verilog class reference container
 // There are no multithreaded locks on this; the base variable must
@@ -1098,14 +1105,16 @@ private:
 public:
     // CONSTRUCTORS
     VlClassRef() = default;
+    // Init with nullptr
+    VlClassRef(VlNull){};
     template <typename... T_Args>
     VlClassRef(VlDeleter& deleter, T_Args&&... args)
         : m_objp{new T_Class{std::forward<T_Args>(args)...}} {
         m_objp->m_deleter = &deleter;
         refCountInc();
     }
-    // cppcheck-suppress noExplicitConstructor
-    VlClassRef(T_Class* objp)
+    // Explicit to avoid implicit conversion from 0
+    explicit VlClassRef(T_Class* objp)
         : m_objp{objp} {
         refCountInc();
     }
@@ -1145,10 +1154,16 @@ public:
         m_objp = vlstd::exchange(moved.m_objp, nullptr);
         return *this;
     }
+    // Assign with nullptr
+    VlClassRef& operator=(VlNull) {
+        refCountDec();
+        m_objp = nullptr;
+        return *this;
+    }
     // Dynamic caster
     template <typename T_OtherClass>
     VlClassRef<T_OtherClass> dynamicCast() const {
-        return dynamic_cast<T_OtherClass*>(m_objp);
+        return VlClassRef<T_OtherClass>{dynamic_cast<T_OtherClass*>(m_objp)};
     }
     // Dereference operators
     T_Class& operator*() const { return *m_objp; }

src/V3EmitCConstInit.h

@@ -101,7 +101,9 @@ protected:
         const V3Number& num = nodep->num();
         UASSERT_OBJ(!num.isFourState(), nodep, "4-state value in constant pool");
         const AstNodeDType* const dtypep = nodep->dtypep();
-        if (num.isString()) {
+        if (num.isNull()) {
+            puts("VlNull{}");
+        } else if (num.isString()) {
             // Note: putsQuoted does not track indentation, so we use this instead
             puts("\"");
             puts(num.toString());

src/V3EmitCFunc.cpp

@@ -483,7 +483,9 @@ void EmitCFunc::emitCvtWideArray(AstNode* nodep, AstNode* fromp) {
 
 void EmitCFunc::emitConstant(AstConst* nodep, AstVarRef* assigntop, const string& assignString) {
     // Put out constant set to the specified variable, or given variable in a string
-    if (nodep->num().isFourState()) {
+    if (nodep->num().isNull()) {
+        puts("VlNull{}");
+    } else if (nodep->num().isFourState()) {
         nodep->v3warn(E_UNSUPPORTED, "Unsupported: 4-state numbers in this context");
     } else if (nodep->num().isString()) {
         putbs("std::string{");

src/V3Number.cpp

@@ -506,7 +506,6 @@ string V3Number::ascii(bool prefixed, bool cleanVerilog) const {
             out << "'";
             if (bitIs0(0)) {
                 out << '0';
-                if (isNull()) out << "[null]";
             } else if (bitIs1(0)) {
                 out << '1';
             } else if (bitIsZ(0)) {
@@ -542,7 +541,13 @@ string V3Number::ascii(bool prefixed, bool cleanVerilog) const {
         // Always deal with 4 bits at once.  Note no 4-state, it's above.
         out << displayed("%0h");
     }
-    if (isNull() && VL_UNCOVERABLE(!isEqZero())) out << "-%E-null-not-zero";
+    if (isNull()) {
+        if (VL_UNCOVERABLE(!isEqZero())) {
+            out << "-%E-null-not-zero";
+        } else {
+            out << " [null]";
+        }
+    }
     return out.str();
 }
 
@@ -2169,7 +2174,9 @@ V3Number& V3Number::opAssignNonXZ(const V3Number& lhs, bool ignoreXZ) {
     // to itself; V3Simulate does this when hits "foo=foo;"
     // So no: NUM_ASSERT_OP_ARGS1(lhs);
     if (this != &lhs) {
-        if (isString()) {
+        if (VL_UNLIKELY(lhs.isNull())) {
+            m_data.m_isNull = true;
+        } else if (isString()) {
             if (VL_UNLIKELY(!lhs.isString())) {
                 // Non-compatible types, erase value.
                 m_data.str() = "";

src/V3Width.cpp

@@ -5086,6 +5086,7 @@ private:
                     // (get an ASSIGN with EXTEND on the lhs instead of rhs)
                 }
                 if (!portp->basicp() || portp->basicp()->isOpaque()) {
+                    checkClassAssign(nodep, "Function Argument", pinp, portp->dtypep());
                     userIterate(pinp, WidthVP(portp->dtypep(), FINAL).p());
                 } else {
                     iterateCheckAssign(nodep, "Function Argument", pinp, FINAL, portp->dtypep());
@@ -5803,6 +5804,15 @@ private:
         return false;  // No change
     }
 
+    void checkClassAssign(AstNode* nodep, const char* side, AstNode* rhsp,
+                          AstNodeDType* lhsDTypep) {
+        if (VN_IS(lhsDTypep, ClassRefDType) && !VN_IS(rhsp->dtypep(), ClassRefDType)) {
+            if (auto* const constp = VN_CAST(rhsp, Const)) {
+                if (constp->num().isNull()) return;
+            }
+            nodep->v3error(side << " expects a " << lhsDTypep->prettyTypeName());
+        }
+    }
     static bool similarDTypeRecurse(AstNodeDType* node1p, AstNodeDType* node2p) {
         return node1p->skipRefp()->similarDType(node2p->skipRefp());
     }
@@ -5885,6 +5895,7 @@ private:
         // if (debug()) nodep->dumpTree(cout, "-checkass: ");
         UASSERT_OBJ(stage == FINAL, nodep, "Bad width call");
         // We iterate and size the RHS based on the result of RHS evaluation
+        checkClassAssign(nodep, side, rhsp, lhsDTypep);
         const bool lhsStream
             = (VN_IS(nodep, NodeAssign) && VN_IS(VN_AS(nodep, NodeAssign)->lhsp(), NodeStream));
         rhsp = iterateCheck(nodep, side, rhsp, ASSIGN, FINAL, lhsDTypep,

test_regress/t/t_class_assign_bad.out

@@ -0,0 +1,17 @@
+%Error: t/t_class_assign_bad.v:16:9: Assign RHS expects a CLASSREFDTYPE 'Cls'
+                                   : ... In instance t
+   16 |       c = 0;
+      |         ^
+%Error: t/t_class_assign_bad.v:17:9: Assign RHS expects a CLASSREFDTYPE 'Cls'
+                                   : ... In instance t
+   17 |       c = 1;
+      |         ^
+%Error: t/t_class_assign_bad.v:18:7: Function Argument expects a CLASSREFDTYPE 'Cls'
+                                   : ... In instance t
+   18 |       t(0);
+      |       ^
+%Error: t/t_class_assign_bad.v:19:7: Function Argument expects a CLASSREFDTYPE 'Cls'
+                                   : ... In instance t
+   19 |       t(1);
+      |       ^
+%Error: Exiting due to

test_regress/t/t_class_assign_bad.pl

@@ -0,0 +1,19 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2022 by Antmicro Ltd. 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(simulator => 1);
+
+compile(
+    fails => 1,
+    expect_filename => $Self->{golden_filename},
+    );
+
+ok(1);
+1;

test_regress/t/t_class_assign_bad.v

@@ -0,0 +1,21 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2022 by Antmicro Ltd.
+// SPDX-License-Identifier: CC0-1.0
+
+class Cls;
+endclass : Cls
+
+module t (/*AUTOARG*/);
+   Cls c;
+
+   task t(Cls c); endtask
+
+   initial begin
+      c = 0;
+      c = 1;
+      t(0);
+      t(1);
+   end
+endmodule