Skip to content

Commit 4a73b36

Browse files
authored
Switch back to the system llvm-symbolizer. (#4410)
Undoes most of #4347, because of [performance complaints](https://discord.com/channels/655572317891461132/707150492370862090/1295527235133898772). With a 30-ish frame stack trace and `-c dbg`, my installed `llvm-symbolizer` still seems slow (~6s), but the hermetic `llvm-symbolizer` adds ~4s (i.e., ~10s total). I don't think we can easily force the hermetic version to build in opt configuration, so I'm backing it out.
1 parent 77facdd commit 4a73b36

File tree

8 files changed

+12
-22
lines changed

8 files changed

+12
-22
lines changed

MODULE.bazel.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bazel/cc_toolchains/clang_configuration.bzl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ def _configure_clang_toolchain_impl(repository_ctx):
220220
"{CLANG_VERSION_FOR_CACHE}": clang_version_for_cache.replace('"', "_").replace("\\", "_"),
221221
"{CLANG_VERSION}": str(clang_version),
222222
"{LLVM_BINDIR}": str(ar_path.dirname),
223+
"{LLVM_SYMBOLIZER}": str(ar_path.dirname.get_child("llvm-symbolizer")),
223224
"{SYSROOT}": str(sysroot_dir),
224225
},
225226
executable = False,

bazel/cc_toolchains/clang_detected_variables.tpl.bzl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ This file gets processed by a repository rule, substituting the
99
"""
1010

1111
llvm_bindir = "{LLVM_BINDIR}"
12+
llvm_symbolizer = "{LLVM_SYMBOLIZER}"
1213
clang_bindir = "{CLANG_BINDIR}"
1314
clang_version = {CLANG_VERSION}
1415
clang_version_for_cache = "{CLANG_VERSION_FOR_CACHE}"

bazel/cc_toolchains/defs.bzl

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,28 +4,26 @@
44

55
"""Provides helpers for cc rules. Intended for general consumption."""
66

7-
# The hermetic llvm-symbolizer target.
8-
_llvm_symbolizer = "@llvm-project//llvm:llvm-symbolizer"
7+
load("@bazel_cc_toolchain//:clang_detected_variables.bzl", "llvm_symbolizer")
98

109
def cc_env():
1110
"""Returns standard environment settings for a cc_binary.
1211
13-
In use, this should set both `data` and `env`, as in:
12+
In use, this looks like:
1413
1514
```
16-
load("//bazel/cc_toolchains:defs.bzl", "cc_env", "cc_env_data")
15+
load("//bazel/cc_toolchains:defs.bzl", "cc_env")
1716
1817
cc_binary(
1918
...
20-
data = cc_env_data(),
2119
env = cc_env(),
2220
)
2321
```
2422
2523
We're currently setting this on a target-by-target basis, mainly because
2624
it's difficult to modify default behaviors.
2725
"""
28-
env = {"LLVM_SYMBOLIZER_PATH": "$(location {0})".format(_llvm_symbolizer)}
26+
env = {"LLVM_SYMBOLIZER_PATH": llvm_symbolizer}
2927

3028
# On macOS, there's a nano zone allocation warning due to asan (arises
3129
# in fastbuild/dbg). This suppresses the warning in `bazel run`.
@@ -37,10 +35,3 @@ def cc_env():
3735
"//bazel/cc_toolchains:macos_asan": env.update({"MallocNanoZone": "0"}),
3836
"//conditions:default": env,
3937
})
40-
41-
def cc_env_data():
42-
"""Returns data needed for cc_env().
43-
44-
Set up as a function mainly for parity, and in case we need future changes.
45-
"""
46-
return [_llvm_symbolizer]

explorer/BUILD

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
44

55
load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library")
6-
load("//bazel/cc_toolchains:defs.bzl", "cc_env", "cc_env_data")
6+
load("//bazel/cc_toolchains:defs.bzl", "cc_env")
77
load("//testing/file_test:rules.bzl", "file_test")
88

99
package(default_visibility = [
@@ -36,7 +36,6 @@ cc_library(
3636
cc_binary(
3737
name = "explorer",
3838
srcs = ["main_bin.cpp"],
39-
data = cc_env_data(),
4039
env = cc_env(),
4140
# Running clang-tidy is slow, and explorer is currently feature frozen, so
4241
# don't spend time linting it.

toolchain/driver/BUILD

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
44

55
load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test")
6-
load("//bazel/cc_toolchains:defs.bzl", "cc_env", "cc_env_data")
6+
load("//bazel/cc_toolchains:defs.bzl", "cc_env")
77
load("//testing/fuzzing:rules.bzl", "cc_fuzz_test")
88

99
package(default_visibility = ["//visibility:public"])
@@ -170,7 +170,6 @@ cc_fuzz_test(
170170
cc_binary(
171171
name = "carbon",
172172
srcs = ["driver_main.cpp"],
173-
data = cc_env_data(),
174173
env = cc_env(),
175174
deps = [
176175
":driver",

toolchain/install/BUILD

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
44

55
load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library", "cc_test")
6-
load("//bazel/cc_toolchains:defs.bzl", "cc_env", "cc_env_data")
6+
load("//bazel/cc_toolchains:defs.bzl", "cc_env")
77
load("install_filegroups.bzl", "install_filegroup", "install_symlink", "install_target", "make_install_filegroups")
88
load("pkg_helpers.bzl", "pkg_naming_variables", "pkg_tar_and_test")
99
load("run_tool.bzl", "run_tool")
@@ -142,7 +142,7 @@ pkg_tar_and_test(
142142
# Support `bazel run` on specific binaries.
143143
run_tool(
144144
name = "run_carbon",
145-
data = [":install_data"] + cc_env_data(),
145+
data = [":install_data"],
146146
env = cc_env(),
147147
tool = "prefix_root/bin/carbon",
148148
)

toolchain/testing/BUILD

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
44

55
load("@rules_cc//cc:defs.bzl", "cc_library", "cc_test")
6-
load("//bazel/cc_toolchains:defs.bzl", "cc_env", "cc_env_data")
6+
load("//bazel/cc_toolchains:defs.bzl", "cc_env")
77
load("//testing/file_test:rules.bzl", "file_test")
88

99
package(default_visibility = ["//visibility:public"])
@@ -28,7 +28,6 @@ file_test(
2828
size = "small",
2929
timeout = "moderate", # Taking >60 seconds in GitHub actions
3030
srcs = ["file_test.cpp"],
31-
data = cc_env_data(),
3231
env = cc_env(),
3332
tests = [
3433
"//toolchain/check:testdata",

0 commit comments

Comments
 (0)