From e011bffeab3f16180e2dd2c5b6cde1e4d57a70c4 Mon Sep 17 00:00:00 2001 From: Jeffrey Song Date: Mon, 13 Jul 2026 02:27:46 -0700 Subject: [PATCH] Copy MT message string arguments Context: The multithreaded finish, stop, fatal, and warning wrappers deferred raw const char pointers into the evaluation message queue. Temporary caller strings could be destroyed before the main thread consumed the message, while nullable arguments also require preserving the distinction between nullptr and a non-null empty string. Changes: - Copy nullable filename, hierarchy, and message arguments before posting each deferred callback. - Reconstruct the original pointer nullness when invoking the existing single-threaded handlers, including non-null empty strings. - Add an AddressSanitizer regression with independent warning, finish, stop, and fatal modes covering temporary strings, empty strings, and null arguments. - Check pointer nullness separately from copied string contents. Evidence: - Each wrapper mode fails against origin/master with a heap-use-after-free and passes with this change under AddressSanitizer and C++14. - A temporary mutation that collapses empty filenames to null is rejected by all four modes with a filename-nullness mismatch. - Seven adjacent multithreaded tests and focused distribution checks passed during candidate verification. - clang-format 18.1.8 and git diff --check pass. Boundary: The change only owns string lifetime and nullable-string identity across the MT message queue. It does not alter finish, stop, fatal, or warning policy. --- include/verilated.cpp | 32 +++- .../t/t_verilated_mt_msg_lifetime.cpp | 176 ++++++++++++++++++ test_regress/t/t_verilated_mt_msg_lifetime.py | 67 +++++++ 3 files changed, 271 insertions(+), 4 deletions(-) create mode 100644 test_regress/t/t_verilated_mt_msg_lifetime.cpp create mode 100755 test_regress/t/t_verilated_mt_msg_lifetime.py diff --git a/include/verilated.cpp b/include/verilated.cpp index d981f7c0a..520917dcf 100644 --- a/include/verilated.cpp +++ b/include/verilated.cpp @@ -243,26 +243,50 @@ void vl_warn(const char* filename, int linenum, const char* hier, const char* ms // Wrapper to call certain functions via messages when multithreaded void VL_FINISH_MT(const char* filename, int linenum, const char* hier) VL_MT_SAFE { + const bool haveFilename = filename; + const bool haveHier = hier; + const std::string filenameStr{haveFilename ? filename : ""}; + const std::string hierStr{haveHier ? hier : ""}; VerilatedThreadMsgQueue::post(VerilatedMsg{[=]() { // - vl_finish(filename, linenum, hier); + vl_finish(haveFilename ? filenameStr.c_str() : nullptr, linenum, + haveHier ? hierStr.c_str() : nullptr); }}); } void VL_STOP_MT(const char* filename, int linenum, const char* hier, bool maybe) VL_MT_SAFE { + const bool haveFilename = filename; + const bool haveHier = hier; + const std::string filenameStr{haveFilename ? filename : ""}; + const std::string hierStr{haveHier ? hier : ""}; VerilatedThreadMsgQueue::post(VerilatedMsg{[=]() { // - vl_stop_maybe(filename, linenum, hier, maybe); + vl_stop_maybe(haveFilename ? filenameStr.c_str() : nullptr, linenum, + haveHier ? hierStr.c_str() : nullptr, maybe); }}); } void VL_FATAL_MT(const char* filename, int linenum, const char* hier, const char* msg) VL_MT_SAFE { + const bool haveFilename = filename; + const bool haveHier = hier; + const bool haveMsg = msg; + const std::string filenameStr{haveFilename ? filename : ""}; + const std::string hierStr{haveHier ? hier : ""}; + const std::string msgStr{haveMsg ? msg : ""}; VerilatedThreadMsgQueue::post(VerilatedMsg{[=]() { // - vl_fatal(filename, linenum, hier, msg); + vl_fatal(haveFilename ? filenameStr.c_str() : nullptr, linenum, + haveHier ? hierStr.c_str() : nullptr, haveMsg ? msgStr.c_str() : nullptr); }}); } void VL_WARN_MT(const char* filename, int linenum, const char* hier, const char* msg) VL_MT_SAFE { + const bool haveFilename = filename; + const bool haveHier = hier; + const bool haveMsg = msg; + const std::string filenameStr{haveFilename ? filename : ""}; + const std::string hierStr{haveHier ? hier : ""}; + const std::string msgStr{haveMsg ? msg : ""}; VerilatedThreadMsgQueue::post(VerilatedMsg{[=]() { // - vl_warn(filename, linenum, hier, msg); + vl_warn(haveFilename ? filenameStr.c_str() : nullptr, linenum, + haveHier ? hierStr.c_str() : nullptr, haveMsg ? msgStr.c_str() : nullptr); }}); } diff --git a/test_regress/t/t_verilated_mt_msg_lifetime.cpp b/test_regress/t/t_verilated_mt_msg_lifetime.cpp new file mode 100644 index 000000000..22fe098cb --- /dev/null +++ b/test_regress/t/t_verilated_mt_msg_lifetime.cpp @@ -0,0 +1,176 @@ +// -*- mode: C++; c-file-style: "cc-mode" -*- +// DESCRIPTION: Verilator: MT message wrappers copy string arguments +// +// 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-FileCopyrightText: 2026 Wilson Snyder +// SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +#include "verilated.h" +#define VERILATOR_VERILATED_CPP_ +#include "verilated_imp.h" + +#include +#include +#include +#include + +double sc_time_stamp() { return 0.0; } + +struct MessageRecord final { + const bool filenameNull; + const int line; + const bool hierNull; + const bool messageNull; + const std::string filename; + const std::string hier; + const std::string message; + + MessageRecord(const char* filenamep, int linenum, const char* hierp, const char* messagep) + : filenameNull{!filenamep} + , line{linenum} + , hierNull{!hierp} + , messageNull{!messagep} + , filename{filenamep ? filenamep : ""} + , hier{hierp ? hierp : ""} + , message{messagep ? messagep : ""} {} +}; + +static std::vector s_finishRecords; +static std::vector s_stopRecords; +static std::vector s_fatalRecords; +static std::vector s_warnRecords; + +void vl_finish(const char* filenamep, int linenum, const char* hierp) { + s_finishRecords.emplace_back(filenamep, linenum, hierp, nullptr); +} + +void vl_stop(const char* filenamep, int linenum, const char* hierp) { + s_stopRecords.emplace_back(filenamep, linenum, hierp, nullptr); +} + +void vl_fatal(const char* filenamep, int linenum, const char* hierp, const char* messagep) { + s_fatalRecords.emplace_back(filenamep, linenum, hierp, messagep); +} + +void vl_warn(const char* filenamep, int linenum, const char* hierp, const char* messagep) { + s_warnRecords.emplace_back(filenamep, linenum, hierp, messagep); +} + +static void fail(const std::string& text) { + std::cerr << "%Error: " << text << '\n'; + std::abort(); +} + +static void checkRecord(const MessageRecord& record, int line, bool filenameNull, + const std::string& filename, bool hierNull, const std::string& hier, + bool messageNull, const std::string& message) { + if (record.line != line) fail("line mismatch"); + if (record.filenameNull != filenameNull) fail("filename nullness mismatch"); + if (record.filename != filename) fail("filename mismatch"); + if (record.hierNull != hierNull) fail("hierarchy nullness mismatch"); + if (record.hier != hier) fail("hierarchy mismatch"); + if (record.messageNull != messageNull) fail("message nullness mismatch"); + if (record.message != message) fail("message mismatch"); +} + +static std::string longString(const char* prefix, char fill, const char* suffix = "") { + return std::string{prefix} + std::string(512, fill) + suffix; +} + +static void queueWarning() { + const std::string filename = longString("mt_msg_lifetime_warn_source_", 'w', ".sv"); + const std::string hier = longString("mt_msg_lifetime_warn_hier_", 'h'); + const std::string message = longString("mt_msg_lifetime_warn_message_", 'm'); + VL_WARN_MT(filename.c_str(), 122, hier.c_str(), message.c_str()); + VL_WARN_MT("", 172, "", ""); + VL_WARN_MT(nullptr, 222, nullptr, nullptr); +} + +static void queueFinish() { + const std::string filename = longString("mt_msg_lifetime_finish_source_", 'f', ".sv"); + const std::string hier = longString("mt_msg_lifetime_finish_hier_", 'h'); + VL_FINISH_MT(filename.c_str(), 123, hier.c_str()); + VL_FINISH_MT("", 173, ""); + VL_FINISH_MT(nullptr, 223, nullptr); +} + +static void queueStop() { + const std::string filename = longString("mt_msg_lifetime_stop_source_", 's', ".sv"); + const std::string hier = longString("mt_msg_lifetime_stop_hier_", 'h'); + VL_STOP_MT(filename.c_str(), 124, hier.c_str(), false); + VL_STOP_MT("", 174, "", false); + VL_STOP_MT(nullptr, 224, nullptr, false); +} + +static void queueFatal() { + const std::string filename = longString("mt_msg_lifetime_fatal_source_", 'f', ".sv"); + const std::string hier = longString("mt_msg_lifetime_fatal_hier_", 'h'); + const std::string message = longString("mt_msg_lifetime_fatal_message_", 'm'); + VL_FATAL_MT(filename.c_str(), 125, hier.c_str(), message.c_str()); + VL_FATAL_MT("", 175, "", ""); + VL_FATAL_MT(nullptr, 225, nullptr, nullptr); +} + +static void churnHeap() { + for (int i = 0; i < 4096; ++i) { + const std::string poison = longString("mt_msg_lifetime_poison_", 'x'); + if (poison.empty()) std::abort(); + } +} + +static void checkRecords(const std::vector& records, int line, + const std::string& filename, const std::string& hier, + const std::string& message, bool haveMessage) { + if (records.size() != 3) fail("message count mismatch"); + checkRecord(records[0], line, false, filename, false, hier, !haveMessage, message); + checkRecord(records[1], line + 50, false, "", false, "", !haveMessage, ""); + checkRecord(records[2], line + 100, true, "", true, "", true, ""); +} + +int main(int argc, char** argv) { + if (argc != 2) fail("expected one wrapper mode"); + const std::string mode = argv[1]; + + VerilatedContext context; + Verilated::threadContextp(&context); + VerilatedEvalMsgQueue evalMsgQ; + Verilated::mtaskId(1); + + if (mode == "warn") { + queueWarning(); + } else if (mode == "finish") { + queueFinish(); + } else if (mode == "stop") { + queueStop(); + } else if (mode == "fatal") { + queueFatal(); + } else { + fail("unknown wrapper mode"); + } + churnHeap(); + + Verilated::endOfThreadMTask(&evalMsgQ); + Verilated::endOfEval(&evalMsgQ); + + if (mode == "warn") { + checkRecords(s_warnRecords, 122, longString("mt_msg_lifetime_warn_source_", 'w', ".sv"), + longString("mt_msg_lifetime_warn_hier_", 'h'), + longString("mt_msg_lifetime_warn_message_", 'm'), true); + } else if (mode == "finish") { + checkRecords(s_finishRecords, 123, + longString("mt_msg_lifetime_finish_source_", 'f', ".sv"), + longString("mt_msg_lifetime_finish_hier_", 'h'), "", false); + } else if (mode == "stop") { + checkRecords(s_stopRecords, 124, longString("mt_msg_lifetime_stop_source_", 's', ".sv"), + longString("mt_msg_lifetime_stop_hier_", 'h'), "", false); + } else { + checkRecords(s_fatalRecords, 125, longString("mt_msg_lifetime_fatal_source_", 'f', ".sv"), + longString("mt_msg_lifetime_fatal_hier_", 'h'), + longString("mt_msg_lifetime_fatal_message_", 'm'), true); + } + + std::cout << "MT_MSG_LIFETIME_DONE " << mode << '\n'; + return 0; +} diff --git a/test_regress/t/t_verilated_mt_msg_lifetime.py b/test_regress/t/t_verilated_mt_msg_lifetime.py new file mode 100755 index 000000000..be117d94d --- /dev/null +++ b/test_regress/t/t_verilated_mt_msg_lifetime.py @@ -0,0 +1,67 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: MT message wrappers copy string arguments +# +# 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-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import os +import shlex +import subprocess +import sys + +import vltest_bootstrap + +test.scenarios("vlt") + +exe = test.obj_dir + "/t_verilated_mt_msg_lifetime" +probe_cpp = test.obj_dir + "/asan_probe.cpp" +probe_exe = test.obj_dir + "/asan_probe" +run_log = test.obj_dir + "/run.log" + +cxx_cmd = shlex.split(os.environ["CXX"]) +test.write_wholefile(probe_cpp, "int main() { return 0; }\n") +probe_cmd = cxx_cmd + ["-fsanitize=address", "-pthread", probe_cpp, "-o", probe_exe] +if sys.platform == "darwin": + probe_cmd.append("-Wl,-U,__Z18vlFlushSolverStatsv") +probe = subprocess.run(probe_cmd, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False) +if probe.returncode: + test.skip("C++ compiler does not support AddressSanitizer") + +cmd = [ + *cxx_cmd, + "-std=gnu++14", + "-O0", + "-g", + "-fsanitize=address", + "-fno-omit-frame-pointer", + "-DVL_USER_FATAL", + "-DVL_USER_FINISH", + "-DVL_USER_STOP", + "-DVL_USER_WARN", + "-I" + os.environ["VERILATOR_ROOT"] + "/include", + "-I" + os.environ["VERILATOR_ROOT"] + "/include/vltstd", + "t/t_verilated_mt_msg_lifetime.cpp", + os.environ["VERILATOR_ROOT"] + "/include/verilated.cpp", + os.environ["VERILATOR_ROOT"] + "/include/verilated_threads.cpp", + "-pthread", + "-o", + exe, +] +if sys.platform == "darwin": + cmd.append("-Wl,-U,__Z18vlFlushSolverStatsv") + +test.run(cmd=cmd, logfile=test.obj_dir + "/cxx_compile.log") + +test.setenv("ASAN_OPTIONS", "abort_on_error=0:detect_leaks=0") +for mode in ("warn", "finish", "stop", "fatal"): + mode_log = run_log + "." + mode + test.run(cmd=[exe, mode], logfile=mode_log) + test.file_grep(mode_log, r"MT_MSG_LIFETIME_DONE " + mode) + +test.passes()