Enforce calls to lookup virtual methods do so on valid resolutions.
Bug: 140016938
Bug: 140088797
Change-Id: Iea4d8e898bd41624b078aaf08ee05bd0ff06e742
diff --git a/src/main/java/com/android/tools/r8/graph/AppInfo.java b/src/main/java/com/android/tools/r8/graph/AppInfo.java
index 2e352a2..047c498 100644
--- a/src/main/java/com/android/tools/r8/graph/AppInfo.java
+++ b/src/main/java/com/android/tools/r8/graph/AppInfo.java
@@ -3,7 +3,11 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph;
+import com.android.tools.r8.graph.ResolutionResult.ArrayCloneMethodResult;
+import com.android.tools.r8.graph.ResolutionResult.ClassNotFoundResult;
+import com.android.tools.r8.graph.ResolutionResult.IncompatibleClassResult;
import com.android.tools.r8.graph.ResolutionResult.MultiResult;
+import com.android.tools.r8.graph.ResolutionResult.NoSuchMethodResult;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.google.common.collect.ImmutableMap;
@@ -265,7 +269,7 @@
}
DexClass definition = definitionFor(holder);
if (definition == null) {
- return ResolutionResult.EmptyResult.get();
+ return ClassNotFoundResult.INSTANCE;
}
return resolveMethod(definition, method);
}
@@ -305,7 +309,7 @@
assert checkIfObsolete();
assert holder.isArrayType();
if (method.name == dexItemFactory.cloneMethodName) {
- return ResolutionResult.EmptyResult.get();
+ return ArrayCloneMethodResult.INSTANCE;
} else {
return resolveMethodOnClass(dexItemFactory.objectType, method);
}
@@ -328,9 +332,12 @@
return resolveMethodOnArray(holder, method);
}
DexClass clazz = definitionFor(holder);
+ if (clazz == null) {
+ return ClassNotFoundResult.INSTANCE;
+ }
// Step 1: If holder is an interface, resolution fails with an ICCE. We return null.
- if (clazz == null || clazz.isInterface()) {
- return ResolutionResult.EmptyResult.get();
+ if (clazz.isInterface()) {
+ return IncompatibleClassResult.INSTANCE;
}
return resolveMethodOnClass(clazz, method);
}
@@ -403,7 +410,7 @@
return result;
}
// Return any of the non-default methods.
- return anyTarget == null ? ResolutionResult.EmptyResult.get() : anyTarget;
+ return anyTarget == null ? NoSuchMethodResult.INSTANCE : anyTarget;
}
/**
@@ -459,14 +466,17 @@
public ResolutionResult resolveMethodOnInterface(DexType holder, DexMethod desc) {
assert checkIfObsolete();
if (holder.isArrayType()) {
- return ResolutionResult.EmptyResult.get();
+ return IncompatibleClassResult.INSTANCE;
}
// Step 1: Lookup interface.
DexClass definition = definitionFor(holder);
// If the definition is not an interface, resolution fails with an ICCE. We just return the
// empty result here.
- if (definition == null || !definition.isInterface()) {
- return ResolutionResult.EmptyResult.get();
+ if (definition == null) {
+ return ClassNotFoundResult.INSTANCE;
+ }
+ if (!definition.isInterface()) {
+ return IncompatibleClassResult.INSTANCE;
}
return resolveMethodOnInterface(definition, desc);
}
@@ -482,8 +492,7 @@
// Step 3: Look for matching method on object class.
DexClass objectClass = definitionFor(dexItemFactory.objectType);
if (objectClass == null) {
- // TODO(herhut): This should never happen. How do we handle missing classes?
- return ResolutionResult.EmptyResult.get();
+ return ClassNotFoundResult.INSTANCE;
}
result = objectClass.lookupMethod(desc);
if (result != null && result.accessFlags.isPublic() && !result.accessFlags.isAbstract()) {
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index d43583a..808fcc3 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -1274,6 +1274,13 @@
}
@Override
+ public boolean isValidVirtualTarget(InternalOptions options) {
+ return options.canUseNestBasedAccess()
+ ? (!accessFlags.isStatic() && !accessFlags.isConstructor())
+ : isVirtualMethod();
+ }
+
+ @Override
public DexEncodedMethod asResultOfResolve() {
checkIfObsolete();
return this;
diff --git a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
index 4aeae4e..83a4132 100644
--- a/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
+++ b/src/main/java/com/android/tools/r8/graph/ResolutionResult.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.graph;
+import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Sets;
import java.util.Collections;
@@ -12,6 +13,7 @@
public interface ResolutionResult {
+ // TODO(b/140214802): Remove this method as its usage is questionable.
DexEncodedMethod asResultOfResolve();
DexEncodedMethod asSingleTarget();
@@ -22,8 +24,15 @@
void forEachTarget(Consumer<DexEncodedMethod> consumer);
+ boolean isValidVirtualTarget(InternalOptions options);
+
+ default Set<DexEncodedMethod> lookupVirtualDispatchTargets(
+ boolean isInterface, AppInfoWithSubtyping appInfo) {
+ return isInterface ? lookupInterfaceTargets(appInfo) : lookupVirtualTargets(appInfo);
+ }
+
default Set<DexEncodedMethod> lookupVirtualTargets(AppInfoWithSubtyping appInfo) {
- // TODO(b/140016938): Don't allow this lookup on non-virtual resolutions.
+ assert isValidVirtualTarget(appInfo.app().options);
// First add the target for receiver type method.type.
Set<DexEncodedMethod> result = Sets.newIdentityHashSet();
forEachTarget(result::add);
@@ -46,7 +55,7 @@
}
default Set<DexEncodedMethod> lookupInterfaceTargets(AppInfoWithSubtyping appInfo) {
- // TODO(b/140016938): Don't allow this lookup on non-virtual resolutions.
+ assert isValidVirtualTarget(appInfo.app().options);
Set<DexEncodedMethod> result = Sets.newIdentityHashSet();
if (hasSingleTarget()) {
// Add default interface methods to the list of targets.
@@ -122,6 +131,16 @@
}
@Override
+ public boolean isValidVirtualTarget(InternalOptions options) {
+ for (DexEncodedMethod method : methods) {
+ if (!method.isValidVirtualTarget(options)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ @Override
public DexEncodedMethod asResultOfResolve() {
// Resolution may return any of the targets that were found.
return methods.get(0);
@@ -149,17 +168,7 @@
}
}
- class EmptyResult implements ResolutionResult {
-
- private static final EmptyResult SINGLETON = new EmptyResult();
-
- private EmptyResult() {
- // Intentionally left empty.
- }
-
- static EmptyResult get() {
- return SINGLETON;
- }
+ abstract class EmptyResult implements ResolutionResult {
@Override
public DexEncodedMethod asResultOfResolve() {
@@ -196,4 +205,50 @@
return null;
}
}
+
+ class ArrayCloneMethodResult extends EmptyResult {
+
+ static final ArrayCloneMethodResult INSTANCE = new ArrayCloneMethodResult();
+
+ private ArrayCloneMethodResult() {
+ // Intentionally left empty.
+ }
+
+ @Override
+ public boolean isValidVirtualTarget(InternalOptions options) {
+ return true;
+ }
+ }
+
+ abstract class FailedResolutionResult extends EmptyResult {
+
+ @Override
+ public boolean isValidVirtualTarget(InternalOptions options) {
+ return false;
+ }
+ }
+
+ class ClassNotFoundResult extends FailedResolutionResult {
+ static final ClassNotFoundResult INSTANCE = new ClassNotFoundResult();
+
+ private ClassNotFoundResult() {
+ // Intentionally left empty.
+ }
+ }
+
+ class IncompatibleClassResult extends FailedResolutionResult {
+ static final IncompatibleClassResult INSTANCE = new IncompatibleClassResult();
+
+ private IncompatibleClassResult() {
+ // Intentionally left empty.
+ }
+ }
+
+ class NoSuchMethodResult extends FailedResolutionResult {
+ static final NoSuchMethodResult INSTANCE = new NoSuchMethodResult();
+
+ private NoSuchMethodResult() {
+ // Intentionally left empty.
+ }
+ }
}
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilder.java
index 2322b7b..d1ca480 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/CallGraphBuilder.java
@@ -191,9 +191,10 @@
method -> {
ResolutionResult resolution =
appView.appInfo().resolveMethod(method.holder, method, isInterface);
- return isInterface
- ? resolution.lookupInterfaceTargets(appView.appInfo())
- : resolution.lookupVirtualTargets(appView.appInfo());
+ if (resolution.isValidVirtualTarget(appView.options())) {
+ return resolution.lookupVirtualDispatchTargets(isInterface, appView.appInfo());
+ }
+ return null;
});
if (possibleTargets != null) {
boolean likelySpuriousCallEdge =
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
index 32305eb..074160b 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/InliningConstraints.java
@@ -164,10 +164,6 @@
DexMethod lookup = graphLense.lookupMethod(method);
return forVirtualInvoke(
lookup,
- appView
- .appInfo()
- .resolveMethodOnInterface(lookup.holder, lookup)
- .lookupInterfaceTargets(appView.appInfo()),
invocationContext,
true);
}
@@ -199,10 +195,6 @@
DexMethod lookup = graphLense.lookupMethod(method);
return forVirtualInvoke(
lookup,
- appView
- .appInfo()
- .resolveMethodOnClass(lookup.holder, lookup)
- .lookupVirtualTargets(appView.appInfo()),
invocationContext,
false);
}
@@ -351,20 +343,20 @@
private ConstraintWithTarget forVirtualInvoke(
DexMethod method,
- Collection<DexEncodedMethod> targets,
DexType invocationContext,
boolean isInterface) {
if (method.holder.isArrayType()) {
return ConstraintWithTarget.ALWAYS;
}
- if (targets == null) {
- return ConstraintWithTarget.NEVER;
- }
// Perform resolution and derive inlining constraints based on the accessibility of the
// resolution result.
ResolutionResult resolutionResult =
appView.appInfo().resolveMethod(method.holder, method, isInterface);
+ if (!resolutionResult.isValidVirtualTarget(appView.options())) {
+ return ConstraintWithTarget.NEVER;
+ }
+
DexEncodedMethod resolutionTarget = resolutionResult.asResultOfResolve();
if (resolutionTarget == null) {
// This will fail at runtime.
@@ -393,6 +385,8 @@
// For each of the actual potential targets, derive constraints based on the accessibility
// of the method itself.
+ Collection<DexEncodedMethod> targets =
+ resolutionResult.lookupVirtualDispatchTargets(isInterface, appView.appInfo());
for (DexEncodedMethod target : targets) {
methodHolder = graphLense.lookupType(target.method.holder);
assert appView.definitionFor(methodHolder) != null;
diff --git a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
index b0885a2..9df98de 100644
--- a/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
+++ b/src/main/java/com/android/tools/r8/shaking/AppInfoWithLiveness.java
@@ -867,7 +867,7 @@
// First get the target for the holder type.
ResolutionResult topMethod = resolveMethodOnClass(holder, method);
// We might hit none or multiple targets. Both make this fail at runtime.
- if (!topMethod.hasSingleTarget() || !topMethod.asSingleTarget().isVirtualMethod()) {
+ if (!topMethod.hasSingleTarget() || !topMethod.isValidVirtualTarget(app().options)) {
method.setSingleVirtualMethodCache(refinedReceiverType, null);
return null;
}
@@ -1021,6 +1021,9 @@
// First check that there is a target for this invoke-interface to hit. If there is none,
// this will fail at runtime.
ResolutionResult topTarget = resolveMethodOnInterface(holder, method);
+ if (!topTarget.isValidVirtualTarget(app().options)) {
+ return null;
+ }
if (topTarget.asResultOfResolve() == null) {
return null;
}
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 2bf674b..dc1a70e 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -1618,16 +1618,14 @@
DexEncodedMethod resolutionTarget =
findAndMarkResolutionTarget(method, interfaceInvoke, reason);
- if (resolutionTarget == null) {
+ if (resolutionTarget == null || !resolutionTarget.isValidVirtualTarget(options)) {
// There is no valid resolution, so any call will lead to a runtime exception.
return;
}
assert interfaceInvoke == holder.isInterface();
Set<DexEncodedMethod> possibleTargets =
- holder.isInterface()
- ? resolutionTarget.lookupInterfaceTargets(appInfo)
- : resolutionTarget.lookupVirtualTargets(appInfo);
+ resolutionTarget.lookupVirtualDispatchTargets(interfaceInvoke, appInfo);
if (possibleTargets == null || possibleTargets.isEmpty()) {
return;
}
@@ -1745,13 +1743,20 @@
// Therefore, we need to continue resolution from the super type until we find a virtual method.
if (resolutionTarget.isPrivateMethod() || resolutionTarget.isStatic()) {
assert !interfaceInvoke || resolutionTargetClass.isInterface();
- markPossiblyValidTarget(method, reason, resolutionTarget, resolutionTargetClass);
+ DexEncodedMethod possiblyValidTarget =
+ markPossiblyValidTarget(method, reason, resolutionTarget, resolutionTargetClass);
+ if (possiblyValidTarget != null) {
+ // Since some Art runtimes may actually end up targeting this method, it is returned as
+ // the basis of lookup for the enqueuing of virtual dispatches. Not doing so may cause it
+ // to be marked abstract, thus leading to an AbstractMethodError on said Art runtimes.
+ return possiblyValidTarget;
+ }
}
return resolutionTarget;
}
- private void markPossiblyValidTarget(
+ private DexEncodedMethod markPossiblyValidTarget(
DexMethod method,
KeepReason reason,
DexEncodedMethod resolutionTarget,
@@ -1763,16 +1768,17 @@
resolutionTargetClass.superType, method, resolutionTargetClass.isInterface())
.asResultOfResolve();
if (resolutionTarget == null) {
- return;
+ return null;
}
resolutionTargetClass = appInfo.definitionFor(resolutionTarget.method.holder);
if (resolutionTargetClass == null) {
- return;
+ return null;
}
}
if (resolutionTargetClass.isProgramClass()) {
markMethodAsTargeted(resolutionTarget, reason);
}
+ return resolutionTarget;
}
private DexMethod generatedEnumValuesMethod(DexClass enumClass) {
diff --git a/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java b/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
index 11265a9..f91f1bd 100644
--- a/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/SingleTargetLookupTest.java
@@ -3,6 +3,8 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.resolution;
+import static org.junit.Assert.assertTrue;
+
import com.android.tools.r8.AsmTestBase;
import com.android.tools.r8.dex.ApplicationReader;
import com.android.tools.r8.graph.AppInfo;
@@ -14,6 +16,7 @@
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.graph.DexMethod;
import com.android.tools.r8.graph.DexType;
+import com.android.tools.r8.graph.ResolutionResult;
import com.android.tools.r8.resolution.singletarget.Main;
import com.android.tools.r8.resolution.singletarget.one.AbstractSubClass;
import com.android.tools.r8.resolution.singletarget.one.AbstractTopClass;
@@ -165,46 +168,87 @@
@Parameters(name = "{1}.{0} -> {2}")
public static List<Object[]> getData() {
- return ImmutableList.copyOf(new Object[][]{
- singleTarget("singleTargetAtTop", AbstractTopClass.class),
- singleTargetWithAbstracts("singleShadowingOverride", AbstractTopClass.class,
- AbstractSubClass.class, AbstractTopClass.class),
- manyTargets("abstractTargetAtTop", AbstractTopClass.class, AbstractTopClass.class,
- SubSubClassOne.class, SubSubClassTwo.class),
- singleTargetWithAbstracts("overridenInAbstractClassOnly", AbstractTopClass.class,
- AbstractTopClass.class, SubSubClassThree.class),
- onlyUnreachableTargets("overridenInAbstractClassOnly", SubSubClassThree.class,
- SubSubClassThree.class),
- manyTargets("overriddenInTwoSubTypes", AbstractTopClass.class, AbstractTopClass.class,
- SubSubClassOne.class, SubSubClassTwo.class),
- manyTargets("definedInTwoSubTypes", AbstractTopClass.class, AbstractTopClass.class,
- SubSubClassOne.class, SubSubClassTwo.class),
- onlyUnreachableTargets("staticMethod", AbstractTopClass.class, AbstractTopClass.class),
- manyTargets("overriddenInTwoSubTypes", OtherAbstractTopClass.class,
- OtherAbstractTopClass.class, OtherSubSubClassOne.class, OtherSubSubClassTwo.class),
- manyTargets("abstractOverriddenInTwoSubTypes", OtherAbstractTopClass.class,
- OtherAbstractTopClass.class, OtherSubSubClassOne.class, OtherSubSubClassTwo.class),
- manyTargets("overridesOnDifferentLevels", OtherAbstractTopClass.class,
- OtherAbstractTopClass.class, OtherSubSubClassOne.class, OtherAbstractSubClassTwo.class),
- singleTarget("defaultMethod", AbstractTopClass.class, InterfaceWithDefault.class),
- manyTargets("overriddenDefault", AbstractTopClass.class, InterfaceWithDefault.class,
- SubSubClassTwo.class),
- singleTarget("overriddenDefault", SubSubClassTwo.class),
- singleTarget("overriddenByIrrelevantInterface", AbstractTopClass.class),
- singleTarget("overriddenByIrrelevantInterface", SubSubClassOne.class,
- AbstractTopClass.class),
- manyTargets("overriddenInOtherInterface", AbstractTopClass.class,
- InterfaceWithDefault.class,
- IrrelevantInterfaceWithDefault.class),
- manyTargets("overriddenInOtherInterface", SubSubClassOne.class,
- InterfaceWithDefault.class,
- IrrelevantInterfaceWithDefault.class),
- manyTargets("abstractMethod", ThirdAbstractTopClass.class, ThirdAbstractTopClass.class,
- ThirdSubClassOne.class),
- singleTarget("instanceMethod", ThirdAbstractTopClass.class, ThirdAbstractTopClass.class),
- singleTarget(
- "otherInstanceMethod", ThirdAbstractTopClass.class, ThirdAbstractTopClass.class),
- });
+ return ImmutableList.copyOf(
+ new Object[][] {
+ singleTarget("singleTargetAtTop", AbstractTopClass.class),
+ singleTargetWithAbstracts(
+ "singleShadowingOverride",
+ AbstractTopClass.class,
+ AbstractSubClass.class,
+ AbstractTopClass.class),
+ manyTargets(
+ "abstractTargetAtTop",
+ AbstractTopClass.class,
+ AbstractTopClass.class,
+ SubSubClassOne.class,
+ SubSubClassTwo.class),
+ singleTargetWithAbstracts(
+ "overridenInAbstractClassOnly",
+ AbstractTopClass.class,
+ AbstractTopClass.class,
+ SubSubClassThree.class),
+ onlyUnreachableTargets(
+ "overridenInAbstractClassOnly", SubSubClassThree.class, SubSubClassThree.class),
+ manyTargets(
+ "overriddenInTwoSubTypes",
+ AbstractTopClass.class,
+ AbstractTopClass.class,
+ SubSubClassOne.class,
+ SubSubClassTwo.class),
+ manyTargets(
+ "definedInTwoSubTypes",
+ AbstractTopClass.class,
+ AbstractTopClass.class,
+ SubSubClassOne.class,
+ SubSubClassTwo.class),
+ onlyUnreachableTargets("staticMethod", AbstractTopClass.class),
+ manyTargets(
+ "overriddenInTwoSubTypes",
+ OtherAbstractTopClass.class,
+ OtherAbstractTopClass.class,
+ OtherSubSubClassOne.class,
+ OtherSubSubClassTwo.class),
+ manyTargets(
+ "abstractOverriddenInTwoSubTypes",
+ OtherAbstractTopClass.class,
+ OtherAbstractTopClass.class,
+ OtherSubSubClassOne.class,
+ OtherSubSubClassTwo.class),
+ manyTargets(
+ "overridesOnDifferentLevels",
+ OtherAbstractTopClass.class,
+ OtherAbstractTopClass.class,
+ OtherSubSubClassOne.class,
+ OtherAbstractSubClassTwo.class),
+ singleTarget("defaultMethod", AbstractTopClass.class, InterfaceWithDefault.class),
+ manyTargets(
+ "overriddenDefault",
+ AbstractTopClass.class,
+ InterfaceWithDefault.class,
+ SubSubClassTwo.class),
+ singleTarget("overriddenDefault", SubSubClassTwo.class),
+ singleTarget("overriddenByIrrelevantInterface", AbstractTopClass.class),
+ singleTarget(
+ "overriddenByIrrelevantInterface", SubSubClassOne.class, AbstractTopClass.class),
+ manyTargets(
+ "overriddenInOtherInterface",
+ AbstractTopClass.class,
+ InterfaceWithDefault.class,
+ IrrelevantInterfaceWithDefault.class),
+ manyTargets(
+ "overriddenInOtherInterface",
+ SubSubClassOne.class,
+ InterfaceWithDefault.class,
+ IrrelevantInterfaceWithDefault.class),
+ manyTargets(
+ "abstractMethod",
+ ThirdAbstractTopClass.class,
+ ThirdAbstractTopClass.class,
+ ThirdSubClassOne.class),
+ singleTarget("instanceMethod", ThirdAbstractTopClass.class, ThirdAbstractTopClass.class),
+ singleTarget(
+ "otherInstanceMethod", ThirdAbstractTopClass.class, ThirdAbstractTopClass.class),
+ });
}
public static DexMethod buildMethod(Class clazz, String name, AppInfo appInfo) {
@@ -247,12 +291,16 @@
DexMethod method = buildMethod(invokeReceiver, methodName, appInfo);
Assert.assertNotNull(
appInfo.resolveMethod(toType(invokeReceiver, appInfo), method).asResultOfResolve());
- Set<DexEncodedMethod> targets =
- appInfo.resolveMethod(method.holder, method).lookupVirtualTargets(appInfo);
- Set<DexType> targetHolders = targets.stream().map(m -> m.method.holder)
- .collect(Collectors.toSet());
- Assert.assertEquals(allTargetHolders.size(), targetHolders.size());
- Assert.assertTrue(
- allTargetHolders.stream().map(t -> toType(t, appInfo)).allMatch(targetHolders::contains));
+ ResolutionResult resolutionResult = appInfo.resolveMethod(method.holder, method);
+ if (resolutionResult.isValidVirtualTarget(appInfo.app().options)) {
+ Set<DexEncodedMethod> targets = resolutionResult.lookupVirtualTargets(appInfo);
+ Set<DexType> targetHolders =
+ targets.stream().map(m -> m.method.holder).collect(Collectors.toSet());
+ Assert.assertEquals(allTargetHolders.size(), targetHolders.size());
+ assertTrue(
+ allTargetHolders.stream().map(t -> toType(t, appInfo)).allMatch(targetHolders::contains));
+ } else {
+ assertTrue(allTargetHolders.isEmpty());
+ }
}
}
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodTest.java
index d0806db..e914a7b 100644
--- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodTest.java
@@ -5,7 +5,7 @@
import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.TestBase;
@@ -19,7 +19,6 @@
import com.google.common.collect.ImmutableList;
import java.io.IOException;
import java.util.List;
-import java.util.Set;
import java.util.concurrent.ExecutionException;
import org.junit.Assert;
import org.junit.BeforeClass;
@@ -83,8 +82,9 @@
@Test
public void lookupSingleTarget() {
- DexEncodedMethod resolved =
- appInfo.resolveMethodOnClass(methodOnB.holder, methodOnB).asResultOfResolve();
+ ResolutionResult resolutionResult = appInfo.resolveMethodOnClass(methodOnB.holder, methodOnB);
+ assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options));
+ DexEncodedMethod resolved = resolutionResult.asSingleTarget();
assertEquals(methodOnA, resolved.method);
DexEncodedMethod singleVirtualTarget =
appInfo.lookupSingleVirtualTarget(methodOnB, methodOnB.holder);
@@ -94,14 +94,9 @@
@Test
public void lookupVirtualTargets() {
ResolutionResult resolutionResult = appInfo.resolveMethodOnClass(methodOnB.holder, methodOnB);
- DexEncodedMethod resolved = resolutionResult.asResultOfResolve();
+ DexEncodedMethod resolved = resolutionResult.asSingleTarget();
assertEquals(methodOnA, resolved.method);
- // This behavior is up for debate as the resolution target is A.f which is private static, thus
- // the runtime behavior will be to throw a runtime exception. Thus it would be reasonable to
- // return the empty set as no targets can actually be hit at runtime.
- Set<DexEncodedMethod> targets = resolutionResult.lookupVirtualTargets(appInfo);
- assertTrue("Expected " + methodOnA, targets.stream().anyMatch(m -> m.method == methodOnA));
- assertTrue("Expected " + methodOnC, targets.stream().anyMatch(m -> m.method == methodOnC));
+ assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options));
}
@Test
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodWithVirtualParentTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodWithVirtualParentTest.java
index 6c61cc5..8d867d8 100644
--- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodWithVirtualParentTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodWithVirtualParentTest.java
@@ -5,7 +5,7 @@
import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
import com.android.tools.r8.AsmTestBase;
import com.android.tools.r8.R8TestRunResult;
@@ -20,7 +20,6 @@
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.google.common.collect.ImmutableList;
import java.util.List;
-import java.util.Set;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -144,12 +143,9 @@
@Test
public void lookupVirtualTargets() {
ResolutionResult resolutionResult = appInfo.resolveMethodOnClass(methodOnB.holder, methodOnB);
- DexEncodedMethod resolved = resolutionResult.asResultOfResolve();
+ DexEncodedMethod resolved = resolutionResult.asSingleTarget();
assertEquals(methodOnA, resolved.method);
- // See comment in VirtualOverrideOfPrivateStaticMethodTest.lookupVirtualTargets().
- Set<DexEncodedMethod> targets = resolutionResult.lookupVirtualTargets(appInfo);
- assertTrue("Expected " + methodOnA, targets.stream().anyMatch(m -> m.method == methodOnA));
- assertTrue("Expected " + methodOnC, targets.stream().anyMatch(m -> m.method == methodOnC));
+ assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options));
}
@Test
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java
index 0be4846..f968c03 100644
--- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentInterfaceTest.java
@@ -5,7 +5,7 @@
import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
import com.android.tools.r8.AsmTestBase;
import com.android.tools.r8.R8TestRunResult;
@@ -19,7 +19,6 @@
import com.android.tools.r8.utils.AndroidApiLevel;
import com.google.common.collect.ImmutableList;
import java.util.List;
-import java.util.Set;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -120,24 +119,22 @@
@Test
public void lookupSingleTarget() {
- DexEncodedMethod resolved =
- appInfo.resolveMethodOnInterface(methodOnB.holder, methodOnB).asResultOfResolve();
+ ResolutionResult resolutionResult =
+ appInfo.resolveMethodOnInterface(methodOnB.holder, methodOnB);
+ DexEncodedMethod resolved = resolutionResult.asSingleTarget();
assertEquals(methodOnB, resolved.method);
+ assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options));
DexEncodedMethod singleVirtualTarget =
appInfo.lookupSingleInterfaceTarget(methodOnB, methodOnB.holder);
- // TODO(b/140088797): This should not conclude a single target.
- Assert.assertNotNull(singleVirtualTarget);
+ Assert.assertNull(singleVirtualTarget);
}
@Test
public void lookupVirtualTargets() {
ResolutionResult resolutionResult = appInfo.resolveMethodOnInterface(methodOnB.holder, methodOnB);
- DexEncodedMethod resolved = resolutionResult.asResultOfResolve();
+ DexEncodedMethod resolved = resolutionResult.asSingleTarget();
assertEquals(methodOnB, resolved.method);
- Set<DexEncodedMethod> targets = resolutionResult.lookupInterfaceTargets(appInfo);
- // TODO(b/140088797): This should not conclude a single target.
- assertTrue("Expected " + methodOnC, targets.stream().anyMatch(m -> m.method == methodOnC));
- assertEquals(1, targets.size());
+ assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options));
}
@Test
@@ -162,17 +159,30 @@
@Test
public void runR8() throws Exception {
+ runR8(true);
+ }
+
+ @Test
+ public void runR8NoMerging() throws Exception {
+ runR8(false);
+ }
+
+ public void runR8(boolean enableVerticalClassMerging) throws Exception {
R8TestRunResult runResult =
testForR8(parameters.getBackend())
.addProgramClasses(CLASSES)
.addProgramClassFileData(DUMPS)
.addKeepMainRule(Main.class)
.setMinApi(parameters.getApiLevel())
- // TODO(b/140088797): Once fixed, parameterize on the merger as it appears to fail.
- .addOptionsModification(o -> o.enableVerticalClassMerging = false)
+ .addOptionsModification(o -> o.enableVerticalClassMerging = enableVerticalClassMerging)
.run(parameters.getRuntime(), Main.class);
- // TODO(b/140088797): Due to the single target lookup R8 incorrectly inlines the call C.f().
- runResult.assertSuccessWithOutputLines("Called C.f");
+ if (enableVerticalClassMerging) {
+ // Vertical class merging will merge B and C and change the instruction to invoke-virtual
+ // causing the legacy ART runtime behavior to match the expected error.
+ runResult.assertFailureWithErrorThatMatches(containsString("IncompatibleClassChangeError"));
+ } else {
+ checkResult(runResult);
+ }
}
private void checkResult(TestRunResult<?> runResult) {
diff --git a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java
index 7bec778..20c3cab 100644
--- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java
@@ -5,7 +5,7 @@
import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
import com.android.tools.r8.AsmTestBase;
import com.android.tools.r8.R8TestRunResult;
@@ -20,7 +20,6 @@
import com.android.tools.r8.shaking.AppInfoWithLiveness;
import com.google.common.collect.ImmutableList;
import java.util.List;
-import java.util.Set;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -167,9 +166,10 @@
@Test
public void lookupSingleTarget() {
- DexEncodedMethod resolved =
- appInfo.resolveMethodOnClass(methodOnB.holder, methodOnB).asResultOfResolve();
+ ResolutionResult resolutionResult = appInfo.resolveMethodOnClass(methodOnB.holder, methodOnB);
+ DexEncodedMethod resolved = resolutionResult.asSingleTarget();
assertEquals(methodOnA, resolved.method);
+ assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options));
DexEncodedMethod singleVirtualTarget =
appInfo.lookupSingleVirtualTarget(methodOnB, methodOnB.holder);
Assert.assertNull(singleVirtualTarget);
@@ -180,10 +180,7 @@
ResolutionResult resolutionResult = appInfo.resolveMethodOnClass(methodOnB.holder, methodOnB);
DexEncodedMethod resolved = resolutionResult.asResultOfResolve();
assertEquals(methodOnA, resolved.method);
- // See comment in VirtualOverrideOfPrivateStaticMethodTest.lookupVirtualTargets().
- Set<DexEncodedMethod> targets = resolutionResult.lookupVirtualTargets(appInfo);
- assertTrue("Expected " + methodOnA, targets.stream().anyMatch(m -> m.method == methodOnA));
- assertTrue("Expected " + methodOnC, targets.stream().anyMatch(m -> m.method == methodOnC));
+ assertFalse(resolutionResult.isValidVirtualTarget(appInfo.app().options));
}
@Test
