diff --git a/cc/library_sdk_member.go b/cc/library_sdk_member.go index 2f1554427..a1de84bc5 100644 --- a/cc/library_sdk_member.go +++ b/cc/library_sdk_member.go @@ -221,10 +221,7 @@ var includeDirProperties = []includeDirsProperty{ // Add properties that may, or may not, be arch specific. func addPossiblyArchSpecificProperties(sdkModuleContext android.ModuleContext, builder android.SnapshotBuilder, libInfo *nativeLibInfoProperties, outputProperties android.BpPropertySet) { - if libInfo.SanitizeNever { - sanitizeSet := outputProperties.AddPropertySet("sanitize") - sanitizeSet.AddProperty("never", true) - } + outputProperties.AddProperty("sanitize", &libInfo.Sanitize) // Copy the generated library to the snapshot and add a reference to it in the .bp module. if libInfo.outputFile != nil { @@ -373,8 +370,10 @@ type nativeLibInfoProperties struct { // not vary by arch so cannot be android specific. StubsVersion string `sdk:"ignored-on-host"` - // Value of SanitizeProperties.Sanitize.Never. Needs to be propagated for CRT objects. - SanitizeNever bool `android:"arch_variant"` + // Value of SanitizeProperties.Sanitize. Several - but not all - of these + // affect the expanded variants. All are propagated to avoid entangling the + // sanitizer logic with the snapshot generation. + Sanitize SanitizeUserProps `android:"arch_variant"` // outputFile is not exported as it is always arch specific. outputFile android.Path @@ -423,8 +422,8 @@ func (p *nativeLibInfoProperties) PopulateFromVariant(ctx android.SdkMemberConte p.StubsVersion = ccModule.StubsVersion() } - if ccModule.sanitize != nil && proptools.Bool(ccModule.sanitize.Properties.Sanitize.Never) { - p.SanitizeNever = true + if ccModule.sanitize != nil { + p.Sanitize = ccModule.sanitize.Properties.Sanitize } } diff --git a/cc/sanitize.go b/cc/sanitize.go index 174dcfea5..8c3e97af3 100644 --- a/cc/sanitize.go +++ b/cc/sanitize.go @@ -139,56 +139,59 @@ func (t sanitizerType) incompatibleWithCfi() bool { return t == asan || t == fuzzer || t == hwasan } -type SanitizeProperties struct { - // enable AddressSanitizer, ThreadSanitizer, or UndefinedBehaviorSanitizer - Sanitize struct { - Never *bool `android:"arch_variant"` +type SanitizeUserProps struct { + Never *bool `android:"arch_variant"` - // main sanitizers - Address *bool `android:"arch_variant"` - Thread *bool `android:"arch_variant"` - Hwaddress *bool `android:"arch_variant"` + // main sanitizers + Address *bool `android:"arch_variant"` + Thread *bool `android:"arch_variant"` + Hwaddress *bool `android:"arch_variant"` - // local sanitizers + // local sanitizers + Undefined *bool `android:"arch_variant"` + All_undefined *bool `android:"arch_variant"` + Misc_undefined []string `android:"arch_variant"` + Fuzzer *bool `android:"arch_variant"` + Safestack *bool `android:"arch_variant"` + Cfi *bool `android:"arch_variant"` + Integer_overflow *bool `android:"arch_variant"` + Scudo *bool `android:"arch_variant"` + Scs *bool `android:"arch_variant"` + + // A modifier for ASAN and HWASAN for write only instrumentation + Writeonly *bool `android:"arch_variant"` + + // Sanitizers to run in the diagnostic mode (as opposed to the release mode). + // Replaces abort() on error with a human-readable error message. + // Address and Thread sanitizers always run in diagnostic mode. + Diag struct { Undefined *bool `android:"arch_variant"` - All_undefined *bool `android:"arch_variant"` - Misc_undefined []string `android:"arch_variant"` - Fuzzer *bool `android:"arch_variant"` - Safestack *bool `android:"arch_variant"` Cfi *bool `android:"arch_variant"` Integer_overflow *bool `android:"arch_variant"` - Scudo *bool `android:"arch_variant"` - Scs *bool `android:"arch_variant"` + Misc_undefined []string `android:"arch_variant"` + No_recover []string + } - // A modifier for ASAN and HWASAN for write only instrumentation - Writeonly *bool `android:"arch_variant"` + // value to pass to -fsanitize-recover= + Recover []string - // Sanitizers to run in the diagnostic mode (as opposed to the release mode). - // Replaces abort() on error with a human-readable error message. - // Address and Thread sanitizers always run in diagnostic mode. - Diag struct { - Undefined *bool `android:"arch_variant"` - Cfi *bool `android:"arch_variant"` - Integer_overflow *bool `android:"arch_variant"` - Misc_undefined []string `android:"arch_variant"` - No_recover []string - } + // value to pass to -fsanitize-blacklist + Blocklist *string +} - // value to pass to -fsanitize-recover= - Recover []string - - // value to pass to -fsanitize-blacklist - Blocklist *string - } `android:"arch_variant"` - - SanitizerEnabled bool `blueprint:"mutated"` - SanitizeDep bool `blueprint:"mutated"` - MinimalRuntimeDep bool `blueprint:"mutated"` - BuiltinsDep bool `blueprint:"mutated"` - UbsanRuntimeDep bool `blueprint:"mutated"` - InSanitizerDir bool `blueprint:"mutated"` - Sanitizers []string `blueprint:"mutated"` - DiagSanitizers []string `blueprint:"mutated"` +type SanitizeProperties struct { + // Enable AddressSanitizer, ThreadSanitizer, UndefinedBehaviorSanitizer, and + // others. Please see SanitizerUserProps in build/soong/cc/sanitize.go for + // details. + Sanitize SanitizeUserProps `android:"arch_variant"` + SanitizerEnabled bool `blueprint:"mutated"` + SanitizeDep bool `blueprint:"mutated"` + MinimalRuntimeDep bool `blueprint:"mutated"` + BuiltinsDep bool `blueprint:"mutated"` + UbsanRuntimeDep bool `blueprint:"mutated"` + InSanitizerDir bool `blueprint:"mutated"` + Sanitizers []string `blueprint:"mutated"` + DiagSanitizers []string `blueprint:"mutated"` } type sanitize struct { diff --git a/sdk/cc_sdk_test.go b/sdk/cc_sdk_test.go index 8c9e2285d..a76b07d12 100644 --- a/sdk/cc_sdk_test.go +++ b/sdk/cc_sdk_test.go @@ -435,8 +435,10 @@ include/Test.h -> include/include/Test.h ) } -// Verify that when the shared library has some common and some arch specific properties that the generated -// snapshot is optimized properly. +// Verify that when the shared library has some common and some arch specific +// properties that the generated snapshot is optimized properly. Substruct +// handling is tested with the sanitize clauses (but note there's a lot of +// built-in logic in sanitize.go that can affect those flags). func TestSnapshotWithCcSharedLibraryCommonProperties(t *testing.T) { result := testSdkWithCc(t, ` sdk { @@ -451,9 +453,18 @@ func TestSnapshotWithCcSharedLibraryCommonProperties(t *testing.T) { "aidl/foo/bar/Test.aidl", ], export_include_dirs: ["include"], + sanitize: { + fuzzer: false, + integer_overflow: true, + diag: { undefined: false }, + }, arch: { arm64: { export_system_include_dirs: ["arm64/include"], + sanitize: { + hwaddress: true, + integer_overflow: false, + }, }, }, stl: "none", @@ -471,13 +482,26 @@ cc_prebuilt_library_shared { stl: "none", compile_multilib: "both", export_include_dirs: ["include/include"], + sanitize: { + fuzzer: false, + diag: { + undefined: false, + }, + }, arch: { arm64: { srcs: ["arm64/lib/mynativelib.so"], export_system_include_dirs: ["arm64/include/arm64/include"], + sanitize: { + hwaddress: true, + integer_overflow: false, + }, }, arm: { srcs: ["arm/lib/mynativelib.so"], + sanitize: { + integer_overflow: true, + }, }, }, } @@ -488,13 +512,26 @@ cc_prebuilt_library_shared { stl: "none", compile_multilib: "both", export_include_dirs: ["include/include"], + sanitize: { + fuzzer: false, + diag: { + undefined: false, + }, + }, arch: { arm64: { srcs: ["arm64/lib/mynativelib.so"], export_system_include_dirs: ["arm64/include/arm64/include"], + sanitize: { + hwaddress: true, + integer_overflow: false, + }, }, arm: { srcs: ["arm/lib/mynativelib.so"], + sanitize: { + integer_overflow: true, + }, }, }, } @@ -506,7 +543,7 @@ sdk_snapshot { `), checkAllCopyRules(` include/Test.h -> include/include/Test.h -.intermediates/mynativelib/android_arm64_armv8-a_shared/mynativelib.so -> arm64/lib/mynativelib.so +.intermediates/mynativelib/android_arm64_armv8-a_shared_hwasan/mynativelib.so -> arm64/lib/mynativelib.so arm64/include/Arm64Test.h -> arm64/include/arm64/include/Arm64Test.h .intermediates/mynativelib/android_arm_armv7-a-neon_shared/mynativelib.so -> arm/lib/mynativelib.so`), ) diff --git a/sdk/update.go b/sdk/update.go index 537ab13cb..65baa5ed6 100644 --- a/sdk/update.go +++ b/sdk/update.go @@ -1348,7 +1348,8 @@ type isHostVariant interface { // A property that can be optimized by the commonValueExtractor. type extractorProperty struct { - // The name of the field for this property. + // The name of the field for this property. It is a "."-separated path for + // fields in non-anonymous substructs. name string // Filter that can use metadata associated with the properties being optimized @@ -1385,18 +1386,18 @@ type commonValueExtractor struct { func newCommonValueExtractor(propertiesStruct interface{}) *commonValueExtractor { structType := getStructValue(reflect.ValueOf(propertiesStruct)).Type() extractor := &commonValueExtractor{} - extractor.gatherFields(structType, nil) + extractor.gatherFields(structType, nil, "") return extractor } // Gather the fields from the supplied structure type from which common values will // be extracted. // -// This is recursive function. If it encounters an embedded field (no field name) -// that is a struct then it will recurse into that struct passing in the accessor -// for the field. That will then be used in the accessors for the fields in the -// embedded struct. -func (e *commonValueExtractor) gatherFields(structType reflect.Type, containingStructAccessor fieldAccessorFunc) { +// This is recursive function. If it encounters a struct then it will recurse +// into it, passing in the accessor for the field and the struct name as prefix +// for the nested fields. That will then be used in the accessors for the fields +// in the embedded struct. +func (e *commonValueExtractor) gatherFields(structType reflect.Type, containingStructAccessor fieldAccessorFunc, namePrefix string) { for f := 0; f < structType.NumField(); f++ { field := structType.Field(f) if field.PkgPath != "" { @@ -1426,7 +1427,7 @@ func (e *commonValueExtractor) gatherFields(structType reflect.Type, containingS // Save a copy of the field index for use in the function. fieldIndex := f - name := field.Name + name := namePrefix + field.Name fieldGetter := func(value reflect.Value) reflect.Value { if containingStructAccessor != nil { @@ -1448,9 +1449,15 @@ func (e *commonValueExtractor) gatherFields(structType reflect.Type, containingS return value.Field(fieldIndex) } - if field.Type.Kind() == reflect.Struct && field.Anonymous { - // Gather fields from the embedded structure. - e.gatherFields(field.Type, fieldGetter) + if field.Type.Kind() == reflect.Struct { + // Gather fields from the nested or embedded structure. + var subNamePrefix string + if field.Anonymous { + subNamePrefix = namePrefix + } else { + subNamePrefix = name + "." + } + e.gatherFields(field.Type, fieldGetter, subNamePrefix) } else { property := extractorProperty{ name, @@ -1514,7 +1521,8 @@ func (c dynamicMemberPropertiesContainer) String() string { // Iterates over each exported field (capitalized name) and checks to see whether they // have the same value (using DeepEquals) across all the input properties. If it does not then no // change is made. Otherwise, the common value is stored in the field in the commonProperties -// and the field in each of the input properties structure is set to its default value. +// and the field in each of the input properties structure is set to its default value. Nested +// structs are visited recursively and their non-struct fields are compared. func (e *commonValueExtractor) extractCommonProperties(commonProperties interface{}, inputPropertiesSlice interface{}) error { commonPropertiesValue := reflect.ValueOf(commonProperties) commonStructValue := commonPropertiesValue.Elem()