Skip to content

Commit b0e275b

Browse files
committed
fix(fixup): specs-grammar-safe loader detection + macOS keeps cfg-trust semantics
Two regressions the first CI round caught (exactly the environments local verification can't fake): 1. gcc specs corruption (linux): the rewritten detect walked the loader path to 'whitespace/:;' — but specs embed the baked loader inside %-spec conditionals (%{mmusl:...;:/baked/ld-linux-x86-64.so.2}), so the scan swallowed closing braces and the rewrite corrupted the spec grammar ('braced spec body ... is invalid' from every g++ run after). Now: path-character whitelist scan, skip pristine /lib* multilib defaults, unit-tested against the real spec grammar (incl. aarch64 loader names). detect_baked_loader is exported for the tests. Fixup rev bumped (hermetic-2) so payloads stamped by the broken pass re-run the fixup. 2. macOS host/cfg semantics: the cfg-bypass host flags and the -nostdinc++/-stdlib=libc++ cfg regeneration are LINUX semantics; a bare macOS link has no libc++abi handling (that lives in the main build's needs_explicit_libcxx path) and died with undefined __cxa_* / __gxx_personality_v0. build.mcpp host compiles keep trusting the cfg off-Linux, and the macOS cfg keeps its historical shape (--sysroot=<sdk> + payload libc++ headers only).
1 parent 17ecd64 commit b0e275b

3 files changed

Lines changed: 123 additions & 40 deletions

File tree

src/build/build_program.cppm

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export module mcpp.build.build_program;
1414

