Skip to content

Commit

Permalink
Fix oso_prefix paths (#347)
Browse files Browse the repository at this point in the history
When bazel executions actions in the sandbox, pwd is a path like this:

```
/private/var/tmp/_bazel_ksmiley/92410d0e3373f265902564b48493ac12/sandbox/darwin-sandbox/8/execroot/_main
```

Inside this directory are the inputs, which are symlinks to outside of
this pwd:

```
/private/var/tmp/_bazel_ksmiley/92410d0e3373f265902564b48493ac12/sandbox/darwin-sandbox/1/execroot/_main/bazel-out/darwin_arm64-dbg/bin/_objs/main/main.o -> /private/var/tmp/_bazel_ksmiley/92410d0e3373f265902564b48493ac12/execroot/_main/bazel-out/darwin_arm64-dbg/bin/_objs/main/main.o
```

ld64 resolves the absolute path to this object file, including resolving
symlinks, _before_ it strips the -oso_prefix argument. Because of this
the previous `__BAZEL_EXECUTION_ROOT__` replacement, which was the
sandbox pwd, isn't actually a prefix of the canonicalized object file
path, leading to -oso_prefix being a no-op, and absolute paths ending up
in the binary.

I think many folks didn't see this locally because it's common for Apple
projects to disable sandboxing for local development for perf reasons,
and that's when you're likely to have this debug info in the binary in
the first place.

With this change we add a new substitution that calculates the
sandbox-independent prefix and passes that along instead.
  • Loading branch information
keith authored Sep 30, 2024
1 parent 2241cae commit 8ee7a2d
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 10 deletions.
2 changes: 1 addition & 1 deletion crosstool/cc_toolchain_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2145,7 +2145,7 @@ please file an issue at https://github.com/bazelbuild/apple_support/issues/new
flag_sets = [
flag_set(
actions = _DYNAMIC_LINK_ACTIONS,
flag_groups = [flag_group(flags = ["-Wl,-oso_prefix,__BAZEL_EXECUTION_ROOT__/"])],
flag_groups = [flag_group(flags = ["-Wl,-oso_prefix,__BAZEL_EXECUTION_ROOT_NO_SANDBOX__/"])],
),
],
)
Expand Down
35 changes: 27 additions & 8 deletions crosstool/wrapped_clang.cc
Original file line number Diff line number Diff line change
Expand Up @@ -259,12 +259,14 @@ static std::unique_ptr<TempFile> WriteResponseFile(

void ProcessArgument(const std::string arg, const std::string developer_dir,
const std::string sdk_root, const std::string cwd,
bool relative_ast_path, std::string &linked_binary,
std::string &dsym_path, std::string toolchain_path,
const std::string nosandbox_root, bool relative_ast_path,
std::string &linked_binary, std::string &dsym_path,
std::string toolchain_path,
std::function<void(const std::string &)> consumer);

bool ProcessResponseFile(const std::string arg, const std::string developer_dir,
const std::string sdk_root, const std::string cwd,
const std::string nosandbox_root,
bool relative_ast_path, std::string &linked_binary,
std::string &dsym_path, std::string toolchain_path,
std::function<void(const std::string &)> consumer) {
Expand All @@ -280,7 +282,7 @@ bool ProcessResponseFile(const std::string arg, const std::string developer_dir,
// Arguments in response files might be quoted/escaped, so we need to
// unescape them ourselves.
ProcessArgument(Unescape(arg_from_file), developer_dir, sdk_root, cwd,
relative_ast_path, linked_binary, dsym_path,
nosandbox_root, relative_ast_path, linked_binary, dsym_path,
toolchain_path, consumer);
}

Expand Down Expand Up @@ -335,12 +337,13 @@ std::string GetToolchainPath(const std::string &toolchain_id) {

void ProcessArgument(const std::string arg, const std::string developer_dir,
const std::string sdk_root, const std::string cwd,
bool relative_ast_path, std::string &linked_binary,
std::string &dsym_path, std::string toolchain_path,
const std::string nosandbox_root, bool relative_ast_path,
std::string &linked_binary, std::string &dsym_path,
std::string toolchain_path,
std::function<void(const std::string &)> consumer) {
auto new_arg = arg;
if (arg[0] == '@') {
if (ProcessResponseFile(arg, developer_dir, sdk_root, cwd,
if (ProcessResponseFile(arg, developer_dir, sdk_root, cwd, nosandbox_root,
relative_ast_path, linked_binary, dsym_path,
toolchain_path, consumer)) {
return;
Expand All @@ -355,6 +358,8 @@ void ProcessArgument(const std::string arg, const std::string developer_dir,
}

FindAndReplace("__BAZEL_EXECUTION_ROOT__", cwd, &new_arg);
FindAndReplace("__BAZEL_EXECUTION_ROOT_NO_SANDBOX__", nosandbox_root,
&new_arg);
FindAndReplace("__BAZEL_XCODE_DEVELOPER_DIR__", developer_dir, &new_arg);
FindAndReplace("__BAZEL_XCODE_SDKROOT__", sdk_root, &new_arg);
if (!toolchain_path.empty()) {
Expand Down Expand Up @@ -424,15 +429,29 @@ int main(int argc, char *argv[]) {
std::vector<std::string> invocation_args = {"/usr/bin/xcrun", tool_name};
std::vector<std::string> processed_args = {};

auto wrapper_path = std::filesystem::path(argv[0]);
auto nosandbox_root = GetCurrentDirectory();
if (std::filesystem::is_symlink(wrapper_path)) {
auto resolved_wrapper = std::filesystem::read_symlink(wrapper_path);
int components_to_remove =
std::distance(wrapper_path.begin(), wrapper_path.end());
for (int i = 0; i < components_to_remove; i++) {
resolved_wrapper = resolved_wrapper.parent_path();
}

nosandbox_root = resolved_wrapper.string();
}

bool relative_ast_path = getenv("RELATIVE_AST_PATH") != nullptr;
auto consumer = [&](const std::string &arg) {
processed_args.push_back(arg);
};
for (int i = 1; i < argc; i++) {
std::string arg(argv[i]);

ProcessArgument(arg, developer_dir, sdk_root, cwd, relative_ast_path,
linked_binary, dsym_path, toolchain_path, consumer);
ProcessArgument(arg, developer_dir, sdk_root, cwd, nosandbox_root,
relative_ast_path, linked_binary, dsym_path, toolchain_path,
consumer);
}

char *modulemap = getenv("APPLE_SUPPORT_MODULEMAP");
Expand Down
22 changes: 22 additions & 0 deletions test/binary_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,28 @@ def binary_test_suite(name):
target_under_test = "//test/test_data:macos_binary",
)

apple_verification_test(
name = "{}_relative_oso_test".format(name),
tags = [name],
build_type = "device",
compilation_mode = "dbg",
cpus = {"macos_cpus": "arm64"},
expected_platform_type = "macos",
verifier_script = "//test/shell:verify_relative_oso.sh",
target_under_test = "//test/test_data:cc_test_binary",
)

apple_verification_test(
name = "{}_relative_oso_test_no_sandbox".format(name),
tags = [name],
build_type = "device",
compilation_mode = "dbg",
cpus = {"macos_cpus": "arm64"},
expected_platform_type = "macos",
verifier_script = "//test/shell:verify_relative_oso.sh",
target_under_test = "//test/test_data:cc_test_binary_no_sandbox",
)

apple_verification_test(
name = "{}_macos_arm64e_binary_test".format(name),
tags = [name],
Expand Down
5 changes: 4 additions & 1 deletion test/shell/BUILD
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
exports_files(["verify_binary.sh"])
exports_files([
"verify_binary.sh",
"verify_relative_oso.sh",
])

filegroup(
name = "for_bazel_tests",
Expand Down
10 changes: 10 additions & 0 deletions test/shell/verify_relative_oso.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#!/bin/bash

set -euo pipefail

readonly binary="%{binary}s"

# [ 51] 00001016 66 (N_OSO ) 00 0001 0000000000000000 'bazel-out/darwin_arm64-dbg-macos-arm64-min13.0-applebin_macos-ST-659b080861c8/bin/test/test_data/libcc_main.a(main.o)'
output=$(dsymutil -s "$binary" | grep N_OSO)
echo "$output" | grep " 'bazel-out" \
|| (echo "has non-relative N_OSO entries: $output" >&2 && exit 1)
6 changes: 6 additions & 0 deletions test/test_data/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ cc_binary(
tags = TARGETS_UNDER_TEST_TAGS,
)

cc_binary(
name = "cc_test_binary_no_sandbox",
srcs = ["main.cc"],
tags = TARGETS_UNDER_TEST_TAGS + ["no-sandbox"],
)

cc_library(
name = "cc_main",
srcs = ["main.cc"],
Expand Down

0 comments on commit 8ee7a2d

Please sign in to comment.