Merge "Propagate all sanitizer flags in SDK snapshots."

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 {
+	outputProperties.AddProperty("sanitize", &libInfo.Sanitize)
-		sanitizeSet := outputProperties.AddPropertySet("sanitize")
-		sanitizeSet.AddProperty("never", true)
-	}
 
 	// 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.
+	// Value of SanitizeProperties.Sanitize. Several - but not all - of these
-	SanitizeNever bool `android:"arch_variant"`
+	// 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) {
+	if ccModule.sanitize != nil {
-		p.SanitizeNever = true
+		p.Sanitize = ccModule.sanitize.Properties.Sanitize
 	}
 }
 

cc/sanitize.go

@@ -139,56 +139,59 @@ func (t sanitizerType) incompatibleWithCfi() bool {
 	return t == asan || t == fuzzer || t == hwasan
 }
 
-type SanitizeProperties struct {
+type SanitizeUserProps struct {
-	// enable AddressSanitizer, ThreadSanitizer, or UndefinedBehaviorSanitizer
+	Never *bool `android:"arch_variant"`
-	Sanitize struct {
-		Never *bool `android:"arch_variant"`
 
-		// main sanitizers
+	// main sanitizers
-		Address   *bool `android:"arch_variant"`
+	Address   *bool `android:"arch_variant"`
-		Thread    *bool `android:"arch_variant"`
+	Thread    *bool `android:"arch_variant"`
-		Hwaddress *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"`
+		Misc_undefined   []string `android:"arch_variant"`
-		Scs              *bool    `android:"arch_variant"`
+		No_recover       []string
+	}
 
-		// A modifier for ASAN and HWASAN for write only instrumentation
+	// value to pass to -fsanitize-recover=
-		Writeonly *bool `android:"arch_variant"`
+	Recover []string
 
-		// Sanitizers to run in the diagnostic mode (as opposed to the release mode).
+	// value to pass to -fsanitize-blacklist
-		// Replaces abort() on error with a human-readable error message.
+	Blocklist *string
-		// 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-recover=
+type SanitizeProperties struct {
-		Recover []string
+	// Enable AddressSanitizer, ThreadSanitizer, UndefinedBehaviorSanitizer, and
-
+	// others. Please see SanitizerUserProps in build/soong/cc/sanitize.go for
-		// value to pass to -fsanitize-blacklist
+	// details.
-		Blocklist *string
+	Sanitize          SanitizeUserProps `android:"arch_variant"`
-	} `android:"arch_variant"`
+	SanitizerEnabled  bool              `blueprint:"mutated"`
-
+	SanitizeDep       bool              `blueprint:"mutated"`
-	SanitizerEnabled  bool     `blueprint:"mutated"`
+	MinimalRuntimeDep bool              `blueprint:"mutated"`
-	SanitizeDep       bool     `blueprint:"mutated"`
+	BuiltinsDep       bool              `blueprint:"mutated"`
-	MinimalRuntimeDep bool     `blueprint:"mutated"`
+	UbsanRuntimeDep   bool              `blueprint:"mutated"`
-	BuiltinsDep       bool     `blueprint:"mutated"`
+	InSanitizerDir    bool              `blueprint:"mutated"`
-	UbsanRuntimeDep   bool     `blueprint:"mutated"`
+	Sanitizers        []string          `blueprint:"mutated"`
-	InSanitizerDir    bool     `blueprint:"mutated"`
+	DiagSanitizers    []string          `blueprint:"mutated"`
-	Sanitizers        []string `blueprint:"mutated"`
-	DiagSanitizers    []string `blueprint:"mutated"`
 }
 
 type sanitize struct {

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
+// Verify that when the shared library has some common and some arch specific
-// snapshot is optimized properly.
+// 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`),
 	)

sdk/update.go

@@ -1346,7 +1346,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
@@ -1383,18 +1384,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)
+// This is recursive function. If it encounters a struct then it will recurse
-// that is a struct then it will recurse into that struct passing in the accessor
+// into it, passing in the accessor for the field and the struct name as prefix
-// for the field. That will then be used in the accessors for the fields in the
+// for the nested fields. That will then be used in the accessors for the fields
-// embedded struct.
+// in the embedded struct.
-func (e *commonValueExtractor) gatherFields(structType reflect.Type, containingStructAccessor fieldAccessorFunc) {
+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 != "" {
@@ -1424,7 +1425,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 {
@@ -1446,9 +1447,15 @@ func (e *commonValueExtractor) gatherFields(structType reflect.Type, containingS
 			return value.Field(fieldIndex)
 		}
 
-		if field.Type.Kind() == reflect.Struct && field.Anonymous {
+		if field.Type.Kind() == reflect.Struct {
-			// Gather fields from the embedded structure.
+			// Gather fields from the nested or embedded structure.
-			e.gatherFields(field.Type, fieldGetter)
+			var subNamePrefix string
+			if field.Anonymous {
+				subNamePrefix = namePrefix
+			} else {
+				subNamePrefix = name + "."
+			}
+			e.gatherFields(field.Type, fieldGetter, subNamePrefix)
 		} else {
 			property := extractorProperty{
 				name,
@@ -1512,7 +1519,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()