Skip to content

Commit a6348e0

Browse files
committed
fix(toolchain): a record naming a payload that is gone is not an authority
CI's log gave the answer directly: probe: runtime 'glibc@2.39' is not installed in this home The machine had one glibc payload, 2.44, and a compatibility record naming 2.39. specs, cfg and PT_INTERP are all RECORDS of a past state -- written at some install, some fixup -- and nothing revisits them when the payload underneath is replaced. The probe matches exactly and never falls back, quite deliberately, so it refused; no payload paths resolved; no --dynamic-linker reached the link line; the artifact took the host loader and the hermetic check rejected it. Each record is now checked against the payloads actually installed, and a record naming an absent one is skipped rather than trusted. Where two payloads exist and a record names one of them, that still wins -- it is the value the artifact would load. Where nothing present is named, silence. Two more, both from the full local e2e rather than the targeted subset: - musl targets were getting the glibc payload's lib dir and loader. The sysroot branch pushed them unconditionally, but a musl sysroot is self-contained and the glibc payload there belongs to the HOST toolchain, merely probed alongside. ld then pulled glibc's static libc.a into a musl link: `undefined reference to _DYNAMIC` out of dl-reloc-static-pie.o (e2e 103). The musl guard the header branch already had now covers libraries and the loader too. - The new "vendored xlings is behind the pin" note went to stdout, and `mcpp test --json` promises every stdout line is NDJSON (e2e 155). It is a remark about the environment, not output of the command; stderr. tests/unit/test_runtime_binding_fallback.cpp +3 (8 total) 66 unit tests; e2e 103/155/184/201/86/28/30 pass.
1 parent 7b3b540 commit a6348e0

4 files changed

Lines changed: 112 additions & 26 deletions

File tree

src/fallback/xlings_binary.cppm

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
// 3. Fail with user-facing instructions
77

88
module;
9+
#include <cstdio>
910
#include <cstdlib>
1011
#include <cctype>
1112

