Mitigate issues from invoke-virtual to classes that were once interfaces
Bug: 206891715
Change-Id: I7532d3f51acacd16a3cde62f65e8e881cd9aebd8
diff --git a/src/main/java/com/android/tools/r8/graph/analysis/InvokeVirtualToInterfaceVerifyErrorWorkaround.java b/src/main/java/com/android/tools/r8/graph/analysis/InvokeVirtualToInterfaceVerifyErrorWorkaround.java
new file mode 100644
index 0000000..7c17e4c
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/graph/analysis/InvokeVirtualToInterfaceVerifyErrorWorkaround.java
@@ -0,0 +1,89 @@
+// 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.graph.analysis;
+
+import com.android.tools.r8.graph.AppInfoWithClassHierarchy;
+import com.android.tools.r8.graph.AppView;
+import com.android.tools.r8.graph.DexMethod;
+import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.ProgramMethod;
+import com.android.tools.r8.shaking.Enqueuer;
+import com.android.tools.r8.shaking.KeepInfo.Joiner;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.InternalOptions;
+
+/**
+ * Workaround for situations where an interface in the framework has been changed into a class. When
+ * compiling against a recent android.jar, virtual invokes to such classes may resolve to interface
+ * methods on older API levels, which can lead to verification errors (see b/206891715).
+ *
+ * <p>To mitigate this issue, we pin methods that have virtual invokes to such classes. This ensures
+ * that we don't move these virtual invokes elsewhere (e.g., due to inlining or class merging),
+ * which would otherwise lead to verification errors in other classes.
+ */
+// TODO(b/206891715): This only mitigates the issue. The user may still need to manually outline
+// virtual invokes to classes that was once an interface. To avoid this in general (including D8)
+// the compiler should outline the problematic invokes.
+public class InvokeVirtualToInterfaceVerifyErrorWorkaround implements EnqueuerInvokeAnalysis {
+
+ private final DexType androidHardwareCamera2CameraDeviceType;
+ private final Enqueuer enqueuer;
+ private final InternalOptions options;
+
+ public InvokeVirtualToInterfaceVerifyErrorWorkaround(
+ AppView<? extends AppInfoWithClassHierarchy> appView, Enqueuer enqueuer) {
+ this.androidHardwareCamera2CameraDeviceType =
+ appView.dexItemFactory().createType("Landroid/hardware/camera2/CameraDevice;");
+ this.enqueuer = enqueuer;
+ this.options = appView.options();
+ }
+
+ public static void register(
+ AppView<? extends AppInfoWithClassHierarchy> appView, Enqueuer enqueuer) {
+ if (!isNoop(appView)) {
+ enqueuer.registerInvokeAnalysis(
+ new InvokeVirtualToInterfaceVerifyErrorWorkaround(appView, enqueuer));
+ }
+ }
+
+ private static boolean isNoop(AppView<? extends AppInfoWithClassHierarchy> appView) {
+ return appView.options().getMinApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.L);
+ }
+
+ @Override
+ public void traceInvokeVirtual(DexMethod invokedMethod, ProgramMethod context) {
+ if (isInterfaceInSomeApiLevel(invokedMethod.getHolderType())) {
+ enqueuer.getKeepInfo().joinMethod(context, Joiner::disallowOptimization);
+ }
+ }
+
+ private boolean isInterfaceInSomeApiLevel(DexType type) {
+ // CameraDevice was added as a class in API 21 (L), but was defined as an interface in the
+ // framework before then.
+ return type == androidHardwareCamera2CameraDeviceType
+ && (options.isGeneratingClassFiles()
+ || options.getMinApiLevel().isLessThan(AndroidApiLevel.L));
+ }
+
+ @Override
+ public void traceInvokeDirect(DexMethod invokedMethod, ProgramMethod context) {
+ // Intentionally empty.
+ }
+
+ @Override
+ public void traceInvokeInterface(DexMethod invokedMethod, ProgramMethod context) {
+ // Intentionally empty.
+ }
+
+ @Override
+ public void traceInvokeStatic(DexMethod invokedMethod, ProgramMethod context) {
+ // Intentionally empty.
+ }
+
+ @Override
+ public void traceInvokeSuper(DexMethod invokedMethod, ProgramMethod context) {
+ // Intentionally empty.
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index 33af67f..02c18fc 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -81,6 +81,7 @@
import com.android.tools.r8.graph.analysis.EnqueuerExceptionGuardAnalysis;
import com.android.tools.r8.graph.analysis.EnqueuerInstanceOfAnalysis;
import com.android.tools.r8.graph.analysis.EnqueuerInvokeAnalysis;
+import com.android.tools.r8.graph.analysis.InvokeVirtualToInterfaceVerifyErrorWorkaround;
import com.android.tools.r8.ir.analysis.proto.ProtoEnqueuerUseRegistry;
import com.android.tools.r8.ir.analysis.proto.schema.ProtoEnqueuerExtension;
import com.android.tools.r8.ir.code.ArrayPut;
@@ -231,11 +232,11 @@
private final boolean forceProguardCompatibility;
private final Mode mode;
- private Set<EnqueuerAnalysis> analyses = Sets.newIdentityHashSet();
- private Set<EnqueuerInvokeAnalysis> invokeAnalyses = Sets.newIdentityHashSet();
- private Set<EnqueuerInstanceOfAnalysis> instanceOfAnalyses = Sets.newIdentityHashSet();
- private Set<EnqueuerExceptionGuardAnalysis> exceptionGuardAnalyses = Sets.newIdentityHashSet();
- private Set<EnqueuerCheckCastAnalysis> checkCastAnalyses = Sets.newIdentityHashSet();
+ private Set<EnqueuerAnalysis> analyses = new LinkedHashSet<>();
+ private Set<EnqueuerInvokeAnalysis> invokeAnalyses = new LinkedHashSet<>();
+ private Set<EnqueuerInstanceOfAnalysis> instanceOfAnalyses = new LinkedHashSet<>();
+ private Set<EnqueuerExceptionGuardAnalysis> exceptionGuardAnalyses = new LinkedHashSet<>();
+ private Set<EnqueuerCheckCastAnalysis> checkCastAnalyses = new LinkedHashSet<>();
// Don't hold a direct pointer to app info (use appView).
private AppInfoWithClassHierarchy appInfo;
@@ -470,6 +471,7 @@
: null;
if (mode.isInitialOrFinalTreeShaking()) {
+ InvokeVirtualToInterfaceVerifyErrorWorkaround.register(appView, this);
if (options.protoShrinking().enableGeneratedMessageLiteShrinking) {
registerAnalysis(new ProtoEnqueuerExtension(appView));
}
@@ -528,7 +530,7 @@
return this;
}
- private Enqueuer registerInvokeAnalysis(EnqueuerInvokeAnalysis analysis) {
+ public Enqueuer registerInvokeAnalysis(EnqueuerInvokeAnalysis analysis) {
invokeAnalyses.add(analysis);
return this;
}
@@ -671,6 +673,10 @@
return clazz;
}
+ public MutableKeepInfoCollection getKeepInfo() {
+ return keepInfo;
+ }
+
public KeepClassInfo getKeepInfo(DexProgramClass clazz) {
return keepInfo.getClassInfo(clazz);
}
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index 2da3135..672fd24 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -569,6 +569,10 @@
private final boolean enableTreeShaking;
private final boolean enableMinification;
+ public AndroidApiLevel getMinApiLevel() {
+ return minApiLevel;
+ }
+
public boolean isOptimizing() {
return hasProguardConfiguration() && getProguardConfiguration().isOptimizing();
}
diff --git a/src/test/java/com/android/tools/r8/workaround/InvokeVirtualToInterfaceVerifyErrorWorkaroundTest.java b/src/test/java/com/android/tools/r8/workaround/InvokeVirtualToInterfaceVerifyErrorWorkaroundTest.java
new file mode 100644
index 0000000..3cc8749
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/workaround/InvokeVirtualToInterfaceVerifyErrorWorkaroundTest.java
@@ -0,0 +1,125 @@
+// 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.workaround;
+
+import static com.android.tools.r8.utils.codeinspector.CodeMatchers.invokesMethodWithName;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.android.tools.r8.utils.DescriptorUtils;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import java.io.IOException;
+import java.util.List;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class InvokeVirtualToInterfaceVerifyErrorWorkaroundTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection parameters() {
+ return getTestParameters().withAllRuntimesAndApiLevels().build();
+ }
+
+ @Test
+ public void test() throws Exception {
+ testForR8(parameters.getBackend())
+ .addProgramClasses(A.class)
+ .addProgramClassFileData(getProgramClassFileData())
+ .addKeepClassAndMembersRules(Main.class)
+ // CameraDevice is not present in rt.jar or android.jar when API<L.
+ .applyIf(
+ parameters.isCfRuntime() || parameters.getApiLevel().isLessThan(AndroidApiLevel.L),
+ testBuilder -> testBuilder.addDontWarn("android.hardware.camera2.CameraDevice"))
+ // CameraDeviceUser can only be merged with A when min API>=L.
+ .addHorizontallyMergedClassesInspector(
+ inspector ->
+ inspector
+ .applyIf(
+ parameters.isDexRuntime()
+ && parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.L),
+ i -> i.assertIsCompleteMergeGroup(A.class, CameraDeviceUser.class))
+ .assertNoOtherClassesMerged())
+ .setMinApi(parameters.getApiLevel())
+ .compile()
+ // CameraDeviceUser.m() can only be inlined when min API>=L.
+ .inspect(
+ inspector -> {
+ if (parameters.isDexRuntime()
+ && parameters.getApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.L)) {
+ assertThat(inspector.clazz(CameraDeviceUser.class), isAbsent());
+ } else {
+ ClassSubject cameraDeviceUserClassSubject = inspector.clazz(CameraDeviceUser.class);
+ assertThat(cameraDeviceUserClassSubject, isPresent());
+
+ MethodSubject mMethodSubject =
+ cameraDeviceUserClassSubject.uniqueMethodWithName("m");
+ assertThat(mMethodSubject, isPresent());
+ assertThat(mMethodSubject, invokesMethodWithName("close"));
+ }
+ });
+ }
+
+ private static List<byte[]> getProgramClassFileData() throws IOException {
+ String oldDescriptor = DescriptorUtils.javaTypeToDescriptor(CameraDevice.class.getTypeName());
+ String newDescriptor = "Landroid/hardware/camera2/CameraDevice;";
+ return ImmutableList.of(
+ transformer(Main.class)
+ .replaceClassDescriptorInMembers(oldDescriptor, newDescriptor)
+ .replaceClassDescriptorInMethodInstructions(oldDescriptor, newDescriptor)
+ .transform(),
+ transformer(CameraDeviceUser.class)
+ .replaceClassDescriptorInMembers(oldDescriptor, newDescriptor)
+ .replaceClassDescriptorInMethodInstructions(oldDescriptor, newDescriptor)
+ .transform());
+ }
+
+ // The following classes are transformed to use android.hardware.camera2.CameraDevice.
+ static class Main {
+
+ public static void main(String[] args) {
+ int minApi = args.length;
+ if (minApi >= 21) {
+ new CameraDeviceUser().m(getCameraDevice());
+ } else {
+ System.out.println(new A());
+ }
+ }
+
+ // @Keep
+ static CameraDevice getCameraDevice() {
+ return null;
+ }
+ }
+
+ static class A {}
+
+ static class CameraDeviceUser {
+
+ public void m(CameraDevice cd) {
+ if (cd != null) {
+ cd.close();
+ }
+ }
+ }
+
+ abstract static class CameraDevice {
+
+ abstract void close();
+ }
+}
