diff --git a/examples/cc_asan_test_with_reset/BUILD.bazel b/examples/cc_asan_test_with_automatic_reset/BUILD.bazel similarity index 95% rename from examples/cc_asan_test_with_reset/BUILD.bazel rename to examples/cc_asan_test_with_automatic_reset/BUILD.bazel index 710ebfb..ff023a3 100644 --- a/examples/cc_asan_test_with_reset/BUILD.bazel +++ b/examples/cc_asan_test_with_automatic_reset/BUILD.bazel @@ -44,7 +44,7 @@ cc_library( # original top-level settings, that is, without AddressSanitizer. cc_asan_test_reset( name = "large_dep_reset", - exports = "//cc_asan_test_with_reset/third_party:large_dep", + exports = "//cc_asan_test_with_automatic_reset/third_party:large_dep", ) original_settings( diff --git a/examples/cc_asan_test_with_automatic_reset/asan_test.cpp b/examples/cc_asan_test_with_automatic_reset/asan_test.cpp new file mode 100644 index 0000000..d4339f5 --- /dev/null +++ b/examples/cc_asan_test_with_automatic_reset/asan_test.cpp @@ -0,0 +1,9 @@ +#include "cc_asan_test_with_automatic_reset/lib.h" + +#include + +int main() { + trigger_asan(); + std::cerr << "Did not get expected AddressSanitizer error, is the test instrumented correctly?" << std::endl; + return 1; +} diff --git a/examples/cc_asan_test_with_reset/asan_test.def b/examples/cc_asan_test_with_automatic_reset/asan_test.def similarity index 100% rename from examples/cc_asan_test_with_reset/asan_test.def rename to examples/cc_asan_test_with_automatic_reset/asan_test.def diff --git a/examples/cc_asan_test_with_reset/cc_asan_test.bzl b/examples/cc_asan_test_with_automatic_reset/cc_asan_test.bzl similarity index 100% rename from examples/cc_asan_test_with_reset/cc_asan_test.bzl rename to examples/cc_asan_test_with_automatic_reset/cc_asan_test.bzl diff --git a/examples/cc_asan_test_with_automatic_reset/lib.cpp b/examples/cc_asan_test_with_automatic_reset/lib.cpp new file mode 100644 index 0000000..b3ebcd0 --- /dev/null +++ b/examples/cc_asan_test_with_automatic_reset/lib.cpp @@ -0,0 +1,8 @@ +#include "cc_asan_test_with_automatic_reset/lib.h" + +int trigger_asan() { + int* array = new int[100] {}; + delete[] array; + // Triggers a heap-use-after-free error. + return array[0]; +} diff --git a/examples/cc_asan_test_with_reset/lib.h b/examples/cc_asan_test_with_automatic_reset/lib.h similarity index 100% rename from examples/cc_asan_test_with_reset/lib.h rename to examples/cc_asan_test_with_automatic_reset/lib.h diff --git a/examples/cc_asan_test_with_automatic_reset/third_party/BUILD.bazel b/examples/cc_asan_test_with_automatic_reset/third_party/BUILD.bazel new file mode 100644 index 0000000..eb384c1 --- /dev/null +++ b/examples/cc_asan_test_with_automatic_reset/third_party/BUILD.bazel @@ -0,0 +1,7 @@ +# This is a mock of a large third-party dependency that should not be built with +# AddressSanitizer to reduce build time. +cc_library( + name = "large_dep", + srcs = ["large_dep.cpp"], + visibility = ["//cc_asan_test_with_automatic_reset:__pkg__"], +) diff --git a/examples/cc_asan_test_with_reset/third_party/large_dep.cpp b/examples/cc_asan_test_with_automatic_reset/third_party/large_dep.cpp similarity index 100% rename from examples/cc_asan_test_with_reset/third_party/large_dep.cpp rename to examples/cc_asan_test_with_automatic_reset/third_party/large_dep.cpp diff --git a/examples/cc_asan_test_with_manual_reset/BUILD.bazel b/examples/cc_asan_test_with_manual_reset/BUILD.bazel new file mode 100644 index 0000000..43b48c7 --- /dev/null +++ b/examples/cc_asan_test_with_manual_reset/BUILD.bazel @@ -0,0 +1,33 @@ +load("@with_cfg.bzl", "original_settings") +load(":cc_asan_test.bzl", "cc_asan_test", "cc_asan_test_reset") + +cc_asan_test( + name = "asan_test", + srcs = ["asan_test.cpp"], + env = { + # Effectively invert the exit code so that the test passes if and only + # if the expected ASAN error is detected. + "ASAN_OPTIONS": "exitcode=0:abort_on_error=0", + }, + deps = [ + ":lib", + ], +) + +cc_library( + name = "lib", + srcs = ["lib.cpp"], + hdrs = ["lib.h"], + deps = [":large_dep_reset"], +) + +# This ensures that the "large" third-party dependency is built with the +# original top-level settings, that is, without AddressSanitizer. +cc_asan_test_reset( + name = "large_dep_reset", + exports = "//cc_asan_test_with_manual_reset/third_party:large_dep", +) + +original_settings( + name = "cc_asan_test_original_settings", +) diff --git a/examples/cc_asan_test_with_reset/asan_test.cpp b/examples/cc_asan_test_with_manual_reset/asan_test.cpp similarity index 78% rename from examples/cc_asan_test_with_reset/asan_test.cpp rename to examples/cc_asan_test_with_manual_reset/asan_test.cpp index 7e167b6..6fe21fd 100644 --- a/examples/cc_asan_test_with_reset/asan_test.cpp +++ b/examples/cc_asan_test_with_manual_reset/asan_test.cpp @@ -1,4 +1,4 @@ -#include "cc_asan_test_with_reset/lib.h" +#include "cc_asan_test_with_manual_reset/lib.h" #include diff --git a/examples/cc_asan_test_with_manual_reset/cc_asan_test.bzl b/examples/cc_asan_test_with_manual_reset/cc_asan_test.bzl new file mode 100644 index 0000000..6239f06 --- /dev/null +++ b/examples/cc_asan_test_with_manual_reset/cc_asan_test.bzl @@ -0,0 +1,11 @@ +load("@with_cfg.bzl", "with_cfg") + +_builder = with_cfg(native.cc_test) +_builder.extend("copt", ["-fsanitize=address"]) +_builder.extend("linkopt", select({ + # link.exe doesn't require or recognize -fsanitize=address and would emit a warning. + Label("@rules_cc//cc/compiler:msvc-cl"): [], + "//conditions:default": ["-fsanitize=address"], +})) +_builder.resettable(Label(":cc_asan_test_original_settings")) +cc_asan_test, cc_asan_test_reset = _builder.build() diff --git a/examples/cc_asan_test_with_reset/lib.cpp b/examples/cc_asan_test_with_manual_reset/lib.cpp similarity index 73% rename from examples/cc_asan_test_with_reset/lib.cpp rename to examples/cc_asan_test_with_manual_reset/lib.cpp index 9f9336a..cc38b9d 100644 --- a/examples/cc_asan_test_with_reset/lib.cpp +++ b/examples/cc_asan_test_with_manual_reset/lib.cpp @@ -1,4 +1,4 @@ -#include "cc_asan_test_with_reset/lib.h" +#include "cc_asan_test_with_manual_reset/lib.h" int trigger_asan() { int* array = new int[100] {}; diff --git a/examples/cc_asan_test_with_manual_reset/lib.h b/examples/cc_asan_test_with_manual_reset/lib.h new file mode 100644 index 0000000..f1f566e --- /dev/null +++ b/examples/cc_asan_test_with_manual_reset/lib.h @@ -0,0 +1,3 @@ +#pragma once + +int trigger_asan(); diff --git a/examples/cc_asan_test_with_reset/third_party/BUILD.bazel b/examples/cc_asan_test_with_manual_reset/third_party/BUILD.bazel similarity index 75% rename from examples/cc_asan_test_with_reset/third_party/BUILD.bazel rename to examples/cc_asan_test_with_manual_reset/third_party/BUILD.bazel index 3a84e46..5b705cb 100644 --- a/examples/cc_asan_test_with_reset/third_party/BUILD.bazel +++ b/examples/cc_asan_test_with_manual_reset/third_party/BUILD.bazel @@ -3,5 +3,5 @@ cc_library( name = "large_dep", srcs = ["large_dep.cpp"], - visibility = ["//cc_asan_test_with_reset:__pkg__"], + visibility = ["//cc_asan_test_with_manual_reset:__pkg__"], ) diff --git a/examples/cc_asan_test_with_manual_reset/third_party/large_dep.cpp b/examples/cc_asan_test_with_manual_reset/third_party/large_dep.cpp new file mode 100644 index 0000000..05dc62e --- /dev/null +++ b/examples/cc_asan_test_with_manual_reset/third_party/large_dep.cpp @@ -0,0 +1,10 @@ +// This file is set up to fail when compiled with AddressSanitizer. We use it to +// simulate a large third-party library that shouldn't be instrumented with +// sanitizers. + +#ifndef __has_feature + #define __has_feature(x) 0 // Compatibility with non-clang compilers. +#endif +#if __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__) +#error "large_dep.cpp should not be compiled with ASAN" +#endif diff --git a/with_cfg/private/BUILD.bazel b/with_cfg/private/BUILD.bazel index 01e5547..0248be7 100644 --- a/with_cfg/private/BUILD.bazel +++ b/with_cfg/private/BUILD.bazel @@ -43,6 +43,15 @@ bzl_library( ], ) +bzl_library( + name = "extend", + srcs = ["extend.bzl"], + visibility = ["//with_cfg:__subpackages__"], + deps = [ + ":setting", + ], +) + bzl_library( name = "frontend", srcs = ["frontend.bzl"], @@ -138,6 +147,7 @@ bzl_library( visibility = ["//with_cfg:__subpackages__"], deps = [ ":builder", + ":extend", ":providers", ":rule_defaults", ], diff --git a/with_cfg/private/builder.bzl b/with_cfg/private/builder.bzl index d39b8af..f9f2f85 100644 --- a/with_cfg/private/builder.bzl +++ b/with_cfg/private/builder.bzl @@ -1,3 +1,5 @@ +load("@bazel_features//:features.bzl", "bazel_features") +load(":extend.bzl", "make_transitioned_rule") load(":frontend.bzl", "get_frontend") load(":select.bzl", "map_attr") load(":setting.bzl", "validate_and_get_attr_name") @@ -85,6 +87,24 @@ def _build(*, rule_info, values, operations, mutable_has_been_built, mutable_ori operations = operations, original_settings_label = original_settings_label, ) + + # Resetting attributes is not yet supported with the extended rule approach. + if bazel_features.rules.rule_extension_apis_available and rule_info.supports_extension and not attrs_to_reset: + transitioned_rule = make_transitioned_rule( + rule_info = rule_info, + transition = transition, + values = values, + ) + if original_settings_label: + transitioning_alias = make_transitioning_alias( + providers = rule_info.providers, + transition = transition, + values = values, + original_settings_label = original_settings_label, + ) + return transitioned_rule, transitioning_alias + return transitioned_rule, None + transitioning_alias = make_transitioning_alias( providers = rule_info.providers, transition = transition, diff --git a/with_cfg/private/extend.bzl b/with_cfg/private/extend.bzl new file mode 100644 index 0000000..33bf547 --- /dev/null +++ b/with_cfg/private/extend.bzl @@ -0,0 +1,25 @@ +load(":setting.bzl", "get_attr_type", "validate_and_get_attr_name") + +visibility("private") + +def make_transitioned_rule(*, rule_info, transition, values): + settings_attrs = { + validate_and_get_attr_name(setting): getattr(attr, get_attr_type(value))() + for setting, value in values.items() + } + return rule( + implementation = _transitioned_rule_impl, + parent = rule_info.kind, + cfg = transition, + initializer = lambda **kwargs: _initializer_base(kwargs = kwargs, values = values), + attrs = settings_attrs, + ) + +def _transitioned_rule_impl(ctx): + return ctx.super() + +def _initializer_base(*, kwargs, values): + return kwargs | { + validate_and_get_attr_name(setting): value + for setting, value in values.items() + } diff --git a/with_cfg/private/providers.bzl b/with_cfg/private/providers.bzl index 46a2bee..52dd266 100644 --- a/with_cfg/private/providers.bzl +++ b/with_cfg/private/providers.bzl @@ -9,6 +9,7 @@ RuleInfo = provider(fields = [ "kind", "native", "providers", + "supports_extension", "supports_inheritance", "test", ]) diff --git a/with_cfg/private/setting.bzl b/with_cfg/private/setting.bzl index 688a7a0..69e4b08 100644 --- a/with_cfg/private/setting.bzl +++ b/with_cfg/private/setting.bzl @@ -42,11 +42,11 @@ def validate_and_get_attr_name(setting): fail("Custom build settings can only be referenced via Labels, did you mean Label({})?".format(repr(stripped_setting))) if setting.startswith("-"): fail("\"{}\" is not a valid setting, did you mean \"{}\"?".format(setting, stripped_setting)) - if setting == "features": - # features is a special case as it is both a command-line option (--features) and a - # special attribute on all rules. Avoids "built-in attributes cannot be overridden". - return "features_attr" - return setting + + # Avoid collisions with existing attributes in two cases: + # 1. The setting is `features`, which is also an attribute common to all rules. + # 2. Rule extensions are used and the extended rule has an attribute with the same name. + return "with_cfg_" + setting else: fail("Expected setting to be a Label or a string, got: {} ({})".format(repr(setting), type(setting))) diff --git a/with_cfg/private/transition.bzl b/with_cfg/private/transition.bzl index 932a89d..0a007b3 100644 --- a/with_cfg/private/transition.bzl +++ b/with_cfg/private/transition.bzl @@ -26,7 +26,9 @@ def _make_transition_impl(*, operations, original_settings_label): ) def _transition_base_impl(settings, attr, *, operations, original_settings_label): - if original_settings_label and attr.internal_only_reset: + # internal_only_reset may be missing if this transition is attached to an extended rule rather + # than a transitioning alias. + if original_settings_label and getattr(attr, "internal_only_reset", False): original_settings = settings[str(original_settings_label)] if original_settings: # The reset rule is used in the transitive closure of the transitioning rule. Reset the diff --git a/with_cfg/private/transitioning_alias.bzl b/with_cfg/private/transitioning_alias.bzl index 9edc4cc..e5f3a5b 100644 --- a/with_cfg/private/transitioning_alias.bzl +++ b/with_cfg/private/transitioning_alias.bzl @@ -12,10 +12,12 @@ def make_transitioning_alias(*, providers, transition, values, original_settings if original_settings_label: resettable_attrs = { "internal_only_reset": attr.bool(default = True), + # Fail if the original_settings_label doesn't have the correct provider. "_original_settings": attr.label( default = original_settings_label, providers = [OriginalSettingsInfo], ), + # Fail if --experimental_output_directory_naming_scheme=diff_against_dynamic_baseline isn't set. "_check_for_diff_against_dynamic_baseline": attr.label( default = ":resettable_check_for_diff_against_dynamic_baseline", ), diff --git a/with_cfg/private/with_cfg.bzl b/with_cfg/private/with_cfg.bzl index 451325e..a710dd2 100644 --- a/with_cfg/private/with_cfg.bzl +++ b/with_cfg/private/with_cfg.bzl @@ -128,6 +128,7 @@ def with_cfg( providers = DEFAULT_PROVIDERS + extra_providers, native = _is_native(kind), supports_inheritance = _supports_inheritance(kind), + supports_extension = _supports_extension(kind), ) return make_builder(rule_info) @@ -160,5 +161,16 @@ def _supports_inheritance(kind): # Legacy macros don't support inheritance. return not str(kind).startswith("