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

@@ -4,6 +4,7 @@
 
 #include "common/exe_path.h"
 
+#include <optional>
 #include <string>
 #include <utility>
 
@@ -13,14 +14,48 @@
 
 namespace Carbon {
 
-auto FindExecutablePath(llvm::StringRef argv0) -> std::string {
-  if (!llvm::sys::fs::exists(argv0)) {
-    if (llvm::ErrorOr<std::string> 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();
+  }
+
+  // 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<std::string> path = llvm::sys::findProgramByName(argv0_ref);
+      path && RealPathMatches(path->c_str(), exe_path)) {
+    return std::move(*path);
   }
 
-  return argv0.str();
+  // As a fallback, use exe_path.
+  return exe_path;
 }
 
 }  // namespace Carbon

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
 

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);
 }
 

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",
     ],
 )

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 <iterator>
+
+#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> {
+  std::string filename = argv0.filename();
+  if (filename != "carbon" && filename != "carbon-busybox") {
+    return filename;
+  }
+  return std::nullopt;
+}
+
+auto GetBusyboxInfo(const char* argv0) -> ErrorOr<BusyboxInfo> {
+  // 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: `<prefix>/bin/carbon`
+    // - Other tools will be: `<prefix>/lib/carbon/<group>/bin/<tool>`
+    //
+    // 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

toolchain/install/busybox_info.h

@@ -6,16 +6,16 @@
 #define CARBON_TOOLCHAIN_INSTALL_BUSYBOX_INFO_H_
 
 #include <filesystem>
-#include <iterator>
 #include <optional>
 #include <string>
 
 #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<BusyboxInfo> {
-  // 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: `<prefix>/bin/carbon`
-    // - Other tools will be: `<prefix>/lib/carbon/<group>/bin/<tool>`
-    //
-    // 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<BusyboxInfo>;
 
 }  // namespace Carbon
 

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));

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<int> {
   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<int> {
     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<llvm::StringRef> args;