Prevent internal error for coverpoints that references a covergroup formal parameter

Signed-off-by: Matthew Ballance <matt.ballance@gmail.com>

src/V3Covergroup.cpp

@@ -1692,21 +1692,23 @@ class FunctionalCoverageVisitor final : public VNVisitor {
     }
 
     // VISITORS
-    AstNode* findEnclosingMemberRef(AstClass* cgClassp) {
+    AstNode* findUnsupportedCoverpointRef(AstClass* cgClassp) {
         // An embedded covergroup is lowered into a sibling AstClass that has no handle to
-        // the enclosing object.  A coverpoint/iff/cross expression that references a
+        // the enclosing object.  Two coverpoint/iff/cross shapes cannot be lowered and would
-        // (non-static) member of the enclosing class therefore emits C++ that accesses the
+        // otherwise emit uncompilable C++ or crash V3Broken; detect them so the caller can
-        // member as if it were static ("invalid use of non-static data member").  Detect
+        // skip lowering with a clean COVERIGN warning.  Returns the first offending node
-        // such references so the caller can skip lowering with a clean warning instead of
+        // (or nullptr), preferring the enclosing-member case for message clarity.
-        // producing uncompilable code.  Returns the first offending node, or nullptr.
         // Collect the covergroup class's own member variables (sample/constructor args);
         // references to those are legitimate.
         std::set<const AstVar*> ownVars;
         for (AstNode* itemp = cgClassp->membersp(); itemp; itemp = itemp->nextp()) {
             if (const AstVar* const varp = VN_CAST(itemp, Var)) ownVars.insert(varp);
         }
+        // Pass 1: a (non-static) member of the enclosing class reached with no handle.  The
+        // member would be emitted as if it were static ("invalid use of non-static data
+        // member").
         AstNode* offenderp = nullptr;
-        const auto scan = [&](AstNode* rootp) {
+        const auto scanEnclosing = [&](AstNode* rootp) {
             rootp->foreach([&](AstVarRef* refp) {
                 if (offenderp) return;
                 const AstVar* const varp = refp->varp();  // Always set post-LinkDot
@@ -1716,8 +1718,26 @@ class FunctionalCoverageVisitor final : public VNVisitor {
                 if (varp->isClassMember() && !ownVars.count(varp)) offenderp = refp;
             });
         };
-        for (AstCoverpoint* cpp : m_coverpoints) scan(cpp);
+        for (AstCoverpoint* cpp : m_coverpoints) scanEnclosing(cpp);
-        for (AstCoverCross* crossp : m_coverCrosses) scan(crossp);
+        for (AstCoverCross* crossp : m_coverCrosses) scanEnclosing(crossp);
+        if (offenderp) return offenderp;
+        // Pass 2: the coverpoint expression dereferences a class handle to reach a member,
+        // e.g. a parameterized covergroup 'covergroup cg(cls st); coverpoint st.test;'.  The
+        // lowered covergroup class cannot resolve the handle argument, so lowering leaves a
+        // dangling VarRef and aborts in V3Broken ("Broken link ... VARREF").  AstMemberSel is
+        // class-handle member access only (packed-struct selects use AstStructSel and options
+        // use AstCoverOption), so scanning the value/iff expression will not flag supported
+        // coverpoints.
+        const auto scanHandleDeref = [&](AstNode* rootp) {
+            if (!rootp) return;
+            rootp->foreach([&](AstMemberSel* selp) {
+                if (!offenderp) offenderp = selp;
+            });
+        };
+        for (AstCoverpoint* cpp : m_coverpoints) {
+            scanHandleDeref(cpp->exprp());
+            scanHandleDeref(cpp->iffp());
+        }
         return offenderp;
     }
 
@@ -1802,15 +1822,20 @@ class FunctionalCoverageVisitor final : public VNVisitor {
             iterateChildren(nodep);
 
             // Option B safety net for embedded covergroups: if a coverpoint/iff/cross
-            // references a member of the enclosing class, lowering would emit uncompilable
+            // references a member of the enclosing class (no handle to the enclosing
-            // C++ (no handle to the enclosing instance).  Skip this covergroup with a clean
+            // instance), or dereferences a class-handle argument of a parameterized
-            // warning rather than crashing the C++ compile.  (Full support - an enclosing
+            // covergroup, lowering would emit uncompilable C++ or crash V3Broken with a
-            // back-pointer - is the planned follow-up.)
+            // dangling VarRef.  Skip this covergroup with a clean warning rather than
-            if (AstNode* const offenderp = findEnclosingMemberRef(nodep)) {
+            // failing.  (Full support - an enclosing back-pointer and handle-argument
+            // lowering - is the planned follow-up.)
+            if (AstNode* const offenderp = findUnsupportedCoverpointRef(nodep)) {
+                const bool viaHandle = VN_IS(offenderp, MemberSel);
                 offenderp->v3warn(COVERIGN,
-                                  "Unsupported: 'covergroup' coverpoint referencing enclosing "
+                                  "Unsupported: 'covergroup' coverpoint "
-                                  "class member; ignoring covergroup "
+                                      << (viaHandle ? "dereferencing a class handle member "
-                                      << nodep->prettyNameQ());
+                                                      "(parameterized covergroup)"
+                                                    : "referencing enclosing class member")
+                                      << "; ignoring covergroup " << nodep->prettyNameQ());
                 for (AstCoverpoint* cpp : m_coverpoints) {
                     VL_DO_DANGLING(pushDeletep(cpp->unlinkFrBack()), cpp);
                 }

test_regress/t/t_covergroup_embedded_unsup.out

@@ -1,7 +1,11 @@
-%Warning-COVERIGN: t/t_covergroup_embedded_unsup.v:27:34: Unsupported: 'covergroup' coverpoint referencing enclosing class member; ignoring covergroup '__vlAnonCG_cov_trans'
+%Warning-COVERIGN: t/t_covergroup_embedded_unsup.v:31:34: Unsupported: 'covergroup' coverpoint referencing enclosing class member; ignoring covergroup '__vlAnonCG_cov_trans'
                                                         : ... note: In instance 't'
-   27 |     trans_start_addr: coverpoint trans_collected.addr {option.auto_bin_max = 16;}
+   31 |     trans_start_addr: coverpoint trans_collected.addr {option.auto_bin_max = 16;}
       |                                  ^~~~~~~~~~~~~~~
                    ... For warning description see https://verilator.org/warn/COVERIGN?v=latest
                    ... Use "/* verilator lint_off COVERIGN */" and lint_on around source to disable this message.
+%Warning-COVERIGN: t/t_covergroup_embedded_unsup.v:54:23: Unsupported: 'covergroup' coverpoint dereferencing a class handle member (parameterized covergroup); ignoring covergroup '__vlAnonCG_cov_param'
+                                                        : ... note: In instance 't'
+   54 |     cp: coverpoint st.test;
+      |                       ^~~~
 %Error: Exiting due to

test_regress/t/t_covergroup_embedded_unsup.v

@@ -5,13 +5,17 @@
 // SPDX-FileCopyrightText: 2026 Wilson Snyder
 // SPDX-License-Identifier: CC0-1.0
 
-// Test the graceful-degradation safety net for embedded covergroups (the dominant
+// Test the graceful-degradation safety net for covergroups that cannot yet be lowered.
-// UVM pattern: a covergroup declared inside a class whose coverpoints reference the
+// Two unsupported shapes must degrade to a clean COVERIGN warning (rather than emitting
-// enclosing object's members).  Such a covergroup is lowered into a sibling class
+// uncompilable C++ or crashing) until full support exists:
-// with no handle to the enclosing instance, so emitting it would produce
+//   1. Embedded covergroup (the dominant UVM pattern): coverpoints reference members of
-// uncompilable C++ ("invalid use of non-static data member").  Until the enclosing
+//      the enclosing object.  The covergroup is lowered into a sibling class with no
-// back-pointer feature exists, Verilator must emit a clean COVERIGN warning and skip
+//      handle to the enclosing instance, so emitting it would produce uncompilable C++
-// lowering the covergroup, rather than crashing the C++ compile.
+//      ("invalid use of non-static data member").
+//   2. Parameterized covergroup: a coverpoint dereferences a class-handle argument
+//      ('coverpoint st.test').  The lowered class cannot resolve the handle argument, so
+//      lowering leaves a dangling VarRef and aborts in V3Broken ("Broken link ... VARREF").
+// See https://github.com/verilator/verilator/issues/7853
 
 class ubus_transfer;
   bit [15:0] addr;
@@ -35,10 +39,34 @@ class ubus_master_monitor;
   endfunction
 endclass
 
+class coverage_state;
+  bit [3:0] test;
+  bit [3:0] test2;
+endclass
+
+class parameterized_monitor;
+  coverage_state cs;
+
+  // Parameterized covergroup: the coverpoints dereference the class-handle argument 'st'.
+  // Two handle-dereferencing coverpoints ensure the safety net reports only the first
+  // offender (a second AstMemberSel is seen with the offender already latched).
+  covergroup cov_param(coverage_state st);
+    cp: coverpoint st.test;
+    cp2: coverpoint st.test2;
+  endgroup
+
+  function new();
+    cs = new;
+    cov_param = new(cs);
+  endfunction
+endclass
+
 module t;
   ubus_master_monitor m;
+  parameterized_monitor p;
   initial begin
     m = new;
+    p = new;
     $write("*-* All Finished *-*\n");
     $finish;
   end