From 4b99e3bd4957adb2121ee45c838644783443cbf9 Mon Sep 17 00:00:00 2001 From: Songchun Fan Date: Thu, 11 Jun 2020 14:57:02 -0700 Subject: [PATCH] [pm] minor refactoring minor refactoring per comments of ag/11844801. Test: atest CtsExtractNativeLibsHostTestCases Test: atest CtsDynamicLinkerTestCases BUG: 158795052 Change-Id: I0e96e489e48995d27ebf880005fb01cfac6b045b --- .../internal/content/NativeLibraryHelper.java | 2 ++ .../android/server/pm/PackageAbiHelper.java | 8 +++-- .../server/pm/PackageAbiHelperImpl.java | 31 ++++++++++++------- .../server/pm/PackageManagerService.java | 24 +++++++------- .../src/com/android/server/pm/ScanTests.java | 6 ++-- 5 files changed, 41 insertions(+), 30 deletions(-) diff --git a/core/java/com/android/internal/content/NativeLibraryHelper.java b/core/java/com/android/internal/content/NativeLibraryHelper.java index 476198b5311a0..bbfd07b64ccfe 100644 --- a/core/java/com/android/internal/content/NativeLibraryHelper.java +++ b/core/java/com/android/internal/content/NativeLibraryHelper.java @@ -358,6 +358,8 @@ public class NativeLibraryHelper { createNativeLibrarySubdir(subDir); } + // Even if extractNativeLibs is false, we still need to check if the native libs in the APK + // are valid. This is done in the native code. int copyRet = copyNativeBinaries(handle, subDir, supportedAbi); if (copyRet != PackageManager.INSTALL_SUCCEEDED) { return copyRet; diff --git a/services/core/java/com/android/server/pm/PackageAbiHelper.java b/services/core/java/com/android/server/pm/PackageAbiHelper.java index e355bb9f6e60e..30b1c2c93a452 100644 --- a/services/core/java/com/android/server/pm/PackageAbiHelper.java +++ b/services/core/java/com/android/server/pm/PackageAbiHelper.java @@ -16,6 +16,7 @@ package com.android.server.pm; +import android.annotation.NonNull; import android.annotation.Nullable; import android.util.Pair; @@ -27,6 +28,8 @@ import com.android.server.pm.parsing.pkg.ParsedPackage; import java.io.File; import java.util.Set; + + // TODO: Move to .parsing sub-package @VisibleForTesting public interface PackageAbiHelper { @@ -34,7 +37,8 @@ public interface PackageAbiHelper { * Derive and get the location of native libraries for the given package, * which varies depending on where and how the package was installed. */ - NativeLibraryPaths getNativeLibraryPaths(AndroidPackage pkg, PackageSetting pkgSetting, + @NonNull + NativeLibraryPaths deriveNativeLibraryPaths(AndroidPackage pkg, boolean isUpdatedSystemApp, File appLib32InstallDir); /** @@ -51,7 +55,7 @@ public interface PackageAbiHelper { * If {@code extractLibs} is true, native libraries are extracted from the app if required. */ Pair derivePackageAbi(AndroidPackage pkg, boolean isUpdatedSystemApp, - String cpuAbiOverride, boolean extractLibs) throws PackageManagerException; + String cpuAbiOverride) throws PackageManagerException; /** * Calculates adjusted ABIs for a set of packages belonging to a shared user so that they all diff --git a/services/core/java/com/android/server/pm/PackageAbiHelperImpl.java b/services/core/java/com/android/server/pm/PackageAbiHelperImpl.java index fc58968a73253..8af7e1f4f6d1e 100644 --- a/services/core/java/com/android/server/pm/PackageAbiHelperImpl.java +++ b/services/core/java/com/android/server/pm/PackageAbiHelperImpl.java @@ -131,16 +131,16 @@ final class PackageAbiHelperImpl implements PackageAbiHelper { } @Override - public NativeLibraryPaths getNativeLibraryPaths(AndroidPackage pkg, PackageSetting pkgSetting, - File appLib32InstallDir) { + public NativeLibraryPaths deriveNativeLibraryPaths(AndroidPackage pkg, + boolean isUpdatedSystemApp, File appLib32InstallDir) { // Trying to derive the paths, thus need the raw ABI info from the parsed package, and the // current state in PackageSetting is irrelevant. - return getNativeLibraryPaths(new Abis(pkg.getPrimaryCpuAbi(), pkg.getSecondaryCpuAbi()), + return deriveNativeLibraryPaths(new Abis(pkg.getPrimaryCpuAbi(), pkg.getSecondaryCpuAbi()), appLib32InstallDir, pkg.getCodePath(), pkg.getBaseCodePath(), pkg.isSystem(), - pkgSetting.getPkgState().isUpdatedSystemApp()); + isUpdatedSystemApp); } - private static NativeLibraryPaths getNativeLibraryPaths(final Abis abis, + private static NativeLibraryPaths deriveNativeLibraryPaths(final Abis abis, final File appLib32InstallDir, final String codePath, final String sourceDir, final boolean isSystemApp, final boolean isUpdatedSystemApp) { final File codeFile = new File(codePath); @@ -296,22 +296,19 @@ final class PackageAbiHelperImpl implements PackageAbiHelper { @Override public Pair derivePackageAbi(AndroidPackage pkg, - boolean isUpdatedSystemApp, String cpuAbiOverride, boolean extractLibs) + boolean isUpdatedSystemApp, String cpuAbiOverride) throws PackageManagerException { // Give ourselves some initial paths; we'll come back for another // pass once we've determined ABI below. String pkgRawPrimaryCpuAbi = AndroidPackageUtils.getRawPrimaryCpuAbi(pkg); String pkgRawSecondaryCpuAbi = AndroidPackageUtils.getRawSecondaryCpuAbi(pkg); - final NativeLibraryPaths initialLibraryPaths = getNativeLibraryPaths( + final NativeLibraryPaths initialLibraryPaths = deriveNativeLibraryPaths( new Abis(pkgRawPrimaryCpuAbi, pkgRawSecondaryCpuAbi), PackageManagerService.sAppLib32InstallDir, pkg.getCodePath(), pkg.getBaseCodePath(), pkg.isSystem(), isUpdatedSystemApp); - // We shouldn't attempt to extract libs from system app when it was not updated. - if (pkg.isSystem() && !isUpdatedSystemApp) { - extractLibs = false; - } + final boolean extractLibs = shouldExtractLibs(pkg, isUpdatedSystemApp); final String nativeLibraryRootStr = initialLibraryPaths.nativeLibraryRootDir; final boolean useIsaSpecificSubdirs = initialLibraryPaths.nativeLibraryRootRequiresIsa; @@ -455,11 +452,21 @@ final class PackageAbiHelperImpl implements PackageAbiHelper { final Abis abis = new Abis(primaryCpuAbi, secondaryCpuAbi); return new Pair<>(abis, - getNativeLibraryPaths(abis, PackageManagerService.sAppLib32InstallDir, + deriveNativeLibraryPaths(abis, PackageManagerService.sAppLib32InstallDir, pkg.getCodePath(), pkg.getBaseCodePath(), pkg.isSystem(), isUpdatedSystemApp)); } + private boolean shouldExtractLibs(AndroidPackage pkg, boolean isUpdatedSystemApp) { + // We shouldn't extract libs if the package is a library or if extractNativeLibs=false + boolean extractLibs = !AndroidPackageUtils.isLibrary(pkg) && pkg.isExtractNativeLibs(); + // We shouldn't attempt to extract libs from system app when it was not updated. + if (pkg.isSystem() && !isUpdatedSystemApp) { + extractLibs = false; + } + return extractLibs; + } + /** * Adjusts ABIs for a set of packages belonging to a shared user so that they all match. * i.e, so that all packages can be run inside a single process if required. diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 7ac60c4c80646..9e2f124d75784 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -11477,15 +11477,14 @@ public class PackageManagerService extends IPackageManager.Stub } final String cpuAbiOverride = deriveAbiOverride(request.cpuAbiOverride, pkgSetting); + final boolean isUpdatedSystemApp = pkgSetting.getPkgState().isUpdatedSystemApp(); if ((scanFlags & SCAN_NEW_INSTALL) == 0) { if (needToDeriveAbi) { Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "derivePackageAbi"); - final boolean extractNativeLibs = !AndroidPackageUtils.isLibrary(parsedPackage); final Pair derivedAbi = - packageAbiHelper.derivePackageAbi(parsedPackage, - pkgSetting.getPkgState().isUpdatedSystemApp(), cpuAbiOverride, - extractNativeLibs); + packageAbiHelper.derivePackageAbi(parsedPackage, isUpdatedSystemApp, + cpuAbiOverride); derivedAbi.first.applyTo(parsedPackage); derivedAbi.second.applyTo(parsedPackage); Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER); @@ -11495,15 +11494,15 @@ public class PackageManagerService extends IPackageManager.Stub // structure. Try to detect abi based on directory structure. String pkgRawPrimaryCpuAbi = AndroidPackageUtils.getRawPrimaryCpuAbi(parsedPackage); - if (parsedPackage.isSystem() && !pkgSetting.getPkgState().isUpdatedSystemApp() && - pkgRawPrimaryCpuAbi == null) { + if (parsedPackage.isSystem() && !isUpdatedSystemApp + && pkgRawPrimaryCpuAbi == null) { final PackageAbiHelper.Abis abis = packageAbiHelper.getBundledAppAbis( parsedPackage); abis.applyTo(parsedPackage); abis.applyTo(pkgSetting); final PackageAbiHelper.NativeLibraryPaths nativeLibraryPaths = - packageAbiHelper.getNativeLibraryPaths(parsedPackage, pkgSetting, - sAppLib32InstallDir); + packageAbiHelper.deriveNativeLibraryPaths(parsedPackage, + isUpdatedSystemApp, sAppLib32InstallDir); nativeLibraryPaths.applyTo(parsedPackage); } } else { @@ -11514,8 +11513,8 @@ public class PackageManagerService extends IPackageManager.Stub .setSecondaryCpuAbi(secondaryCpuAbiFromSettings); final PackageAbiHelper.NativeLibraryPaths nativeLibraryPaths = - packageAbiHelper.getNativeLibraryPaths(parsedPackage, - pkgSetting, sAppLib32InstallDir); + packageAbiHelper.deriveNativeLibraryPaths(parsedPackage, + isUpdatedSystemApp, sAppLib32InstallDir); nativeLibraryPaths.applyTo(parsedPackage); if (DEBUG_ABI_SELECTION) { @@ -11540,7 +11539,7 @@ public class PackageManagerService extends IPackageManager.Stub // ABIs we determined during compilation, but the path will depend on the final // package path (after the rename away from the stage path). final PackageAbiHelper.NativeLibraryPaths nativeLibraryPaths = - packageAbiHelper.getNativeLibraryPaths(parsedPackage, pkgSetting, + packageAbiHelper.deriveNativeLibraryPaths(parsedPackage, isUpdatedSystemApp, sAppLib32InstallDir); nativeLibraryPaths.applyTo(parsedPackage); } @@ -17427,7 +17426,6 @@ public class PackageManagerService extends IPackageManager.Stub scanFlags |= SCAN_NO_DEX; try { - final boolean extractNativeLibs = !AndroidPackageUtils.isLibrary(parsedPackage); PackageSetting pkgSetting; synchronized (mLock) { pkgSetting = mSettings.getPackageLPr(pkgName); @@ -17442,7 +17440,7 @@ public class PackageManagerService extends IPackageManager.Stub final Pair derivedAbi = mInjector.getAbiHelper().derivePackageAbi(parsedPackage, isUpdatedSystemAppFromExistingSetting || isUpdatedSystemAppInferred, - abiOverride, extractNativeLibs); + abiOverride); derivedAbi.first.applyTo(parsedPackage); derivedAbi.second.applyTo(parsedPackage); } catch (PackageManagerException pme) { diff --git a/services/tests/servicestests/src/com/android/server/pm/ScanTests.java b/services/tests/servicestests/src/com/android/server/pm/ScanTests.java index 09f946d4b107e..e7eff00c472e2 100644 --- a/services/tests/servicestests/src/com/android/server/pm/ScanTests.java +++ b/services/tests/servicestests/src/com/android/server/pm/ScanTests.java @@ -102,13 +102,13 @@ public class ScanTests { @Before public void setupDefaultAbiBehavior() throws Exception { when(mMockPackageAbiHelper.derivePackageAbi( - any(AndroidPackage.class), anyBoolean(), nullable(String.class), anyBoolean())) + any(AndroidPackage.class), anyBoolean(), nullable(String.class))) .thenReturn(new Pair<>( new PackageAbiHelper.Abis("derivedPrimary", "derivedSecondary"), new PackageAbiHelper.NativeLibraryPaths( "derivedRootDir", true, "derivedNativeDir", "derivedNativeDir2"))); - when(mMockPackageAbiHelper.getNativeLibraryPaths( - any(AndroidPackage.class), any(PackageSetting.class), any(File.class))) + when(mMockPackageAbiHelper.deriveNativeLibraryPaths( + any(AndroidPackage.class), anyBoolean(), any(File.class))) .thenReturn(new PackageAbiHelper.NativeLibraryPaths( "getRootDir", true, "getNativeDir", "getNativeDir2" ));