Add nested class loader subcontext at the proper hierarchy level.

When adding a subcontext in a class loader context tree, there are two
possible cases: 1) the root of the subcontext is itself a <uses-library>
and should be present as a node in the tree, or 2) the root is not a
<uses-library>, but some of its dependencies are -- in that case they
should be disconnected from the root, and the resulting forrest should
be added at the top-level.

Example:

  1) C is a <uses-library>:

     A
     ├── B
     └── C
         ├── D
         └── E
             └── F

  2) C is not a <uses-library>:

     A
     ├── B
     ├── D
     └── E
         └── F

Before the patch subcontexts for transitive dependencies were added
before the subcontext for the direct dependency (even if it was a
<uses-library>, resulting in case-2 hierarchy when case-1 should have
been used. Previosuly this didn't matter because class loader context
was a flat set of libraries, but now it matters because class loader
context is a tree.

This patch changes the order in which libraries are added, so that
direct dependencies are added before transitive ones. The context adding
method now accepts an "implicit root" parameter, so that when adding
transitive dependencies it can check if the corresponding direct
dependency is a <uses-library> and already present in the context.

Partially constructed class loader context is now propagated top-down
into aapt.buildActions, so that the method can use existing part of the
context to decide where the missing part should be connected.

Test: lunch aosp_cf_x86_phone-userdebug && m
Bug: 132357300
Change-Id: I649aff9e27494306885a4f4fc90226c399636b57

dexpreopt/class_loader_context.go

@@ -171,7 +171,23 @@ func (clcMap ClassLoaderContextMap) AddContextForSdk(ctx android.ModuleInstallPa
 }
 
 // Merge the other class loader context map into this one, do not override existing entries.
-func (clcMap ClassLoaderContextMap) AddContextMap(otherClcMap ClassLoaderContextMap) {
+// The implicitRootLib parameter is the name of the library for which the other class loader
+// context map was constructed. If the implicitRootLib is itself a <uses-library>, it should be
+// already present in the class loader context (with the other context as its subcontext) -- in
+// that case do not re-add the other context. Otherwise add the other context at the top-level.
+func (clcMap ClassLoaderContextMap) AddContextMap(otherClcMap ClassLoaderContextMap, implicitRootLib string) {
+	if otherClcMap == nil {
+		return
+	}
+
+	// If the implicit root of the merged map is already present as one of top-level subtrees, do
+	// not merge it second time.
+	for _, clc := range clcMap[AnySdkVersion] {
+		if clc.Name == implicitRootLib {
+			return
+		}
+	}
+
 	for sdkVer, otherClcs := range otherClcMap {
 		for _, otherClc := range otherClcs {
 			alreadyHave := false

dexpreopt/class_loader_context_test.go

@@ -80,7 +80,12 @@ func TestCLC(t *testing.T) {
 	// from conditional context.
 	m.AddContextForSdk(ctx, 42, "f", buildPath(ctx, "f"), installPath(ctx, "f"), nil)
 	m.AddContextForSdk(ctx, AnySdkVersion, "f", buildPath(ctx, "f"), installPath(ctx, "f"), nil)
-	m.AddContextMap(m3)
+
+	// Merge map with implicit root library that is among toplevel contexts => does nothing.
+	m.AddContextMap(m1, "c")
+	// Merge map with implicit root library that is not among toplevel contexts => all subcontexts
+	// of the other map are added as toplevel contexts.
+	m.AddContextMap(m3, "m_g")
 
 	// Compatibility libraries with unknown install paths get default paths.
 	m.AddContextForSdk(ctx, 29, AndroidHidlManager, buildPath(ctx, AndroidHidlManager), nil, nil)

java/aar.go

@@ -109,7 +109,6 @@ type aapt struct {
 	useEmbeddedNativeLibs   bool
 	useEmbeddedDex          bool
 	usesNonSdkApis          bool
-	sdkLibraries            dexpreopt.ClassLoaderContextMap
 	hasNoCode               bool
 	LoggingParent           string
 	resourceFiles           android.Paths
@@ -259,12 +258,11 @@ var extractAssetsRule = pctx.AndroidStaticRule("extractAssets",
 		CommandDeps: []string{"${config.Zip2ZipCmd}"},
 	})
 
-func (a *aapt) buildActions(ctx android.ModuleContext, sdkContext sdkContext, extraLinkFlags ...string) {
+func (a *aapt) buildActions(ctx android.ModuleContext, sdkContext sdkContext,
+	sdkLibraries dexpreopt.ClassLoaderContextMap, extraLinkFlags ...string) {
 
-	transitiveStaticLibs, transitiveStaticLibManifests, staticRRODirs, assetPackages, libDeps, libFlags, sdkLibraries :=
+	transitiveStaticLibs, transitiveStaticLibManifests, staticRRODirs, assetPackages, libDeps, libFlags :=
-		aaptLibs(ctx, sdkContext)
+		aaptLibs(ctx, sdkContext, sdkLibraries)
-
-	a.sdkLibraries = sdkLibraries
 
 	// App manifest file
 	manifestFile := proptools.StringDefault(a.aaptProperties.Manifest, "AndroidManifest.xml")
@@ -391,29 +389,31 @@ func (a *aapt) buildActions(ctx android.ModuleContext, sdkContext sdkContext, ex
 }
 
 // aaptLibs collects libraries from dependencies and sdk_version and converts them into paths
-func aaptLibs(ctx android.ModuleContext, sdkContext sdkContext) (transitiveStaticLibs, transitiveStaticLibManifests android.Paths,
+func aaptLibs(ctx android.ModuleContext, sdkContext sdkContext, sdkLibraries dexpreopt.ClassLoaderContextMap) (
-	staticRRODirs []rroDir, assets, deps android.Paths, flags []string, sdkLibraries dexpreopt.ClassLoaderContextMap) {
+	transitiveStaticLibs, transitiveStaticLibManifests android.Paths, staticRRODirs []rroDir, assets, deps android.Paths, flags []string) {
 
 	var sharedLibs android.Paths
 
+	if sdkLibraries == nil {
+		// Not all callers need to compute class loader context, those who don't just pass nil.
+		// Create a temporary class loader context here (it will be computed, but not used).
+		sdkLibraries = make(dexpreopt.ClassLoaderContextMap)
+	}
+
 	sdkDep := decodeSdkDep(ctx, sdkContext)
 	if sdkDep.useFiles {
 		sharedLibs = append(sharedLibs, sdkDep.jars...)
 	}
 
-	sdkLibraries = make(dexpreopt.ClassLoaderContextMap)
-
 	ctx.VisitDirectDeps(func(module android.Module) {
+		depName := ctx.OtherModuleName(module)
+
 		var exportPackage android.Path
 		aarDep, _ := module.(AndroidLibraryDependency)
 		if aarDep != nil {
 			exportPackage = aarDep.ExportPackage()
 		}
 
-		if dep, ok := module.(Dependency); ok {
-			sdkLibraries.AddContextMap(dep.ExportedSdkLibs())
-		}
-
 		switch ctx.OtherModuleDependencyTag(module) {
 		case instrumentationForTag:
 			// Nothing, instrumentationForTag is treated as libTag for javac but not for aapt2.
@@ -439,7 +439,7 @@ func aaptLibs(ctx android.ModuleContext, sdkContext sdkContext) (transitiveStati
 				transitiveStaticLibs = append(transitiveStaticLibs, aarDep.ExportedStaticPackages()...)
 				transitiveStaticLibs = append(transitiveStaticLibs, exportPackage)
 				transitiveStaticLibManifests = append(transitiveStaticLibManifests, aarDep.ExportedManifests()...)
-				sdkLibraries.AddContextMap(aarDep.ExportedSdkLibs())
+				sdkLibraries.AddContextMap(aarDep.ExportedSdkLibs(), depName)
 				if aarDep.ExportedAssets().Valid() {
 					assets = append(assets, aarDep.ExportedAssets().Path())
 				}
@@ -457,6 +457,12 @@ func aaptLibs(ctx android.ModuleContext, sdkContext sdkContext) (transitiveStati
 				}
 			}
 		}
+
+		// Add nested dependencies after processing the direct dependency: if it is a <uses-library>,
+		// nested context is added as its subcontext, and should not be re-added at the top-level.
+		if dep, ok := module.(Dependency); ok {
+			sdkLibraries.AddContextMap(dep.ExportedSdkLibs(), depName)
+		}
 	})
 
 	deps = append(deps, sharedLibs...)
@@ -473,7 +479,7 @@ func aaptLibs(ctx android.ModuleContext, sdkContext sdkContext) (transitiveStati
 	transitiveStaticLibs = android.FirstUniquePaths(transitiveStaticLibs)
 	transitiveStaticLibManifests = android.FirstUniquePaths(transitiveStaticLibManifests)
 
-	return transitiveStaticLibs, transitiveStaticLibManifests, staticRRODirs, assets, deps, flags, sdkLibraries
+	return transitiveStaticLibs, transitiveStaticLibManifests, staticRRODirs, assets, deps, flags
 }
 
 type AndroidLibrary struct {
@@ -508,8 +514,8 @@ func (a *AndroidLibrary) DepsMutator(ctx android.BottomUpMutatorContext) {
 
 func (a *AndroidLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	a.aapt.isLibrary = true
-	a.aapt.buildActions(ctx, sdkContext(a))
+	a.exportedSdkLibs = make(dexpreopt.ClassLoaderContextMap)
-	a.exportedSdkLibs = a.aapt.sdkLibraries
+	a.aapt.buildActions(ctx, sdkContext(a), a.exportedSdkLibs)
 
 	a.hideApexVariantFromMake = !ctx.Provider(android.ApexInfoProvider).(android.ApexInfo).IsForPlatform()
 
@@ -781,12 +787,11 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	linkFlags = append(linkFlags, "--manifest "+a.manifest.String())
 	linkDeps = append(linkDeps, a.manifest)
 
-	transitiveStaticLibs, staticLibManifests, staticRRODirs, transitiveAssets, libDeps, libFlags, sdkLibraries :=
+	transitiveStaticLibs, staticLibManifests, staticRRODirs, transitiveAssets, libDeps, libFlags :=
-		aaptLibs(ctx, sdkContext(a))
+		aaptLibs(ctx, sdkContext(a), nil)
 
 	_ = staticLibManifests
 	_ = staticRRODirs
-	_ = sdkLibraries
 
 	linkDeps = append(linkDeps, libDeps...)
 	linkFlags = append(linkFlags, libFlags...)

java/app.go

@@ -565,9 +565,8 @@ func (a *AndroidApp) aaptBuildActions(ctx android.ModuleContext) {
 	aaptLinkFlags = append(aaptLinkFlags, a.additionalAaptFlags...)
 
 	a.aapt.splitNames = a.appProperties.Package_splits
-	a.aapt.sdkLibraries = a.exportedSdkLibs
 	a.aapt.LoggingParent = String(a.overridableAppProperties.Logging_parent)
-	a.aapt.buildActions(ctx, sdkContext(a), aaptLinkFlags...)
+	a.aapt.buildActions(ctx, sdkContext(a), a.exportedSdkLibs, aaptLinkFlags...)
 
 	// apps manifests are handled by aapt, don't let Module see them
 	a.properties.Manifest = nil
@@ -601,7 +600,7 @@ func (a *AndroidApp) installPath(ctx android.ModuleContext) android.InstallPath
 	return android.PathForModuleInstall(ctx, installDir, a.installApkName+".apk")
 }
 
-func (a *AndroidApp) dexBuildActions(ctx android.ModuleContext, sdkLibs dexpreopt.ClassLoaderContextMap) android.Path {
+func (a *AndroidApp) dexBuildActions(ctx android.ModuleContext) android.Path {
 	a.dexpreopter.installPath = a.installPath(ctx)
 	if a.dexProperties.Uncompress_dex == nil {
 		// If the value was not force-set by the user, use reasonable default based on the module.
@@ -609,10 +608,8 @@ func (a *AndroidApp) dexBuildActions(ctx android.ModuleContext, sdkLibs dexpreop
 	}
 	a.dexpreopter.uncompressedDex = *a.dexProperties.Uncompress_dex
 	a.dexpreopter.enforceUsesLibs = a.usesLibrary.enforceUsesLibraries()
-	a.dexpreopter.classLoaderContexts = a.usesLibrary.classLoaderContextForUsesLibDeps(ctx)
+	a.dexpreopter.classLoaderContexts = a.exportedSdkLibs
-	a.dexpreopter.classLoaderContexts.AddContextMap(sdkLibs)
 	a.dexpreopter.manifestFile = a.mergedManifestFile
-	a.exportedSdkLibs = make(dexpreopt.ClassLoaderContextMap)
 
 	if ctx.ModuleName() != "framework-res" {
 		a.Module.compile(ctx, a.aaptSrcJar)
@@ -782,6 +779,8 @@ func (a *AndroidApp) generateAndroidBuildActions(ctx android.ModuleContext) {
 		a.aapt.noticeFile = a.noticeOutputs.HtmlGzOutput
 	}
 
+	a.exportedSdkLibs = a.usesLibrary.classLoaderContextForUsesLibDeps(ctx)
+
 	// Process all building blocks, from AAPT to certificates.
 	a.aaptBuildActions(ctx)
 
@@ -789,7 +788,7 @@ func (a *AndroidApp) generateAndroidBuildActions(ctx android.ModuleContext) {
 	a.usesLibrary.freezeEnforceUsesLibraries()
 
 	// Add implicit SDK libraries to <uses-library> list.
-	for _, usesLib := range a.aapt.sdkLibraries.UsesLibs() {
+	for _, usesLib := range a.exportedSdkLibs.UsesLibs() {
 		a.usesLibrary.addLib(usesLib, inList(usesLib, dexpreopt.OptionalCompatUsesLibs))
 	}
 
@@ -806,7 +805,7 @@ func (a *AndroidApp) generateAndroidBuildActions(ctx android.ModuleContext) {
 	a.linter.resources = a.aapt.resourceFiles
 	a.linter.buildModuleReportZip = ctx.Config().UnbundledBuildApps()
 
-	dexJarFile := a.dexBuildActions(ctx, a.aapt.sdkLibraries)
+	dexJarFile := a.dexBuildActions(ctx)
 
 	jniLibs, certificateDeps := collectAppDeps(ctx, a, a.shouldEmbedJnis(ctx), !Bool(a.appProperties.Jni_uses_platform_apis))
 	jniJarFile := a.jniBuildActions(jniLibs, ctx)
@@ -1848,7 +1847,7 @@ func (r *RuntimeResourceOverlay) GenerateAndroidBuildActions(ctx android.ModuleC
 		aaptLinkFlags = append(aaptLinkFlags,
 			"--rename-overlay-target-package "+*r.overridableProperties.Target_package_name)
 	}
-	r.aapt.buildActions(ctx, r, aaptLinkFlags...)
+	r.aapt.buildActions(ctx, r, nil, aaptLinkFlags...)
 
 	// Sign the built package
 	_, certificates := collectAppDeps(ctx, r, false, false)

java/java.go

@@ -1039,7 +1039,7 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps {
 			case libTag, instrumentationForTag:
 				deps.classpath = append(deps.classpath, dep.HeaderJars()...)
 				// sdk lib names from dependencies are re-exported
-				j.exportedSdkLibs.AddContextMap(dep.ExportedSdkLibs())
+				j.exportedSdkLibs.AddContextMap(dep.ExportedSdkLibs(), otherName)
 				deps.aidlIncludeDirs = append(deps.aidlIncludeDirs, dep.AidlIncludeDirs()...)
 				pluginJars, pluginClasses := dep.ExportedPlugins()
 				addPlugins(&deps, pluginJars, pluginClasses...)
@@ -1051,7 +1051,7 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps {
 				deps.staticHeaderJars = append(deps.staticHeaderJars, dep.HeaderJars()...)
 				deps.staticResourceJars = append(deps.staticResourceJars, dep.ResourceJars()...)
 				// sdk lib names from dependencies are re-exported
-				j.exportedSdkLibs.AddContextMap(dep.ExportedSdkLibs())
+				j.exportedSdkLibs.AddContextMap(dep.ExportedSdkLibs(), otherName)
 				deps.aidlIncludeDirs = append(deps.aidlIncludeDirs, dep.AidlIncludeDirs()...)
 				pluginJars, pluginClasses := dep.ExportedPlugins()
 				addPlugins(&deps, pluginJars, pluginClasses...)
@@ -2735,7 +2735,7 @@ func (j *Import) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 			case libTag, staticLibTag:
 				flags.classpath = append(flags.classpath, dep.HeaderJars()...)
 				// sdk lib names from dependencies are re-exported
-				j.exportedSdkLibs.AddContextMap(dep.ExportedSdkLibs())
+				j.exportedSdkLibs.AddContextMap(dep.ExportedSdkLibs(), otherName)
 			case bootClasspathTag:
 				flags.bootClasspath = append(flags.bootClasspath, dep.HeaderJars()...)
 			}