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

Commit 864e1b45 authored by Paul Duffin's avatar Paul Duffin
Browse files

Retry: Detect invalid arch specific properties in snapshot 

Previously, the snapshot code did not know whether a specific property
could be arch specific or not and assumed that they all were which
meant that it could generate snapshots containing arch specific values
for properties that are not arch specific and so would fail when
unpacked.

This change requires arch specific fields in SdkMemberProperties to be
tagged as such using `android:"arch_variant"` (just as in module input
property structures). Any property without that must have properties
that are common across all variants.

Bug: 155628860
Test: m nothing
Change-Id: I3df60f0b53ba02ec2c55a80c7da058eac5909d26
parent a71fe150

android/sdk.go

+7 −1
Original line number Diff line number Diff line
@@ -342,9 +342,15 @@ type SdkMemberType interface { 
	//

	// * The variant property structs are analysed to find exported (capitalized) fields which

	//   have common values. Those fields are cleared and the common value added to the common

	//   properties. A field annotated with a tag of `sdk:"keep"` will be treated as if it

	//   properties.

	//

	//   A field annotated with a tag of `sdk:"keep"` will be treated as if it

	//   was not capitalized, i.e. not optimized for common values.

	//

	//   A field annotated with a tag of `android:"arch_variant"` will be allowed to have

	//   values that differ by arch, fields not tagged as such must have common values across

	//   all variants.

	//

	// * The sdk module type populates the BpModule structure, creating the arch specific

	//   structure and calls AddToPropertySet(...) on the properties struct to add the member

	//   specific properties in the correct place in the structure.

cc/library_sdk_member.go

+5 −5
Original line number Diff line number Diff line
@@ -307,7 +307,7 @@ type nativeLibInfoProperties struct { 
	// The list of possibly common exported include dirs.

	//

	// This field is exported as its contents may not be arch specific.

	ExportedIncludeDirs android.Paths

	ExportedIncludeDirs android.Paths `android:"arch_variant"`



	// The list of arch specific exported generated include dirs.

	//
@@ -322,23 +322,23 @@ type nativeLibInfoProperties struct { 
	// The list of possibly common exported system include dirs.

	//

	// This field is exported as its contents may not be arch specific.

	ExportedSystemIncludeDirs android.Paths

	ExportedSystemIncludeDirs android.Paths `android:"arch_variant"`



	// The list of possibly common exported flags.

	//

	// This field is exported as its contents may not be arch specific.

	ExportedFlags []string

	ExportedFlags []string `android:"arch_variant"`



	// The set of shared libraries

	//

	// This field is exported as its contents may not be arch specific.

	SharedLibs []string

	SharedLibs []string `android:"arch_variant"`



	// The set of system shared libraries. Note nil and [] are semantically

	// distinct - see BaseLinkerProperties.System_shared_libs.

	//

	// This field is exported as its contents may not be arch specific.

	SystemSharedLibs []string

	SystemSharedLibs []string `android:"arch_variant"`



	// The specific stubs version for the lib variant, or empty string if stubs

	// are not in use.

java/java.go

+1 −1
Original line number Diff line number Diff line
@@ -1906,7 +1906,7 @@ func (mt *librarySdkMemberType) CreateVariantPropertiesStruct() android.SdkMembe 
type librarySdkMemberProperties struct {

	android.SdkMemberPropertiesBase



	JarToExport     android.Path

	JarToExport     android.Path `android:"arch_variant"`

	AidlIncludeDirs android.Paths

}

sdk/sdk_test.go

+28 −5
Original line number Diff line number Diff line
@@ -226,8 +226,8 @@ func TestSDkInstall(t *testing.T) { 
}



type EmbeddedPropertiesStruct struct {

	S_Embedded_Common    string

	S_Embedded_Different string

	S_Embedded_Common    string `android:"arch_variant"`

	S_Embedded_Different string `android:"arch_variant"`

}



type testPropertiesStruct struct {
@@ -235,11 +235,11 @@ type testPropertiesStruct struct { 
	private     string

	Public_Kept string `sdk:"keep"`

	S_Common    string

	S_Different string

	S_Different string `android:"arch_variant"`

	A_Common    []string

	A_Different []string

	A_Different []string `android:"arch_variant"`

	F_Common    *bool

	F_Different *bool

	F_Different *bool `android:"arch_variant"`

	EmbeddedPropertiesStruct

}
@@ -346,3 +346,26 @@ func TestCommonValueOptimization(t *testing.T) { 
		},

		structs[1])

}



func TestCommonValueOptimization_InvalidArchSpecificVariants(t *testing.T) {

	common := &testPropertiesStruct{name: "common"}

	structs := []propertiesContainer{

		&testPropertiesStruct{

			name:     "struct-0",

			S_Common: "should-be-but-is-not-common0",

		},

		&testPropertiesStruct{

			name:     "struct-1",

			S_Common: "should-be-but-is-not-common1",

		},

	}



	extractor := newCommonValueExtractor(common)



	h := TestHelper{t}



	err := extractor.extractCommonProperties(common, structs)

	h.AssertErrorMessageEquals("unexpected error", `field "S_Common" is not tagged as "arch_variant" but has arch specific properties:

    "struct-0" has value "should-be-but-is-not-common0"

    "struct-1" has value "should-be-but-is-not-common1"`, err)

}

sdk/update.go

+29 −3
Original line number Diff line number Diff line
@@ -1225,6 +1225,9 @@ type extractorProperty struct { 


	// The empty value for the field.

	emptyValue reflect.Value



	// True if the property can support arch variants false otherwise.

	archVariant bool

}



func (p extractorProperty) String() string {
@@ -1303,6 +1306,7 @@ func (e *commonValueExtractor) gatherFields(structType reflect.Type, containingS 
				name,

				fieldGetter,

				reflect.Zero(field.Type),

				proptools.HasTag(field, "android", "arch_variant"),

			}

			e.properties = append(e.properties, property)

		}
@@ -1370,10 +1374,16 @@ func (e *commonValueExtractor) extractCommonProperties(commonProperties interfac 
		fieldGetter := property.getter



		// Check to see if all the structures have the same value for the field. The commonValue

		// is nil on entry to the loop and if it is nil on exit then there is no common value,

		// otherwise it points to the common value.

		// is nil on entry to the loop and if it is nil on exit then there is no common value or

		// all the values have been filtered out, otherwise it points to the common value.

		var commonValue *reflect.Value



		// Assume that all the values will be the same.

		//

		// While similar to this is not quite the same as commonValue == nil. If all the values

		// have been filtered out then this will be false but commonValue == nil will be true.

		valuesDiffer := false



		for i := 0; i < sliceValue.Len(); i++ {

			container := sliceValue.Index(i).Interface().(propertiesContainer)

			itemValue := reflect.ValueOf(container.optimizableProperties())
@@ -1387,12 +1397,13 @@ func (e *commonValueExtractor) extractCommonProperties(commonProperties interfac 
				// no value in common so break out.

				if !reflect.DeepEqual(fieldValue.Interface(), commonValue.Interface()) {

					commonValue = nil

					valuesDiffer = true

					break

				}

			}

		}



		// If the fields all have a common value then store it in the common struct field

		// If the fields all have common value then store it in the common struct field

		// and set the input struct's field to the empty value.

		if commonValue != nil {

			emptyValue := property.emptyValue
@@ -1404,6 +1415,21 @@ func (e *commonValueExtractor) extractCommonProperties(commonProperties interfac 
				fieldValue.Set(emptyValue)

			}

		}



		if valuesDiffer && !property.archVariant {

			// The values differ but the property does not support arch variants so it

			// is an error.

			var details strings.Builder

			for i := 0; i < sliceValue.Len(); i++ {

				container := sliceValue.Index(i).Interface().(propertiesContainer)

				itemValue := reflect.ValueOf(container.optimizableProperties())

				fieldValue := fieldGetter(itemValue)



				_, _ = fmt.Fprintf(&details, "\n    %q has value %q", container.String(), fieldValue.Interface())

			}



			return fmt.Errorf("field %q is not tagged as \"arch_variant\" but has arch specific properties:%s", property.String(), details.String())

		}

	}



	return nil