Disable de-virtualizing of invoke if adding to main dex
Bug: 177571333
Change-Id: Iffe4d43a2dec019a5c7499c56dba1c16952a998c
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index d08eb3e..0621d6f 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -322,7 +322,9 @@
this.identifierNameStringMarker = null;
}
this.devirtualizer =
- options.enableDevirtualization ? new Devirtualizer(appViewWithLiveness) : null;
+ options.enableDevirtualization
+ ? new Devirtualizer(appViewWithLiveness, mainDexClasses)
+ : null;
this.typeChecker = new TypeChecker(appViewWithLiveness, VerifyTypesHelper.create(appView));
this.d8NestBasedAccessDesugaring = null;
this.serviceLoaderRewriter =
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java b/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java
index 674488f..7179327 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/Devirtualizer.java
@@ -25,6 +25,7 @@
import com.android.tools.r8.ir.code.InvokeVirtual;
import com.android.tools.r8.ir.code.Value;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
+import com.android.tools.r8.shaking.MainDexTracingResult;
import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -47,10 +48,13 @@
public class Devirtualizer {
private final AppView<AppInfoWithLiveness> appView;
+ private final MainDexTracingResult mainDexTracingResult;
private final InternalOptions options;
- public Devirtualizer(AppView<AppInfoWithLiveness> appView) {
+ public Devirtualizer(
+ AppView<AppInfoWithLiveness> appView, MainDexTracingResult mainDexTracingResult) {
this.appView = appView;
+ this.mainDexTracingResult = mainDexTracingResult;
this.options = appView.options();
}
@@ -148,7 +152,9 @@
// Rebind the invoke to the most specific target.
DexMethod invokedMethod = invoke.getInvokedMethod();
DexClassAndMethod reboundTarget = rebindSuperInvokeToMostSpecific(invokedMethod, context);
- if (reboundTarget != null && reboundTarget.getReference() != invokedMethod) {
+ if (reboundTarget != null
+ && reboundTarget.getReference() != invokedMethod
+ && !isRebindingNewClassIntoMainDex(invokedMethod, reboundTarget.getReference())) {
it.replaceCurrentInstruction(
new InvokeSuper(
reboundTarget.getReference(),
@@ -190,6 +196,11 @@
continue;
}
+ // Ensure that we are not adding a new main dex root
+ if (isRebindingNewClassIntoMainDex(invoke.getInvokedMethod(), target.getReference())) {
+ continue;
+ }
+
InvokeVirtual devirtualizedInvoke =
new InvokeVirtual(target.getReference(), invoke.outValue(), invoke.inValues());
it.replaceCurrentInstruction(devirtualizedInvoke);
@@ -382,4 +393,14 @@
// Change the invoke-virtual instruction to target the refined resolution result instead.
return newResolutionResult.getResolvedMethod().method;
}
+
+ private boolean isRebindingNewClassIntoMainDex(
+ DexMethod originalMethod, DexMethod reboundMethod) {
+ if (!mainDexTracingResult.isRoot(originalMethod.holder)
+ && !appView.appInfo().getMainDexClasses().contains(originalMethod.holder)) {
+ return false;
+ }
+ return !mainDexTracingResult.isRoot(reboundMethod.holder)
+ && !appView.appInfo().getMainDexClasses().contains(reboundMethod.holder);
+ }
}
diff --git a/src/main/java/com/android/tools/r8/shaking/MainDexTracingResult.java b/src/main/java/com/android/tools/r8/shaking/MainDexTracingResult.java
index 3af323c..5bf287f 100644
--- a/src/main/java/com/android/tools/r8/shaking/MainDexTracingResult.java
+++ b/src/main/java/com/android/tools/r8/shaking/MainDexTracingResult.java
@@ -146,6 +146,10 @@
return getRoots().contains(definition.getContextType());
}
+ public boolean isRoot(DexType type) {
+ return getRoots().contains(type);
+ }
+
public boolean isDependency(ProgramDefinition definition) {
return getDependencies().contains(definition.getContextType());
}
diff --git a/src/test/java/com/android/tools/r8/TestParametersBuilder.java b/src/test/java/com/android/tools/r8/TestParametersBuilder.java
index 9b84425..ffa34fe 100644
--- a/src/test/java/com/android/tools/r8/TestParametersBuilder.java
+++ b/src/test/java/com/android/tools/r8/TestParametersBuilder.java
@@ -115,6 +115,11 @@
return withDexRuntimesStartingFromIncluding(DexVm.Version.V5_1_1);
}
+ /** Add all available DEX runtimes that do not support native multidex. */
+ public TestParametersBuilder withMainDexRuntimes() {
+ return withDexRuntimesEndingAtExcluding(DexVm.Version.V5_1_1);
+ }
+
/** Add all available DEX runtimes starting from and including {@param startInclusive}. */
public TestParametersBuilder withDexRuntimesStartingFromIncluding(DexVm.Version startInclusive) {
return withDexRuntimeFilter(vm -> startInclusive.isOlderThanOrEqual(vm));
diff --git a/src/test/java/com/android/tools/r8/maindexlist/MainDexDevirtualizerTest.java b/src/test/java/com/android/tools/r8/maindexlist/MainDexDevirtualizerTest.java
new file mode 100644
index 0000000..6cb7ab4
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/maindexlist/MainDexDevirtualizerTest.java
@@ -0,0 +1,178 @@
+// Copyright (c) 2021, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package com.android.tools.r8.maindexlist;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndNotRenamed;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndRenamed;
+import static org.hamcrest.CoreMatchers.hasItem;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.IsNot.not;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
+
+import com.android.tools.r8.NeverClassInline;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.NoVerticalClassMerging;
+import com.android.tools.r8.R8FullTestBuilder;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.ThrowableConsumer;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.utils.Box;
+import com.android.tools.r8.utils.StringUtils;
+import com.android.tools.r8.utils.codeinspector.CheckCastInstructionSubject;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.InstructionSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.BiConsumer;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class MainDexDevirtualizerTest extends TestBase {
+
+ private final TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection data() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ public MainDexDevirtualizerTest(TestParameters parameters) {
+ this.parameters = parameters;
+ }
+
+ @Test
+ public void testR8() throws Exception {
+ assumeTrue(parameters.isCfRuntime() || !parameters.getDexRuntimeVersion().isDalvik());
+ runTest(
+ testBuilder -> {},
+ (inspector, mainDexClasses) -> {
+ assertTrue(mainDexClasses.isEmpty());
+ // Verify that the call to I.foo in Main.class has been changed to A.foo by checking for a
+ // cast.
+ ClassSubject clazz = inspector.clazz(Main.class);
+ assertThat(clazz, isPresentAndNotRenamed());
+ MethodSubject main = clazz.uniqueMethodWithName("main");
+ assertThat(main, isPresent());
+ List<CheckCastInstructionSubject> checkCasts =
+ main.streamInstructions()
+ .filter(InstructionSubject::isCheckCast)
+ .map(InstructionSubject::asCheckCast)
+ .collect(Collectors.toList());
+ assertEquals(1, checkCasts.size());
+ ClassSubject a = inspector.clazz(A.class);
+ assertThat(a, isPresentAndRenamed());
+ assertEquals(
+ a.getFinalDescriptor(), checkCasts.get(0).getType().getDescriptor().toString());
+ });
+ }
+
+ @Test
+ public void testMainDexClasses() throws Exception {
+ assumeTrue(parameters.isDexRuntime());
+ assumeTrue(parameters.getDexRuntimeVersion().isDalvik());
+ runTest(
+ r8FullTestBuilder ->
+ r8FullTestBuilder.addMainDexListClasses(I.class, Provider.class, Main.class),
+ this::inspect);
+ }
+
+ @Test
+ public void testMainDexTracing() throws Exception {
+ assumeTrue(parameters.isDexRuntime());
+ assumeTrue(parameters.getDexRuntimeVersion().isDalvik());
+ runTest(
+ r8FullTestBuilder -> r8FullTestBuilder.addMainDexClassRules(Main.class, I.class),
+ this::inspect);
+ }
+
+ private void runTest(
+ ThrowableConsumer<R8FullTestBuilder> setMainDexConsumer,
+ BiConsumer<CodeInspector, List<String>> resultConsumer)
+ throws Exception {
+ Box<String> mainDexStringList = new Box<>("");
+ testForR8(parameters.getBackend())
+ .addProgramClasses(I.class, Provider.class, A.class, Main.class)
+ .enableNoVerticalClassMergingAnnotations()
+ .enableInliningAnnotations()
+ .enableNeverClassInliningAnnotations()
+ .addKeepMainRule(Main.class)
+ .addKeepClassRulesWithAllowObfuscation(I.class)
+ .setMinApi(parameters.getApiLevel())
+ .applyIf(
+ parameters.isDexRuntime() && parameters.getDexRuntimeVersion().isDalvik(),
+ builder ->
+ builder.setMainDexListConsumer(ToolHelper.consumeString(mainDexStringList::set)))
+ .apply(setMainDexConsumer)
+ .run(parameters.getRuntime(), Main.class)
+ .apply(
+ result ->
+ resultConsumer.accept(
+ result.inspector(),
+ mainDexStringList.get().equals("")
+ ? new ArrayList<>()
+ : StringUtils.splitLines(mainDexStringList.get())));
+ }
+
+ private void inspect(CodeInspector inspector, List<String> mainDexClasses) {
+ assertEquals(4, inspector.allClasses().size());
+ assertEquals(3, mainDexClasses.size());
+ inspectClassInMainDex(Main.class, inspector, mainDexClasses);
+ inspectClassInMainDex(I.class, inspector, mainDexClasses);
+ inspectClassInMainDex(Provider.class, inspector, mainDexClasses);
+ ClassSubject aClass = inspector.clazz(A.class);
+ assertThat(aClass, isPresentAndRenamed());
+ assertThat(mainDexClasses, not(hasItem(aClass.getFinalBinaryName() + ".class")));
+ }
+
+ private void inspectClassInMainDex(
+ Class<?> clazz, CodeInspector inspector, List<String> mainDexClasses) {
+ ClassSubject classSubject = inspector.clazz(clazz);
+ assertThat(classSubject, isPresent());
+ assertThat(mainDexClasses, hasItem(classSubject.getFinalBinaryName() + ".class"));
+ }
+
+ @NoVerticalClassMerging
+ public interface I {
+
+ @NeverInline
+ void foo();
+ }
+
+ public static class Provider {
+ @NeverInline
+ public static I getImpl() {
+ return new A(); // <-- We will call-site optimize getImpl() to always return A.
+ }
+ }
+
+ @NeverClassInline
+ public static class A implements I {
+
+ @Override
+ @NeverInline
+ public void foo() {
+ System.out.println("A::foo");
+ }
+ }
+
+ public static class Main {
+
+ public static void main(String[] args) {
+ // The de-virtualizer will try and rebind from I.foo to A.foo.
+ Provider.getImpl().foo();
+ }
+ }
+}
