From 284b14981d2a130786b5cc55c952bb97bd04a3a0 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Mon, 7 Oct 2024 11:19:49 -0700 Subject: [PATCH] Fix our default `fastbuild` Bazel config (#4363) 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. --- .bazelrc | 10 ++-- .../clang_cc_toolchain_config.bzl | 50 ++++++++++++++----- bazel/malloc/BUILD | 39 +++++++++++++++ 3 files changed, 83 insertions(+), 16 deletions(-) create mode 100644 bazel/malloc/BUILD diff --git a/.bazelrc b/.bazelrc index a781c60934e50..4a7893c037bdd 100644 --- a/.bazelrc +++ b/.bazelrc @@ -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 @@ -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 diff --git a/bazel/cc_toolchains/clang_cc_toolchain_config.bzl b/bazel/cc_toolchains/clang_cc_toolchain_config.bzl index 1f2d5f07c5584..22ffd21bd2a41 100644 --- a/bazel/cc_toolchains/clang_cc_toolchain_config.bzl +++ b/bazel/cc_toolchains/clang_cc_toolchain_config.bzl @@ -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 @@ -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. @@ -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( @@ -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 = [ @@ -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), @@ -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, diff --git a/bazel/malloc/BUILD b/bazel/malloc/BUILD new file mode 100644 index 0000000000000..2d6d519037d9a --- /dev/null +++ b/bazel/malloc/BUILD @@ -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"], + }), +)