From 8ee7a2df6ef9a169395866f614869510b4d586e8 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Mon, 30 Sep 2024 14:36:16 -0700 Subject: [PATCH] Fix oso_prefix paths (#347) 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. --- crosstool/cc_toolchain_config.bzl | 2 +- crosstool/wrapped_clang.cc | 35 ++++++++++++++++++++++++------- test/binary_tests.bzl | 22 +++++++++++++++++++ test/shell/BUILD | 5 ++++- test/shell/verify_relative_oso.sh | 10 +++++++++ test/test_data/BUILD | 6 ++++++ 6 files changed, 70 insertions(+), 10 deletions(-) create mode 100755 test/shell/verify_relative_oso.sh diff --git a/crosstool/cc_toolchain_config.bzl b/crosstool/cc_toolchain_config.bzl index a643c0f..23743d1 100644 --- a/crosstool/cc_toolchain_config.bzl +++ b/crosstool/cc_toolchain_config.bzl @@ -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__/"])], ), ], ) diff --git a/crosstool/wrapped_clang.cc b/crosstool/wrapped_clang.cc index 7b58d34..2cc3aa5 100644 --- a/crosstool/wrapped_clang.cc +++ b/crosstool/wrapped_clang.cc @@ -259,12 +259,14 @@ static std::unique_ptr 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 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 consumer) { @@ -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); } @@ -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 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; @@ -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()) { @@ -424,6 +429,19 @@ int main(int argc, char *argv[]) { std::vector invocation_args = {"/usr/bin/xcrun", tool_name}; std::vector 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); @@ -431,8 +449,9 @@ int main(int argc, char *argv[]) { 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"); diff --git a/test/binary_tests.bzl b/test/binary_tests.bzl index db5d9b5..47c1dea 100644 --- a/test/binary_tests.bzl +++ b/test/binary_tests.bzl @@ -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], diff --git a/test/shell/BUILD b/test/shell/BUILD index bfcadeb..b4fa83e 100644 --- a/test/shell/BUILD +++ b/test/shell/BUILD @@ -1,4 +1,7 @@ -exports_files(["verify_binary.sh"]) +exports_files([ + "verify_binary.sh", + "verify_relative_oso.sh", +]) filegroup( name = "for_bazel_tests", diff --git a/test/shell/verify_relative_oso.sh b/test/shell/verify_relative_oso.sh new file mode 100755 index 0000000..4132c5e --- /dev/null +++ b/test/shell/verify_relative_oso.sh @@ -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) diff --git a/test/test_data/BUILD b/test/test_data/BUILD index 328f07c..f87568f 100644 --- a/test/test_data/BUILD +++ b/test/test_data/BUILD @@ -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"],