From f9abec0987feb81132201179217ad0ea66ae4074 Mon Sep 17 00:00:00 2001 From: Trevor Radcliffe Date: Mon, 14 Aug 2023 19:16:32 +0000 Subject: [PATCH] Block CFI on static libraries Bug: 295805467 Test: Unit tests and inspecting generated BUILD files Change-Id: I1bbd2f48ad384e0b5b6f7cc1458b12ded2748e8f --- bazel/properties.go | 36 +++++++++++++++++++ bp2build/cc_library_conversion_test.go | 10 +----- bp2build/cc_library_shared_conversion_test.go | 4 +-- bp2build/cc_library_static_conversion_test.go | 18 +++------- cc/Android.bp | 1 + cc/bp2build.go | 4 +-- cc/constants.go | 19 ++++++++++ cc/library.go | 4 +++ 8 files changed, 70 insertions(+), 26 deletions(-) create mode 100644 cc/constants.go diff --git a/bazel/properties.go b/bazel/properties.go index 9c63bc04b..29e44afb3 100644 --- a/bazel/properties.go +++ b/bazel/properties.go @@ -1296,6 +1296,42 @@ func (sla StringListAttribute) IsEmpty() bool { return len(sla.Value) == 0 && !sla.HasConfigurableValues() } +// RemoveFromAllConfigs removes all instances of the specified value from all configurations +// of the givenStringListAttribute +func (sla *StringListAttribute) RemoveFromAllConfigs(toRemove string) { + if removed, removalResult := removeFromList(toRemove, sla.Value); removed { + if len(removalResult) > 0 { + sla.Value = removalResult + } else { + sla.Value = nil + } + } + for axis, slsv := range sla.ConfigurableValues { + for config, sl := range slsv { + if removed, removalResult := removeFromList(toRemove, sl); removed { + if len(removalResult) > 0 { + sla.SetSelectValue(axis, config, removalResult) + } else { + sla.SetSelectValue(axis, config, nil) + } + } + } + } +} + +func removeFromList(s string, list []string) (bool, []string) { + result := make([]string, 0, len(list)) + var removed bool + for _, item := range list { + if item != s { + result = append(result, item) + } else { + removed = true + } + } + return removed, result +} + type configurableStringLists map[ConfigurationAxis]stringListSelectValues func (csl configurableStringLists) Append(other configurableStringLists) { diff --git a/bp2build/cc_library_conversion_test.go b/bp2build/cc_library_conversion_test.go index 28dbf7ead..82b9e4ab8 100644 --- a/bp2build/cc_library_conversion_test.go +++ b/bp2build/cc_library_conversion_test.go @@ -4785,7 +4785,6 @@ cc_library { }`, ExpectedBazelTargets: []string{ MakeBazelTarget("cc_library_static", "foo_bp2build_cc_library_static", AttrNameToString{ - "features": `["android_cfi"]`, "local_includes": `["."]`, }), MakeBazelTarget("cc_library_shared", "foo", AttrNameToString{ @@ -4814,10 +4813,6 @@ cc_library { }`, ExpectedBazelTargets: []string{ MakeBazelTarget("cc_library_static", "foo_bp2build_cc_library_static", AttrNameToString{ - "features": `select({ - "//build/bazel/platforms/os:android": ["android_cfi"], - "//conditions:default": [], - })`, "local_includes": `["."]`, }), MakeBazelTarget("cc_library_shared", "foo", AttrNameToString{ @@ -4848,10 +4843,7 @@ cc_library { }`, ExpectedBazelTargets: []string{ MakeBazelTarget("cc_library_static", "foo_bp2build_cc_library_static", AttrNameToString{ - "features": `[ - "android_cfi", - "android_cfi_assembly_support", - ]`, + "features": `["android_cfi_assembly_support"]`, "local_includes": `["."]`, }), MakeBazelTarget("cc_library_shared", "foo", AttrNameToString{ diff --git a/bp2build/cc_library_shared_conversion_test.go b/bp2build/cc_library_shared_conversion_test.go index 90b13b03f..b9df2e786 100644 --- a/bp2build/cc_library_shared_conversion_test.go +++ b/bp2build/cc_library_shared_conversion_test.go @@ -1522,7 +1522,7 @@ func TestCcLibrarySharedWithCfiAndCfiAssemblySupport(t *testing.T) { runCcLibrarySharedTestCase(t, Bp2buildTestCase{ Description: "cc_library_shared has correct features when cfi is enabled with cfi assembly support", Blueprint: ` -cc_library_static { +cc_library_shared { name: "foo", sanitize: { cfi: true, @@ -1532,7 +1532,7 @@ cc_library_static { }, }`, ExpectedBazelTargets: []string{ - MakeBazelTarget("cc_library_static", "foo", AttrNameToString{ + MakeBazelTarget("cc_library_shared", "foo", AttrNameToString{ "features": `[ "android_cfi", "android_cfi_assembly_support", diff --git a/bp2build/cc_library_static_conversion_test.go b/bp2build/cc_library_static_conversion_test.go index 89ec8f9a7..e7a75b42e 100644 --- a/bp2build/cc_library_static_conversion_test.go +++ b/bp2build/cc_library_static_conversion_test.go @@ -2135,9 +2135,9 @@ cc_library_static { }) } -func TestCcLibraryStaticWithCfi(t *testing.T) { +func TestCcLibraryStaticNoCfi(t *testing.T) { runCcLibraryStaticTestCase(t, Bp2buildTestCase{ - Description: "cc_library_static has correct features when cfi is enabled", + Description: "cc_library_static never explicitly enables CFI", Blueprint: ` cc_library_static { name: "foo", @@ -2147,7 +2147,6 @@ cc_library_static { }`, ExpectedBazelTargets: []string{ MakeBazelTarget("cc_library_static", "foo", AttrNameToString{ - "features": `["android_cfi"]`, "local_includes": `["."]`, }), }, @@ -2156,7 +2155,7 @@ cc_library_static { func TestCcLibraryStaticWithCfiOsSpecific(t *testing.T) { runCcLibraryStaticTestCase(t, Bp2buildTestCase{ - Description: "cc_library_static has correct features when cfi is enabled for specific variants", + Description: "cc_library_static never explicitly enables CFI even for specific variants", Blueprint: ` cc_library_static { name: "foo", @@ -2170,10 +2169,6 @@ cc_library_static { }`, ExpectedBazelTargets: []string{ MakeBazelTarget("cc_library_static", "foo", AttrNameToString{ - "features": `select({ - "//build/bazel/platforms/os:android": ["android_cfi"], - "//conditions:default": [], - })`, "local_includes": `["."]`, }), }, @@ -2182,7 +2177,7 @@ cc_library_static { func TestCcLibraryStaticWithCfiAndCfiAssemblySupport(t *testing.T) { runCcLibraryStaticTestCase(t, Bp2buildTestCase{ - Description: "cc_library_static has correct features when cfi is enabled with cfi_assembly_support", + Description: "cc_library_static will specify cfi_assembly_support feature but not cfi feature", Blueprint: ` cc_library_static { name: "foo", @@ -2195,10 +2190,7 @@ cc_library_static { }`, ExpectedBazelTargets: []string{ MakeBazelTarget("cc_library_static", "foo", AttrNameToString{ - "features": `[ - "android_cfi", - "android_cfi_assembly_support", - ]`, + "features": `["android_cfi_assembly_support"]`, "local_includes": `["."]`, }), }, diff --git a/cc/Android.bp b/cc/Android.bp index c32d85490..8f6709f90 100644 --- a/cc/Android.bp +++ b/cc/Android.bp @@ -30,6 +30,7 @@ bootstrap_go_package { "cc.go", "ccdeps.go", "check.go", + "constants.go", "coverage.go", "gen.go", "generated_cc_library.go", diff --git a/cc/bp2build.go b/cc/bp2build.go index 7f78e283d..bbdf67284 100644 --- a/cc/bp2build.go +++ b/cc/bp2build.go @@ -1953,9 +1953,9 @@ func bp2buildSanitizerFeatures(ctx android.BazelConversionPathContext, m *Module 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") + features = append(features, "-"+cfiFeatureName) } else if proptools.Bool(sanitizerProps.Sanitize.Cfi) { - features = append(features, "android_cfi") + features = append(features, cfiFeatureName) if proptools.Bool(sanitizerProps.Sanitize.Config.Cfi_assembly_support) { features = append(features, "android_cfi_assembly_support") } diff --git a/cc/constants.go b/cc/constants.go new file mode 100644 index 000000000..f188d4e16 --- /dev/null +++ b/cc/constants.go @@ -0,0 +1,19 @@ +// Copyright 2016 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package cc + +var ( + cfiFeatureName = "android_cfi" +) diff --git a/cc/library.go b/cc/library.go index 2d4d60440..c5a922291 100644 --- a/cc/library.go +++ b/cc/library.go @@ -331,6 +331,7 @@ func libraryBp2Build(ctx android.TopDownMutatorContext, m *Module) { sharedFeatures.DeduplicateAxesFromBase() staticFeatures := baseAttributes.features.Clone().Append(staticAttrs.Features) staticFeatures.DeduplicateAxesFromBase() + staticFeatures.RemoveFromAllConfigs(cfiFeatureName) staticCommonAttrs := staticOrSharedAttributes{ Srcs: *srcs.Clone().Append(staticAttrs.Srcs), @@ -2946,6 +2947,9 @@ func sharedOrStaticLibraryBp2Build(ctx android.TopDownMutatorContext, module *Mo features := baseAttributes.features.Clone().Append(libSharedOrStaticAttrs.Features) features.DeduplicateAxesFromBase() + if isStatic { + features.RemoveFromAllConfigs(cfiFeatureName) + } commonAttrs := staticOrSharedAttributes{ Srcs: compilerAttrs.srcs,