Fix incorrect handling of API outline stubs
* Don't include exception guards in method API level calculation. These
classes will be stubbed if required and therefore always present at
runtime and not causing soft verification errors. The previous inclusion
had the sideeffect of inlining the outlined instantiation.
* Don't remove library classes from library during compilation for
classes which are stubbed. Doing so could cause the the guard to be
removed as the class becomes a program class only which is not
registered as allocated in the object allocating info (when the
instantiation was processed the class was still a library class).
* Include an API level check in the SingleCallerInliner
Bug: b/342961827
Change-Id: I3576730761f1ed3fe00c291f8e41df06d07ff9f8
diff --git a/src/main/java/com/android/tools/r8/graph/DirectMappedDexApplication.java b/src/main/java/com/android/tools/r8/graph/DirectMappedDexApplication.java
index 1f97831..52ae66a 100644
--- a/src/main/java/com/android/tools/r8/graph/DirectMappedDexApplication.java
+++ b/src/main/java/com/android/tools/r8/graph/DirectMappedDexApplication.java
@@ -238,7 +238,11 @@
public void addProgramClassPotentiallyOverridingNonProgramClass(DexProgramClass clazz) {
addProgramClass(clazz);
pendingClasspathRemovalIfPresent.add(clazz.type);
- if (libraryClasses.containsKey(clazz.type)) {
+ // When java.lang.Record is added remove the library class, as this program class will
+ // eventually be renamed to com.android.tools.r8.RecordTag so there should not be a
+ // multi-resolution result when looking up java.lang.Record.
+ if (clazz.type.isIdenticalTo(options.dexItemFactory().recordType)
+ && libraryClasses.containsKey(clazz.type)) {
ensureMutableLibraryClassesMap();
libraryClasses.remove(clazz.type);
}
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java b/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
index b6e87c2..f5e2f36 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/DeadCodeRemover.java
@@ -3,11 +3,11 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.ir.optimize;
-import static com.android.tools.r8.graph.DexProgramClass.asProgramClassOrNull;
import com.android.tools.r8.errors.Unreachable;
import com.android.tools.r8.graph.AppView;
-import com.android.tools.r8.graph.DexProgramClass;
+import com.android.tools.r8.graph.ClassResolutionResult;
+import com.android.tools.r8.graph.DexClass;
import com.android.tools.r8.graph.DexType;
import com.android.tools.r8.ir.code.BasicBlock;
import com.android.tools.r8.ir.code.CatchHandlers;
@@ -224,7 +224,8 @@
if (block.hasCatchHandlers()) {
if (block.canThrow()) {
if (appView.enableWholeProgramOptimizations()) {
- Collection<CatchHandler<BasicBlock>> deadCatchHandlers = getDeadCatchHandlers(block);
+ Collection<CatchHandler<BasicBlock>> deadCatchHandlers =
+ getDeadCatchHandlers(code, block);
if (!deadCatchHandlers.isEmpty()) {
for (CatchHandler<BasicBlock> catchHandler : deadCatchHandlers) {
catchHandler.target.unlinkCatchHandlerForGuard(catchHandler.guard);
@@ -249,10 +250,8 @@
return mayHaveIntroducedUnreachableBlocks;
}
- /**
- * Returns the catch handlers of the given block that are dead, if any.
- */
- private Collection<CatchHandler<BasicBlock>> getDeadCatchHandlers(BasicBlock block) {
+ /** Returns the catch handlers of the given block that are dead, if any. */
+ private Collection<CatchHandler<BasicBlock>> getDeadCatchHandlers(IRCode code, BasicBlock block) {
AppInfoWithLiveness appInfoWithLiveness = appView.appInfo().withLiveness();
ImmutableList.Builder<CatchHandler<BasicBlock>> builder = ImmutableList.builder();
CatchHandlers<BasicBlock> catchHandlers = block.getCatchHandlers();
@@ -275,11 +274,19 @@
continue;
}
- // We can exploit that a catch handler must be dead if its guard is never instantiated
- // directly or indirectly.
if (appInfoWithLiveness != null) {
- DexProgramClass clazz = asProgramClassOrNull(appView.definitionFor(guard));
- if (clazz != null && !appInfoWithLiveness.isInstantiatedDirectlyOrIndirectly(clazz)) {
+ ClassResolutionResult result =
+ appView.definitionForWithResolutionResult(guard, code.context().getContextClass());
+ if (!result.hasClassResolutionResult() || result.isMultipleClassResolutionResult()) {
+ // With a multi resolution result one of the results is a library class, so the guard
+ // cannot be removed.
+ continue;
+ }
+ // We can exploit that a catch handler must be dead if its guard is never instantiated
+ // directly or indirectly.
+ DexClass clazz = result.toSingleClassWithProgramOverLibrary();
+ if (clazz.isProgramClass()
+ && !appInfoWithLiveness.isInstantiatedDirectlyOrIndirectly(clazz.asProgramClass())) {
builder.add(new CatchHandler<>(guard, target));
continue;
}
diff --git a/src/main/java/com/android/tools/r8/optimize/singlecaller/SingleCallerScanner.java b/src/main/java/com/android/tools/r8/optimize/singlecaller/SingleCallerScanner.java
index fa0bdfd..205bd4b 100644
--- a/src/main/java/com/android/tools/r8/optimize/singlecaller/SingleCallerScanner.java
+++ b/src/main/java/com/android/tools/r8/optimize/singlecaller/SingleCallerScanner.java
@@ -18,6 +18,7 @@
import com.android.tools.r8.lightir.LirInstructionView;
import com.android.tools.r8.lightir.LirOpcodeUtils;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.utils.AndroidApiLevelUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.ObjectUtils;
import com.android.tools.r8.utils.ThreadUtils;
@@ -47,7 +48,8 @@
singleCallerMethodCandidates.removeIf(
(callee, caller) ->
callee.getDefinition().isLibraryMethodOverride().isPossiblyTrue()
- || !appView.getKeepInfo(callee).isSingleCallerInliningAllowed(options));
+ || !appView.getKeepInfo(callee).isSingleCallerInliningAllowed(options)
+ || !AndroidApiLevelUtils.isApiSafeForInlining(caller, callee, appView.options()));
return traceInstructions(singleCallerMethodCandidates, executorService);
}
diff --git a/src/main/java/com/android/tools/r8/shaking/ComputeApiLevelUseRegistry.java b/src/main/java/com/android/tools/r8/shaking/ComputeApiLevelUseRegistry.java
index bd53f9a..ea26098 100644
--- a/src/main/java/com/android/tools/r8/shaking/ComputeApiLevelUseRegistry.java
+++ b/src/main/java/com/android/tools/r8/shaking/ComputeApiLevelUseRegistry.java
@@ -131,7 +131,7 @@
@Override
public void registerTypeReference(DexType type) {
- // Type references are OK as long as we do not have a use on them
+ // Type references are OK as long as we do not have a use on them.
}
@Override
@@ -141,7 +141,9 @@
@Override
public void registerExceptionGuard(DexType guard) {
- setMaxApiReferenceLevel(guard);
+ // Type references as exception guard are OK unless unknown. Library exception guards will be
+ // stubbed.
+ setMaxApiReferenceLevelIfUnknown(guard);
}
@Override
@@ -170,6 +172,15 @@
}
}
+ private void setMaxApiReferenceLevelIfUnknown(DexType type) {
+ if (isEnabled) {
+ ComputedApiLevel computedApiLevel = apiLevelCompute.computeApiLevelForLibraryReference(type);
+ if (computedApiLevel.isUnknownApiLevel()) {
+ maxApiReferenceLevel = maxApiReferenceLevel.max(computedApiLevel);
+ }
+ }
+ }
+
@SuppressWarnings("HidingField")
public ComputedApiLevel getMaxApiReferenceLevel() {
return maxApiReferenceLevel;
diff --git a/src/test/java/com/android/tools/r8/apimodel/ApiModelMockExceptionInstantiateAndCatchTest.java b/src/test/java/com/android/tools/r8/apimodel/ApiModelMockExceptionInstantiateAndCatchTest.java
index a45b22e..e212a7a 100644
--- a/src/test/java/com/android/tools/r8/apimodel/ApiModelMockExceptionInstantiateAndCatchTest.java
+++ b/src/test/java/com/android/tools/r8/apimodel/ApiModelMockExceptionInstantiateAndCatchTest.java
@@ -160,12 +160,11 @@
testForR8(parameters.getBackend())
.apply(this::setupTestBuilder)
.addKeepMainRule(Main.class)
- .addDontObfuscate()
.compile()
.applyIf(
exceptionPresentAtRuntime(), b -> b.addBootClasspathClasses(LibraryException.class))
.run(parameters.getRuntime(), Main.class)
- .apply(this::checkUnexpectedOutput)
+ .apply(this::checkOutput)
.inspect(this::inspect);
}
@@ -181,21 +180,6 @@
}
}
- private void checkUnexpectedOutput(SingleTestRunResult<?> runResult) {
- if (exceptionPresentAtRuntime()) {
- if (parameters.isCfRuntime()
- || parameters.getApiLevel().isGreaterThanOrEqualTo(mockExceptionLevel)) {
- runResult.assertSuccessWithOutputLines("Valid behaviour");
- } else {
- // TODO(b/342961827): This should not happen.
- runResult.assertSuccessWithOutputLines(
- LibraryException.class.getTypeName() + ": Failed, true");
- }
- } else {
- runResult.assertSuccessWithOutputLines("java.lang.NoClassDefFoundError, false");
- }
- }
-
// Only present from api level P.
public static class LibraryException extends Exception {
public LibraryException(String message) {
diff --git a/src/test/java/com/android/tools/r8/apimodel/ApiModelNoInliningOfTryCatchReferenceTest.java b/src/test/java/com/android/tools/r8/apimodel/ApiModelNoInliningOfTryCatchReferenceTest.java
index a9ca217..6f52939 100644
--- a/src/test/java/com/android/tools/r8/apimodel/ApiModelNoInliningOfTryCatchReferenceTest.java
+++ b/src/test/java/com/android/tools/r8/apimodel/ApiModelNoInliningOfTryCatchReferenceTest.java
@@ -50,24 +50,16 @@
.apply(ApiModelingTestHelper::disableOutliningAndStubbing)
.enableInliningAnnotations()
.addHorizontallyMergedClassesInspector(
- horizontallyMergedClassesInspector -> {
- if (parameters.isDexRuntime()
- && parameters.getApiLevel().isGreaterThanOrEqualTo(exceptionApiLevel)) {
+ horizontallyMergedClassesInspector ->
horizontallyMergedClassesInspector.assertIsCompleteMergeGroup(
- TestClass.class, Caller.class);
- } else {
- horizontallyMergedClassesInspector.assertNoClassesMerged();
- }
- })
+ TestClass.class, Caller.class))
.apply(
ApiModelingTestHelper.addTracedApiReferenceLevelCallBack(
(reference, apiLevel) -> {
if (reference.equals(Reference.methodFromMethod(tryCatch))) {
+ // The exception catch guard does not contribute to the modelled API level.
assertEquals(
- exceptionApiLevel.max(
- parameters.isCfRuntime()
- ? AndroidApiLevel.B
- : parameters.getApiLevel()),
+ parameters.isCfRuntime() ? AndroidApiLevel.B : parameters.getApiLevel(),
apiLevel);
}
}))