@@ -68,15 +69,22 @@ acquire_xlings_binary(const std::filesystem::path& destBin, bool quiet = false,
6869
// restore. Look before leaping.
6970
auto candidate = candidate_source_version();
7071
if (candidate.empty() || !version_is_older(have, candidate)) {
72+
// stderr, not stdout. This is a remark about the environment,
73+
// not output of the command that happens to be running -- and
74+
// `mcpp test --json` promises every stdout line is NDJSON, a
75+
// promise this line broke the moment a machine fell behind the
76+
// pin (e2e 155).
7177
if (!quiet)
72-
std::println("{:>12} vendored xlings {} is older than the "
78+
std::println(stderr,
79+
"{:>12} vendored xlings {} is older than the "
7380
"pinned {}, but no newer source is available "
7481
"(keeping it; run `xlings self update`)",
7582
"Note", have, pinnedVersion);
7683
return destBin;
7784
}
7885
if (!quiet)
79-
std::println("{:>12} vendored xlings {} -> {} (pinned {})",
86+
std::println(stderr,
87+
"{:>12} vendored xlings {} -> {} (pinned {})",
8088
"Updating", have, candidate, pinnedVersion);
8189
std::error_code rec;
8290
std::filesystem::remove(destBin, rec);

src/toolchain/linkmodel.cppm

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,11 +344,21 @@ ToolchainLinkModel resolve_link_model(const Toolchain& tc) {
344344
// point — the baked copy was a per-toolchain-install decision that no
345345
// longer matched the per-build one, and it accumulated one dead entry
346346
// per home that ever touched the shared payload.
347-
if (tc.payloadPaths && !tc.payloadPaths->glibcLib.empty()) {
347+
//
348+
// Never for a musl target. Its sysroot is self-contained -- it brings
349+
// its own libc, CRT and loader -- and the glibc payload here belongs
350+
// to the HOST toolchain, which merely happens to be probed alongside.
351+
// Putting it on the link path let ld pull glibc's static libc.a into a
352+
// musl link: `undefined reference to _DYNAMIC`, `hidden symbol
353+
// _DYNAMIC isn't defined`, from dl-reloc-static-pie.o. The musl branch
354+
// below already says this about headers; it holds for libraries and
355+
// the loader too.
356+
if (!is_musl_target(tc)
357+
&& tc.payloadPaths && !tc.payloadPaths->glibcLib.empty()) {
348358
lm.loader = resolve_loader(tc.payloadPaths->glibcLib, tc.targetTriple);
349359
lm.libDirs.push_back(tc.payloadPaths->glibcLib);
350360
}
351-
if (!tc.binaryPath.empty()) {
361+
if (!is_musl_target(tc) && !tc.binaryPath.empty()) {
352362
std::error_code lec;
353363
auto gccLib = tc.binaryPath.parent_path().parent_path() / "lib64";
354364
if (std::filesystem::exists(gccLib, lec))

src/toolchain/post_install.cppm

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,34 @@ std::string baked_runtime_binding(const std::filesystem::path& compilerBin) {
614614
return "glibc@" + ver;
615615
};
616616

617+
// Which glibc payloads are actually installed. Every source below is
618+
// checked against this, because all of them are RECORDS of a past state:
619+
// specs and cfg were written at some install, PT_INTERP at some fixup. A
620+
// payload can be replaced afterwards -- upgraded, garbage-collected -- and
621+
// the record keeps naming what used to be there.
622+
//
623+
// That is not hypothetical. CI resolved `glibc@2.39` from a record while
624+
// the only payload on disk was 2.44, so the exact-match probe correctly
625+
// refused, no payload paths resolved, and the artifact fell through to the
626+
// host loader. A fossil that names something no longer present is not an
627+
// authority; it is just old.
628+
std::vector<std::string> installed;
629+
if (auto xpkgs = mcpp::xlings::paths::xpkgs_from_compiler(compilerBin)) {
630+
for (auto it = std::filesystem::directory_iterator(*xpkgs / "xim-x-glibc", ec);
631+
!ec && it != std::filesystem::directory_iterator{}; it.increment(ec)) {
632+
if (!it->is_directory(ec)) continue;
633+
auto v = it->path().filename().string();
634+
if (!v.empty() && v.front() != '.') installed.push_back(v);
635+
}
636+
}
637+
auto still_there = [&](const std::string& binding) {
638+
if (binding.empty()) return false;
639+
const auto at = binding.find('@');
640+
if (at == std::string::npos) return false;
641+
auto ver = binding.substr(at + 1);
642+
return std::ranges::find(installed, ver) != installed.end();
643+
};
644+
617645
auto read_file = [&](const std::filesystem::path& p) -> std::string {
618646
if (!std::filesystem::exists(p, ec)) return {};
619647
std::ifstream is(p);
@@ -624,15 +652,15 @@ std::string baked_runtime_binding(const std::filesystem::path& compilerBin) {
624652
// clang: the sibling <driver>.cfg.
625653
auto cfg = compilerBin.parent_path()
626654
/ (compilerBin.stem().string() + ".cfg");
627-
if (auto r = from_text(read_file(cfg)); !r.empty()) return r;
655+
if (auto r = from_text(read_file(cfg)); still_there(r)) return r;
628656

629657
// gcc: lib/gcc/<triple>/<ver>/specs, one level of globbing each.
630658
auto gccRoot = compilerBin.parent_path().parent_path();
631659
for (auto t = std::filesystem::directory_iterator(gccRoot / "lib" / "gcc", ec);
632660
!ec && t != std::filesystem::directory_iterator{}; t.increment(ec)) {
633661
for (auto v = std::filesystem::directory_iterator(t->path(), ec);
634662
!ec && v != std::filesystem::directory_iterator{}; v.increment(ec)) {
635-
if (auto r = from_text(read_file(v->path() / "specs")); !r.empty())
663+
if (auto r = from_text(read_file(v->path() / "specs")); still_there(r))
636664
return r;
637665
}
638666
}
@@ -649,7 +677,7 @@ std::string baked_runtime_binding(const std::filesystem::path& compilerBin) {
649677
// PT_INTERP is produced by the patchelf walk, which still runs on every
650678
// install, and it names the glibc payload this toolchain was aligned to --
651679
// the same fact the specs held, from a mechanism that has not gone away.
652-
if (auto r = from_text(read_elf_interp(compilerBin)); !r.empty())
680+
if (auto r = from_text(read_elf_interp(compilerBin)); still_there(r))
653681
return r;
654682

655683
// Last: the installed payload set, but ONLY when it is a singleton.
@@ -664,26 +692,17 @@ std::string baked_runtime_binding(const std::filesystem::path& compilerBin) {
664692
//
665693
// Two or more and this stays silent. That is the case the incident was,
666694
// and it is the case the subos must answer.
667-
if (auto xpkgs = mcpp::xlings::paths::xpkgs_from_compiler(compilerBin)) {
668-
std::vector<std::string> versions;
669-
for (auto it = std::filesystem::directory_iterator(*xpkgs / "xim-x-glibc", ec);
670-
!ec && it != std::filesystem::directory_iterator{}; it.increment(ec)) {
671-
if (!it->is_directory(ec)) continue;
672-
auto v = it->path().filename().string();
673-
if (!v.empty() && v.front() != '.') versions.push_back(v);
674-
}
675-
if (versions.size() == 1) {
676-
mcpp::log::verbose("probe", std::format(
677-
"runtime binding glibc@{} — the only glibc payload installed, "
678-
"so there is nothing to choose between", versions[0]));
679-
return "glibc@" + versions[0];
680-
}
681-
if (versions.size() > 1)
682-
mcpp::log::verbose("probe", std::format(
683-
"{} glibc payloads installed and nothing declares which one "
684-
"this build binds; declining rather than picking",
685-
versions.size()));
695+
if (installed.size() == 1) {
696+
mcpp::log::verbose("probe", std::format(
697+
"runtime binding glibc@{} — the only glibc payload installed, so "
698+
"there is nothing to choose between", installed[0]));
699+
return "glibc@" + installed[0];
686700
}
701+
if (installed.size() > 1)
702+
mcpp::log::verbose("probe", std::format(
703+
"{} glibc payloads installed and nothing still present declares "
704+
"which one this build binds; declining rather than picking",
705+
installed.size()));
687706
return {};
688707
}
689708

tests/unit/test_runtime_binding_fallback.cpp

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,53 @@ TEST(RuntimeBindingFallback, EmptyPathIsSilence) {
6666
EXPECT_EQ(tc::baked_runtime_binding({}), "");
6767
}
6868

69+
// A record that names a payload which is no longer installed.
70+
//
71+
// specs, cfg and PT_INTERP are all RECORDS of a past state: written at some
72+
// install or some fixup, and never revisited when the payload underneath is
73+
// replaced. CI hit exactly this -- a record said glibc@2.39 while the only
74+
// payload on disk was 2.44 -- and because the probe matches exactly and never
75+
// falls back, nothing resolved, no loader reached the link line, and the
76+
// artifact took the host's. A fossil naming something absent is not an
77+
// authority; it is just old, and the resolution must carry on past it.
78+
struct HomeWithSpecs {
79+
std::filesystem::path root, compiler;
80+
HomeWithSpecs(std::initializer_list<const char*> glibcVersions,
81+
std::string_view specsNames) {
82+
root = std::filesystem::temp_directory_path()
83+
/ std::format("mcpp_fossil_{}", std::random_device{}());
84+
auto xpkgs = root / "data" / "xpkgs";
85+
auto gccRoot = xpkgs / "xim-x-gcc" / "16.1.0";
86+
compiler = gccRoot / "bin" / "g++";
87+
std::filesystem::create_directories(compiler.parent_path());
88+
std::ofstream(compiler) << "not an elf";
89+
for (auto v : glibcVersions)
90+
std::filesystem::create_directories(xpkgs / "xim-x-glibc" / v / "lib64");
91+
auto sd = gccRoot / "lib" / "gcc" / "x86_64-linux-gnu" / "16.1.0";
92+
std::filesystem::create_directories(sd);
93+
std::ofstream(sd / "specs")
94+
<< "*link:\n%{!static:--dynamic-linker " << xpkgs.string()
95+
<< "/xim-x-glibc/" << specsNames << "/lib64/ld-linux-x86-64.so.2}\n";
96+
}
97+
~HomeWithSpecs() { std::error_code ec; std::filesystem::remove_all(root, ec); }
98+
};
99+
100+
TEST(RuntimeBindingFallback, RecordNamingAnAbsentPayloadIsSkipped) {
101+
HomeWithSpecs h{{"2.44"}, "2.39"};
102+
// Not "glibc@2.39": that payload is gone. The singleton rule answers.
103+
EXPECT_EQ(tc::baked_runtime_binding(h.compiler), "glibc@2.44");
104+
}
105+
106+
TEST(RuntimeBindingFallback, RecordNamingAPresentPayloadWins) {
107+
HomeWithSpecs h{{"2.39", "2.44"}, "2.39"};
108+
// Two installed, so the singleton rule cannot answer -- but the record
109+
// names one that IS there, and that is what the artifact would load.
110+
EXPECT_EQ(tc::baked_runtime_binding(h.compiler), "glibc@2.39");
111+
}
112+
113+
TEST(RuntimeBindingFallback, StaleRecordWithNothingToFallBackOnIsSilence) {
114+
HomeWithSpecs h{{"2.39", "2.44"}, "2.28"};
115+
EXPECT_EQ(tc::baked_runtime_binding(h.compiler), "");
116+
}
117+
69118
} // namespace

0 commit comments

Comments
 (0)