From 74d6fc0b9f2dc04a9ff46805d2b7468f4d84d345 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Fri, 27 Sep 2024 13:41:55 -0700 Subject: [PATCH] Update LLVM and use a hermetic llvm-symbolizer (#4347) This only sets the symbolizer for our more used targets; not sure if there's a great way to set it everywhere (I suppose I could try wrapping cc_binary etc rules if there's a strong preference). There is a downside here, symbolizing a fastbuild crash seems to take about 3s. Not sure if there's a good way to get a faster llvm-symbolizer execution...? I tried running with the new LLVM update without the LLVM_SYMBOLIZER_PATH, and it looks like that's insufficient. With the settings, I now get readable crashes: ``` #9 0x000055dc007d1724 void Carbon::Internal::CheckFail{"FATAL"}, Carbon::TemplateString<27>{"toolchain/driver/driver.cpp"}, 84, Carbon::TemplateString<0>{}, Carbon::TemplateString<3>{"err"}>() (/usr/local/google/home/jperkins/.cache/bazel/_bazel_jperkins/85deb7d9d96f7e0e80b42618a55969d7/sandbox/linux-sandbox/9383/execroot/_main/bazel-out/k8-fastbuild/bin/toolchain/testing/file_test.runfiles/_main/toolchain/testing/file_test+0x2894724) ``` Note the LLVM update is for https://github.com/llvm/llvm-project/pull/109021 --- MODULE.bazel | 6 ++--- MODULE.bazel.lock | 2 +- bazel/cc_toolchains/clang_configuration.bzl | 1 - .../clang_detected_variables.tpl.bzl | 1 - bazel/cc_toolchains/defs.bzl | 23 ++++++++++++++++--- explorer/BUILD | 3 ++- toolchain/driver/BUILD | 3 ++- toolchain/install/BUILD | 7 +++++- toolchain/install/run_tool.bzl | 3 ++- toolchain/install/run_tool.py | 3 +++ toolchain/testing/BUILD | 3 +++ 11 files changed, 42 insertions(+), 13 deletions(-) diff --git a/MODULE.bazel b/MODULE.bazel index 95c86091cbf0..420033d44275 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -135,8 +135,8 @@ bazel_dep(name = "zstd", version = "1.5.6", repo_name = "llvm_zstd") # We pin to specific upstream commits and try to track top-of-tree reasonably # closely rather than pinning to a specific release. -# HEAD as of 2024-09-06. -llvm_project_version = "876b0e60feb6ee4eabb1c8b52881117ce93b3c4c" +# HEAD as of 2024-09-25. +llvm_project_version = "29b92d07746fac26cd64c914bc9c5c3833974f6d" # Load a repository for the raw llvm-project, pre-overlay. http_archive( @@ -147,7 +147,7 @@ http_archive( "@carbon//bazel/llvm_project:0001_Patch_for_mallinfo2_when_using_Bazel_build_system.patch", "@carbon//bazel/llvm_project:0002_Added_Bazel_build_for_compiler_rt_fuzzer.patch", ], - sha256 = "7d9e6a4d1e419657b84a4bdf71d07bbb44d7c290847f7d15a841040af0d58a7b", + sha256 = "3e8e93e3749454af4b64f7f34b792a4748b62fc533bca1703d33b2b04e34eb70", strip_prefix = "llvm-project-{0}".format(llvm_project_version), urls = ["https://github.com/llvm/llvm-project/archive/{0}.tar.gz".format(llvm_project_version)], ) diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index ba6b1ebc4cae..5a1eeeb3ed2a 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -129,7 +129,7 @@ "moduleExtensions": { "//bazel/cc_toolchains:clang_configuration.bzl%clang_toolchain_extension": { "general": { - "bzlTransitiveDigest": "K2JE5G8tvZ+UBmAPF5s/YSUNg53pTvAZi2ause87buQ=", + "bzlTransitiveDigest": "zhZDHLTJB1MzuZXa9Ro50GY6j6P/cJVHY+vpabJkz8w=", "usagesDigest": "FiqDwj5QoiCFb1PRYvajLeyuRjRXbsy2CVC+VsEEt7Q=", "recordedFileInputs": {}, "recordedDirentsInputs": {}, diff --git a/bazel/cc_toolchains/clang_configuration.bzl b/bazel/cc_toolchains/clang_configuration.bzl index 741623848519..936dbe979a38 100644 --- a/bazel/cc_toolchains/clang_configuration.bzl +++ b/bazel/cc_toolchains/clang_configuration.bzl @@ -220,7 +220,6 @@ def _configure_clang_toolchain_impl(repository_ctx): "{CLANG_VERSION_FOR_CACHE}": clang_version_for_cache.replace('"', "_").replace("\\", "_"), "{CLANG_VERSION}": str(clang_version), "{LLVM_BINDIR}": str(ar_path.dirname), - "{LLVM_SYMBOLIZER}": str(ar_path.dirname.get_child("llvm-symbolizer")), "{SYSROOT}": str(sysroot_dir), }, executable = False, diff --git a/bazel/cc_toolchains/clang_detected_variables.tpl.bzl b/bazel/cc_toolchains/clang_detected_variables.tpl.bzl index 3cb83b275950..372474694cfc 100644 --- a/bazel/cc_toolchains/clang_detected_variables.tpl.bzl +++ b/bazel/cc_toolchains/clang_detected_variables.tpl.bzl @@ -9,7 +9,6 @@ This file gets processed by a repository rule, substituting the """ llvm_bindir = "{LLVM_BINDIR}" -llvm_symbolizer = "{LLVM_SYMBOLIZER}" clang_bindir = "{CLANG_BINDIR}" clang_version = {CLANG_VERSION} clang_version_for_cache = "{CLANG_VERSION_FOR_CACHE}" diff --git a/bazel/cc_toolchains/defs.bzl b/bazel/cc_toolchains/defs.bzl index 3025d17f2452..3f4b6458dd54 100644 --- a/bazel/cc_toolchains/defs.bzl +++ b/bazel/cc_toolchains/defs.bzl @@ -4,11 +4,28 @@ """Provides helpers for cc rules. Intended for general consumption.""" -load("@bazel_cc_toolchain//:clang_detected_variables.bzl", "llvm_symbolizer") +# The hermetic llvm-symbolizer target. +llvm_symbolizer = "@llvm-project//llvm:llvm-symbolizer" def cc_env(): - """Returns standard environment settings for a cc_binary.""" - env = {"LLVM_SYMBOLIZER_PATH": llvm_symbolizer} + """Returns standard environment settings for a cc_binary. + + In use, this should set both `data` and `env`, as in: + + ``` + load("//bazel/cc_toolchains:defs.bzl", "cc_env", "llvm_symbolizer") + + cc_binary( + ... + data = [llvm_symbolizer], + env = cc_env(), + ) + ``` + + We're currently setting this on a target-by-target basis, mainly because + it's difficult to modify default behaviors. + """ + env = {"LLVM_SYMBOLIZER_PATH": "$(location {0})".format(llvm_symbolizer)} # On macOS, there's a nano zone allocation warning due to asan (arises # in fastbuild/dbg). This suppresses the warning in `bazel run`. diff --git a/explorer/BUILD b/explorer/BUILD index 89aced90f742..ee3bc8c44d45 100644 --- a/explorer/BUILD +++ b/explorer/BUILD @@ -3,7 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library") -load("//bazel/cc_toolchains:defs.bzl", "cc_env") +load("//bazel/cc_toolchains:defs.bzl", "cc_env", "llvm_symbolizer") load("//testing/file_test:rules.bzl", "file_test") package(default_visibility = [ @@ -36,6 +36,7 @@ cc_library( cc_binary( name = "explorer", srcs = ["main_bin.cpp"], + data = [llvm_symbolizer], env = cc_env(), # Running clang-tidy is slow, and explorer is currently feature frozen, so # don't spend time linting it. diff --git a/toolchain/driver/BUILD b/toolchain/driver/BUILD index 638f192742b6..be332e65ca37 100644 --- a/toolchain/driver/BUILD +++ b/toolchain/driver/BUILD @@ -3,7 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test") -load("//bazel/cc_toolchains:defs.bzl", "cc_env") +load("//bazel/cc_toolchains:defs.bzl", "cc_env", "llvm_symbolizer") load("//testing/fuzzing:rules.bzl", "cc_fuzz_test") package(default_visibility = ["//visibility:public"]) @@ -167,6 +167,7 @@ cc_fuzz_test( cc_binary( name = "carbon", srcs = ["driver_main.cpp"], + data = [llvm_symbolizer], env = cc_env(), deps = [ ":driver", diff --git a/toolchain/install/BUILD b/toolchain/install/BUILD index 28c2610f5f6b..9594b823e688 100644 --- a/toolchain/install/BUILD +++ b/toolchain/install/BUILD @@ -3,6 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test") +load("//bazel/cc_toolchains:defs.bzl", "cc_env", "llvm_symbolizer") load("install_filegroups.bzl", "install_filegroup", "install_symlink", "install_target", "make_install_filegroups") load("pkg_helpers.bzl", "pkg_naming_variables", "pkg_tar_and_test") load("run_tool.bzl", "run_tool") @@ -141,6 +142,10 @@ pkg_tar_and_test( # Support `bazel run` on specific binaries. run_tool( name = "run_carbon", - data = [":install_data"], + data = [ + ":install_data", + llvm_symbolizer, + ], + env = cc_env(), tool = "prefix_root/bin/carbon", ) diff --git a/toolchain/install/run_tool.bzl b/toolchain/install/run_tool.bzl index 3b65723ce06f..b95f570dbc27 100644 --- a/toolchain/install/run_tool.bzl +++ b/toolchain/install/run_tool.bzl @@ -6,7 +6,7 @@ load("@rules_python//python:defs.bzl", "py_binary") -def run_tool(name, tool, data): +def run_tool(name, tool, data, env): # TODO: Fix the driver file discovery in order to allow symlinks. py_binary( name = name, @@ -14,4 +14,5 @@ def run_tool(name, tool, data): srcs = ["run_tool.py"], args = ["$(location {})".format(tool)], data = [tool] + data, + env = env, ) diff --git a/toolchain/install/run_tool.py b/toolchain/install/run_tool.py index 732c673b25a1..f99b0d0d6977 100644 --- a/toolchain/install/run_tool.py +++ b/toolchain/install/run_tool.py @@ -14,6 +14,9 @@ SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception import os import sys +from pathlib import Path if __name__ == "__main__": + symbolizer = str(Path(os.environ["LLVM_SYMBOLIZER_PATH"]).absolute()) + os.environ["LLVM_SYMBOLIZER_PATH"] = symbolizer os.execv(sys.argv[1], sys.argv[1:]) diff --git a/toolchain/testing/BUILD b/toolchain/testing/BUILD index 4df14fc334a9..f6df4d81aa42 100644 --- a/toolchain/testing/BUILD +++ b/toolchain/testing/BUILD @@ -3,6 +3,7 @@ # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test") +load("//bazel/cc_toolchains:defs.bzl", "cc_env", "llvm_symbolizer") load("//testing/file_test:rules.bzl", "file_test") package(default_visibility = ["//visibility:public"]) @@ -11,6 +12,8 @@ file_test( name = "file_test", size = "small", srcs = ["file_test.cpp"], + data = [llvm_symbolizer], + env = cc_env(), tests = [ "//toolchain/check:testdata", "//toolchain/codegen:testdata",