tequilaOS/platform_build_soong
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Fix data race in dex_bootjars

Browse source 
Move fields in bootImageConfig that are written during
GenerateAndroidBuildActions into a provider to avoid a data race.

Test: go test -race ./...
Change-Id: I0d6783530843889b96f32d748bda7824493d2e32
This commit is contained in:
Colin Cross 2024-01-17 12:14:44 -08:00
parent bcf53701b7
commit 84ed511ceb
2 changed files with 52 additions and 40 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

71
java/dexpreopt_bootjars.go
View file

@ -238,8 +238,7 @@ func init() {
// //
// WARNING: All fields in this struct should be initialized in the genBootImageConfigs function. // WARNING: All fields in this struct should be initialized in the genBootImageConfigs function.
// Failure to do so can lead to data races if there is no synchronization enforced ordering between // Failure to do so can lead to data races if there is no synchronization enforced ordering between
// the writer and the reader. Fields which break this rule are marked as deprecated and should be // the writer and the reader.
// removed and replaced with something else, e.g. providers.
type bootImageConfig struct { type bootImageConfig struct {
// If this image is an extension, the image that it extends. // If this image is an extension, the image that it extends.
extends *bootImageConfig extends *bootImageConfig
@ -279,16 +278,6 @@ type bootImageConfig struct {
// File path to a zip archive with all image files (or nil, if not needed). // File path to a zip archive with all image files (or nil, if not needed).
zip android.WritablePath zip android.WritablePath
// Rules which should be used in make to install the outputs.
//
// Deprecated: Not initialized correctly, see struct comment.
profileInstalls android.RuleBuilderInstalls
// Path to the license metadata file for the module that built the profile.
//
// Deprecated: Not initialized correctly, see struct comment.
profileLicenseMetadataFile android.OptionalPath
// Target-dependent fields. // Target-dependent fields.
variants []*bootImageVariant variants []*bootImageVariant
@ -602,6 +591,7 @@ func (d *dexpreoptBootJars) GenerateAndroidBuildActions(ctx android.ModuleContex
imageConfigs := genBootImageConfigs(ctx) imageConfigs := genBootImageConfigs(ctx)
d.defaultBootImage = defaultBootImageConfig(ctx) d.defaultBootImage = defaultBootImageConfig(ctx)
d.otherImages = make([]*bootImageConfig, 0, len(imageConfigs)-1) d.otherImages = make([]*bootImageConfig, 0, len(imageConfigs)-1)
var profileInstalls android.RuleBuilderInstalls
for _, name := range getImageNames() { for _, name := range getImageNames() {
config := imageConfigs[name] config := imageConfigs[name]
if config != d.defaultBootImage { if config != d.defaultBootImage {
@ -610,11 +600,19 @@ func (d *dexpreoptBootJars) GenerateAndroidBuildActions(ctx android.ModuleContex
if !config.isEnabled(ctx) { if !config.isEnabled(ctx) {
continue continue
} }
generateBootImage(ctx, config) installs := generateBootImage(ctx, config)
profileInstalls = append(profileInstalls, installs...)
if config == d.defaultBootImage { if config == d.defaultBootImage {
bootFrameworkProfileRule(ctx, config) _, installs := bootFrameworkProfileRule(ctx, config)
profileInstalls = append(profileInstalls, installs...)
} }
} }
if len(profileInstalls) > 0 {
android.SetProvider(ctx, profileInstallInfoProvider, profileInstallInfo{
profileInstalls: profileInstalls,
profileLicenseMetadataFile: android.OptionalPathForPath(ctx.LicenseMetadataFile()),
})
}
} }
// GenerateSingletonBuildActions generates build rules for the dexpreopt config for Make. // GenerateSingletonBuildActions generates build rules for the dexpreopt config for Make.
@ -635,7 +633,7 @@ func shouldBuildBootImages(config android.Config, global *dexpreopt.GlobalConfig
return true return true
} }
func generateBootImage(ctx android.ModuleContext, imageConfig *bootImageConfig) { func generateBootImage(ctx android.ModuleContext, imageConfig *bootImageConfig) android.RuleBuilderInstalls {
apexJarModulePairs := getModulesForImage(ctx, imageConfig) apexJarModulePairs := getModulesForImage(ctx, imageConfig)
// Copy module dex jars to their predefined locations. // Copy module dex jars to their predefined locations.
@ -644,12 +642,12 @@ func generateBootImage(ctx android.ModuleContext, imageConfig *bootImageConfig)
// Build a profile for the image config from the profile at the default path. The profile will // Build a profile for the image config from the profile at the default path. The profile will
// then be used along with profiles imported from APEXes to build the boot image. // then be used along with profiles imported from APEXes to build the boot image.
profile := bootImageProfileRule(ctx, imageConfig) profile, profileInstalls := bootImageProfileRule(ctx, imageConfig)
// If dexpreopt of boot image jars should be skipped, stop after generating a profile. // If dexpreopt of boot image jars should be skipped, stop after generating a profile.
global := dexpreopt.GetGlobalConfig(ctx) global := dexpreopt.GetGlobalConfig(ctx)
if SkipDexpreoptBootJars(ctx) || (global.OnlyPreoptArtBootImage && imageConfig.name != "art") { if SkipDexpreoptBootJars(ctx) || (global.OnlyPreoptArtBootImage && imageConfig.name != "art") {
return return profileInstalls
} }
// Build boot image files for the android variants. // Build boot image files for the android variants.
@ -663,6 +661,8 @@ func generateBootImage(ctx android.ModuleContext, imageConfig *bootImageConfig)
// Create a `dump-oat-<image-name>` rule that runs `oatdump` for debugging purposes. // Create a `dump-oat-<image-name>` rule that runs `oatdump` for debugging purposes.
dumpOatRules(ctx, imageConfig) dumpOatRules(ctx, imageConfig)
return profileInstalls
} }
type apexJarModulePair struct { type apexJarModulePair struct {
@ -1177,9 +1177,19 @@ func bootImageProfileRuleCommon(ctx android.ModuleContext, name string, dexFiles
return profile return profile
} }
func bootImageProfileRule(ctx android.ModuleContext, image *bootImageConfig) android.WritablePath { type profileInstallInfo struct {
// Rules which should be used in make to install the outputs.
profileInstalls android.RuleBuilderInstalls
// Path to the license metadata file for the module that built the profile.
profileLicenseMetadataFile android.OptionalPath
}
var profileInstallInfoProvider = blueprint.NewProvider[profileInstallInfo]()
func bootImageProfileRule(ctx android.ModuleContext, image *bootImageConfig) (android.WritablePath, android.RuleBuilderInstalls) {
if !image.isProfileGuided() { if !image.isProfileGuided() {
return nil return nil, nil
} }
profile := bootImageProfileRuleCommon(ctx, image.name, image.dexPathsDeps.Paths(), image.getAnyAndroidVariant().dexLocationsDeps) profile := bootImageProfileRuleCommon(ctx, image.name, image.dexPathsDeps.Paths(), image.getAnyAndroidVariant().dexLocationsDeps)
@ -1187,21 +1197,19 @@ func bootImageProfileRule(ctx android.ModuleContext, image *bootImageConfig) and
if image == defaultBootImageConfig(ctx) { if image == defaultBootImageConfig(ctx) {
rule := android.NewRuleBuilder(pctx, ctx) rule := android.NewRuleBuilder(pctx, ctx)
rule.Install(profile, "/system/etc/boot-image.prof") rule.Install(profile, "/system/etc/boot-image.prof")
image.profileInstalls = append(image.profileInstalls, rule.Installs()...) return profile, rule.Installs()
image.profileLicenseMetadataFile = android.OptionalPathForPath(ctx.LicenseMetadataFile())
} }
return profile, nil
return profile
} }
// bootFrameworkProfileRule generates the rule to create the boot framework profile and // bootFrameworkProfileRule generates the rule to create the boot framework profile and
// returns a path to the generated file. // returns a path to the generated file.
func bootFrameworkProfileRule(ctx android.ModuleContext, image *bootImageConfig) android.WritablePath { func bootFrameworkProfileRule(ctx android.ModuleContext, image *bootImageConfig) (android.WritablePath, android.RuleBuilderInstalls) {
globalSoong := dexpreopt.GetGlobalSoongConfig(ctx) globalSoong := dexpreopt.GetGlobalSoongConfig(ctx)
global := dexpreopt.GetGlobalConfig(ctx) global := dexpreopt.GetGlobalConfig(ctx)
if global.DisableGenerateProfile || ctx.Config().UnbundledBuild() { if global.DisableGenerateProfile || ctx.Config().UnbundledBuild() {
return nil return nil, nil
} }
defaultProfile := "frameworks/base/config/boot-profile.txt" defaultProfile := "frameworks/base/config/boot-profile.txt"
@ -1221,10 +1229,7 @@ func bootFrameworkProfileRule(ctx android.ModuleContext, image *bootImageConfig)
rule.Install(profile, "/system/etc/boot-image.bprof") rule.Install(profile, "/system/etc/boot-image.bprof")
rule.Build("bootFrameworkProfile", "profile boot framework jars") rule.Build("bootFrameworkProfile", "profile boot framework jars")
image.profileInstalls = append(image.profileInstalls, rule.Installs()...) return profile, rule.Installs()
image.profileLicenseMetadataFile = android.OptionalPathForPath(ctx.LicenseMetadataFile())
return profile
} }
func dumpOatRules(ctx android.ModuleContext, image *bootImageConfig) { func dumpOatRules(ctx android.ModuleContext, image *bootImageConfig) {
@ -1292,9 +1297,11 @@ func (d *dexpreoptBootJars) MakeVars(ctx android.MakeVarsContext) {
image := d.defaultBootImage image := d.defaultBootImage
if image != nil { if image != nil {
ctx.Strict("DEXPREOPT_IMAGE_PROFILE_BUILT_INSTALLED", image.profileInstalls.String()) if profileInstallInfo, ok := android.SingletonModuleProvider(ctx, d, profileInstallInfoProvider); ok {
if image.profileLicenseMetadataFile.Valid() { ctx.Strict("DEXPREOPT_IMAGE_PROFILE_BUILT_INSTALLED", profileInstallInfo.profileInstalls.String())
ctx.Strict("DEXPREOPT_IMAGE_PROFILE_LICENSE_METADATA", image.profileLicenseMetadataFile.String()) if profileInstallInfo.profileLicenseMetadataFile.Valid() {
ctx.Strict("DEXPREOPT_IMAGE_PROFILE_LICENSE_METADATA", profileInstallInfo.profileLicenseMetadataFile.String())
}
} }
if SkipDexpreoptBootJars(ctx) { if SkipDexpreoptBootJars(ctx) {

21
java/dexpreopt_config_testing.go
View file

@ -523,7 +523,7 @@ func checkArtBootImageConfig(t *testing.T, result *android.TestResult, mutated b
}, },
} }
checkBootImageConfig(t, imageConfig, mutated, expected) checkBootImageConfig(t, result, imageConfig, mutated, expected)
} }
// getFrameworkImageConfig gets the framework bootImageConfig that was created during the test. // getFrameworkImageConfig gets the framework bootImageConfig that was created during the test.
@ -904,7 +904,7 @@ func checkFrameworkBootImageConfig(t *testing.T, result *android.TestResult, mut
profileLicenseMetadataFile: expectedLicenseMetadataFile, profileLicenseMetadataFile: expectedLicenseMetadataFile,
} }
checkBootImageConfig(t, imageConfig, mutated, expected) checkBootImageConfig(t, result, imageConfig, mutated, expected)
} }
// getMainlineImageConfig gets the framework bootImageConfig that was created during the test. // getMainlineImageConfig gets the framework bootImageConfig that was created during the test.
@ -1183,7 +1183,7 @@ func CheckMainlineBootImageConfig(t *testing.T, result *android.TestResult) {
profileLicenseMetadataFile: expectedLicenseMetadataFile, profileLicenseMetadataFile: expectedLicenseMetadataFile,
} }
checkBootImageConfig(t, imageConfig, false, expected) checkBootImageConfig(t, result, imageConfig, false, expected)
} }
// clearMutatedFields clears fields in the expectedConfig that correspond to fields in the // clearMutatedFields clears fields in the expectedConfig that correspond to fields in the
@ -1211,19 +1211,19 @@ func clearMutatedFields(expected *expectedConfig) {
// zero value so that they will match the unmodified values in the boot image. // zero value so that they will match the unmodified values in the boot image.
// //
// It runs the checks in an image specific subtest of the current test. // It runs the checks in an image specific subtest of the current test.
func checkBootImageConfig(t *testing.T, imageConfig *bootImageConfig, mutated bool, expected *expectedConfig) { func checkBootImageConfig(t *testing.T, result *android.TestResult, imageConfig *bootImageConfig, mutated bool, expected *expectedConfig) {
if !mutated { if !mutated {
clearMutatedFields(expected) clearMutatedFields(expected)
} }
t.Run(imageConfig.name, func(t *testing.T) { t.Run(imageConfig.name, func(t *testing.T) {
nestedCheckBootImageConfig(t, imageConfig, expected) nestedCheckBootImageConfig(t, result, imageConfig, mutated, expected)
}) })
} }
// nestedCheckBootImageConfig does the work of comparing the image against the expected values and // nestedCheckBootImageConfig does the work of comparing the image against the expected values and
// is run in an image specific subtest. // is run in an image specific subtest.
func nestedCheckBootImageConfig(t *testing.T, imageConfig *bootImageConfig, expected *expectedConfig) { func nestedCheckBootImageConfig(t *testing.T, result *android.TestResult, imageConfig *bootImageConfig, mutated bool, expected *expectedConfig) {
android.AssertStringEquals(t, "name", expected.name, imageConfig.name) android.AssertStringEquals(t, "name", expected.name, imageConfig.name)
android.AssertStringEquals(t, "stem", expected.stem, imageConfig.stem) android.AssertStringEquals(t, "stem", expected.stem, imageConfig.stem)
android.AssertPathRelativeToTopEquals(t, "dir", expected.dir, imageConfig.dir) android.AssertPathRelativeToTopEquals(t, "dir", expected.dir, imageConfig.dir)
@ -1234,8 +1234,13 @@ func nestedCheckBootImageConfig(t *testing.T, imageConfig *bootImageConfig, expe
android.AssertPathsRelativeToTopEquals(t, "dexPathsDeps", expected.dexPathsDeps, imageConfig.dexPathsDeps.Paths()) android.AssertPathsRelativeToTopEquals(t, "dexPathsDeps", expected.dexPathsDeps, imageConfig.dexPathsDeps.Paths())
// dexPathsByModule is just a different representation of the other information in the config. // dexPathsByModule is just a different representation of the other information in the config.
android.AssertPathRelativeToTopEquals(t, "zip", expected.zip, imageConfig.zip) android.AssertPathRelativeToTopEquals(t, "zip", expected.zip, imageConfig.zip)
assertInstallsEqual(t, "profileInstalls", expected.profileInstalls, imageConfig.profileInstalls)
android.AssertStringEquals(t, "profileLicenseMetadataFile", expected.profileLicenseMetadataFile, imageConfig.profileLicenseMetadataFile.RelativeToTop().String()) if !mutated {
dexBootJarModule := result.ModuleForTests("dex_bootjars", "android_common")
profileInstallInfo, _ := android.SingletonModuleProvider(result, dexBootJarModule.Module(), profileInstallInfoProvider)
assertInstallsEqual(t, "profileInstalls", expected.profileInstalls, profileInstallInfo.profileInstalls)
android.AssertStringEquals(t, "profileLicenseMetadataFile", expected.profileLicenseMetadataFile, profileInstallInfo.profileLicenseMetadataFile.RelativeToTop().String())
}
android.AssertIntEquals(t, "variant count", 4, len(imageConfig.variants)) android.AssertIntEquals(t, "variant count", 4, len(imageConfig.variants))
for i, variant := range imageConfig.variants { for i, variant := range imageConfig.variants {