Skip to content

Commit cc146a6

Browse files
committed
fix: isolate private glibc gcc launcher
1 parent 7b5b7a8 commit cc146a6

6 files changed

Lines changed: 191 additions & 30 deletions

File tree

src/build/ninja_backend.cppm

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ std::unique_ptr<Backend> make_ninja_backend();
6464
std::string emit_ninja_string(const BuildPlan& plan);
6565
std::string filter_ninja_output(std::string_view output,
6666
std::span<const std::string> commandPrefixes);
67+
std::string compiler_launcher_contents(const std::filesystem::path& loader,
68+
const std::filesystem::path& compiler,
69+
const std::vector<std::filesystem::path>& dirs);
6770

6871
// Advice appended to a failed build whose linker output names a replaceable
6972
// function nothing in the graph defines. Empty when there is nothing to add.
@@ -81,6 +84,21 @@ std::string link_failure_advice(std::string_view output);
8184

8285
namespace mcpp::build {
8386

87+
std::string compiler_launcher_contents(const std::filesystem::path& loader,
88+
const std::filesystem::path& compiler,
89+
const std::vector<std::filesystem::path>& dirs) {
90+
std::string libraryPath;
91+
for (auto const& dir : dirs) {
92+
if (!libraryPath.empty()) libraryPath += ':';
93+
libraryPath += dir.string();
94+
}
95+
return std::format(
96+
"#!/bin/sh\nexec {} --library-path {} {} \"$@\"\n",
97+
mcpp::platform::shell::quote(loader.string()),
98+
mcpp::platform::shell::quote(libraryPath),
99+
mcpp::platform::shell::quote(compiler.string()));
100+
}
101+
84102
namespace {
85103

86104
std::string escape_ninja_path(const std::filesystem::path& p) {
@@ -288,25 +306,20 @@ const std::vector<std::filesystem::path>& compiler_invocation_dirs(const BuildPl
288306
}
289307

290308
bool needs_compiler_launcher(const BuildPlan& plan) {
291-
return mcpp::platform::is_linux && !compiler_invocation_dirs(plan).empty();
309+
return mcpp::platform::is_linux
310+
&& plan.toolchain.compiler == mcpp::toolchain::CompilerId::GCC
311+
&& !plan.toolchain.compilerInvocationLoader.empty();
292312
}
293313

294314
std::filesystem::path compiler_launcher_path(const BuildPlan& plan, bool cxx) {
295315
return plan.outputDir / (cxx ? "mcpp-cxx" : "mcpp-cc");
296316
}
297317

298318
void write_compiler_launcher(const std::filesystem::path& path,
319+
const std::filesystem::path& loader,
299320
const std::filesystem::path& compiler,
300321
const std::vector<std::filesystem::path>& dirs) {
301-
std::string libraryPath;
302-
for (auto const& dir : dirs) {
303-
if (!libraryPath.empty()) libraryPath += ':';
304-
libraryPath += dir.string();
305-
}
306-
write_file(path, std::format(
307-
"#!/bin/sh\nLD_LIBRARY_PATH={}\nexport LD_LIBRARY_PATH\nexec {} \"$@\"\n",
308-
mcpp::platform::shell::quote(libraryPath),
309-
mcpp::platform::shell::quote(compiler.string())));
322+
write_file(path, compiler_launcher_contents(loader, compiler, dirs));
310323

311324
std::error_code ec;
312325
std::filesystem::permissions(path, std::filesystem::perms::owner_exec,
@@ -627,8 +640,9 @@ std::string emit_ninja_string(const BuildPlan& plan) {
627640
const bool need_ios_init_shim = flags.needsStreamInitShim;
628641
auto compiler_command = [&](const std::filesystem::path& binary, bool cxx) {
629642
if (needs_compiler_launcher(plan))
630-
return escape_ninja_path(compiler_launcher_path(plan, cxx));
631-
return escape_ninja_path(binary);
643+
return shell_quote_arg(escape_ninja_chars(
644+
compiler_launcher_path(plan, cxx).string()));
645+
return shell_quote_arg(escape_ninja_chars(binary.string()));
632646
};
633647
append(std::format("cxx = {}\n", compiler_command(flags.cxxBinary, /*cxx=*/true)));
634648
// clang-scan-deps receives the driver after `--` as argv, not as a shell
@@ -2325,8 +2339,10 @@ std::expected<BuildResult, BuildError> NinjaBackend::build(const BuildPlan& plan
23252339
if (needs_compiler_launcher(plan)) {
23262340
const auto& dirs = compiler_invocation_dirs(plan);
23272341
write_compiler_launcher(compiler_launcher_path(plan, /*cxx=*/true),
2342+
plan.toolchain.compilerInvocationLoader,
23282343
flags.cxxBinary, dirs);
23292344
write_compiler_launcher(compiler_launcher_path(plan, /*cxx=*/false),
2345+
plan.toolchain.compilerInvocationLoader,
23302346
flags.ccBinary, dirs);
23312347
}
23322348
stage("write-compiler-launcher");

src/toolchain/detect.cppm

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ detect(const std::filesystem::path& explicit_compiler,
6161
tc.compilerRuntimeDirs = discover_compiler_runtime_dirs(tc.binaryPath);
6262
tc.compilerInvocationRuntimeDirs =
6363
discover_compiler_invocation_runtime_dirs(tc.binaryPath, runtimeBinding);
64+
auto loader = discover_compiler_invocation_loader(tc.binaryPath, runtimeBinding);
65+
if (!loader) return std::unexpected(loader.error());
66+
if (*loader)
67+
tc.compilerInvocationLoader = std::move(**loader);
6468
auto envPrefix = compiler_env_prefix(tc);
6569

6670
auto ver_r = run_capture(std::format("{}{} --version 2>&1",

src/toolchain/model.cppm

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,15 +103,21 @@ struct Toolchain {
103103
// Runtime directories inherited by Ninja and its whole process tree. Keep
104104
// private libc out of this list: Ninja launches each edge through /bin/sh.
105105
std::vector<std::filesystem::path> compilerRuntimeDirs;
106-
// Directories required when starting a compiler driver. On Linux this may
107-
// include the bound glibc payload, but it is applied inside each compiler
108-
// command after Ninja's shell has already started.
106+
// Directories required when starting a compiler driver. For native managed
107+
// GCC on Linux, the private loader below consumes these after Ninja's
108+
// shell has already started.
109109
std::vector<std::filesystem::path> compilerInvocationRuntimeDirs;
110+
// The private ELF loader for a native managed GCC and its bound glibc.
111+
// Empty for every other toolchain: they retain their ordinary driver
112+
// invocation. Keeping this separate from the runtime directories matters
113+
// because an LD_LIBRARY_PATH export reaches GCC's host as/ld children.
114+
std::filesystem::path compilerInvocationLoader;
110115
std::vector<std::filesystem::path> linkRuntimeDirs; // -L/-rpath dirs for produced binaries
111116
// Environment the toolchain's tools need when invoked (set on the ninja
112-
// process, inherited by compiler/linker children). Empty for GCC/Clang
113-
// (their LD_LIBRARY_PATH need goes through compilerRuntimeDirs); the
114-
// MSVC backend fills INCLUDE/LIB/PATH here (design §5.1).
117+
// process, inherited by compiler/linker children). Empty for GCC/Clang;
118+
// native managed GCC uses compilerInvocationLoader instead of exporting
119+
// its bound glibc. The MSVC backend fills INCLUDE/LIB/PATH here (design
120+
// §5.1).
115121
// (Own struct, not std::pair — GCC 16 modules choke on a std::pair
116122
// member added to this exported class: "failed to load pendings".)
117123
std::vector<EnvVar> envOverrides;

src/toolchain/probe.cppm

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ std::vector<std::filesystem::path>
3737
discover_compiler_invocation_runtime_dirs(const std::filesystem::path& compilerBin,
3838
std::string_view runtimeBinding);
3939

40+
std::expected<std::optional<std::filesystem::path>, DetectError>
41+
discover_compiler_invocation_loader(const std::filesystem::path& compilerBin,
42+
std::string_view runtimeBinding);
43+
4044
std::vector<std::filesystem::path>
4145
discover_link_runtime_dirs(const std::filesystem::path& compilerBin,
4246
std::string_view targetTriple);
@@ -107,6 +111,10 @@ bool is_native_managed_gcc(const std::filesystem::path& compilerBin) {
107111
return packageVersion.parent_path().filename() == "xim-x-gcc";
108112
}
109113

114+
bool is_elf_loader_name(std::string_view name) {
115+
return name.starts_with("ld-linux-") && name.find(".so") != std::string_view::npos;
116+
}
117+
110118
} // namespace
111119

112120
std::optional<std::filesystem::path>
@@ -247,6 +255,58 @@ discover_compiler_invocation_runtime_dirs(const std::filesystem::path& compilerB
247255
return dirs;
248256
}
249257

258+
std::expected<std::optional<std::filesystem::path>, DetectError>
259+
discover_compiler_invocation_loader(const std::filesystem::path& compilerBin,
260+
std::string_view runtimeBinding) {
261+
if constexpr (!mcpp::platform::is_linux) {
262+
return std::optional<std::filesystem::path>{};
263+
}
264+
265+
if (!runtimeBinding.starts_with("glibc@") || !is_native_managed_gcc(compilerBin))
266+
return std::optional<std::filesystem::path>{};
267+
268+
auto glibc = payload_root_for_binding(compilerBin, runtimeBinding);
269+
if (!glibc) {
270+
return std::unexpected(DetectError{std::format(
271+
"native managed GCC '{}' requires bound {} but its payload is unavailable",
272+
compilerBin.string(), std::string(runtimeBinding))});
273+
}
274+
275+
std::vector<std::filesystem::path> candidates;
276+
std::error_code ec;
277+
for (auto const& dir : {*glibc / "lib64", *glibc / "lib"}) {
278+
if (!std::filesystem::is_directory(dir, ec)) {
279+
ec.clear();
280+
continue;
281+
}
282+
for (std::filesystem::directory_iterator it(dir, ec), end; !ec && it != end;
283+
it.increment(ec)) {
284+
const auto& candidate = it->path();
285+
if (!is_elf_loader_name(candidate.filename().string())
286+
|| !std::filesystem::is_regular_file(candidate, ec)) {
287+
ec.clear();
288+
continue;
289+
}
290+
auto resolved = std::filesystem::weakly_canonical(candidate, ec);
291+
if (ec) {
292+
ec.clear();
293+
resolved = std::filesystem::absolute(candidate, ec);
294+
if (ec) resolved = candidate;
295+
}
296+
if (std::find(candidates.begin(), candidates.end(), resolved) == candidates.end())
297+
candidates.push_back(std::move(resolved));
298+
}
299+
ec.clear();
300+
}
301+
302+
if (candidates.size() != 1) {
303+
return std::unexpected(DetectError{std::format(
304+
"native managed GCC '{}' requires exactly one ld-linux-*.so* in bound {} payload; found {}",
305+
compilerBin.string(), std::string(runtimeBinding), candidates.size())});
306+
}
307+
return std::optional<std::filesystem::path>{candidates.front()};
308+
}
309+
250310
std::vector<std::filesystem::path>
251311
discover_link_runtime_dirs(const std::filesystem::path& compilerBin,
252312
std::string_view targetTriple) {
@@ -270,6 +330,11 @@ discover_link_runtime_dirs(const std::filesystem::path& compilerBin,
270330
}
271331

272332
std::string compiler_env_prefix(const Toolchain& tc) {
333+
if (!tc.compilerInvocationLoader.empty()) {
334+
return std::format("{} --library-path {} ",
335+
mcpp::xlings::shq(tc.compilerInvocationLoader.string()),
336+
mcpp::xlings::shq(join_colon_paths(tc.compilerInvocationRuntimeDirs)));
337+
}
273338
return env_prefix_for_dirs(tc.compilerInvocationRuntimeDirs.empty()
274339
? tc.compilerRuntimeDirs : tc.compilerInvocationRuntimeDirs);
275340
}

tests/unit/test_ninja_backend.cpp

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -797,7 +797,7 @@ TEST(NinjaBackend, ClangScanRuleWritesViaDashOWithoutShellRedirection) {
797797
<< "scan rule must not use shell redirection: " << rule;
798798
}
799799

800-
TEST(NinjaBackend, ClangScanUsesRawDriverWhenCompilerNeedsRuntimeLauncher) {
800+
TEST(NinjaBackend, ClangKeepsItsRawDriverWhenItHasRuntimeDirectories) {
801801
auto plan = minimal_plan();
802802
plan.toolchain.compiler = mcpp::toolchain::CompilerId::Clang;
803803
plan.toolchain.binaryPath = "/opt/xim-x-llvm/bin/clang++";
@@ -813,14 +813,9 @@ TEST(NinjaBackend, ClangScanUsesRawDriverWhenCompilerNeedsRuntimeLauncher) {
813813

814814
auto ninja = emit_ninja_string(plan);
815815

816-
if constexpr (mcpp::platform::is_linux) {
817-
EXPECT_NE(ninja.find("cxx = /tmp/mcpp-ninja-test/target/test/mcpp-cxx"),
818-
std::string::npos) << ninja;
819-
EXPECT_EQ(ninja.find("LD_LIBRARY_PATH"), std::string::npos) << ninja;
820-
} else {
821-
EXPECT_NE(ninja.find("cxx = /opt/xim-x-llvm/bin/clang++"), std::string::npos)
822-
<< ninja;
823-
}
816+
EXPECT_NE(ninja.find("cxx = /opt/xim-x-llvm/bin/clang++"), std::string::npos)
817+
<< ninja;
818+
EXPECT_EQ(ninja.find("mcpp-cxx"), std::string::npos) << ninja;
824819
EXPECT_NE(ninja.find("cxx_driver = /opt/xim-x-llvm/bin/clang++"), std::string::npos)
825820
<< ninja;
826821
auto scanRule = ninja.find("rule cxx_scan");
@@ -832,6 +827,51 @@ TEST(NinjaBackend, ClangScanUsesRawDriverWhenCompilerNeedsRuntimeLauncher) {
832827
EXPECT_EQ(rule.find("LD_LIBRARY_PATH"), std::string::npos) << rule;
833828
}
834829

830+
TEST(NinjaBackend, PrivateGccLauncherUsesItsLoaderWithoutExportingLibraryPath) {
831+
if constexpr (!mcpp::platform::is_linux)
832+
GTEST_SKIP() << "private ELF loaders are Linux-only";
833+
834+
auto script = compiler_launcher_contents(
835+
"/opt/xim-x-glibc/2.44/lib/ld-linux-x86-64.so.2",
836+
"/opt/xim-x-gcc/16.1.0/bin/g++",
837+
{"/opt/xim-x-glibc/2.44/lib", "/opt/xim-x-gcc/16.1.0/lib64"});
838+
839+
EXPECT_NE(script.find("exec '/opt/xim-x-glibc/2.44/lib/ld-linux-x86-64.so.2' --library-path "),
840+
std::string::npos) << script;
841+
EXPECT_NE(script.find("'/opt/xim-x-glibc/2.44/lib:/opt/xim-x-gcc/16.1.0/lib64' "),
842+
std::string::npos) << script;
843+
EXPECT_NE(script.find("'/opt/xim-x-gcc/16.1.0/bin/g++' \"$@\""), std::string::npos)
844+
<< script;
845+
EXPECT_EQ(script.find("LD_LIBRARY_PATH"), std::string::npos) << script;
846+
}
847+
848+
TEST(NinjaBackend, PrivateGccLauncherPathIsNinjaEscapedThenShellQuoted) {
849+
if constexpr (!mcpp::platform::is_linux)
850+
GTEST_SKIP() << "private ELF loaders are Linux-only";
851+
852+
auto plan = minimal_plan();
853+
plan.outputDir = "/tmp/mcpp output$dir:with'quote";
854+
plan.toolchain.compilerInvocationLoader =
855+
"/opt/xim-x-glibc/2.44/lib/ld-linux-x86-64.so.2";
856+
plan.toolchain.compilerInvocationRuntimeDirs = {"/opt/xim-x-glibc/2.44/lib"};
857+
plan.compileUnits.push_back({
858+
.source = "src/main.c",
859+
.kind = mcpp::SourceKind::C,
860+
.object = "obj/main.o",
861+
.packageName = "objc_rule_test",
862+
});
863+
864+
auto ninja = emit_ninja_string(plan);
865+
auto expectedCxx = shell_quote_arg(escape_ninja_chars(
866+
(plan.outputDir / "mcpp-cxx").string()));
867+
auto expectedCc = shell_quote_arg(escape_ninja_chars(
868+
(plan.outputDir / "mcpp-cc").string()));
869+
870+
EXPECT_NE(ninja.find("cxx = " + expectedCxx), std::string::npos) << ninja;
871+
EXPECT_NE(ninja.find("cc = " + expectedCc), std::string::npos) << ninja;
872+
EXPECT_EQ(ninja.find("LD_LIBRARY_PATH"), std::string::npos) << ninja;
873+
}
874+
835875
// The `cmd /c` wrapper was the only place mcpp put a shell between ninja and
836876
// a compiler invocation. Keep it gone: it is what re-imposes the 8191 ceiling.
837877
TEST(NinjaBackend, NoRuleWrapsItsCommandInCmdSlashC) {

tests/unit/test_toolchain_detect.cpp

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ TEST(ToolchainDetect, IgnoresTargetRuntimeLibraryPathDuringProbe) {
9494
EXPECT_EQ(tc->targetTriple, "x86_64-unknown-linux-gnu");
9595
}
9696

97-
TEST(ToolchainDetect, CompilerRuntimeDirsUseTheBoundGlibcPayload) {
97+
TEST(ToolchainDetect, NativeGccUsesTheBoundGlibcLoaderForProbes) {
9898
auto root = std::filesystem::temp_directory_path()
9999
/ std::format("mcpp_compiler_runtime_{}", std::random_device{}());
100100
TempDirGuard cleanup{root};
@@ -107,10 +107,29 @@ TEST(ToolchainDetect, CompilerRuntimeDirsUseTheBoundGlibcPayload) {
107107
std::filesystem::create_directories(wanted);
108108
std::filesystem::create_directories(other);
109109

110+
auto loader = wanted / "ld-linux-x86-64.so.2";
111+
auto trace = root / "loader.trace";
112+
std::ofstream loaderOs(loader);
113+
loaderOs << R"(#!/usr/bin/env bash
114+
if [[ "$1" != "--library-path" || "$2" != ")" << wanted.string() << R"(" ]]; then
115+
echo "bad loader invocation: $*" >&2
116+
exit 127
117+
fi
118+
printf '%s\n' "$*" >> ")" << trace.string() << R"("
119+
shift 3
120+
exec env -u LD_LIBRARY_PATH "$@"
121+
)";
122+
loaderOs.close();
123+
std::filesystem::permissions(
124+
loader,
125+
std::filesystem::perms::owner_exec
126+
| std::filesystem::perms::owner_read
127+
| std::filesystem::perms::owner_write);
128+
110129
std::ofstream os(compiler);
111130
os << R"(#!/usr/bin/env bash
112-
if [[ "${LD_LIBRARY_PATH:-}" != *")" << wanted.string() << R"("* ]]; then
113-
echo "missing bound glibc directory" >&2
131+
if [[ -n "${LD_LIBRARY_PATH:-}" ]]; then
132+
echo "probe inherited LD_LIBRARY_PATH" >&2
114133
exit 127
115134
fi
116135
case "$1" in
@@ -127,6 +146,7 @@ esac
127146

128147
auto tc = detect(compiler, "glibc@2.44");
129148
ASSERT_TRUE(tc.has_value()) << tc.error().message;
149+
EXPECT_EQ(tc->compilerInvocationLoader, loader);
130150
EXPECT_NE(std::find(tc->compilerInvocationRuntimeDirs.begin(),
131151
tc->compilerInvocationRuntimeDirs.end(), wanted),
132152
tc->compilerInvocationRuntimeDirs.end());
@@ -135,6 +155,13 @@ esac
135155
tc->compilerInvocationRuntimeDirs.end());
136156
EXPECT_EQ(std::find(tc->compilerRuntimeDirs.begin(), tc->compilerRuntimeDirs.end(), wanted),
137157
tc->compilerRuntimeDirs.end());
158+
EXPECT_EQ(compiler_env_prefix(*tc).find("LD_LIBRARY_PATH"), std::string::npos);
159+
160+
std::ifstream traceIn(trace);
161+
std::string traceText((std::istreambuf_iterator<char>(traceIn)), {});
162+
EXPECT_NE(traceText.find("--version"), std::string::npos) << traceText;
163+
EXPECT_NE(traceText.find("-dumpmachine"), std::string::npos) << traceText;
164+
EXPECT_NE(traceText.find("-print-sysroot"), std::string::npos) << traceText;
138165

139166
auto mingw = xpkgs / "xim-x-mingw-cross-gcc" / "16.1.0" / "bin"
140167
/ "x86_64-w64-mingw32-g++";
@@ -143,6 +170,9 @@ esac
143170

144171
auto mingwDirs = discover_compiler_invocation_runtime_dirs(mingw, "glibc@2.44");
145172
EXPECT_EQ(std::find(mingwDirs.begin(), mingwDirs.end(), wanted), mingwDirs.end());
173+
auto mingwLoader = discover_compiler_invocation_loader(mingw, "glibc@2.44");
174+
ASSERT_TRUE(mingwLoader.has_value()) << mingwLoader.error().message;
175+
EXPECT_FALSE(mingwLoader->has_value());
146176
}
147177
#endif // defined(__linux__)
148178

0 commit comments

Comments
 (0)