1515
import std;
1616
import mcpp.manifest;
17+
import mcpp.platform;
1718
import mcpp.platform.process;
1819
import mcpp.toolchain.fingerprint; // hash_file / hash_string (FNV-1a, 16 hex)
1920
import mcpp.toolchain.linkmodel; // shared C-library / clang-cfg-bypass model
@@ -119,12 +120,17 @@ std::vector<std::string> host_base_flags(const mcpp::toolchain::Toolchain& tc) {
119120
std::vector<std::string> f;
120121
const auto lm = mcpp::toolchain::resolve_link_model(tc);
121122

122-
// Clang with a bundled cfg: bypass it (--no-default-config) and provide
123-
// everything explicitly, same as the main build — the cfg is an
124-
// install-time-generated artifact whose content varies per machine and
125-
// install path, so trusting it here while bypassing it in the main build
126-
// meant two different toolchains for the same project.
123+
// Clang with a bundled cfg on LINUX: bypass it (--no-default-config) and
124+
// provide everything explicitly, same as the main build — the cfg is an
125+
// install-time-generated artifact, so trusting it here while bypassing
126+
// it in the main build meant two different toolchains for one project.
127+
// On macOS/Windows keep trusting the cfg: the macOS link additionally
128+
// needs the platform's libc++abi/unwind handling that the main build's
129+
// needs_explicit_libcxx path owns (duplicating it for a host compile
130+
// produced undefined __cxa_*/__gxx_personality_v0), and the fixup
131+
// pipeline regenerates the cfg deterministically anyway.
127132
if (mcpp::toolchain::is_clang(tc)) {
133+
if constexpr (!mcpp::platform::is_linux) return f;
128134
const auto dm = mcpp::toolchain::resolve_clang_driver(tc);
129135
if (dm.hasCfg) {
130136
f.push_back("--no-default-config");

src/toolchain/post_install.cppm

Lines changed: 52 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -92,28 +92,37 @@ export void patchelf_walk(const std::filesystem::path& dir,
9292
// gcc specs file. xim bakes the installing user's XLINGS_HOME into specs at
9393
// install time, so the DIR varies per machine, and the loader NAME varies
9494
// per arch — detect both instead of hardcoding either.
95-
std::string detect_baked_loader(const std::string& specsContent) {
95+
export std::string detect_baked_loader(const std::string& specsContent) {
96+
// Path-character whitelist. Specs embed loader paths inside %-spec
97+
// syntax (`%{mmusl:...;:/baked/dir/ld-linux-x86-64.so.2}`), so scanning
98+
// to "whitespace or :;" is NOT a valid boundary — it would swallow the
99+
// closing braces, and replacing that string corrupts the spec grammar
100+
// ("braced spec body ... is invalid" from every subsequent g++ run).
101+
auto is_path_char = [](char c) {
102+
return std::isalnum(static_cast<unsigned char>(c))
103+
|| c == '/' || c == '.' || c == '-' || c == '_' || c == '+';
104+
};
105+
106+
// The baked GNU loader is the ld-linux entry whose directory is NOT a
107+
// standard /lib* location — specs also contain pristine defaults
108+
// (/lib/ld-linux.so.2, /libx32/…) for other multilib branches that must
109+
// never be rewritten.
96110
constexpr std::string_view kLoaderMark = "/ld-linux-";
97-
auto pos = specsContent.find(kLoaderMark);
98-
if (pos == std::string::npos) return "";
99-
// Walk backwards to find start of the absolute path…
100-
auto start = pos;
101-
while (start > 0 && specsContent[start - 1] != ' '
102-
&& specsContent[start - 1] != ':'
103-
&& specsContent[start - 1] != ';'
104-
&& specsContent[start - 1] != '\n') {
105-
--start;
111+
for (std::size_t pos = specsContent.find(kLoaderMark);
112+
pos != std::string::npos;
113+
pos = specsContent.find(kLoaderMark, pos + 1)) {
114+
auto start = pos;
115+
while (start > 0 && is_path_char(specsContent[start - 1])) --start;
116+
auto end = pos + 1;
117+
while (end < specsContent.size() && is_path_char(specsContent[end])) ++end;
118+
auto loader = specsContent.substr(start, end - start);
119+
if (loader.empty() || loader[0] != '/') continue;
120+
auto dir = std::filesystem::path(loader).parent_path().string();
121+
if (dir == "/lib" || dir == "/lib64" || dir == "/lib32" || dir == "/libx32")
122+
continue; // pristine multilib default, not a baked path
123+
return loader;
106124
}
107-
// …and forwards to its end.
108-
auto end = pos + kLoaderMark.size();
109-
while (end < specsContent.size()
110-
&& !std::isspace(static_cast<unsigned char>(specsContent[end]))
111-
&& specsContent[end] != ':' && specsContent[end] != ';') {
112-
++end;
113-
}
114-
auto loader = specsContent.substr(start, end - start);
115-
if (loader.empty() || loader[0] != '/') return "";
116-
return loader;
125+
return "";
117126
}
118127

119128
void fixup_gcc_specs(const std::filesystem::path& gccPkgRoot,
@@ -199,9 +208,18 @@ export void fixup_clang_cfg(const std::filesystem::path& payloadRoot,
199208
}
200209

201210
std::string common, cxxOnly;
211+
auto cxxInclude = payloadRoot / "include" / "c++" / "v1";
202212
if constexpr (mcpp::platform::is_macos) {
213+
// macOS keeps its historical cfg semantics: the C library and the
214+
// C++ runtime LINK both come from the SDK; only the libc++ HEADERS
215+
// come from the payload. Do NOT add -nostdinc++/-stdlib=libc++
216+
// here — a bare cfg-driven link has no libc++abi handling (that
217+
// lives in the main build's needs_explicit_libcxx path) and dies
218+
// with undefined __cxa_* / __gxx_personality_v0.
203219
if (auto sdk = mcpp::platform::macos::sdk_path())
204220
common += "--sysroot=" + sdk->string() + "\n";
221+
if (std::filesystem::exists(cxxInclude))
222+
cxxOnly += "-isystem " + cxxInclude.string() + "\n";
205223
} else {
206224
if (!glibcLibDir.empty()) {
207225
auto loader = resolve_loader(glibcLibDir, triple);
@@ -212,21 +230,20 @@ export void fixup_clang_cfg(const std::filesystem::path& payloadRoot,
212230
common += "-Wl,--enable-new-dtags,-rpath," + glibcLibDir.string() + "\n";
213231
}
214232
common += "-fuse-ld=lld\n--rtlib=compiler-rt\n--unwindlib=libunwind\n";
215-
}
216233

217-
auto cxxInclude = payloadRoot / "include" / "c++" / "v1";
218-
if (std::filesystem::exists(cxxInclude)) {
219-
cxxOnly += "-nostdinc++\n-stdlib=libc++\n";
220-
cxxOnly += "-isystem " + cxxInclude.string() + "\n";
221-
}
222-
if (!triple.empty()) {
223-
auto tripleInclude = payloadRoot / "include" / triple / "c++" / "v1";
224-
if (std::filesystem::exists(tripleInclude))
225-
cxxOnly += "-isystem " + tripleInclude.string() + "\n";
226-
auto tripleLib = payloadRoot / "lib" / triple;
227-
if (std::filesystem::exists(tripleLib)) {
228-
cxxOnly += "-L" + tripleLib.string() + "\n";
229-
cxxOnly += "-Wl,-rpath," + tripleLib.string() + "\n";
234+
if (std::filesystem::exists(cxxInclude)) {
235+
cxxOnly += "-nostdinc++\n-stdlib=libc++\n";
236+
cxxOnly += "-isystem " + cxxInclude.string() + "\n";
237+
}
238+
if (!triple.empty()) {
239+
auto tripleInclude = payloadRoot / "include" / triple / "c++" / "v1";
240+
if (std::filesystem::exists(tripleInclude))
241+
cxxOnly += "-isystem " + tripleInclude.string() + "\n";
242+
auto tripleLib = payloadRoot / "lib" / triple;
243+
if (std::filesystem::exists(tripleLib)) {
244+
cxxOnly += "-L" + tripleLib.string() + "\n";
245+
cxxOnly += "-Wl,-rpath," + tripleLib.string() + "\n";
246+
}
230247
}
231248
}
232249

@@ -334,7 +351,7 @@ void llvm_post_install_fixup(const mcpp::config::GlobalConfig& cfg,
334351
// runtime libs. Idempotent via a content-fingerprinted marker.
335352
//
336353
// Bump when the fixup logic changes so existing installs re-run it.
337-
constexpr std::string_view kFixupRev = "hermetic-1";
354+
constexpr std::string_view kFixupRev = "hermetic-2";
338355

339356
export void ensure_post_install_fixup(const mcpp::config::GlobalConfig& cfg,
340357
const std::filesystem::path& payloadRoot,

tests/unit/test_post_install.cpp

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
#include <gtest/gtest.h>
2+
3+
import std;
4+
import mcpp.toolchain.post_install;
5+
6+
// detect_baked_loader parses gcc SPECS GRAMMAR, not plain text. The baked
7+
// loader path is embedded inside %-spec conditionals, e.g.
8+
// %{mmusl:/lib/ld-musl-x86_64.so.1;:/baked/dir/ld-linux-x86-64.so.2}
9+
// A scanner that treats "whitespace or :;" as the boundary swallows the
10+
// closing braces; replacing that string then corrupts the spec grammar and
11+
// EVERY subsequent g++ invocation dies with "braced spec body ... is
12+
// invalid" (observed on CI). These tests pin the exact grammar shape.
13+
14+
namespace {
15+
16+
using mcpp::toolchain::detect_baked_loader;
17+
18+
// Realistic *link_spec fragment as xim bakes it (64-bit branch rewritten to
19+
// the installing user's home; other multilib branches pristine).
20+
const std::string kBakedSpecs =
21+
"*link:\n"
22+
"%{m16|m32|mx32:;:-m elf_x86_64} %{shared:-shared} %{!shared: %{!static: "
23+
"%{m16|m32:-dynamic-linker %{muclibc:/lib/ld-uClibc.so.0;:%{mbionic:/system/bin/linker;:"
24+
"%{mmusl:/lib/ld-musl-i386.so.1;:/lib/ld-linux.so.2}}}} "
25+
"%{m16|m32|mx32:;:-dynamic-linker %{muclibc:/lib/ld64-uClibc.so.0;:%{mbionic:/system/bin/linker64;:"
26+
"%{mmusl:/lib/ld-musl-x86_64.so.1;:/opt/other-home/.xlings/data/xpkgs/xim-x-glibc/2.39/lib64/ld-linux-x86-64.so.2}}}}} "
27+
"%{static:-static}}\n";
28+
29+
TEST(DetectBakedLoader, ExtractsExactPathWithoutSpecBraces) {
30+
auto got = detect_baked_loader(kBakedSpecs);
31+
EXPECT_EQ(got,
32+
"/opt/other-home/.xlings/data/xpkgs/xim-x-glibc/2.39/lib64/ld-linux-x86-64.so.2");
33+
// The regression: any brace in the result corrupts the specs on rewrite.
34+
EXPECT_EQ(got.find('}'), std::string::npos);
35+
EXPECT_EQ(got.find('{'), std::string::npos);
36+
}
37+
38+
TEST(DetectBakedLoader, IgnoresPristineMultilibDefaults) {
39+
// An unbaked spec (all-standard /lib*/ paths) must not be rewritten.
40+
const std::string pristine =
41+
"%{mmusl:/lib/ld-musl-x86_64.so.1;:/lib64/ld-linux-x86-64.so.2} "
42+
"%{m16|m32:-dynamic-linker /lib/ld-linux.so.2} "
43+
"%{mx32:-dynamic-linker /libx32/ld-linux-x32.so.2}";
44+
EXPECT_EQ(detect_baked_loader(pristine), "");
45+
}
46+
47+
TEST(DetectBakedLoader, Aarch64LoaderNameDetected) {
48+
const std::string specs =
49+
"-dynamic-linker %{mmusl:/lib/ld-musl-aarch64.so.1;:"
50+
"/srv/build/.xlings/xpkgs/xim-x-glibc/2.39/lib/ld-linux-aarch64.so.1}";
51+
EXPECT_EQ(detect_baked_loader(specs),
52+
"/srv/build/.xlings/xpkgs/xim-x-glibc/2.39/lib/ld-linux-aarch64.so.1");
53+
}
54+
55+
TEST(DetectBakedLoader, EmptyWhenNoGnuLoaderPresent) {
56+
EXPECT_EQ(detect_baked_loader("no loaders here"), "");
57+
EXPECT_EQ(detect_baked_loader("%{mmusl:/lib/ld-musl-x86_64.so.1}"), "");
58+
}
59+
60+
} // namespace

0 commit comments

Comments
 (0)