Merge "Handle non-standard invoke-super in class merging"
diff --git a/src/main/java/com/android/tools/r8/graph/DefaultUseRegistry.java b/src/main/java/com/android/tools/r8/graph/DefaultUseRegistry.java
new file mode 100644
index 0000000..7f31098
--- /dev/null
+++ b/src/main/java/com/android/tools/r8/graph/DefaultUseRegistry.java
@@ -0,0 +1,63 @@
+// Copyright (c) 2018, 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;
+
+public class DefaultUseRegistry extends UseRegistry {
+
+ @Override
+ public boolean registerInvokeVirtual(DexMethod method) {
+ return true;
+ }
+
+ @Override
+ public boolean registerInvokeDirect(DexMethod method) {
+ return true;
+ }
+
+ @Override
+ public boolean registerInvokeStatic(DexMethod method) {
+ return true;
+ }
+
+ @Override
+ public boolean registerInvokeInterface(DexMethod method) {
+ return true;
+ }
+
+ @Override
+ public boolean registerInvokeSuper(DexMethod method) {
+ return true;
+ }
+
+ @Override
+ public boolean registerInstanceFieldWrite(DexField field) {
+ return true;
+ }
+
+ @Override
+ public boolean registerInstanceFieldRead(DexField field) {
+ return true;
+ }
+
+ @Override
+ public boolean registerNewInstance(DexType type) {
+ return true;
+ }
+
+ @Override
+ public boolean registerStaticFieldRead(DexField field) {
+ return true;
+ }
+
+ @Override
+ public boolean registerStaticFieldWrite(DexField field) {
+ return true;
+ }
+
+ @Override
+ public boolean registerTypeReference(DexType type) {
+ return true;
+ }
+}
diff --git a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
index c347dba..5da6f06 100644
--- a/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
+++ b/src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java
@@ -4,6 +4,7 @@
package com.android.tools.r8.shaking;
import com.android.tools.r8.errors.CompilationError;
+import com.android.tools.r8.graph.DefaultUseRegistry;
import com.android.tools.r8.graph.DexAnnotationSet;
import com.android.tools.r8.graph.DexApplication;
import com.android.tools.r8.graph.DexClass;
@@ -68,7 +69,7 @@
private final DexApplication application;
private final AppInfoWithLiveness appInfo;
private final GraphLense graphLense;
- private final Map<DexType, DexType> mergedClasses = new IdentityHashMap<>();
+ private final Map<DexType, DexType> mergedClasses = new HashMap<>();
private final Timing timing;
private Collection<DexMethod> invokes;
@@ -107,6 +108,36 @@
}
}
}
+
+ // Avoid merging two types if this could remove a NoSuchMethodError, as illustrated by the
+ // following example. (Alternatively, it would be possible to merge A and B and rewrite the
+ // "invoke-super A.m" instruction into "invoke-super Object.m" to preserve the error. This
+ // situation should generally not occur in practice, though.)
+ //
+ // class A {}
+ // class B extends A {
+ // public void m() {}
+ // }
+ // class C extends A {
+ // public void m() {
+ // invoke-super "A.m" <- should yield NoSuchMethodError, cannot merge A and B
+ // }
+ // }
+ if (!method.isStaticMethod()) {
+ method.registerCodeReferences(
+ new DefaultUseRegistry() {
+ @Override
+ public boolean registerInvokeSuper(DexMethod target) {
+ DexClass targetClass = appInfo.definitionFor(target.getHolder());
+ if (targetClass != null
+ && targetClass.isProgramClass()
+ && targetClass.lookupVirtualMethod(target) == null) {
+ pinnedTypes.add(target.getHolder());
+ }
+ return true;
+ }
+ });
+ }
}
}
return pinnedTypes;
@@ -401,8 +432,7 @@
// Record that invoke-super instructions in the target class should be redirected to the
// newly created direct method.
- deferredRenamings.mapVirtualMethodToDirectInType(
- virtualMethod.method, resultingDirectMethod.method, target.type);
+ redirectSuperCallsInTarget(virtualMethod.method, resultingDirectMethod.method);
if (shadowedBy == null) {
// In addition to the newly added direct method, create a virtual method such that we do
@@ -491,6 +521,36 @@
return deferredRenamings;
}
+ private void redirectSuperCallsInTarget(DexMethod oldTarget, DexMethod newTarget) {
+ // If we merge class B into class C, and class C contains an invocation super.m(), then it
+ // is insufficient to rewrite "invoke-super B.m()" to "invoke-direct C.m$B()" (the method
+ // C.m$B denotes the direct method that has been created in C for B.m). In particular, there
+ // might be an instruction "invoke-super A.m()" in C that resolves to B.m at runtime (A is
+ // a superclass of B), which also needs to be rewritten to "invoke-direct C.m$B()".
+ //
+ // We handle this by adding a mapping for [target] and all of its supertypes.
+ DexClass holder = target;
+ while (holder != null && holder.isProgramClass()) {
+ DexMethod signatureInHolder =
+ application.dexItemFactory.createMethod(holder.type, oldTarget.proto, oldTarget.name);
+ // Only rewrite the invoke-super call if it does not lead to a NoSuchMethodError.
+ if (resolutionSucceeds(signatureInHolder)) {
+ deferredRenamings.mapVirtualMethodToDirectInType(
+ signatureInHolder, newTarget, target.type);
+ holder = holder.superType != null ? appInfo.definitionFor(holder.superType) : null;
+ } else {
+ break;
+ }
+ }
+ }
+
+ private boolean resolutionSucceeds(DexMethod targetMethod) {
+ DexClass enclosingClass = appInfo.definitionFor(targetMethod.holder);
+ return enclosingClass != null
+ && (enclosingClass.lookupVirtualMethod(targetMethod) != null
+ || appInfo.lookupSuperTarget(targetMethod, enclosingClass.type) != null);
+ }
+
private DexEncodedMethod buildBridgeMethod(
DexEncodedMethod signature, DexMethod invocationTarget) {
DexType holder = target.type;
