From 840efb666118ce15abbfaf49c11828d41ec38a80 Mon Sep 17 00:00:00 2001 From: Ulya Trafimovich Date: Thu, 15 Jul 2021 14:34:40 +0100 Subject: [PATCH] Don't attempt to add stub libraries to class loader context. A Java module may depend on a stub library. In that case an additional dependency on the implementation library is created, and it is used to add the implementation library to class loader context. We should not attempt to add the stubs library as well (previously the attempt to add it happend after the implemention was added to CLC, to the attempt was unsuccessful). Raise an error if someone tries to add the same library with different build/instal paths. Also, rename local variable `implicitSdkLib` to `sdkLib` to better reflect its meaning. Bug: 193425964 Test: $ lunch aosp_cf_x86_64_phone-userdebug && m && launch_cvd $ adb wait-for-device && \ adb root && \ adb logcat | \ grep -E 'ClassLoaderContext [a-z ]+ mismatch' -C1 # empty output, no errors Change-Id: I01c1bdd23f9d118d891d0b806e7e3b4d78896a34 --- dexpreopt/class_loader_context.go | 14 +++++++++--- java/app.go | 9 ++++++++ java/java.go | 37 ++++++++++++++----------------- 3 files changed, 37 insertions(+), 23 deletions(-) diff --git a/dexpreopt/class_loader_context.go b/dexpreopt/class_loader_context.go index 94b9adf76..245af2cea 100644 --- a/dexpreopt/class_loader_context.go +++ b/dexpreopt/class_loader_context.go @@ -286,11 +286,19 @@ func (clcMap ClassLoaderContextMap) addContext(ctx android.ModuleInstallPathCont } subcontexts := nestedClcMap[AnySdkVersion] - // If the library with this name is already present as one of the unconditional top-level - // components, do not re-add it. + // Check if the library with this name is already present in unconditional top-level CLC. for _, clc := range clcMap[sdkVer] { - if clc.Name == lib { + if clc.Name != lib { + // Ok, a different library. + } else if clc.Host == hostPath && clc.Device == devicePath { + // Ok, the same library with the same paths. Don't re-add it, but don't raise an error + // either, as the same library may be reachable via different transitional dependencies. return nil + } else { + // Fail, as someone is trying to add the same library with different paths. This likely + // indicates an error somewhere else, like trying to add a stub library. + return fmt.Errorf("a named %q is already in class loader context,"+ + "but the library paths are different:\t\n", lib) } } diff --git a/java/app.go b/java/app.go index 4456b22dc..4e967ad45 100755 --- a/java/app.go +++ b/java/app.go @@ -1264,6 +1264,15 @@ func (u *usesLibrary) classLoaderContextForUsesLibDeps(ctx android.ModuleContext dep := android.RemoveOptionalPrebuiltPrefix(ctx.OtherModuleName(m)) + // Skip stub libraries. A dependency on the implementation library has been added earlier, + // so it will be added to CLC, but the stub shouldn't be. Stub libraries can be distingushed + // from implementation libraries by their name, which is different as it has a suffix. + if comp, ok := m.(SdkLibraryComponentDependency); ok { + if impl := comp.OptionalSdkLibraryImplementation(); impl != nil && *impl != dep { + return + } + } + if lib, ok := m.(UsesLibraryDependency); ok { libName := dep if ulib, ok := m.(ProvidesUsesLib); ok && ulib.ProvidesUsesLib() != nil { diff --git a/java/java.go b/java/java.go index e74185ec5..a751ae0dc 100644 --- a/java/java.go +++ b/java/java.go @@ -1766,22 +1766,16 @@ func addCLCFromDep(ctx android.ModuleContext, depModule android.Module, return } - // Find out if the dependency is either an SDK library or an ordinary library that is disguised - // as an SDK library by the means of `provides_uses_lib` property. If yes, the library is itself - // a and should be added as a node in the CLC tree, and its CLC should be added - // as subtree of that node. Otherwise the library is not a and should not be - // added to CLC, but the transitive dependencies from its CLC should be added to - // the current CLC. - var implicitSdkLib *string - comp, isComp := depModule.(SdkLibraryComponentDependency) - if isComp { - implicitSdkLib = comp.OptionalImplicitSdkLibrary() - // OptionalImplicitSdkLibrary() may be nil so need to fall through to ProvidesUsesLib(). - } - if implicitSdkLib == nil { - if ulib, ok := depModule.(ProvidesUsesLib); ok { - implicitSdkLib = ulib.ProvidesUsesLib() - } + depName := android.RemoveOptionalPrebuiltPrefix(ctx.OtherModuleName(depModule)) + + var sdkLib *string + if lib, ok := depModule.(SdkLibraryDependency); ok && lib.sharedLibrary() { + // A shared SDK library. This should be added as a top-level CLC element. + sdkLib = &depName + } else if ulib, ok := depModule.(ProvidesUsesLib); ok { + // A non-SDK library disguised as an SDK library by the means of `provides_uses_lib` + // property. This should be handled in the same way as a shared SDK library. + sdkLib = ulib.ProvidesUsesLib() } depTag := ctx.OtherModuleDependencyTag(depModule) @@ -1791,7 +1785,7 @@ func addCLCFromDep(ctx android.ModuleContext, depModule android.Module, // Propagate through static library dependencies, unless it is a component // library (such as stubs). Component libraries have a dependency on their SDK library, // which should not be pulled just because of a static component library. - if implicitSdkLib != nil { + if sdkLib != nil { return } } else { @@ -1799,11 +1793,14 @@ func addCLCFromDep(ctx android.ModuleContext, depModule android.Module, return } - if implicitSdkLib != nil { - clcMap.AddContext(ctx, dexpreopt.AnySdkVersion, *implicitSdkLib, + // If this is an SDK (or SDK-like) library, then it should be added as a node in the CLC tree, + // and its CLC should be added as subtree of that node. Otherwise the library is not a + // and should not be added to CLC, but the transitive dependencies + // from its CLC should be added to the current CLC. + if sdkLib != nil { + clcMap.AddContext(ctx, dexpreopt.AnySdkVersion, *sdkLib, dep.DexJarBuildPath(), dep.DexJarInstallPath(), dep.ClassLoaderContexts()) } else { - depName := ctx.OtherModuleName(depModule) clcMap.AddContextMap(dep.ClassLoaderContexts(), depName) } }