From bd6b076f8062432ff955614fda7d2c9222025b36 Mon Sep 17 00:00:00 2001 From: Ulya Trafimovich Date: Fri, 12 Mar 2021 12:12:12 +0000 Subject: [PATCH 1/2] Allow modules that don't run verify_uses_libraries to have nonempty CLC. There are modules that would have passed verify_uses_library check, but don't run it for some reason (the check gets enabled either with an explicit setting, or if the module has nonempty lists in the build properties). Previously all such modules were assumed to have empty CLC, which is not always true. In particular, compatibility libraries are ignored, which affected e.g. Calendar and messaging apps. This CL gives such apps a chance to have correct CLC. The goal for the future is to enforce verify_library_check by default. Bug: 132357300 Test: lunch aosp_cf_x86_64_phone-userdebug && m Change-Id: Iea3be0fc9d7775c52950848b5a3fd3b7fcd36c53 --- dexpreopt/dexpreopt.go | 60 +++++++++++++++++++++++------------- scripts/construct_context.py | 4 +-- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/dexpreopt/dexpreopt.go b/dexpreopt/dexpreopt.go index 4999bc77e..216e205ad 100644 --- a/dexpreopt/dexpreopt.go +++ b/dexpreopt/dexpreopt.go @@ -259,35 +259,53 @@ func dexpreoptCommand(ctx android.PathContext, globalSoong *GlobalSoongConfig, g Implicits(clcHost). Text("stored_class_loader_context_arg=--stored-class-loader-context=PCL[" + strings.Join(clcTarget, ":") + "]") - } else if module.EnforceUsesLibraries { - // Generate command that saves target SDK version in a shell variable. - manifestOrApk := module.ManifestPath - if manifestOrApk == nil { - // No manifest to extract targetSdkVersion from, hope that dexjar is an APK. + } else { + // There are three categories of Java modules handled here: + // + // - Modules that have passed verify_uses_libraries check. They are AOT-compiled and + // expected to be loaded on device without CLC mismatch errors. + // + // - Modules that have failed the check in relaxed mode, so it didn't cause a build error. + // They are dexpreopted with "verify" filter and not AOT-compiled. + // TODO(b/132357300): ensure that CLC mismatch errors are ignored with "verify" filter. + // + // - Modules that didn't run the check. They are AOT-compiled, but it's unknown if they + // will have CLC mismatch errors on device (the check is disabled by default). + // + // TODO(b/132357300): enable the check by default and eliminate the last category, so that + // no time/space is wasted on AOT-compiling modules that will fail CLC check on device. + + var manifestOrApk android.Path + if module.ManifestPath != nil { + // Ok, there is an XML manifest. + manifestOrApk = module.ManifestPath + } else if filepath.Ext(base) == ".apk" { + // Ok, there is is an APK with the manifest inside. manifestOrApk = module.DexPath } - rule.Command().Text(`target_sdk_version="$(`). - Tool(globalSoong.ManifestCheck). - Flag("--extract-target-sdk-version"). - Input(manifestOrApk). - FlagWithInput("--aapt ", ctx.Config().HostToolPath(ctx, "aapt")). - Text(`)"`) + + // Generate command that saves target SDK version in a shell variable. + if manifestOrApk == nil { + // There is neither an XML manifest nor APK => nowhere to extract targetSdkVersion from. + // Set the latest ("any") version: then construct_context will not add any compatibility + // libraries (if this is incorrect, there will be a CLC mismatch and dexopt on device). + rule.Command().Textf(`target_sdk_version=%d`, AnySdkVersion) + } else { + rule.Command().Text(`target_sdk_version="$(`). + Tool(globalSoong.ManifestCheck). + Flag("--extract-target-sdk-version"). + Input(manifestOrApk). + FlagWithInput("--aapt ", ctx.Config().HostToolPath(ctx, "aapt")). + Text(`)"`) + } // Generate command that saves host and target class loader context in shell variables. clc, paths := ComputeClassLoaderContext(module.ClassLoaderContexts) rule.Command(). - Text("if ! test -s ").Input(module.EnforceUsesLibrariesStatusFile). - Text(` ; then eval "$(`).Tool(globalSoong.ConstructContext). + Text(`eval "$(`).Tool(globalSoong.ConstructContext). Text(` --target-sdk-version ${target_sdk_version}`). Text(clc).Implicits(paths). - Text(`)" ; fi`) - - } else { - // Other libraries or APKs for which the exact list is unknown. - // We assume the class loader context is empty. - rule.Command(). - Text(`class_loader_context_arg=--class-loader-context=PCL[]`). - Text(`stored_class_loader_context_arg=""`) + Text(`)"`) } // Devices that do not have a product partition use a symlink from /product to /system/product. diff --git a/scripts/construct_context.py b/scripts/construct_context.py index 6f9edc429..f0658baa2 100755 --- a/scripts/construct_context.py +++ b/scripts/construct_context.py @@ -66,9 +66,9 @@ def main(): if not args.sdk: raise SystemExit('target sdk version is not set') if not args.host_contexts: - raise SystemExit('host context is not set') + args.host_contexts = [] if not args.target_contexts: - raise SystemExit('target context is not set') + args.target_contexts = [] print(construct_contexts(args)) From dd622951a431eb7eb1e01ffa6c8a878dbb0f75f8 Mon Sep 17 00:00:00 2001 From: Ulya Trafimovich Date: Thu, 25 Mar 2021 12:52:49 +0000 Subject: [PATCH 2/2] Fix AAPT path in dexpreopt. Dexpreopt uses AAPT to parse `targetSdkVersion` from the manifest, so it the generated dexpreopt.sh script that calls AAPT must depend on it, otherwise AAPT might not be built by the time it is used. Tool dependencies are handled via the global Soong config and the DEXPREOPT_GEN_DEPS variable that Soong generates for Make. This config always uses Soong tool paths, like out/soong/host/linux-x86/bin/aapt rather than out/host/linux-x86/bin/aapt. This CL fixes a mistake in dexpreopt rule that used context-dependent AAPT path (so, when called from Make, the dependency was on Soong tool, but a Make tool was actually used, so it failed sometimes). Bug: 132357300 Test: lunch aosp_cf_x86_64_phone-userdebug && m Change-Id: I1f0ab4afac98e6239f324e7f3571d670fd7a36cd --- dexpreopt/dexpreopt.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dexpreopt/dexpreopt.go b/dexpreopt/dexpreopt.go index 216e205ad..81a63b061 100644 --- a/dexpreopt/dexpreopt.go +++ b/dexpreopt/dexpreopt.go @@ -295,7 +295,7 @@ func dexpreoptCommand(ctx android.PathContext, globalSoong *GlobalSoongConfig, g Tool(globalSoong.ManifestCheck). Flag("--extract-target-sdk-version"). Input(manifestOrApk). - FlagWithInput("--aapt ", ctx.Config().HostToolPath(ctx, "aapt")). + FlagWithInput("--aapt ", globalSoong.Aapt). Text(`)"`) }