Add assert when VerilatedContext is mis-deleted (#3121).

Changes

@@ -14,6 +14,7 @@ Verilator 4.223 devel
 **Minor:**
 
 * Support compile time trace signal selection with tracing_on/off (#3323). [Shunyao CAD]
+* Add assert when VerilatedContext is mis-deleted (#3121). [Ruptert Swarbrick]
 * Fix hang with large case statement optimization (#3405). [Mike Urbach]
 * Fix 'with' operator with type casting (#3387). [xiak95]
 

docs/guide/connecting.rst

@@ -110,6 +110,9 @@ model.  Here is a simple example:
              Verilated::commandArgs(argc, argv);   // Remember args
 
              top = new Vtop;             // Create model
+             // Do not instead make Vtop as a file-scope static
+             // variable, as the "C++ static initialization order fiasco"
+             // may cause a crash
 
              top->reset_l = 0;           // Set some inputs
 

examples/make_tracing_c/sim_main.cpp

@@ -34,6 +34,8 @@ int main(int argc, char** argv, char** env) {
     // Using unique_ptr is similar to
     // "VerilatedContext* contextp = new VerilatedContext" then deleting at end.
     const std::unique_ptr<VerilatedContext> contextp{new VerilatedContext};
+    // Do not instead make Vtop as a file-scope static variable, as the
+    // "C++ static initialization order fiasco" may cause a crash
 
     // Set debug level, 0 is off, 9 is highest presently used
     // May be overridden by commandArgs argument parsing

include/verilated.cpp

@@ -2289,7 +2289,17 @@ VerilatedContext::VerilatedContext()
 }
 
 // Must declare here not in interface, as otherwise forward declarations not known
-VerilatedContext::~VerilatedContext() {}
+VerilatedContext::~VerilatedContext() {
+    checkMagic(this);
+    m_magic = 0x1;  // Arbitrary but 0x1 is what Verilator src uses for a deleted pointer
+}
+
+void VerilatedContext::checkMagic(const VerilatedContext* contextp) {
+    if (VL_UNLIKELY(!contextp || contextp->m_magic != MAGIC)) {
+        VL_FATAL_MT("", 0, "",  // LCOV_EXCL_LINE
+                    "Attempt to create model using a bad/deleted VerilatedContext pointer");
+    }
+}
 
 VerilatedContext::Serialized::Serialized() {
     m_timeunit = VL_TIME_UNIT;  // Initial value until overriden by _Vconfigure
@@ -2657,6 +2667,7 @@ const VerilatedScopeNameMap* VerilatedContext::scopeNameMap() VL_MT_SAFE {
 
 VerilatedSyms::VerilatedSyms(VerilatedContext* contextp)
     : _vm_contextp__(contextp ? contextp : Verilated::threadContextp()) {
+    VerilatedContext::checkMagic(_vm_contextp__);
     Verilated::threadContextp(_vm_contextp__);
 #ifdef VL_THREADED
     __Vm_evalMsgQp = new VerilatedEvalMsgQueue;
@@ -2664,6 +2675,7 @@ VerilatedSyms::VerilatedSyms(VerilatedContext* contextp)
 }
 
 VerilatedSyms::~VerilatedSyms() {
+    VerilatedContext::checkMagic(_vm_contextp__);
 #ifdef VL_THREADED
     delete __Vm_evalMsgQp;
 #endif

include/verilated.h

@@ -374,6 +374,10 @@ protected:
     // List of free descriptors in the MCT region [4, 32)
     std::vector<IData> m_fdFreeMct VL_GUARDED_BY(m_fdMutex);
 
+    // Magic to check for bad construction
+    static constexpr uint64_t MAGIC = 0xC35F9A6E5298EE6EULL;  // SHA256 "VerilatedContext"
+    uint64_t m_magic = MAGIC;
+
 private:
     // CONSTRUCTORS
     VL_UNCOPYABLE(VerilatedContext);
@@ -535,6 +539,9 @@ public:  // But for internal use only
     // Internal: Serialization setup
     static constexpr size_t serialized1Size() VL_PURE { return sizeof(m_s); }
     void* serialized1Ptr() VL_MT_UNSAFE { return &m_s; }
+
+    // Internal: Check magic number
+    static void checkMagic(const VerilatedContext* contextp);
 };
 
 //===========================================================================

test_regress/driver.pl

@@ -1892,6 +1892,7 @@ sub _make_main {
             $fh->print("        if (save_time && ${time} == save_time) {\n");
             $fh->print("            save_model(\"$self->{obj_dir}/saved.vltsv\");\n");
             $fh->print("            printf(\"Exiting after save_model\\n\");\n");
+            $fh->print("            topp.reset(nullptr);\n");
             $fh->print("            return 0;\n");
             $fh->print("        }\n");
         }

test_regress/t/t_wrapper_del_context_bad.cpp

@@ -0,0 +1,22 @@
+//
+// DESCRIPTION: Verilator: Verilog Multiple Model Test Module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2022 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+#include <verilated.h>
+
+#include VM_PREFIX_INCLUDE
+
+int main(int argc, char** argv, char** env) {
+    // Create contexts
+    VerilatedContext* contextp{new VerilatedContext};
+
+    delete contextp;  // Test mistake - deleting contextp
+
+    // instantiate verilated design
+    std::unique_ptr<VM_PREFIX> topp{new VM_PREFIX{contextp, "TOP"}};
+
+    return 0;
+}

test_regress/t/t_wrapper_del_context_bad.out

@@ -0,0 +1,2 @@
+%Error: Attempt to create model using a bad/deleted VerilatedContext pointer
+Aborting...

test_regress/t/t_wrapper_del_context_bad.pl

@@ -0,0 +1,25 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use strict; use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Multiple Model Test Module
+#
+# Copyright 2020-2021 by Andreas Kuster. 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);
+
+compile(
+    make_top_shell => 0,
+    make_main => 0,
+    verilator_flags2 => ["--exe $Self->{t_dir}/$Self->{name}.cpp"],
+    );
+
+execute(
+    fails => 1,
+    expect_filename => $Self->{golden_filename},
+    );
+
+ok(1);
+1;

test_regress/t/t_wrapper_del_context_bad.v

@@ -0,0 +1,9 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2022 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+module top;
+   initial $finish;
+endmodule