Skip to content

Commit

Permalink
Fix our default fastbuild Bazel config (#4363)
Browse files Browse the repository at this point in the history
Causes the default development (`fastbuild`) Bazel build and test to
both use ASan, minimal optimizations, and produce good backtraces with
source locations.

This also fixes all of the non-host configs that were deeply broken for
the latest releases of Bazel going back quite some time.

The intent of our our Bazel toolchain config was for a minimally
optimized, minimal debug info, and ASan + UBSan configuration to be the
`fastbuild`, or the default development build of the project. This
matches its inclusion of asserts, etc.

At some point quite some time ago, all of this stopped working. Bazel no
longer has a `nonhost` feature. This was disabled a long time ago,
briefly argued to be re-enabled, but has persistently been removed.
However, since then the host and non-host features have been separated
including the compilation mode, and so none of that is needed now.
Instead, we can use a much simpler and more principled approach to all
of the feature configuration now which this PR implements.

However, we added TCMalloc _after_ all of the ASan stuff became broken,
and so we never saw that it is fundamentally incompatible with ASan. So
this PR also reworks how TCMalloc is used to only apply to `-c opt`
builds on Linux, and it also explicitly disables it when using
`--config=asan`. I've not found a convenient way to tie the malloc
library choice to a toolchain feature in Bazel, so this relies on the
config being used rather than the feature in isolation.

Last but not least, with this change `fastbuild` creates substantially
larger output and so this change also passes several new flags to reduce
the size costs. One is a general improvement from outlining ASan
instrumentation. The others are in a special feature as they reduce the
error message quality for two of the more expensive UBSan checks in
favor of small generated code size. Keeping these last two separate
allows disabling this locally if needed to debug a failure.
  • Loading branch information
chandlerc authored Oct 7, 2024
1 parent 55d8edc commit 284b149
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 16 deletions.
10 changes: 6 additions & 4 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,15 @@ build --strip=never
# Enable Abseil for GoogleTest.
build --define=absl=1

# Enable TCMalloc on Linux in optimized builds.
build --custom_malloc=//bazel/malloc:tcmalloc_if_linux_opt

# Configuration for enabling Address Sanitizer. Note that this is enabled by
# default for fastbuild. The config is provided to enable ASan even in
# optimized or other build configurations.
# optimized or other build configurations. Note that ASan and TCMalloc are
# incompatible so this explicitly forces the system malloc.
build:asan --features=asan
build:asan --custom_malloc=//bazel/malloc:system_malloc

# Configuration for enabling LibFuzzer (along with ASan).
build:fuzzer --features=fuzzer
Expand All @@ -108,9 +113,6 @@ build --action_env=LANG=en_US.UTF-8
build --enable_platform_specific_config
build:linux --define=pfm=1

# Enable TCMalloc on Linux as well.
build:linux --custom_malloc=@tcmalloc//tcmalloc

# Disables `actions.declare_symlink`. Done for cross-environment support.
build --allow_unresolved_symlinks=false

Expand Down
50 changes: 38 additions & 12 deletions bazel/cc_toolchains/clang_cc_toolchain_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -484,8 +484,7 @@ def _impl(ctx):

sanitizer_common_flags = feature(
name = "sanitizer_common_flags",
requires = [feature_set(["nonhost"])],
implies = ["minimal_optimization_flags", "minimal_debug_info_flags", "preserve_call_stacks"],
implies = ["minimal_debug_info_flags", "preserve_call_stacks"],
)

# Separated from the feature above so it can only be included on platforms
Expand All @@ -505,16 +504,19 @@ def _impl(ctx):

asan = feature(
name = "asan",
requires = [feature_set(["nonhost"])],
implies = ["sanitizer_common_flags"],
flag_sets = [flag_set(
actions = all_compile_actions + all_link_actions,
flag_groups = [flag_group(flags = [
"-fsanitize=address,undefined,nullability",
"-fsanitize-address-use-after-scope",
# Outlining is almost always the right tradeoff for our
# sanitizer usage where we're more pressured on generated code
# size than runtime performance.
"-fsanitize-address-outline-instrumentation",
# We don't need the recovery behavior of UBSan as we expect
# builds to be clean. Not recovering is a bit cheaper.
"-fno-sanitize-recover=undefined",
"-fno-sanitize-recover=undefined,nullability",
# Don't embed the full path name for files. This limits the size
# and combined with line numbers is unlikely to result in many
# ambiguities.
Expand All @@ -526,6 +528,24 @@ def _impl(ctx):
)],
)

# A feature that further reduces the generated code size of our the ASan
# feature, but at the cost of lower quality diagnostics. This is enabled
# along with ASan in our fastbuild configuration, but can be disabled
# explicitly to get better error messages.
asan_min_size = feature(
name = "asan_min_size",
requires = [feature_set(["asan"])],
flag_sets = [flag_set(
actions = all_compile_actions + all_link_actions,
flag_groups = [flag_group(flags = [
# Force two UBSan checks that have especially large code size
# cost to use the minimal branch to a trapping instruction model
# instead of the full diagnostic.
"-fsanitize-trap=alignment,null",
])],
)],
)

# Likely due to being unable to use the static-linked and up-to-date
# sanitizer runtimes, we have to disable a number of sanitizers on macOS.
macos_asan_workarounds = feature(
Expand All @@ -540,17 +560,23 @@ def _impl(ctx):
)],
)

enable_asan_in_fastbuild = feature(
name = "enable_asan_in_fastbuild",
# An enabled feature that requires the `fastbuild` compilation. This is used
# to toggle general features on by default, while allowing them to be
# directly enabled and disabled more generally as desired.
enable_in_fastbuild = feature(
name = "enable_in_fastbuild",
enabled = True,
requires = [feature_set(["nonhost", "fastbuild"])],
implies = ["asan"],
requires = [feature_set(["fastbuild"])],
implies = [
"asan",
"asan_min_size",
"minimal_optimization_flags",
"minimal_debug_info_flags",
],
)

fuzzer = feature(
name = "fuzzer",
requires = [feature_set(["nonhost"])],
implies = ["asan"],
flag_sets = [flag_set(
actions = all_compile_actions + all_link_actions,
flag_groups = [flag_group(flags = [
Expand Down Expand Up @@ -999,7 +1025,6 @@ def _impl(ctx):
feature(name = "fastbuild"),
feature(name = "host"),
feature(name = "no_legacy_features"),
feature(name = "nonhost"),
feature(name = "opt"),
feature(name = "supports_dynamic_linker", enabled = ctx.attr.target_os == "linux"),
feature(name = "supports_pic", enabled = True),
Expand All @@ -1018,7 +1043,8 @@ def _impl(ctx):
sysroot_feature,
sanitizer_common_flags,
asan,
enable_asan_in_fastbuild,
asan_min_size,
enable_in_fastbuild,
fuzzer,
layering_check,
module_maps,
Expand Down
39 changes: 39 additions & 0 deletions bazel/malloc/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# 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

load("@bazel_skylib//lib:selects.bzl", "selects")
load("@rules_cc//cc:defs.bzl", "cc_library")

config_setting(
name = "is_linux",
constraint_values = ["@platforms//os:linux"],
)

config_setting(
name = "opt",
values = {"compilation_mode": "opt"},
)

selects.config_setting_group(
name = "linux_opt",
match_all = [
":is_linux",
":opt",
],
)

# A placeholder empty library that can be used as a `malloc` attribute or with
# the `--custom_malloc` bazel flag to have no custom malloc and use the system
# malloc instead.
cc_library(name = "system_malloc")

# A library that enables TCMalloc for optimized builds on Linux. On other
# platforms and configurations, this falls back on the system malloc.
cc_library(
name = "tcmalloc_if_linux_opt",
deps = select({
":linux_opt": ["@tcmalloc//tcmalloc"],
"//conditions:default": [":system_malloc"],
}),
)

0 comments on commit 284b149

Please sign in to comment.