From d9b7f17f372a196efc82112c29efb86abf91e266 Mon Sep 17 00:00:00 2001 From: Trevor Radcliffe Date: Wed, 9 Aug 2023 22:21:38 +0000 Subject: [PATCH] bp2build for sanitizer blocklists long term fix Bug: 286872909 Test: Unit tests Test: b build relevant targets Change-Id: I553091f76fca936006651b1ed22c8fe4d176e18f --- bazel/configurability.go | 23 +++++++++++++ bazel/properties.go | 20 +++++------ bp2build/cc_binary_conversion_test.go | 9 ++++- bp2build/cc_library_conversion_test.go | 18 ++++++++-- bp2build/cc_library_shared_conversion_test.go | 9 ++++- bp2build/cc_library_static_conversion_test.go | 9 ++++- cc/binary.go | 12 ++++--- cc/bp2build.go | 34 +++++++++++++++---- cc/library.go | 3 ++ 9 files changed, 111 insertions(+), 26 deletions(-) diff --git a/bazel/configurability.go b/bazel/configurability.go index 671e5c141..aa58fdc7e 100644 --- a/bazel/configurability.go +++ b/bazel/configurability.go @@ -76,6 +76,8 @@ const ( NonApex = "non_apex" ErrorproneDisabled = "errorprone_disabled" + // TODO: b/294868620 - Remove when completing the bug + SanitizersEnabled = "sanitizers_enabled" ) func PowerSetWithoutEmptySet[T any](items []T) [][]T { @@ -223,6 +225,12 @@ var ( ErrorproneDisabled: "//build/bazel/rules/java/errorprone:errorprone_globally_disabled", ConditionsDefaultConfigKey: ConditionsDefaultSelectKey, } + + // TODO: b/294868620 - Remove when completing the bug + sanitizersEnabledMap = map[string]string{ + SanitizersEnabled: "//build/bazel/rules/cc:sanitizers_enabled", + ConditionsDefaultConfigKey: ConditionsDefaultSelectKey, + } ) // basic configuration types @@ -237,6 +245,8 @@ const ( osAndInApex inApex errorProneDisabled + // TODO: b/294868620 - Remove when completing the bug + sanitizersEnabled ) func osArchString(os string, arch string) string { @@ -253,6 +263,8 @@ func (ct configurationType) String() string { osAndInApex: "os_in_apex", inApex: "in_apex", errorProneDisabled: "errorprone_disabled", + // TODO: b/294868620 - Remove when completing the bug + sanitizersEnabled: "sanitizers_enabled", }[ct] } @@ -287,6 +299,11 @@ func (ct configurationType) validateConfig(config string) { if _, ok := errorProneMap[config]; !ok { panic(fmt.Errorf("Unknown errorprone config: %s", config)) } + // TODO: b/294868620 - Remove when completing the bug + case sanitizersEnabled: + if _, ok := sanitizersEnabledMap[config]; !ok { + panic(fmt.Errorf("Unknown sanitizers_enabled config: %s", config)) + } default: panic(fmt.Errorf("Unrecognized ConfigurationType %d", ct)) } @@ -318,6 +335,9 @@ func (ca ConfigurationAxis) SelectKey(config string) string { return inApexMap[config] case errorProneDisabled: return errorProneMap[config] + // TODO: b/294868620 - Remove when completing the bug + case sanitizersEnabled: + return sanitizersEnabledMap[config] default: panic(fmt.Errorf("Unrecognized ConfigurationType %d", ca.configurationType)) } @@ -338,6 +358,9 @@ var ( InApexAxis = ConfigurationAxis{configurationType: inApex} ErrorProneAxis = ConfigurationAxis{configurationType: errorProneDisabled} + + // TODO: b/294868620 - Remove when completing the bug + SanitizersEnabledAxis = ConfigurationAxis{configurationType: sanitizersEnabled} ) // ProductVariableConfigurationAxis returns an axis for the given product variable diff --git a/bazel/properties.go b/bazel/properties.go index 702c31c4d..e6e6819a7 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -433,7 +433,7 @@ func (la *LabelAttribute) SetSelectValue(axis ConfigurationAxis, config string, switch axis.configurationType { case noConfig: la.Value = &value - case arch, os, osArch, productVariables, osAndInApex: + case arch, os, osArch, productVariables, osAndInApex, sanitizersEnabled: if la.ConfigurableValues == nil { la.ConfigurableValues = make(configurableLabels) } @@ -449,7 +449,7 @@ func (la *LabelAttribute) SelectValue(axis ConfigurationAxis, config string) *La switch axis.configurationType { case noConfig: return la.Value - case arch, os, osArch, productVariables, osAndInApex: + case arch, os, osArch, productVariables, osAndInApex, sanitizersEnabled: return la.ConfigurableValues[axis][config] default: panic(fmt.Errorf("Unrecognized ConfigurationAxis %s", axis)) @@ -519,7 +519,7 @@ func (ba *BoolAttribute) SetSelectValue(axis ConfigurationAxis, config string, v switch axis.configurationType { case noConfig: ba.Value = value - case arch, os, osArch, productVariables, osAndInApex: + case arch, os, osArch, productVariables, osAndInApex, sanitizersEnabled: if ba.ConfigurableValues == nil { ba.ConfigurableValues = make(configurableBools) } @@ -666,7 +666,7 @@ func (ba BoolAttribute) SelectValue(axis ConfigurationAxis, config string) *bool switch axis.configurationType { case noConfig: return ba.Value - case arch, os, osArch, productVariables, osAndInApex: + case arch, os, osArch, productVariables, osAndInApex, sanitizersEnabled: if v, ok := ba.ConfigurableValues[axis][config]; ok { return &v } else { @@ -801,7 +801,7 @@ func (lla *LabelListAttribute) SetSelectValue(axis ConfigurationAxis, config str switch axis.configurationType { case noConfig: lla.Value = list - case arch, os, osArch, productVariables, osAndInApex, inApex, errorProneDisabled: + case arch, os, osArch, productVariables, osAndInApex, inApex, errorProneDisabled, sanitizersEnabled: if lla.ConfigurableValues == nil { lla.ConfigurableValues = make(configurableLabelLists) } @@ -817,7 +817,7 @@ func (lla *LabelListAttribute) SelectValue(axis ConfigurationAxis, config string switch axis.configurationType { case noConfig: return lla.Value - case arch, os, osArch, productVariables, osAndInApex, inApex, errorProneDisabled: + case arch, os, osArch, productVariables, osAndInApex, inApex, errorProneDisabled, sanitizersEnabled: return lla.ConfigurableValues[axis][config] default: panic(fmt.Errorf("Unrecognized ConfigurationAxis %s", axis)) @@ -1175,7 +1175,7 @@ func (sa *StringAttribute) SetSelectValue(axis ConfigurationAxis, config string, switch axis.configurationType { case noConfig: sa.Value = str - case arch, os, osArch, productVariables: + case arch, os, osArch, productVariables, sanitizersEnabled: if sa.ConfigurableValues == nil { sa.ConfigurableValues = make(configurableStrings) } @@ -1191,7 +1191,7 @@ func (sa *StringAttribute) SelectValue(axis ConfigurationAxis, config string) *s switch axis.configurationType { case noConfig: return sa.Value - case arch, os, osArch, productVariables: + case arch, os, osArch, productVariables, sanitizersEnabled: if v, ok := sa.ConfigurableValues[axis][config]; ok { return v } else { @@ -1381,7 +1381,7 @@ func (sla *StringListAttribute) SetSelectValue(axis ConfigurationAxis, config st switch axis.configurationType { case noConfig: sla.Value = list - case arch, os, osArch, productVariables, osAndInApex, errorProneDisabled: + case arch, os, osArch, productVariables, osAndInApex, errorProneDisabled, sanitizersEnabled: if sla.ConfigurableValues == nil { sla.ConfigurableValues = make(configurableStringLists) } @@ -1397,7 +1397,7 @@ func (sla *StringListAttribute) SelectValue(axis ConfigurationAxis, config strin switch axis.configurationType { case noConfig: return sla.Value - case arch, os, osArch, productVariables, osAndInApex, errorProneDisabled: + case arch, os, osArch, productVariables, osAndInApex, errorProneDisabled, sanitizersEnabled: return sla.ConfigurableValues[axis][config] default: panic(fmt.Errorf("Unrecognized ConfigurationAxis %s", axis)) diff --git a/bp2build/cc_binary_conversion_test.go b/bp2build/cc_binary_conversion_test.go index d9a786069..90db36573 100644 --- a/bp2build/cc_binary_conversion_test.go +++ b/bp2build/cc_binary_conversion_test.go @@ -880,8 +880,15 @@ func TestCcBinaryWithSanitizerBlocklist(t *testing.T) { }`, targets: []testBazelTarget{ {"cc_binary", "foo", AttrNameToString{ + "copts": `select({ + "//build/bazel/rules/cc:sanitizers_enabled": ["-fsanitize-ignorelist=$(location foo_blocklist.txt)"], + "//conditions:default": [], + })`, + "additional_compiler_inputs": `select({ + "//build/bazel/rules/cc:sanitizers_enabled": [":foo_blocklist.txt"], + "//conditions:default": [], + })`, "local_includes": `["."]`, - "features": `["sanitizer_blocklist_foo_blocklist_txt"]`, }}, }, }) diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index e5ae73e48..93a8dea7b 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -4206,11 +4206,25 @@ cc_library { `, ExpectedBazelTargets: []string{ MakeBazelTarget("cc_library_static", "foo_bp2build_cc_library_static", AttrNameToString{ - "features": `["sanitizer_blocklist_foo_blocklist_txt"]`, + "copts": `select({ + "//build/bazel/rules/cc:sanitizers_enabled": ["-fsanitize-ignorelist=$(location foo_blocklist.txt)"], + "//conditions:default": [], + })`, + "additional_compiler_inputs": `select({ + "//build/bazel/rules/cc:sanitizers_enabled": [":foo_blocklist.txt"], + "//conditions:default": [], + })`, "local_includes": `["."]`, }), MakeBazelTarget("cc_library_shared", "foo", AttrNameToString{ - "features": `["sanitizer_blocklist_foo_blocklist_txt"]`, + "copts": `select({ + "//build/bazel/rules/cc:sanitizers_enabled": ["-fsanitize-ignorelist=$(location foo_blocklist.txt)"], + "//conditions:default": [], + })`, + "additional_compiler_inputs": `select({ + "//build/bazel/rules/cc:sanitizers_enabled": [":foo_blocklist.txt"], + "//conditions:default": [], + })`, "local_includes": `["."]`, }), }, diff --git a/bp2build/cc_library_shared_conversion_test.go b/bp2build/cc_library_shared_conversion_test.go index ccb426fa7..90b13b03f 100644 --- a/bp2build/cc_library_shared_conversion_test.go +++ b/bp2build/cc_library_shared_conversion_test.go @@ -1225,7 +1225,14 @@ cc_library_shared { `, ExpectedBazelTargets: []string{ MakeBazelTarget("cc_library_shared", "foo", AttrNameToString{ - "features": `["sanitizer_blocklist_foo_blocklist_txt"]`, + "copts": `select({ + "//build/bazel/rules/cc:sanitizers_enabled": ["-fsanitize-ignorelist=$(location foo_blocklist.txt)"], + "//conditions:default": [], + })`, + "additional_compiler_inputs": `select({ + "//build/bazel/rules/cc:sanitizers_enabled": [":foo_blocklist.txt"], + "//conditions:default": [], + })`, "local_includes": `["."]`, }), }, diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index 26baf894c..89ec8f9a7 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/bp2build/cc_library_static_conversion_test.go @@ -1950,7 +1950,14 @@ cc_library_static { `, ExpectedBazelTargets: []string{ MakeBazelTarget("cc_library_static", "foo", AttrNameToString{ - "features": `["sanitizer_blocklist_foo_blocklist_txt"]`, + "copts": `select({ + "//build/bazel/rules/cc:sanitizers_enabled": ["-fsanitize-ignorelist=$(location foo_blocklist.txt)"], + "//conditions:default": [], + })`, + "additional_compiler_inputs": `select({ + "//build/bazel/rules/cc:sanitizers_enabled": [":foo_blocklist.txt"], + "//conditions:default": [], + })`, "local_includes": `["."]`, }), }, diff --git a/cc/binary.go b/cc/binary.go index 5ba33a24a..4606b623e 100644 --- a/cc/binary.go +++ b/cc/binary.go @@ -638,7 +638,8 @@ func binaryBp2buildAttrs(ctx android.TopDownMutatorContext, m *Module) binaryAtt Stl: baseAttrs.stl, Cpp_std: baseAttrs.cppStd, - Additional_linker_inputs: baseAttrs.additionalLinkerInputs, + Additional_linker_inputs: baseAttrs.additionalLinkerInputs, + Additional_compiler_inputs: baseAttrs.additionalCompilerInputs, Strip: stripAttributes{ Keep_symbols: baseAttrs.stripKeepSymbols, @@ -680,10 +681,11 @@ type binaryAttributes struct { Srcs_c bazel.LabelListAttribute Srcs_as bazel.LabelListAttribute - Copts bazel.StringListAttribute - Cppflags bazel.StringListAttribute - Conlyflags bazel.StringListAttribute - Asflags bazel.StringListAttribute + Copts bazel.StringListAttribute + Cppflags bazel.StringListAttribute + Conlyflags bazel.StringListAttribute + Asflags bazel.StringListAttribute + Additional_compiler_inputs bazel.LabelListAttribute Deps bazel.LabelListAttribute Dynamic_deps bazel.LabelListAttribute diff --git a/cc/bp2build.go b/cc/bp2build.go index 9d90a5b5d..601c0c54e 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -65,6 +65,8 @@ type staticOrSharedAttributes struct { Hdrs bazel.LabelListAttribute Copts bazel.StringListAttribute + Additional_compiler_inputs bazel.LabelListAttribute + Deps bazel.LabelListAttribute Implementation_deps bazel.LabelListAttribute Dynamic_deps bazel.LabelListAttribute @@ -508,6 +510,8 @@ type compilerAttributes struct { suffix bazel.StringAttribute fdoProfile bazel.LabelAttribute + + additionalCompilerInputs bazel.LabelListAttribute } type filterOutFn func(string) bool @@ -1016,11 +1020,16 @@ func bp2BuildParseBaseProps(ctx android.Bp2buildMutatorContext, module *Module) (&compilerAttrs).localIncludes.Append(rsLocalIncludes) (&compilerAttrs).localIncludes.Value = android.FirstUniqueStrings(compilerAttrs.localIncludes.Value) - features := compilerAttrs.features.Clone().Append(linkerAttrs.features).Append(bp2buildSanitizerFeatures(ctx, module)) + sanitizerValues := bp2buildSanitizerFeatures(ctx, module) + + features := compilerAttrs.features.Clone().Append(linkerAttrs.features).Append(sanitizerValues.features) features = features.Append(bp2buildLtoFeatures(ctx, module)) features = features.Append(convertHiddenVisibilityToFeatureBase(ctx, module)) features.DeduplicateAxesFromBase() + compilerAttrs.copts = *compilerAttrs.copts.Append(sanitizerValues.copts) + compilerAttrs.additionalCompilerInputs = *compilerAttrs.additionalCompilerInputs.Append(sanitizerValues.additionalCompilerInputs) + addMuslSystemDynamicDeps(ctx, linkerAttrs) return baseAttributes{ @@ -1910,8 +1919,16 @@ func bp2buildBinaryLinkerProps(ctx android.BazelConversionPathContext, m *Module return attrs } -func bp2buildSanitizerFeatures(ctx android.BazelConversionPathContext, m *Module) bazel.StringListAttribute { +type sanitizerValues struct { + features bazel.StringListAttribute + copts bazel.StringListAttribute + additionalCompilerInputs bazel.LabelListAttribute +} + +func bp2buildSanitizerFeatures(ctx android.BazelConversionPathContext, m *Module) sanitizerValues { sanitizerFeatures := bazel.StringListAttribute{} + sanitizerCopts := bazel.StringListAttribute{} + sanitizerCompilerInputs := bazel.LabelListAttribute{} bp2BuildPropParseHelper(ctx, m, &SanitizeProperties{}, func(axis bazel.ConfigurationAxis, config string, props interface{}) { var features []string if sanitizerProps, ok := props.(*SanitizeProperties); ok { @@ -1923,9 +1940,10 @@ func bp2buildSanitizerFeatures(ctx android.BazelConversionPathContext, m *Module } blocklist := sanitizerProps.Sanitize.Blocklist if blocklist != nil { - // Format the blocklist name to be used in a feature name - blocklistFeatureSuffix := strings.Replace(strings.ToLower(*blocklist), ".", "_", -1) - features = append(features, "sanitizer_blocklist_"+blocklistFeatureSuffix) + // TODO: b/294868620 - Change this not to use the special axis when completing the bug + coptValue := fmt.Sprintf("-fsanitize-ignorelist=$(location %s)", *blocklist) + sanitizerCopts.SetSelectValue(bazel.SanitizersEnabledAxis, bazel.SanitizersEnabled, []string{coptValue}) + sanitizerCompilerInputs.SetSelectValue(bazel.SanitizersEnabledAxis, bazel.SanitizersEnabled, bazel.MakeLabelListFromTargetNames([]string{*blocklist})) } if sanitizerProps.Sanitize.Cfi != nil && !proptools.Bool(sanitizerProps.Sanitize.Cfi) { features = append(features, "-android_cfi") @@ -1938,7 +1956,11 @@ func bp2buildSanitizerFeatures(ctx android.BazelConversionPathContext, m *Module sanitizerFeatures.SetSelectValue(axis, config, features) } }) - return sanitizerFeatures + return sanitizerValues{ + features: sanitizerFeatures, + copts: sanitizerCopts, + additionalCompilerInputs: sanitizerCompilerInputs, + } } func bp2buildLtoFeatures(ctx android.BazelConversionPathContext, m *Module) bazel.StringListAttribute { diff --git a/cc/library.go b/cc/library.go index df1dbc5b4..2d4d60440 100644 --- a/cc/library.go +++ b/cc/library.go @@ -349,6 +349,7 @@ func libraryBp2Build(ctx android.TopDownMutatorContext, m *Module) { Runtime_deps: linkerAttrs.runtimeDeps, sdkAttributes: bp2BuildParseSdkAttributes(m), Native_coverage: baseAttributes.Native_coverage, + Additional_compiler_inputs: compilerAttrs.additionalCompilerInputs, } includeAttrs := includesAttributes{ @@ -376,6 +377,7 @@ func libraryBp2Build(ctx android.TopDownMutatorContext, m *Module) { Runtime_deps: linkerAttrs.runtimeDeps, sdkAttributes: bp2BuildParseSdkAttributes(m), Native_coverage: baseAttributes.Native_coverage, + Additional_compiler_inputs: compilerAttrs.additionalCompilerInputs, } staticTargetAttrs := &bazelCcLibraryStaticAttributes{ @@ -2962,6 +2964,7 @@ func sharedOrStaticLibraryBp2Build(ctx android.TopDownMutatorContext, module *Mo sdkAttributes: bp2BuildParseSdkAttributes(module), Runtime_deps: linkerAttrs.runtimeDeps, Native_coverage: baseAttributes.Native_coverage, + Additional_compiler_inputs: compilerAttrs.additionalCompilerInputs, } module.convertTidyAttributes(ctx, &commonAttrs.tidyAttributes)