Fix stale pointers in VerilatedImpData::m_hierMap (#6726)

This commit is contained in:
Christian Hecken 2025-12-07 21:42:29 +01:00 committed by GitHub
parent ae480c5f76
commit 9a23711ff9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 121 additions and 0 deletions

View File

@ -3584,6 +3584,8 @@ void VerilatedHierarchy::remove(const VerilatedScope* fromp, const VerilatedScop
VerilatedImp::hierarchyRemove(fromp, top);
}
void VerilatedHierarchy::clear() { VerilatedImp::hierarchyClear(); }
//===========================================================================
// VerilatedOneThreaded:: Methods

View File

@ -743,6 +743,7 @@ class VerilatedHierarchy final {
public:
static void add(const VerilatedScope* fromp, const VerilatedScope* top);
static void remove(const VerilatedScope* fromp, const VerilatedScope* top);
static void clear();
};
//===========================================================================

View File

@ -524,6 +524,11 @@ public:
const auto it = find(scopes.begin(), scopes.end(), top);
if (it != scopes.end()) scopes.erase(it);
}
static void hierarchyClear() VL_MT_SAFE {
const VerilatedLockGuard lock{s().m_hierMapMutex};
VerilatedHierarchyMap& map = s().m_hierMap;
map.clear();
}
static const VerilatedHierarchyMap* hierarchyMap() VL_MT_SAFE_POSTINIT {
// Thread save only assuming this is called only after model construction completed
return &s().m_hierMap;

View File

@ -626,6 +626,11 @@ void EmitCSyms::emitScopeHier(std::vector<std::string>& stmts, bool destroy) {
stmts.emplace_back("__Vhier." + method + "(" + fromId + ", " + toId + ");");
}
}
if (destroy) {
stmts.emplace_back("// Clear keys from hierarchy map after values have been removed");
stmts.emplace_back("__Vhier.clear();");
}
}
std::vector<std::string> EmitCSyms::getSymCtorStmts() {

View File

@ -0,0 +1,64 @@
// ======================================================================
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty.
// SPDX-License-Identifier: CC0-1.0
// ======================================================================
// DESCRIPTION: Scope hierarchy map clearing upon VerilatedModel destruction
//
// VerilatedImp::s() is a VerilatedImpData singleton that contains an m_hierMap
// whose keys are pointers to VerilatedScope objects. Because it is a
// singleton, it is not automatically destroyed together with the
// VerilatedModel, so this test checks that the VerilatedSyms destructor that is
// invoked upon the VerilatedModel's destruction clears the keys from the map.
// Workaround to be able to include verilated_imp.h, needed to directly test the hierarchy map
#define VERILATOR_VERILATED_CPP_
#include "verilated_imp.h"
#include <verilated.h>
#include "TestVpi.h"
#include "Vt_vpi_hierarchy_clear.h"
#include "Vt_vpi_hierarchy_clear__Syms.h"
#include <memory>
int main(int argc, char** argv) {
const std::unique_ptr<VerilatedContext> contextp{new VerilatedContext};
contextp->commandArgs(argc, argv);
// Vt_vpi_hierarchy_clear::vlSymsp is private, so create a dummy syms object just to be able to
// construct the VerilatedScope objects
std::unique_ptr<Vt_vpi_hierarchy_clear__Syms> dummySymsp
= std::make_unique<Vt_vpi_hierarchy_clear__Syms>(contextp.get(), "dummySymsObject",
nullptr);
std::unique_ptr<VerilatedScope> additionalScope
= std::make_unique<VerilatedScope>(dummySymsp.get(), "t.additional", "additional",
"Additional", -12, VerilatedScope::SCOPE_MODULE);
{
// Construct and destroy
const std::unique_ptr<VM_PREFIX> topp{
new VM_PREFIX{contextp.get(),
// Note null name - we're flattening it out
""}};
// Insert additional scope into map for the first topp
VerilatedImp::hierarchyAdd(additionalScope.get(), nullptr);
const bool scopeFound = (VerilatedImp::hierarchyMap()->find(additionalScope.get())
!= VerilatedImp::hierarchyMap()->end());
CHECK_RESULT_NZ(scopeFound); // NOLINT(concurrency-mt-unsafe)
}
// Test second construction
const std::unique_ptr<VM_PREFIX> topp{new VM_PREFIX{contextp.get(),
// Note null name - we're flattening it out
""}};
// Do not insert additionalScope this time, so it should not be present in the map any more
const bool scopeFound = (VerilatedImp::hierarchyMap()->find(additionalScope.get())
!= VerilatedImp::hierarchyMap()->end());
CHECK_RESULT_Z(scopeFound); // NOLINT(concurrency-mt-unsafe)
return 0;
}

View File

@ -0,0 +1,20 @@
#!/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('vlt_all')
test.compile(make_top_shell=False,
make_main=False,
verilator_flags2=["--exe --vpi", test.pli_filename])
test.execute()
test.passes()

View File

@ -0,0 +1,24 @@
// ======================================================================
// This file ONLY is placed under the Creative Commons Public Domain, for
// any use, without warranty.
// SPDX-License-Identifier: CC0-1.0
// ======================================================================
module t (
clk
);
input clk;
Sub sub (.clk(clk));
endmodule
module Sub (
clk
);
// this comment should get ignored for public-ignore
input clk /* verilator public_flat_rw */;
endmodule