Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b0249577 authored by Martin Stjernholm's avatar Martin Stjernholm
Browse files

Propagate all sanitizer flags in SDK snapshots. 

liblog snapshot needs to sanitizer.address=false to avoid cycle in asan
builds. Adding that separately in library_sdk_member.go would start to
feel like whack-a-mole, so the snapshot generation is instead extended
to handle nested property structs.

This uses the BpPropertySet.AddProperty extension in
https://r.android.com/1423510, and common value optimisation now
recurses into non-anonymous structs, instead of comparing them as a
whole.

Test: m nothing
Test: `m SANITIZE_TARGET=address nothing` with prebuilts/runtime
  present in the manifest and a fresh snapshot made with this
Bug: 151303681
Change-Id: I472554117a488e6c800045cb2ed59377778571a4
parent 191c25f5

cc/library_sdk_member.go

+7 −8
Original line number Diff line number Diff line
@@ -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

	}

}

cc/sanitize.go

+49 −46
Original line number Diff line number Diff line
@@ -139,9 +139,7 @@ func (t sanitizerType) incompatibleWithCfi() bool { 
	return t == asan || t == fuzzer || t == hwasan

}



type SanitizeProperties struct {

	// enable AddressSanitizer, ThreadSanitizer, or UndefinedBehaviorSanitizer

	Sanitize struct {

type SanitizeUserProps struct {

	Never *bool `android:"arch_variant"`



	// main sanitizers
@@ -179,8 +177,13 @@ type SanitizeProperties struct { 


	// value to pass to -fsanitize-blacklist

	Blocklist *string

	} `android:"arch_variant"`

}



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"`

sdk/cc_sdk_test.go

+40 −3
Original line number Diff line number Diff line
@@ -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`),

	)

sdk/update.go

+20 −12
Original line number Diff line number Diff line
@@ -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()