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.
This commit is contained in:
Jeffrey Song 2026-07-13 02:27:46 -07:00
parent dde8de0a0d
commit e011bffeab
3 changed files with 271 additions and 4 deletions

View File

@ -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);
}});
}

View File

@ -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 <cstdlib>
#include <iostream>
#include <string>
#include <vector>
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<MessageRecord> s_finishRecords;
static std::vector<MessageRecord> s_stopRecords;
static std::vector<MessageRecord> s_fatalRecords;
static std::vector<MessageRecord> 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<MessageRecord>& 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;
}

View File

@ -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()