Skip to content

Commit 9f706cd

Browse files
committed
fix(toolchain): store multi-token dialect flags as tokens, not a string to split
macOS CI segfaulted on every build.mcpp test. The crash was inside `contract_env` — a function this branch never touched — corrupting a local vector before the compile even started, and it reproduced only under clang/libc++, never under GCC. Bisecting with a clean target and dep cache each round (a first attempt was worthless: switching mcpp.toml's toolchain without clearing the dependency cache links a libstdc++-built mcpplibs.cmdline into a libc++ mcpp, which crashes for an entirely unrelated reason) narrowed it to a single addition: the `split_ws` helper. Not a call to it — its mere PRESENCE in the anonymous namespace. Deleting it fixed the crash; a trivial unused function in the same spot did not cause one. That is a clang codegen problem, not a logic error, and no amount of reading the diff would have found it. The fix is also the better design. `split_ws` only existed because the dialect table stored "-x c++" and "/nologo /EHsc /utf-8" as ninja-command strings while the build.mcpp path needs argv tokens — so the token boundary was being re-derived at the call site. The table now carries both forms, the argv one as a span over a static array, and the spelling stays in one row. Verified under BOTH toolchains from a clean target and cache: clang 22.1.8 (e2e 89 / 92 / 179 / 181 all pass, previously all segfaults) and GCC 16.1.0 (unit 49/49).
1 parent 3345242 commit 9f706cd

2 files changed

Lines changed: 23 additions & 18 deletions

File tree

src/build/build_program.cppm

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -104,22 +104,6 @@ struct Directives {
104104
std::vector<std::string> rerunEnv; // declared env-var inputs
105105
};
106106

107-
// Split a whitespace-separated flag string into argv tokens. The dialect
108-
// table stores some entries as multi-token strings ("-x c++",
109-
// "/nologo /EHsc /utf-8") because their other consumer is a ninja command
110-
// line, where a single string is what's wanted; an argv vector is not.
111-
std::vector<std::string> split_ws(std::string_view s) {
112-
std::vector<std::string> out;
113-
std::size_t i = 0;
114-
while (i < s.size()) {
115-
while (i < s.size() && (s[i] == ' ' || s[i] == '\t')) ++i;
116-
std::size_t b = i;
117-
while (i < s.size() && s[i] != ' ' && s[i] != '\t') ++i;
118-
if (i > b) out.emplace_back(s.substr(b, i - b));
119-
}
120-
return out;
121-
}
122-
123107
std::string trim(std::string_view s) {
124108
std::size_t b = 0, e = s.size();
125109
while (b < e && (s[b] == ' ' || s[b] == '\t' || s[b] == '\r')) ++b;
@@ -859,7 +843,7 @@ std::expected<void, std::string> run_build_program(
859843
if (msvcHost) {
860844
// /nologo /EHsc /utf-8 — cl.exe needs these to behave like the other
861845
// two drivers do by default (quiet, exceptions on, UTF-8 sources).
862-
for (auto& f : split_ws(dial.alwaysFlags)) compileArgv.push_back(f);
846+
for (auto f : dial.alwaysFlagsArgv) compileArgv.emplace_back(f);
863847
}
864848
compileArgv.push_back(std_flag);
865849
// No optimization: this program runs once per build and its compile time
@@ -871,7 +855,7 @@ std::expected<void, std::string> run_build_program(
871855
for (auto& sf : stdFlags) compileArgv.push_back(sf);
872856
// The `.mcpp` extension is unknown to every driver, so without this the
873857
// file is handed to the linker as a linker script.
874-
for (auto& f : split_ws(dial.forceCxxLang)) compileArgv.push_back(f);
858+
for (auto f : dial.forceCxxLangArgv) compileArgv.emplace_back(f);
875859
compileArgv.push_back(src.string());
876860
if (usesModule || !stdObjects.empty()) {
877861
// Link the module objects (GNU: reset the input language first so the

src/toolchain/dialect.cppm

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ struct CommandDialect {
3232
std::string_view optPrefix; // "-O" | "/O"
3333
std::string_view debugFlags; // "-g" | "/Zi /FS"
3434
std::string_view alwaysFlags; // "" | "/nologo /EHsc /utf-8"
35+
// Same rationale as forceCxxLangArgv: the argv consumer gets tokens, not
36+
// a string it has to split. Both spans point at static arrays below.
37+
std::span<const std::string_view> alwaysFlagsArgv;
3538

3639
// Link and language-selection spellings.
3740
//
@@ -42,7 +45,15 @@ struct CommandDialect {
4245
std::string_view libSearchPrefix; // "-L" | "/LIBPATH:"
4346
// The `.mcpp` extension is unknown to every compiler driver, so the
4447
// language has to be forced or the driver hands the file to the linker.
48+
//
49+
// Two forms of the same thing, because the two consumers need different
50+
// shapes and neither should re-derive the other's: a ninja command line
51+
// wants one string, an argv vector wants tokens. Storing both keeps the
52+
// spelling in one row — splitting the string at the call site would put
53+
// the token boundary in a second place, and cost a helper this file is
54+
// better off without (see the note on `alwaysFlagsArgv`).
4555
std::string_view forceCxxLang; // "-x c++" | "/TP"
56+
std::span<const std::string_view> forceCxxLangArgv;
4657
// Static CRT / runtime. On MSVC this is a compile-time CRT model, not a
4758
// link mode — there is no /MT equivalent of `-static` for the whole image.
4859
std::string_view staticRuntime; // "-static"| "/MT"
@@ -95,6 +106,12 @@ namespace mcpp::toolchain {
95106

96107
namespace {
97108

109+
// Token forms of the multi-token rows. Static arrays so the spans above are
110+
// constexpr-initializable and no consumer has to split a string at runtime.
111+
constexpr std::string_view kGnuForceCxxArgv[] = {"-x", "c++"};
112+
constexpr std::string_view kMsvcForceCxxArgv[] = {"/TP"};
113+
constexpr std::string_view kMsvcAlwaysArgv[] = {"/nologo", "/EHsc", "/utf-8"};
114+
98115
constexpr CommandDialect kGnuDialect{
99116
.id = "gnu",
100117
.includePrefix = "-I",
@@ -105,9 +122,11 @@ constexpr CommandDialect kGnuDialect{
105122
.optPrefix = "-O",
106123
.debugFlags = "-g",
107124
.alwaysFlags = "",
125+
.alwaysFlagsArgv = {},
108126
.libFlag = "-l{}",
109127
.libSearchPrefix = "-L",
110128
.forceCxxLang = "-x c++",
129+
.forceCxxLangArgv = kGnuForceCxxArgv,
111130
.staticRuntime = "-static",
112131
.outputExePrefix = "-o ",
113132
.objExt = ".o",
@@ -130,9 +149,11 @@ constexpr CommandDialect kMsvcDialect{
130149
.optPrefix = "/O",
131150
.debugFlags = "/Zi /FS",
132151
.alwaysFlags = "/nologo /EHsc /utf-8",
152+
.alwaysFlagsArgv = kMsvcAlwaysArgv,
133153
.libFlag = "{}.lib",
134154
.libSearchPrefix = "/LIBPATH:",
135155
.forceCxxLang = "/TP",
156+
.forceCxxLangArgv = kMsvcForceCxxArgv,
136157
.staticRuntime = "/MT",
137158
.outputExePrefix = "/Fe:",
138159
.objExt = ".obj",

0 commit comments

Comments
 (0)