Convert VPI to singleton, part of (#2660).

include/verilated.cpp

@@ -21,6 +21,27 @@
 /// Code available from: https://verilator.org
 ///
 //=========================================================================
+// Internal note:
+//
+// verilated.o may exist both in protect-lib (incrementally linked .a/.so)
+// and the main module.  Both refer the same instance of static
+// variables/VL_THREAD_LOCAL in verilated.o such as Verilated, or
+// VerilatedImpData.  This is important to share that state, but the
+// sharing may cause a double-free error when shutting down because the
+// loader will insert a constructor/destructor at each reference to
+// verilated.o, resulting in at runtime constructors/destructors being
+// called multiple times.
+//
+// To avoid the trouble:
+//   * Statics declared inside functions. The compiler will wrap
+//     the construction in must-be-one-time checks.
+//   * Or, use only C++20 constinit types. (TODO: Make a VL_CONSTINIT).
+//   * Or, use types that are multi-constructor safe.
+//   * Or, the static should be of a union, which will avoid compiler
+//     construction, and appropriately check for duplicate construction.
+//   * Or, code is not linked in protected library. e.g. the VPI
+//     and DPI libraries are not needed there.
+//=========================================================================
 
 #define VERILATOR_VERILATED_CPP_
 

include/verilated_threads.cpp

@@ -19,6 +19,11 @@
 
 #include <cstdio>
 
+//=============================================================================
+// Globals
+
+// Internal note: Globals may multi-construct, see verilated.cpp top.
+
 std::atomic<vluint64_t> VlMTaskVertex::s_yields;
 
 VL_THREAD_LOCAL VlThreadPool::ProfileTrace* VlThreadPool::t_profilep = nullptr;

include/verilated_threads.h

@@ -189,7 +189,7 @@ private:
     VerilatedMutex m_mutex;
     std::condition_variable_any m_cv;
     // Only notify the condition_variable if the worker is waiting
-    bool m_waiting VL_GUARDED_BY(m_mutex);
+    bool m_waiting VL_GUARDED_BY(m_mutex) = false;
 
     // Why a vector? We expect the pending list to be very short, typically
     // 0 or 1 or 2, so popping from the front shouldn't be
@@ -262,6 +262,7 @@ class VlThreadPool final {
     // corrupting the profiling data. It's super cheap to append
     // a VlProfileRec struct on the end of a pre-allocated vector;
     // this is the only cost we pay in real-time during a profiling cycle.
+    // Internal note: Globals may multi-construct, see verilated.cpp top.
     static VL_THREAD_LOCAL ProfileTrace* t_profilep;
     ProfileSet m_allProfiles VL_GUARDED_BY(m_mutex);
     VerilatedMutex m_mutex;

include/verilated_vpi.cpp

@@ -62,6 +62,7 @@ class VerilatedVpio VL_NOT_FINAL {
     static constexpr vluint32_t activeMagic() { return 0xfeed100f; }
 
     // MEM MANGLEMENT
+    // Internal note: Globals may multi-construct, see verilated.cpp top.
     static VL_THREAD_LOCAL vluint8_t* t_freeHead;
 
 public:
@@ -504,17 +505,21 @@ class VerilatedVpiImp final {
     typedef std::list<VerilatedVpiCbHolder> VpioCbList;
     typedef std::map<std::pair<QData, vluint64_t>, VerilatedVpiCbHolder> VpioTimedCbs;
 
+    // All only medium-speed, so use singleton function
     VpioCbList m_cbObjLists[CB_ENUM_MAX_VALUE];  // Callbacks for each supported reason
     VpioTimedCbs m_timedCbs;  // Time based callbacks
     VerilatedVpiError* m_errorInfop = nullptr;  // Container for vpi error info
     VerilatedAssertOneThread m_assertOne;  ///< Assert only called from single thread
     vluint64_t m_nextCallbackId = 1;  // Id to identify callback
 
-    static VerilatedVpiImp s_s;  // Singleton
+    static VerilatedVpiImp& s() {  // Singleton
+        static VerilatedVpiImp s_s;
+        return s_s;
+    }
 
 public:
-    static void assertOneCheck() { s_s.m_assertOne.check(); }
-    static vluint64_t nextCallbackId() { return ++s_s.m_nextCallbackId; }
+    static void assertOneCheck() { s().m_assertOne.check(); }
+    static vluint64_t nextCallbackId() { return ++s().m_nextCallbackId; }
 
     static void cbReasonAdd(vluint64_t id, const s_cb_data* cb_data_p) {
         // The passed cb_data_p was property of the user, so need to recreate
@@ -525,20 +530,20 @@ public:
                                     cb_data_p->reason, id, cb_data_p->obj););
         VerilatedVpioVar* varop = nullptr;
         if (cb_data_p->reason == cbValueChange) varop = VerilatedVpioVar::castp(cb_data_p->obj);
-        s_s.m_cbObjLists[cb_data_p->reason].emplace_back(id, cb_data_p, varop);
+        s().m_cbObjLists[cb_data_p->reason].emplace_back(id, cb_data_p, varop);
     }
     static void cbTimedAdd(vluint64_t id, const s_cb_data* cb_data_p, QData time) {
         // The passed cb_data_p was property of the user, so need to recreate
         VL_DEBUG_IF_PLI(VL_DBG_MSGF("- vpi: vpi_register_cb reason=%d id=%" VL_PRI64
                                     "d delay=%" VL_PRI64 "u\n",
                                     cb_data_p->reason, id, time););
-        s_s.m_timedCbs.emplace(std::piecewise_construct,
+        s().m_timedCbs.emplace(std::piecewise_construct,
                                std::forward_as_tuple(std::make_pair(time, id)),
                                std::forward_as_tuple(id, cb_data_p, nullptr));
     }
     static void cbReasonRemove(vluint64_t id, vluint32_t reason) {
         // Id might no longer exist, if already removed due to call after event, or teardown
-        VpioCbList& cbObjList = s_s.m_cbObjLists[reason];
+        VpioCbList& cbObjList = s().m_cbObjLists[reason];
         // We do not remove it now as we may be iterating the list,
         // instead set to nullptr and will cleanup later
         for (auto& ir : cbObjList) {
@@ -547,13 +552,13 @@ public:
     }
     static void cbTimedRemove(vluint64_t id, QData time) {
         // Id might no longer exist, if already removed due to call after event, or teardown
-        const auto it = s_s.m_timedCbs.find(std::make_pair(time, id));
-        if (VL_LIKELY(it != s_s.m_timedCbs.end())) it->second.invalidate();
+        const auto it = s().m_timedCbs.find(std::make_pair(time, id));
+        if (VL_LIKELY(it != s().m_timedCbs.end())) it->second.invalidate();
     }
     static void callTimedCbs() VL_MT_UNSAFE_ONE {
         assertOneCheck();
         QData time = VL_TIME_Q();
-        for (auto it = s_s.m_timedCbs.begin(); it != s_s.m_timedCbs.end();) {
+        for (auto it = s().m_timedCbs.begin(); it != s().m_timedCbs.end();) {
             if (VL_UNLIKELY(it->first.first <= time)) {
                 VerilatedVpiCbHolder& ho = it->second;
                 const auto last_it = it;
@@ -564,19 +569,19 @@ public:
                     ho.invalidate();  // Timed callbacks are one-shot
                     (ho.cb_rtnp())(ho.cb_datap());
                 }
-                s_s.m_timedCbs.erase(last_it);
+                s().m_timedCbs.erase(last_it);
             } else {
                 ++it;
             }
         }
     }
     static QData cbNextDeadline() {
-        const auto it = s_s.m_timedCbs.cbegin();
-        if (VL_LIKELY(it != s_s.m_timedCbs.cend())) return it->first.first;
+        const auto it = s().m_timedCbs.cbegin();
+        if (VL_LIKELY(it != s().m_timedCbs.cend())) return it->first.first;
         return ~0ULL;  // maxquad
     }
     static bool callCbs(vluint32_t reason) VL_MT_UNSAFE_ONE {
-        VpioCbList& cbObjList = s_s.m_cbObjLists[reason];
+        VpioCbList& cbObjList = s().m_cbObjLists[reason];
         bool called = false;
         if (cbObjList.empty()) return called;
         const auto last = std::prev(cbObjList.end());  // prevent looping over newly added elements
@@ -600,7 +605,7 @@ public:
     }
     static bool callValueCbs() VL_MT_UNSAFE_ONE {
         assertOneCheck();
-        VpioCbList& cbObjList = s_s.m_cbObjLists[cbValueChange];
+        VpioCbList& cbObjList = s().m_cbObjLists[cbValueChange];
         bool called = false;
         typedef std::unordered_set<VerilatedVpioVar*> VpioVarSet;
         VpioVarSet update;  // set of objects to update after callbacks
@@ -640,9 +645,17 @@ public:
     static VerilatedVpiError* error_info() VL_MT_UNSAFE_ONE;  // getter for vpi error info
 };
 
-class VerilatedVpiError final {
-    //// Container for vpi error info
+//======================================================================
+// Statics
+// Internal note: Globals may multi-construct, see verilated.cpp top.
 
+VL_THREAD_LOCAL vluint8_t* VerilatedVpio::t_freeHead = nullptr;
+
+//======================================================================
+// VerilatedVpiError
+/// Internal container for vpi error info
+
+class VerilatedVpiError final {
     t_vpi_error_info m_errorInfo;
     bool m_flag = false;
     char m_buff[VL_VPI_LINE_SIZE_];
@@ -708,11 +721,6 @@ public:
     static const char* strFromVpiProp(PLI_INT32 vpiVal) VL_MT_SAFE;
 };
 
-//======================================================================
-
-VerilatedVpiImp VerilatedVpiImp::s_s;  // Singleton
-VL_THREAD_LOCAL vluint8_t* VerilatedVpio::t_freeHead = nullptr;
-
 //======================================================================
 // VerilatedVpi implementation
 
@@ -742,8 +750,8 @@ PLI_INT32 VerilatedVpioReasonCb::dovpi_remove_cb() {
 
 VerilatedVpiError* VerilatedVpiImp::error_info() VL_MT_UNSAFE_ONE {
     VerilatedVpiImp::assertOneCheck();
-    if (VL_UNLIKELY(!s_s.m_errorInfop)) s_s.m_errorInfop = new VerilatedVpiError();
-    return s_s.m_errorInfop;
+    if (VL_UNLIKELY(!s().m_errorInfop)) s().m_errorInfop = new VerilatedVpiError();
+    return s().m_errorInfop;
 }
 
 //======================================================================

include/verilatedos.h

@@ -39,7 +39,9 @@
 # define VL_ATTR_PRINTF(fmtArgNum) __attribute__((format(printf, (fmtArgNum), (fmtArgNum) + 1)))
 # define VL_ATTR_PURE __attribute__((pure))
 # define VL_ATTR_UNUSED __attribute__((unused))
-# define VL_ATTR_WEAK __attribute__((weak))
+# if !defined(_WIN32) && !defined(__MINGW32__)
+#  define VL_ATTR_WEAK __attribute__((weak))
+# endif
 # define VL_FUNC __func__
 # if defined(__clang__) && defined(VL_THREADED)
 #  define VL_ACQUIRE(...) __attribute__((acquire_capability(__VA_ARGS__)))