From 3070e5cfc60aec1711dd32b0ec5a152382ed8e42 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Thu, 12 Jun 2025 09:05:11 -0700 Subject: [PATCH] Try using getMainExecutable to address argv[0] limitations (#5643) When finding an executable, this validates that the returned binary is a symlink back to the same thing as /proc/self/exe, also using that as a fallback for different things. Looking back at #3912, we started using `findProgramByName` in order to avoid path canonicalization done by `GetMainExecutable`. That created issues as in #5096, wherein an `argv[0]` that's not explicit enough (`llvm-symbolizer` instead of the full path, done in [LLVM's Signals.cpp](https://github.com/llvm/llvm-project/blob/4f60f45130c6bd96c79e468fe9927a29af760f56/llvm/lib/Support/Signals.cpp#L198)) leads to incorrect results (finding an `llvm-symbolizer` in `$PATH`). One option to fix this would be to patch LLVM to provide an absolute path for `llvm-symbolizer`. However, I'll suggest that passing a filename in `argv[0]` is not terribly uncommon, and could be a migration limitation if we force it. The failure mode is also opaque; for example: ``` $ /bin/sh -c "exec -a llvm-symbolizer ./bazel-bin/toolchain/carbon" error: expected carbon-busybox symlink at `/usr/lib/llvm-19/bin/llvm-symbolizer` ``` Combined with the `setenv` of `LLVM_SYMBOLIZER_PATH` in `busybox_main.cpp`, this is intended to fix #5096. --- common/exe_path.cpp | 47 ++++++++-- common/exe_path.h | 7 +- common/exe_path_test.cpp | 21 +++-- toolchain/install/BUILD | 2 + toolchain/install/busybox_info.cpp | 101 +++++++++++++++++++++ toolchain/install/busybox_info.h | 70 +------------- toolchain/install/busybox_info_test.cpp | 116 ++++++++++++++---------- toolchain/install/busybox_main.cpp | 11 ++- 8 files changed, 244 insertions(+), 131 deletions(-) create mode 100644 toolchain/install/busybox_info.cpp diff --git a/common/exe_path.cpp b/common/exe_path.cpp index 4812f2fd42c8..9994c8637f73 100644 --- a/common/exe_path.cpp +++ b/common/exe_path.cpp @@ -4,6 +4,7 @@ #include "common/exe_path.h" +#include #include #include @@ -13,14 +14,48 @@ namespace Carbon { -auto FindExecutablePath(llvm::StringRef argv0) -> std::string { - if (!llvm::sys::fs::exists(argv0)) { - if (llvm::ErrorOr path = llvm::sys::findProgramByName(argv0)) { - return std::move(*path); - } +// Returns true if a found path resolves to the actual executable path. +static auto RealPathMatches(const char* found_path, llvm::StringRef exe_path) + -> bool { + char* buffer = realpath(found_path, nullptr); + if (!buffer) { + return false; + } + bool matches = exe_path == buffer; + free(buffer); + return matches; +} + +auto FindExecutablePath(const char* argv0) -> std::string { + static int static_for_main_addr; + // Note this returns the canonical path, dropping symlink information that we + // might want. As a consequence, we use it as a last resort. However, it's + // also helpful to use to ensure we found the correct tool through other + // means. + std::string exe_path = + llvm::sys::fs::getMainExecutable(argv0, &static_for_main_addr); + + llvm::StringRef argv0_ref = argv0; + + // If `argv[0]` is path-like and points at the executable, use the form in + // `argv[0]`. + if (argv0_ref.contains('/') && RealPathMatches(argv0, exe_path)) { + return argv0_ref.str(); } - return argv0.str(); + // If we can find `argv[0]` in `$PATH`, use the form from that. + // + // For example, `llvm-symbolizer` is subprocessed with `argv[0]` that uses + // this path. If `LLVM_SYMBOLIZER_PATH` is set to Carbon, but + // `llvm-symbolizer` in `$PATH` is a different binary, that can lead to + // problems -- which is why we verify the match. + if (llvm::ErrorOr path = llvm::sys::findProgramByName(argv0_ref); + path && RealPathMatches(path->c_str(), exe_path)) { + return std::move(*path); + } + + // As a fallback, use exe_path. + return exe_path; } } // namespace Carbon diff --git a/common/exe_path.h b/common/exe_path.h index 65685baac062..8bd10d69011b 100644 --- a/common/exe_path.h +++ b/common/exe_path.h @@ -9,12 +9,13 @@ namespace Carbon { -// Computes the executable path for the given `argv[0]` value form `main`. +// Computes the executable path for the given `argv[0]` value from `main`. +// `argv0` is required to be null-terminated. // // A simplistic approach -- if the provided string isn't already a valid path, // we look it up in the PATH environment variable. Doesn't resolve any symlinks -// and if it fails, simply returns the provided `argv0`. -auto FindExecutablePath(llvm::StringRef argv0) -> std::string; +// and if it fails, returns the main executable path. +auto FindExecutablePath(const char* argv0) -> std::string; } // namespace Carbon diff --git a/common/exe_path_test.cpp b/common/exe_path_test.cpp index 0be5080f6a2f..0a09b413ee21 100644 --- a/common/exe_path_test.cpp +++ b/common/exe_path_test.cpp @@ -17,21 +17,28 @@ namespace Carbon { namespace { TEST(ExePath, FailureFallback) { + static int static_for_main_addr; + std::string running_binary = + llvm::sys::fs::getMainExecutable("exe_path_test", &static_for_main_addr); + llvm::SmallString<128> path = llvm::StringRef(getenv("TEST_TMPDIR")); llvm::sys::path::append(path, "non_existant_binary"); - std::string exe_path = FindExecutablePath(path); - EXPECT_EQ(path, exe_path); + std::string exe_path = FindExecutablePath(path.c_str()); + EXPECT_EQ(running_binary, exe_path); } -TEST(ExePath, File) { +TEST(ExePath, Symlink) { + static int static_for_main_addr; + std::string running_binary = + llvm::sys::fs::getMainExecutable("exe_path_test", &static_for_main_addr); + llvm::SmallString<128> path = llvm::StringRef(getenv("TEST_TMPDIR")); llvm::sys::path::append(path, "test_binary"); - int fd = -1; - std::error_code ec = llvm::sys::fs::openFileForWrite(path, fd); + std::error_code ec; + std::filesystem::create_symlink(running_binary, path.c_str(), ec); ASSERT_TRUE(!ec) << "Error code: " << ec; - close(fd); - std::string exe_path = FindExecutablePath(path); + std::string exe_path = FindExecutablePath(path.c_str()); EXPECT_EQ(path, exe_path); } diff --git a/toolchain/install/BUILD b/toolchain/install/BUILD index a8a10fe29704..8fbfd343ddcb 100644 --- a/toolchain/install/BUILD +++ b/toolchain/install/BUILD @@ -78,9 +78,11 @@ cc_library( cc_library( name = "busybox_info", + srcs = ["busybox_info.cpp"], hdrs = ["busybox_info.h"], deps = [ "//common:error", + "//common:exe_path", "@llvm-project//llvm:Support", ], ) diff --git a/toolchain/install/busybox_info.cpp b/toolchain/install/busybox_info.cpp new file mode 100644 index 000000000000..dde460dd593a --- /dev/null +++ b/toolchain/install/busybox_info.cpp @@ -0,0 +1,101 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "toolchain/install/busybox_info.h" + +#include + +#include "common/exe_path.h" +#include "llvm/ADT/StringRef.h" + +namespace Carbon { + +// The mode is set to the initial filename used for `argv[0]`. +static auto GetMode(const std::filesystem::path& argv0) + -> std::optional { + std::string filename = argv0.filename(); + if (filename != "carbon" && filename != "carbon-busybox") { + return filename; + } + return std::nullopt; +} + +auto GetBusyboxInfo(const char* argv0) -> ErrorOr { + // Need storage due to `unsetenv` affecting `getenv` lifetime; using `path` + // for `GetMode`. + std::filesystem::path argv0_path = argv0; + + // Check for an override of `argv[0]` from the environment and apply it. + if (const char* argv0_override = getenv(Argv0OverrideEnv)) { + argv0_path = argv0_override; + unsetenv(Argv0OverrideEnv); + } + + BusyboxInfo info = {.bin_path = FindExecutablePath(argv0_path.c_str()), + .mode = GetMode(argv0_path)}; + + if (info.bin_path.filename() == "carbon-busybox") { + // Check for bazel structure. For example, this makes work: + // /bin/sh -c "exec -a carbon ./bazel-bin/toolchain/carbon" + // /bin/sh -c "exec -a llvm-symbolizer ./bazel-bin/toolchain/carbon" + if (std::filesystem::exists(info.bin_path.string() + ".runfiles")) { + std::string prefix_root = info.bin_path.parent_path().string() + + "/prefix_root/lib/carbon/carbon-busybox"; + if (std::filesystem::exists(prefix_root)) { + info.bin_path = prefix_root; + } + } + return info; + } + + // Now search through any symlinks to locate the installed busybox binary. + while (true) { + // If we've not already reached the busybox, look for it relative to the + // current binary path. This can help more immediately locate an + // installation tree, and avoids walking through a final layer of symlinks + // which may point to content-addressed storage or other parts of a build + // output tree. + // + // We break this into two cases we need to handle: + // - Carbon's CLI will be: `/bin/carbon` + // - Other tools will be: `/lib/carbon//bin/` + // + // We also check that the current path is within a `bin` directory to + // provide best-effort checking for accidentally walking up from symlinks + // that aren't within an installation-shaped tree. + auto parent_path = info.bin_path.parent_path(); + // Strip any `.` path components at the end to simplify processing. + while (parent_path.filename() == ".") { + parent_path = parent_path.parent_path(); + } + if (parent_path.filename() == "bin") { + auto lib_path = info.bin_path.filename() == "carbon" + ? parent_path / ".." / "lib" / "carbon" + : parent_path / ".." / ".."; + auto busybox_path = lib_path / "carbon-busybox"; + std::error_code ec; + if (std::filesystem::exists(busybox_path, ec)) { + info.bin_path = busybox_path; + return info; + } + } + + // Try to walk through another layer of symlinks and see if we can find the + // installation there or are linked directly to the busybox. + std::error_code ec; + auto symlink_target = std::filesystem::read_symlink(info.bin_path, ec); + if (ec) { + return ErrorBuilder() + << "expected carbon-busybox symlink at `" << info.bin_path << "`"; + } + + // Do a path join, to handle relative symlinks. + info.bin_path = parent_path / symlink_target; + if (info.bin_path.filename() == "carbon-busybox") { + return info; + } + } +} + +} // namespace Carbon diff --git a/toolchain/install/busybox_info.h b/toolchain/install/busybox_info.h index 926350e9e2ef..b1a84b86816b 100644 --- a/toolchain/install/busybox_info.h +++ b/toolchain/install/busybox_info.h @@ -6,16 +6,16 @@ #define CARBON_TOOLCHAIN_INSTALL_BUSYBOX_INFO_H_ #include -#include #include #include #include "common/error.h" -#include "llvm/ADT/StringRef.h" namespace Carbon { -constexpr const char* Argv0OverrideEnv = "CARBON_ARGV0_OVERRIDE"; +// An optional override of argv0, particularly used by `//toolchain/carbon` to +// get desired behavior without further special-casing. +inline constexpr const char* Argv0OverrideEnv = "CARBON_ARGV0_OVERRIDE"; struct BusyboxInfo { // The path to `carbon-busybox`. @@ -33,69 +33,7 @@ struct BusyboxInfo { // `lib/carbon/carbon-busybox` within that install. // // If unable to locate a plausible busybox binary, returns an error instead. -inline auto GetBusyboxInfo(llvm::StringRef argv0) -> ErrorOr { - // Check for an override of `argv[0]` from the environment and apply it. - if (const char* argv0_override = getenv(Argv0OverrideEnv)) { - argv0 = argv0_override; - } - - BusyboxInfo info = {.bin_path = argv0.str(), .mode = std::nullopt}; - std::filesystem::path filename = info.bin_path.filename(); - // The mode is set to the initial filename used for `argv[0]`. - if (filename != "carbon" && filename != "carbon-busybox") { - info.mode = filename; - } - - // Now search through any symlinks to locate the installed busybox binary. - while (true) { - filename = info.bin_path.filename(); - if (filename == "carbon-busybox") { - return info; - } - - // If we've not already reached the busybox, look for it relative to the - // current binary path. This can help more immediately locate an - // installation tree, and avoids walking through a final layer of symlinks - // which may point to content-addressed storage or other parts of a build - // output tree. - // - // We break this into two cases we need to handle: - // - Carbon's CLI will be: `/bin/carbon` - // - Other tools will be: `/lib/carbon//bin/` - // - // We also check that the current path is within a `bin` directory to - // provide best-effort checking for accidentally walking up from symlinks - // that aren't within an installation-shaped tree. - auto parent_path = info.bin_path.parent_path(); - // Strip any `.` path components at the end to simplify processing. - while (parent_path.filename() == ".") { - parent_path = parent_path.parent_path(); - } - if (parent_path.filename() == "bin") { - auto lib_path = filename == "carbon" - ? parent_path / ".." / "lib" / "carbon" - : parent_path / ".." / ".."; - auto busybox_path = lib_path / "carbon-busybox"; - std::error_code ec; - if (std::filesystem::exists(busybox_path, ec)) { - info.bin_path = busybox_path; - return info; - } - } - - // Try to walk through another layer of symlinks and see if we can find the - // installation there or are linked directly to the busybox. - std::error_code ec; - auto symlink_target = std::filesystem::read_symlink(info.bin_path, ec); - if (ec) { - return ErrorBuilder() - << "expected carbon-busybox symlink at `" << info.bin_path << "`"; - } - - // Do a path join, to handle relative symlinks. - info.bin_path = parent_path / symlink_target; - } -} +auto GetBusyboxInfo(const char* argv0) -> ErrorOr; } // namespace Carbon diff --git a/toolchain/install/busybox_info_test.cpp b/toolchain/install/busybox_info_test.cpp index dd8010c73815..188e58ab2cd0 100644 --- a/toolchain/install/busybox_info_test.cpp +++ b/toolchain/install/busybox_info_test.cpp @@ -14,6 +14,7 @@ #include "common/check.h" #include "llvm/ADT/ScopeExit.h" +#include "llvm/Support/FileSystem.h" namespace Carbon { namespace { @@ -22,13 +23,18 @@ using ::testing::Eq; class BusyboxInfoTest : public ::testing::Test { public: - // Set up a temp directory for the test case. explicit BusyboxInfoTest() { + // Set up a temp directory for the test case. const char* tmpdir = std::getenv("TEST_TMPDIR"); CARBON_CHECK(tmpdir); dir_ = MakeDir(std::filesystem::absolute( std::filesystem::path(tmpdir) / ::testing::UnitTest::GetInstance()->current_test_info()->name())); + + // Most tests need the running binary for `MakeBusyboxFile`. + static int static_for_main_addr; + running_binary_ = llvm::sys::fs::getMainExecutable("busybox_info_test", + &static_for_main_addr); } // Delete the test case's temp directory. @@ -74,7 +80,7 @@ class BusyboxInfoTest : public ::testing::Test { if (busybox_target) { MakeSymlink(prefix / "lib/carbon/carbon-busybox", *busybox_target); } else { - MakeFile(prefix / "lib/carbon/carbon-busybox"); + MakeBusyboxFile(prefix / "lib/carbon/carbon-busybox"); } MakeDir(prefix / "lib/carbon/llvm/bin"); MakeSymlink(prefix / "lib/carbon/llvm/bin/clang++", "clang"); @@ -84,93 +90,108 @@ class BusyboxInfoTest : public ::testing::Test { return prefix; } + // Makes a fake busybox file. This is a symlink to the running binary because + // we validate to make sure the running binary is one and the same. + auto MakeBusyboxFile(std::filesystem::path file) -> std::filesystem::path { + return MakeSymlink(file, running_binary_); + } + + // The path to the running binary, `busybox_info_test`. This is provided + // because `GetExecutablePath` can fall back to it. + std::string running_binary_; + // The test's temp directory, deleted on destruction. std::filesystem::path dir_; }; TEST_F(BusyboxInfoTest, Direct) { - auto busybox = MakeFile(dir_ / "carbon-busybox"); + auto busybox = MakeBusyboxFile(dir_ / "carbon-busybox"); - auto info = GetBusyboxInfo(busybox.string()); + auto info = GetBusyboxInfo(busybox.c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(busybox)); EXPECT_THAT(info->mode, Eq(std::nullopt)); } TEST_F(BusyboxInfoTest, SymlinkInCurrentDirectory) { - MakeFile(dir_ / "carbon-busybox"); + MakeBusyboxFile(dir_ / "carbon-busybox"); auto target = MakeSymlink(dir_ / "carbon", "carbon-busybox"); - auto info = GetBusyboxInfo(target.string()); + auto info = GetBusyboxInfo(target.c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(dir_ / "carbon-busybox")); EXPECT_THAT(info->mode, Eq(std::nullopt)); } TEST_F(BusyboxInfoTest, SymlinkInCurrentDirectoryWithDot) { - MakeFile(dir_ / "carbon-busybox"); + MakeBusyboxFile(dir_ / "carbon-busybox"); auto target = MakeSymlink(dir_ / "carbon", "./carbon-busybox"); - auto info = GetBusyboxInfo(target.string()); + auto info = GetBusyboxInfo(target.c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(dir_ / "./carbon-busybox")); EXPECT_THAT(info->mode, Eq(std::nullopt)); } TEST_F(BusyboxInfoTest, ExtraSymlink) { - MakeFile(dir_ / "carbon-busybox"); + MakeBusyboxFile(dir_ / "carbon-busybox"); MakeSymlink(dir_ / "c", "carbon-busybox"); auto target = MakeSymlink(dir_ / "carbon", "c"); - auto info = GetBusyboxInfo(target.string()); + auto info = GetBusyboxInfo(target.c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(dir_ / "carbon-busybox")); EXPECT_THAT(info->mode, Eq(std::nullopt)); } TEST_F(BusyboxInfoTest, OriginalSymlinkNameFormsMode) { - MakeFile(dir_ / "carbon-busybox"); + MakeBusyboxFile(dir_ / "carbon-busybox"); MakeSymlink(dir_ / "carbon", "carbon-busybox"); auto clang_target = MakeSymlink(dir_ / "clang", "carbon"); auto clang_plusplus_target = MakeSymlink(dir_ / "clang++", "clang"); - auto info = GetBusyboxInfo(clang_target.string()); + auto info = GetBusyboxInfo(clang_target.c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(dir_ / "carbon-busybox")); EXPECT_THAT(info->mode, Eq("clang")); - info = GetBusyboxInfo(clang_plusplus_target.string()); + info = GetBusyboxInfo(clang_plusplus_target.c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(dir_ / "carbon-busybox")); EXPECT_THAT(info->mode, Eq("clang++")); } -TEST_F(BusyboxInfoTest, BusyboxIsSymlink) { - MakeFile(dir_ / "actual-busybox"); - auto target = MakeSymlink(dir_ / "carbon-busybox", "actual-busybox"); - - auto info = GetBusyboxInfo(target.string()); - ASSERT_TRUE(info.ok()) << info.error(); - EXPECT_THAT(info->bin_path, Eq(target)); - EXPECT_THAT(info->mode, Eq(std::nullopt)); -} - TEST_F(BusyboxInfoTest, BusyboxIsSymlinkToNowhere) { auto target = MakeSymlink(dir_ / "carbon-busybox", "nonexistent"); - auto info = GetBusyboxInfo(target.string()); - ASSERT_TRUE(info.ok()) << info.error(); - EXPECT_THAT(info->bin_path, Eq(dir_ / "carbon-busybox")); - EXPECT_THAT(info->mode, Eq(std::nullopt)); + auto info = GetBusyboxInfo(target.c_str()); + ASSERT_FALSE(info.ok()); + EXPECT_THAT(info.error().message(), + Eq(llvm::formatv("expected carbon-busybox symlink at `{0}`", + running_binary_) + .str())); +} + +TEST_F(BusyboxInfoTest, BusyboxIsWrongFile) { + // This has the correct name, but it doesn't map back to the running binary + // and so is ignored. + auto target = MakeFile(dir_ / "carbon-busybox"); + + auto info = GetBusyboxInfo(target.c_str()); + ASSERT_FALSE(info.ok()); + EXPECT_THAT(info.error().message(), + Eq(llvm::formatv("expected carbon-busybox symlink at `{0}`", + running_binary_) + .str())); } TEST_F(BusyboxInfoTest, RelativeSymlink) { MakeDir(dir_ / "dir1"); - MakeFile(dir_ / "dir1/carbon-busybox"); + MakeBusyboxFile(dir_ / "dir1/carbon-busybox"); MakeDir(dir_ / "dir2"); auto target = MakeSymlink(dir_ / "dir2/carbon", "../dir1/carbon-busybox"); - auto info = GetBusyboxInfo(target.string()); + auto info = GetBusyboxInfo(target.c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(dir_ / "dir2/../dir1/carbon-busybox")); EXPECT_THAT(info->mode, Eq(std::nullopt)); @@ -178,12 +199,12 @@ TEST_F(BusyboxInfoTest, RelativeSymlink) { TEST_F(BusyboxInfoTest, AbsoluteSymlink) { MakeDir(dir_ / "dir1"); - auto busybox = MakeFile(dir_ / "dir1/carbon-busybox"); + auto busybox = MakeBusyboxFile(dir_ / "dir1/carbon-busybox"); ASSERT_TRUE(busybox.is_absolute()); MakeDir(dir_ / "dir2"); auto target = MakeSymlink(dir_ / "dir2/carbon", busybox); - auto info = GetBusyboxInfo(target.string()); + auto info = GetBusyboxInfo(target.c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(busybox)); EXPECT_THAT(info->mode, Eq(std::nullopt)); @@ -192,7 +213,7 @@ TEST_F(BusyboxInfoTest, AbsoluteSymlink) { TEST_F(BusyboxInfoTest, NotBusyboxFile) { auto target = MakeFile(dir_ / "file"); - auto info = GetBusyboxInfo(target.string()); + auto info = GetBusyboxInfo(target.c_str()); EXPECT_FALSE(info.ok()); } @@ -200,30 +221,30 @@ TEST_F(BusyboxInfoTest, NotBusyboxSymlink) { MakeFile(dir_ / "file"); auto target = MakeSymlink(dir_ / "carbon", "file"); - auto info = GetBusyboxInfo(target.string()); + auto info = GetBusyboxInfo(target.c_str()); EXPECT_FALSE(info.ok()); } TEST_F(BusyboxInfoTest, LayerSymlinksInstallTree) { - auto actual_busybox = MakeFile(dir_ / "actual-busybox"); + auto actual_busybox = MakeBusyboxFile(dir_ / "actual-busybox"); // Create a facsimile of the install prefix with even the busybox as a // symlink. Also include potential relative sibling symlinks like `clang++` to // `clang`. auto prefix = MakeInstallTree(dir_ / "test_prefix", actual_busybox); - auto info = GetBusyboxInfo((prefix / "bin/carbon").string()); + auto info = GetBusyboxInfo((prefix / "bin/carbon").c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(prefix / "bin/../lib/carbon/carbon-busybox")); EXPECT_THAT(info->mode, Eq(std::nullopt)); - info = GetBusyboxInfo((prefix / "lib/carbon/llvm/bin/clang").string()); + info = GetBusyboxInfo((prefix / "lib/carbon/llvm/bin/clang").c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(prefix / "lib/carbon/llvm/bin/../../carbon-busybox")); EXPECT_THAT(info->mode, Eq("clang")); - info = GetBusyboxInfo((prefix / "lib/carbon/llvm/bin/clang++").string()); + info = GetBusyboxInfo((prefix / "lib/carbon/llvm/bin/clang++").c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(prefix / "lib/carbon/llvm/bin/../../carbon-busybox")); @@ -237,7 +258,7 @@ TEST_F(BusyboxInfoTest, StopSearchAtFirstSymlinkWithRelativeBusybox) { // A second install, but with its symlinks pointing into the `opt` tree rather // than at its busybox. MakeDir(dir_ / "lib/carbon"); - MakeFile(dir_ / "lib/carbon/carbon-busybox"); + MakeBusyboxFile(dir_ / "lib/carbon/carbon-busybox"); MakeDir(dir_ / "bin"); auto target = MakeSymlink(dir_ / "bin/carbon", "../opt/bin/carbon"); MakeDir(dir_ / "lib/carbon/llvm/bin"); @@ -246,10 +267,10 @@ TEST_F(BusyboxInfoTest, StopSearchAtFirstSymlinkWithRelativeBusybox) { // Starting from the second install uses the relative busybox rather than // traversing the symlink further. - auto info = GetBusyboxInfo(target.string()); + auto info = GetBusyboxInfo(target.c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(dir_ / "bin/../lib/carbon/carbon-busybox")); - info = GetBusyboxInfo(clang_target.string()); + info = GetBusyboxInfo(clang_target.c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(dir_ / "lib/carbon/llvm/bin/../../carbon-busybox")); @@ -271,7 +292,7 @@ TEST_F(BusyboxInfoTest, RejectSymlinkInUnrelatedInstall) { // Check that the busybox doesn't use the relative busybox in this case, and // walks the symlink to find the correct installation. - auto info = GetBusyboxInfo(stray_target.string()); + auto info = GetBusyboxInfo(stray_target.c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(dir_ / "usr/local/bin/../lib/carbon/carbon-busybox")); @@ -281,7 +302,7 @@ TEST_F(BusyboxInfoTest, RejectSymlinkInUnrelatedInstall) { // Check that the busybox doesn't use the relative busybox in this case, and // walks the symlink to find the correct installation. - info = GetBusyboxInfo(stray_target.string()); + info = GetBusyboxInfo(stray_target.c_str()); ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(dir_ / "usr/local/bin/../lib/carbon/carbon-busybox")); @@ -290,14 +311,17 @@ TEST_F(BusyboxInfoTest, RejectSymlinkInUnrelatedInstall) { TEST_F(BusyboxInfoTest, EnvBinaryPathOverride) { // The test should not have this environment variable set. ASSERT_THAT(getenv(Argv0OverrideEnv), Eq(nullptr)); - // Clean up this environment variable when this test finishes. - auto _ = llvm::make_scope_exit([] { unsetenv(Argv0OverrideEnv); }); // Set the environment to our actual busybox. - auto busybox = MakeFile(dir_ / "carbon-busybox"); - setenv(Argv0OverrideEnv, busybox.c_str(), /*overwrite=*/1); + auto busybox = MakeBusyboxFile(dir_ / "carbon-busybox"); + setenv(Argv0OverrideEnv, busybox.c_str(), /*overwrite=*/1); auto info = GetBusyboxInfo("/some/nonexistent/path"); + if (getenv(Argv0OverrideEnv)) { + unsetenv(Argv0OverrideEnv); + ADD_FAILURE() << "GetBusyboxInfo should unset Argv0OverrideEnv"; + } + ASSERT_TRUE(info.ok()) << info.error(); EXPECT_THAT(info->bin_path, Eq(busybox)); EXPECT_THAT(info->mode, Eq(std::nullopt)); diff --git a/toolchain/install/busybox_main.cpp b/toolchain/install/busybox_main.cpp index 5986369dab08..380f94b25115 100644 --- a/toolchain/install/busybox_main.cpp +++ b/toolchain/install/busybox_main.cpp @@ -9,7 +9,6 @@ #include "common/bazel_working_dir.h" #include "common/error.h" -#include "common/exe_path.h" #include "common/init_llvm.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" @@ -26,8 +25,7 @@ static auto Main(int argc, char** argv) -> ErrorOr { InitLLVM init_llvm(argc, argv); // Start by resolving any symlinks. - CARBON_ASSIGN_OR_RETURN(auto busybox_info, - GetBusyboxInfo(FindExecutablePath(argv[0]))); + CARBON_ASSIGN_OR_RETURN(auto busybox_info, GetBusyboxInfo(argv[0])); auto fs = llvm::vfs::getRealFileSystem(); @@ -38,6 +36,13 @@ static auto Main(int argc, char** argv) -> ErrorOr { return Error(*install_paths.error()); } + // If `LLVM_SYMBOLIZER_PATH` is unset, sets it. Signals.cpp would do some more + // path resolution which this overrides in favor of using the busybox itself + // for symbolization. + setenv("LLVM_SYMBOLIZER_PATH", + (install_paths.llvm_install_bin() + "llvm-symbolizer").c_str(), + /*overwrite=*/0); + SetWorkingDirForBazel(); llvm::SmallVector args;