Remove special handling for incorrect ART resolution.
The incorrect resolution happens on ART 5 to 7, thus both Dalvik and recent VMs
will not exhibit the incorrect behavior and any program relying on it should
rather fix the program than expect the compiler to preserve the incorrect
behavior.
Bug: 140013075
Change-Id: I9e3b59746489082b07edc6acd4cd19fd01716329
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 46895aa..183151c 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -2040,56 +2040,11 @@
// invoke. This also ensures preserving the errors detailed below.
if (resolutionTargetClass.isProgramClass()) {
markMethodAsTargeted(resolutionTargetClass.asProgramClass(), resolutionTarget, reason);
-
- // If the method of an invoke-virtual instruction resolves to a private or static method, then
- // the invoke fails with an IllegalAccessError or IncompatibleClassChangeError, respectively.
- //
- // Unfortunately the above is not always the case:
- // Some Art VMs do not fail with an IllegalAccessError or IncompatibleClassChangeError if the
- // method of an invoke-virtual instruction resolves to a private or static method, but instead
- // ignores private and static methods during resolution (see also NonVirtualOverrideTest).
- // 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();
- MarkedResolutionTarget possiblyValidTarget =
- markPossiblyValidTarget(
- method, reason, resolutionTarget, resolutionTargetClass.asProgramClass());
- if (!possiblyValidTarget.isUnresolved()) {
- // 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 new MarkedResolutionTarget(resolutionTargetClass, resolutionTarget);
}
- private MarkedResolutionTarget markPossiblyValidTarget(
- DexMethod method,
- KeepReason reason,
- DexEncodedMethod resolutionTarget,
- DexProgramClass resolutionTargetClass) {
- while (resolutionTarget.isPrivateMethod() || resolutionTarget.isStatic()) {
- resolutionTarget =
- appInfo
- .resolveMethod(
- resolutionTargetClass.superType, method, resolutionTargetClass.isInterface())
- .asResultOfResolve();
- if (resolutionTarget == null) {
- return MarkedResolutionTarget.unresolved();
- }
- resolutionTargetClass = getProgramClassOrNull(resolutionTarget.method.holder);
- if (resolutionTargetClass == null) {
- return MarkedResolutionTarget.unresolved();
- }
- }
- markMethodAsTargeted(resolutionTargetClass, resolutionTarget, reason);
- return new MarkedResolutionTarget(resolutionTargetClass, resolutionTarget);
- }
-
private DexMethod generatedEnumValuesMethod(DexClass enumClass) {
DexType arrayOfEnumClass =
appView
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 952cfe7..6e87ef3 100644
--- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodWithVirtualParentTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfPrivateStaticMethodWithVirtualParentTest.java
@@ -165,7 +165,7 @@
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), Main.class);
}
- checkResult(runResult);
+ checkResult(runResult, false);
}
@Test
@@ -177,11 +177,11 @@
.addKeepMainRule(Main.class)
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), Main.class);
- checkResult(runResult);
+ checkResult(runResult, true);
}
- private void checkResult(TestRunResult<?> runResult) {
- if (expectedToIncorrectlyRun(parameters.getRuntime())) {
+ private void checkResult(TestRunResult<?> runResult, boolean isCorrectedByR8) {
+ if (expectedToIncorrectlyRun(parameters.getRuntime(), isCorrectedByR8)) {
// Do to incorrect resolution, some Art VMs will resolve to Base.f (ignoring A.f) and thus
// virtual dispatch to C.f. See b/140013075.
runResult.assertSuccessWithOutputLines("Called C.f");
@@ -190,8 +190,9 @@
}
}
- private boolean expectedToIncorrectlyRun(TestRuntime runtime) {
- return runtime.isDex()
+ private boolean expectedToIncorrectlyRun(TestRuntime runtime, boolean isCorrectedByR8) {
+ return !isCorrectedByR8
+ && runtime.isDex()
&& runtime.asDex().getVm().isNewerThan(DexVm.ART_4_4_4_HOST)
&& runtime.asDex().getVm().isOlderThanOrEqual(DexVm.ART_7_0_0_HOST);
}
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 c17ed49..968f21e 100644
--- a/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java
+++ b/src/test/java/com/android/tools/r8/resolution/VirtualOverrideOfStaticMethodWithVirtualParentTest.java
@@ -200,7 +200,7 @@
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), Main.class);
}
- checkResult(runResult);
+ checkResult(runResult, false);
}
@Test
@@ -212,11 +212,11 @@
.addKeepMainRule(Main.class)
.setMinApi(parameters.getApiLevel())
.run(parameters.getRuntime(), Main.class);
- checkResult(runResult);
+ checkResult(runResult, true);
}
- private void checkResult(TestRunResult<?> runResult) {
- if (expectedToIncorrectlyRun(parameters.getRuntime())) {
+ private void checkResult(TestRunResult<?> runResult, boolean isCorrectedByR8) {
+ if (expectedToIncorrectlyRun(parameters.getRuntime(), isCorrectedByR8)) {
// Do to incorrect resolution, some Art VMs will resolve to Base.f (ignoring A.f) and thus
// virtual dispatch to C.f. See b/140013075.
runResult.assertSuccessWithOutputLines("Called C.f");
@@ -225,8 +225,9 @@
}
}
- private boolean expectedToIncorrectlyRun(TestRuntime runtime) {
- return runtime.isDex()
+ private boolean expectedToIncorrectlyRun(TestRuntime runtime, boolean isCorrectedByR8) {
+ return !isCorrectedByR8
+ && runtime.isDex()
&& runtime.asDex().getVm().isNewerThan(DexVm.ART_4_4_4_HOST)
&& runtime.asDex().getVm().isOlderThanOrEqual(DexVm.ART_7_0_0_HOST);
}
