Skip to content

Commit 1238c9a

Browse files
committed
fix(build): the macOS ordering shim must not be able to break the link
Two Mach-O facts the first CI round found the hard way: * an __asm__ label is used VERBATIM — clang does not prepend Mach-O's global '_'. The C++ symbol _ZNSt3__18ios_base4InitC1Ev therefore has to be written __ZNSt3__18ios_base4InitC1Ev, and getting it wrong is not a silent no-op: every macOS link failed with 'undefined symbol: ZNSt3__18ios_base4InitC1Ev'. * plain __attribute__((weak)) on a declaration is NOT Mach-O's weak-undefined form, so it did not make the bad reference optional. weak_import is. Both are now correct, but neither is the safety net. The backend only generates the shim TU when the libc++ archive actually defines the symbol — checked by scanning the ranlib index in the archive's first member, no subprocess — so an unexpected libc++ spelling disables the ordering aid and says so, instead of failing the build. A check upstream of the reference cannot break a link the way the reference itself can.
1 parent c7c17e9 commit 1238c9a

3 files changed

Lines changed: 99 additions & 18 deletions

File tree

src/build/distribution.cppm

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,11 @@ struct MechanismInput {
164164
std::string libcxxArchive;
165165
std::string libcxxAbiArchive;
166166
std::string libunwindArchive;
167+
// macOS only: the libc++ archive actually defines the ABI symbol the
168+
// initializer-ordering shim binds to. Checked against the archive rather
169+
// than assumed, so an unexpected spelling disables the shim instead of
170+
// producing an undefined reference at link time.
171+
bool streamInitSymbolPresent = false;
167172
// macOS only: a deployment floor was resolved. The static-libc++
168173
// mechanism exists to make that floor real, so without one there is
169174
// nothing to make real.
@@ -259,7 +264,21 @@ Mechanism resolve(const MechanismInput& in) {
259264
m.unitFlags = " -nostdlib++"
260265
" -Wl,-load_hidden," + in.libcxxArchive +
261266
" -Wl,-load_hidden," + in.libcxxAbiArchive;
262-
m.streamInitShim = true;
267+
// The ordering shim binds a libc++ INTERNAL ABI symbol, so its
268+
// presence is verified against the archive instead of assumed.
269+
// Getting this wrong must not break the link — hence a check
270+
// here rather than a weak reference in the shim: Mach-O's
271+
// weak-undefined form is `weak_import` and applies to dylib
272+
// symbols, so a plain weak declaration would NOT have saved a
273+
// missing archive symbol (it did not: ld64.lld errored outright).
274+
m.streamInitShim = in.streamInitSymbolPresent;
275+
if (!m.streamInitShim) {
276+
m.diagnostic =
277+
"this libc++ does not export the stream initializer mcpp "
278+
"orders first on macOS; a global object whose constructor "
279+
"uses std::cout may crash at startup (mcpp#336). Use "
280+
"cxx_runtime = \"host-coupled\" if you hit it";
281+
}
263282
return m;
264283
}
265284
// HostCoupled
@@ -367,12 +386,22 @@ Mechanism resolve(const MechanismInput& in) {
367386
// WHY C: it needs no standard library, no module flags and no C++ ABI of its
368387
// own — it only has to run before everything else and poke one symbol.
369388
//
370-
// WHY WEAK: `_ZNSt3__18ios_base4InitC1Ev` is a libc++ ABI symbol
371-
// (_LIBCPP_EXPORTED_FROM_ABI), stable for many years but not ours. A weak
372-
// undefined reference means a toolchain that spells it differently links
373-
// exactly as it does today and the shim is a no-op, instead of failing the
374-
// link. It also means the shim does not itself drag iostream.cpp.o out of the
375-
// archive: if the program never touches a stream there is nothing to order.
389+
// WHY THE NAME IS SPELLED WITH TWO UNDERSCORES: an `__asm__` label is used
390+
// VERBATIM — clang does not add Mach-O's global `_` prefix to it. The C++
391+
// symbol `_ZNSt3__18ios_base4InitC1Ev` therefore has to be written
392+
// `__ZNSt3__18ios_base4InitC1Ev` here. Getting this wrong is not a silent
393+
// no-op: ld64.lld reports `undefined symbol: ZNSt3__18ios_base4InitC1Ev` and
394+
// every link fails, which is exactly what the first CI round did.
395+
//
396+
// WHY `weak_import` AND a presence check: Mach-O's weak-undefined form is
397+
// `weak_import` (plain `weak` on a declaration does NOT make an undefined
398+
// reference optional there). Even so, the real safety net is upstream — the
399+
// backend only generates this TU when the archive actually defines the
400+
// symbol, so an unexpected libc++ spelling disables the shim rather than
401+
// breaking the link. The attribute is the second line of defence.
402+
//
403+
// The reference does not itself drag iostream.cpp.o out of the archive: if the
404+
// program never touches a stream, there is nothing to order.
376405
//
377406
// WHY IT WORKS: libc++'s `ios_base::Init::Init()` is not empty — it is a
378407
// guarded function-local static that calls `DoIOSInit::DoIOSInit()`, and THAT
@@ -396,7 +425,8 @@ std::string_view stream_init_shim_source() {
396425
" * Weak: a toolchain without this exact ABI symbol links as before.\n"
397426
" */\n"
398427
"extern void mcpp_libcxx_ios_init(void *)\n"
399-
" __attribute__((weak)) __asm__(\"_ZNSt3__18ios_base4InitC1Ev\");\n"
428+
" __attribute__((weak_import))\n"
429+
" __asm__(\"__ZNSt3__18ios_base4InitC1Ev\");\n"
400430
"\n"
401431
"static char mcpp_libcxx_ios_init_storage[8];\n"
402432
"\n"

src/build/flags.cppm

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -564,12 +564,12 @@ CompileFlags compute_flags(const BuildPlan& plan) {
564564
// macOS packages) or under lib/<llvm-triple>/ (the Linux ones), so try
565565
// both rather than hard-coding one layout. Sorted so the choice cannot
566566
// depend on directory iteration order.
567-
auto find_archive = [&](std::string_view name) -> std::string {
567+
auto find_archive = [&](std::string_view name) -> std::filesystem::path {
568568
if (llvmRootForStdlib.empty()) return {};
569569
std::error_code ec;
570570
auto libDir = llvmRootForStdlib / "lib";
571571
auto direct = libDir / name;
572-
if (std::filesystem::exists(direct, ec)) return escape_path(direct);
572+
if (std::filesystem::exists(direct, ec)) return direct;
573573
std::vector<std::filesystem::path> hits;
574574
for (auto& e : std::filesystem::directory_iterator(libDir, ec)) {
575575
std::error_code de;
@@ -578,7 +578,28 @@ CompileFlags compute_flags(const BuildPlan& plan) {
578578
if (std::filesystem::exists(p, de)) hits.push_back(p);
579579
}
580580
std::ranges::sort(hits);
581-
return hits.empty() ? std::string{} : escape_path(hits.front());
581+
return hits.empty() ? std::filesystem::path{} : hits.front();
582+
};
583+
584+
// Does this archive define a given symbol? Answered by scanning the
585+
// ranlib symbol index, which a BSD/Mach-O archive keeps in its FIRST
586+
// member — so a bounded read of the head is enough and no toolchain
587+
// subprocess is involved. Used for exactly one thing: refusing to
588+
// generate the macOS ordering shim against a libc++ that does not
589+
// export the symbol it binds. That failure mode is not hypothetical —
590+
// the first CI round of #336 turned every macOS link into `undefined
591+
// symbol` — and a check here cannot fail the build the way a bad
592+
// reference in the generated TU can.
593+
auto archive_defines = [](const std::filesystem::path& p,
594+
std::string_view sym) -> bool {
595+
if (p.empty()) return false;
596+
std::ifstream is(p, std::ios::binary);
597+
if (!is) return false;
598+
constexpr std::streamsize kHead = 8 << 20;
599+
std::string head(static_cast<std::size_t>(kHead), '\0');
600+
is.read(head.data(), kHead);
601+
head.resize(static_cast<std::size_t>(is.gcount()));
602+
return head.find(sym) != std::string::npos;
582603
};
583604

584605
dist::MechanismInput mi;
@@ -601,12 +622,21 @@ CompileFlags compute_flags(const BuildPlan& plan) {
601622
|| testsContract == dist::Contract::SelfContained)
602623
&& caps.stdlib_id == "libc++";
603624
if (wantsArchives) {
604-
mi.libcxxArchive = find_archive("libc++.a");
605-
mi.libcxxAbiArchive = find_archive("libc++abi.a");
625+
auto libcxxA = find_archive("libc++.a");
626+
auto libcxxAbiA = find_archive("libc++abi.a");
627+
mi.libcxxArchive = libcxxA.empty() ? std::string{} : escape_path(libcxxA);
628+
mi.libcxxAbiArchive = libcxxAbiA.empty() ? std::string{} : escape_path(libcxxAbiA);
606629
// ELF only: without it the "self-contained" binary still pulls
607630
// libunwind.so.1. Mach-O's libc++abi.a carries its own unwinder.
608-
if (mi.format == dist::Format::Elf)
609-
mi.libunwindArchive = find_archive("libunwind.a");
631+
if (mi.format == dist::Format::Elf) {
632+
auto unwindA = find_archive("libunwind.a");
633+
if (!unwindA.empty()) mi.libunwindArchive = escape_path(unwindA);
634+
} else {
635+
// Searched without the leading underscore so the same needle
636+
// matches the ELF (`_ZN...`) and Mach-O (`__ZN...`) spellings.
637+
mi.streamInitSymbolPresent =
638+
archive_defines(libcxxA, "ZNSt3__18ios_base4InitC1Ev");
639+
}
610640
}
611641

612642
// "Explicit" = a human wrote it down. `static_stdlib = false` counts:

tests/unit/test_distribution.cpp

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ dist::MechanismInput macos_input() {
2222
in.macosFloor = true;
2323
in.libcxxArchive = "/tc/lib/libc++.a";
2424
in.libcxxAbiArchive = "/tc/lib/libc++abi.a";
25+
in.streamInitSymbolPresent = true;
2526
return in;
2627
}
2728

@@ -271,6 +272,7 @@ TEST(Distribution, TableIsTotalAndEveryDowngradeExplainsItself) {
271272
in.libcxxArchive = "/a.a";
272273
in.libcxxAbiArchive = "/b.a";
273274
in.libunwindArchive = "/c.a";
275+
in.streamInitSymbolPresent = true;
274276
}
275277
auto m = dist::resolve(in);
276278
auto where = std::format("contract={} format={} stdlib={} role={} archives={}",
@@ -298,16 +300,35 @@ TEST(Distribution, TableIsTotalAndEveryDowngradeExplainsItself) {
298300
// differently). See issue #336 for the disassembly this rests on.
299301
TEST(Distribution, StreamInitShimKeepsItsThreeLoadBearingProperties) {
300302
auto src = std::string(dist::stream_init_shim_source());
301-
// 1. weak — a toolchain without this ABI symbol must still link.
302-
EXPECT_NE(src.find("__attribute__((weak))"), std::string::npos);
303+
// 1. weak_import — Mach-O's weak-UNDEFINED form. Plain `weak` is not it;
304+
// the first CI round proved that by failing every macOS link.
305+
EXPECT_NE(src.find("__attribute__((weak_import))"), std::string::npos);
303306
// 2. ios_base::Init::Init, NOT DoIOSInit::DoIOSInit — the former is
304307
// guarded by __cxa_guard, so libc++'s own initializer later becomes a
305308
// no-op instead of placement-new'ing over live streams.
306-
EXPECT_NE(src.find("_ZNSt3__18ios_base4InitC1Ev"), std::string::npos);
309+
// ...spelled with TWO leading underscores: an __asm__ label is used
310+
// verbatim, so Mach-O's global `_` prefix has to be written out.
311+
EXPECT_NE(src.find("\"__ZNSt3__18ios_base4InitC1Ev\""), std::string::npos);
307312
EXPECT_EQ(src.find("DoIOSInit"), std::string::npos);
308313
// 3. a constructor, and it must be guarded by the weak null check.
309314
EXPECT_NE(src.find("__attribute__((constructor))"), std::string::npos);
310315
EXPECT_NE(src.find("if (mcpp_libcxx_ios_init)"), std::string::npos);
311316
// C, not C++: no standard library, no module flags, no ABI of its own.
312317
EXPECT_EQ(src.find("#include"), std::string::npos);
313318
}
319+
320+
// The shim binds a libc++ INTERNAL symbol. If that symbol is not in the
321+
// archive, mcpp must NOT generate the reference — an undefined symbol fails
322+
// the link outright (ld64.lld does not treat a weak declaration as an
323+
// optional undefined). The absence is reported instead, because the startup
324+
// hazard is still there.
325+
TEST(Distribution, ShimIsNotGeneratedWhenTheSymbolIsAbsent) {
326+
auto in = macos_input();
327+
in.streamInitSymbolPresent = false;
328+
auto m = dist::resolve(in);
329+
EXPECT_FALSE(m.streamInitShim);
330+
EXPECT_FALSE(m.diagnostic.empty());
331+
// The contract itself is still honored — only the ordering aid is gone.
332+
EXPECT_EQ(m.effective, dist::Contract::SelfContained);
333+
EXPECT_NE(m.unitFlags.find("-load_hidden"), std::string::npos);
334+
}

0 commit comments

Comments
 (0)