Handle non-standard invoke-super in class merging
This CL handles rewriting of invoke-super instructions that would not normally be generated by javac. Necessary for InvokeSuperTest to succeed with vertical class merging enabled.
The issue handled by this CL is the following.
If we merge class C into class D and class D contains an invocation super.m(), then it is insufficient to rewrite all instances of "invoke-super C.m()" in D to "invoke-direct D.m$C()" (the method D.m$C denotes the direct method that has been created in D for C.m). In particular, there might be an instruction "invoke-super B.m()" in D that resolves to C.m at runtime (assume that B is a superclass of C), which also needs to be rewritten to "invoke-direct D.m$C()".
Note that D might also have an instruction "invoke-super A.m()" that would lead to a NoSuchMethodError at runtime (A is a superclass of B). In this case, it is important not to rewrite this instruction into "invoke-direct D.m$C()", since this would change the semantics.
Change-Id: I399fc5a5e26954d71416915b8aaadd6405737e29
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;
