Rewrite code from javac code with for JDK-8272564
Some Dalvik and Art MVs does not support interface invokes to Object
members not explicitly defined on the symbolic reference of the
interface invoke. In these cases rewrite to a virtual invoke with
the symbolic reference java.lang.Object.
Bug: 218298666
Change-Id: I38abdea8d9bbf4d004be1877373b58de161a55a7
diff --git a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
index 686f124..3640575 100644
--- a/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
+++ b/src/main/java/com/android/tools/r8/graph/DexItemFactory.java
@@ -1431,6 +1431,54 @@
|| method.match(waitLong)
|| method.match(waitLongInt);
}
+
+ public DexMethod matchingPublicObjectMember(DexMethod method) {
+ switch (method.getName().byteAt(0)) {
+ case 't':
+ if (method.match(toString)) {
+ return toString;
+ }
+ break;
+ case 'h':
+ if (method.match(hashCode)) {
+ return hashCode;
+ }
+ break;
+ case 'e':
+ if (method.match(equals)) {
+ return equals;
+ }
+ break;
+ case 'g':
+ if (method.match(getClass)) {
+ return getClass;
+ }
+ break;
+ case 'n':
+ if (method.match(notify)) {
+ return notify;
+ }
+ if (method.match(notifyAll)) {
+ return notifyAll;
+ }
+ break;
+ case 'w':
+ if (method.match(wait)) {
+ return wait;
+ }
+ if (method.match(waitLong)) {
+ return waitLong;
+ }
+ if (method.match(waitLongInt)) {
+ return waitLongInt;
+ }
+ break;
+ default:
+ // Methods finalize and clone are not public.
+ return null;
+ }
+ return null;
+ }
}
public class BufferMembers {
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
index 24207a2..5d27d27 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/IRConverter.java
@@ -1091,6 +1091,12 @@
timing.end();
}
+ if (!options.canHaveInvokeInterfaceToObjectMethodBug()) {
+ timing.begin("JDK-8272564 fix rewrite");
+ CodeRewriter.rewriteJdk8272564Fix(code, appView);
+ timing.end();
+ }
+
boolean isDebugMode = options.debug || method.getOptimizationInfo().isReachabilitySensitive();
if (isDebugMode) {
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
index f4f4412..b6988e4 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
@@ -70,6 +70,7 @@
import com.android.tools.r8.ir.code.IntSwitch;
import com.android.tools.r8.ir.code.Invoke;
import com.android.tools.r8.ir.code.InvokeDirect;
+import com.android.tools.r8.ir.code.InvokeInterface;
import com.android.tools.r8.ir.code.InvokeMethod;
import com.android.tools.r8.ir.code.InvokeMethodWithReceiver;
import com.android.tools.r8.ir.code.InvokeNewArray;
@@ -3773,6 +3774,26 @@
}
}
+ // The javac fix for JDK-8272564 has to be rewritten back to invoke-virtual on Object if the
+ // method with an Object signature is not defined on the interface. See
+ // https://bugs.openjdk.java.net/browse/JDK-8272564
+ public static void rewriteJdk8272564Fix(IRCode code, AppView<?> appView) {
+ DexItemFactory dexItemFactory = appView.dexItemFactory();
+ InstructionListIterator it = code.instructionListIterator();
+ while (it.hasNext()) {
+ Instruction instruction = it.next();
+ if (instruction.isInvokeInterface()) {
+ InvokeInterface invoke = instruction.asInvokeInterface();
+ DexMethod method = invoke.getInvokedMethod();
+ DexMethod objectMember = dexItemFactory.objectMembers.matchingPublicObjectMember(method);
+ if (objectMember != null && appView.definitionFor(method) == null) {
+ it.replaceCurrentInstruction(
+ new InvokeVirtual(objectMember, invoke.outValue(), invoke.arguments()));
+ }
+ }
+ }
+ }
+
private static NewInstance findNewInstance(Phi phi) {
Set<Phi> seen = Sets.newIdentityHashSet();
Set<Value> values = Sets.newIdentityHashSet();
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 e9db143..1c31fbe 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -2338,4 +2338,16 @@
public boolean canHaveSuperInvokeBug() {
return getMinApiLevel().isLessThan(AndroidApiLevel.N);
}
+
+ // Some Dalvik and Art MVs does not support interface invokes to Object
+ // members not explicitly defined on the symbolic reference of the
+ // interface invoke. In these cases rewrite to a virtual invoke with
+ // the symbolic reference java.lang.Object.
+ //
+ // javac started generating code like this with the fix for JDK-8272564.
+ //
+ // See b/218298666.
+ public boolean canHaveInvokeInterfaceToObjectMethodBug() {
+ return isGeneratingClassFiles() || getMinApiLevel().isGreaterThanOrEqualTo(AndroidApiLevel.O);
+ }
}
diff --git a/src/test/examplesJava18/jdk8272564/I.java b/src/test/examplesJava18/jdk8272564/I.java
index ba9e252..1694eea 100644
--- a/src/test/examplesJava18/jdk8272564/I.java
+++ b/src/test/examplesJava18/jdk8272564/I.java
@@ -6,4 +6,8 @@
interface I {
public String toString();
+
+ public int hashCode();
+
+ public boolean equals(Object other);
}
diff --git a/src/test/examplesJava18/jdk8272564/Main.java b/src/test/examplesJava18/jdk8272564/Main.java
index cc759ca..c5963fd 100644
--- a/src/test/examplesJava18/jdk8272564/Main.java
+++ b/src/test/examplesJava18/jdk8272564/Main.java
@@ -14,6 +14,34 @@
k.toString();
}
+ // Remaining public methods on Object.
+ static void g(I i, J j, K k) throws InterruptedException {
+ i.hashCode();
+ j.hashCode();
+ k.hashCode();
+ i.equals(new Object());
+ j.equals(new Object());
+ k.equals(new Object());
+ i.getClass();
+ j.getClass();
+ k.getClass();
+ i.notify();
+ j.notify();
+ k.notify();
+ i.notifyAll();
+ j.notifyAll();
+ k.notifyAll();
+ i.wait();
+ j.wait();
+ k.wait();
+ i.wait(1L);
+ j.wait(1L);
+ k.wait(1L);
+ i.wait(1L, 1);
+ j.wait(1L, 1);
+ k.wait(1L, 1);
+ }
+
public static void main(String[] args) {
f(new A(), new B(), new C());
}
diff --git a/src/test/java/com/android/tools/r8/desugar/jdk8272564/Jdk8272564Test.java b/src/test/java/com/android/tools/r8/desugar/jdk8272564/Jdk8272564Test.java
index 5bce7b7..191cc22 100644
--- a/src/test/java/com/android/tools/r8/desugar/jdk8272564/Jdk8272564Test.java
+++ b/src/test/java/com/android/tools/r8/desugar/jdk8272564/Jdk8272564Test.java
@@ -12,8 +12,8 @@
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
import com.android.tools.r8.TestRuntime;
-import com.android.tools.r8.ToolHelper.DexVm.Version;
import com.android.tools.r8.examples.jdk18.jdk8272564.Jdk8272564;
+import com.android.tools.r8.utils.AndroidApiLevel;
import com.android.tools.r8.utils.InternalOptions.TestingOptions;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.android.tools.r8.utils.codeinspector.InstructionSubject;
@@ -42,17 +42,24 @@
public TestParameters parameters;
// With the fix for JDK-8272564 there are no invokevirtual instructions.
- private void assertJdk8272564FixedCode(CodeInspector inspector) throws Exception {
+ private void assertJdk8272564FixedCode(CodeInspector inspector) {
assertTrue(
inspector
.clazz(Jdk8272564.Main.typeName())
.uniqueMethodWithName("f")
.streamInstructions()
.noneMatch(InstructionSubject::isInvokeVirtual));
+ assertTrue(
+ inspector
+ .clazz(Jdk8272564.Main.typeName())
+ .uniqueMethodWithName("g")
+ .streamInstructions()
+ .noneMatch(InstructionSubject::isInvokeVirtual));
}
// Without the fix for JDK-8272564 there is one invokeinterface and 2 invokevirtual instructions.
- private void assertJdk8272564NotFixedCode(CodeInspector inspector) throws Exception {
+ private void assertJdk8272564NotFixedCode(
+ CodeInspector inspector, int invokeVirtualCount, int getClassCount) {
assertEquals(
1,
inspector
@@ -69,6 +76,47 @@
.streamInstructions()
.filter(InstructionSubject::isInvokeVirtual)
.count());
+ assertEquals(
+ 2,
+ inspector
+ .clazz(Jdk8272564.Main.typeName())
+ .uniqueMethodWithName("g")
+ .streamInstructions()
+ .filter(InstructionSubject::isInvokeInterface)
+ .count());
+ assertEquals(
+ 2,
+ inspector
+ .clazz(Jdk8272564.Main.typeName())
+ .uniqueMethodWithName("g")
+ .streamInstructions()
+ .filter(InstructionSubject::isInvokeInterface)
+ .count());
+ assertEquals(
+ invokeVirtualCount,
+ inspector
+ .clazz(Jdk8272564.Main.typeName())
+ .uniqueMethodWithName("g")
+ .streamInstructions()
+ .filter(InstructionSubject::isInvokeVirtual)
+ .count());
+ assertEquals(
+ getClassCount,
+ inspector
+ .clazz(Jdk8272564.Main.typeName())
+ .uniqueMethodWithName("g")
+ .streamInstructions()
+ .filter(InstructionSubject::isInvoke)
+ .filter(instruction -> instruction.getMethod().getName().toString().equals("getClass"))
+ .count());
+ }
+
+ private void assertJdk8272564NotFixedCode(CodeInspector inspector) {
+ assertJdk8272564NotFixedCode(inspector, 22, 3);
+ }
+
+ private void assertJdk8272564NotFixedCodeR8(CodeInspector inspector) {
+ assertJdk8272564NotFixedCode(inspector, 19, 0);
}
@Test
@@ -94,13 +142,10 @@
.addProgramFiles(Jdk8272564.jar())
.run(parameters.getRuntime(), Jdk8272564.Main.typeName())
.applyIf(
- parameters.getRuntime().isDex()
- && parameters.getRuntime().asDex().getVersion().isOlderThanOrEqual(Version.V4_4_4),
- r -> r.assertFailureWithErrorThatThrows(NoSuchMethodError.class),
- parameters.getRuntime().isDex()
- && parameters.getRuntime().asDex().getVersion().isOlderThanOrEqual(Version.V7_0_0),
- r -> r.assertFailureWithErrorThatThrows(IncompatibleClassChangeError.class),
- r -> r.assertSuccess());
+ parameters.isDexRuntime() && parameters.getApiLevel().isLessThan(AndroidApiLevel.O),
+ b -> b.inspect(this::assertJdk8272564NotFixedCode),
+ b -> b.inspect(this::assertJdk8272564FixedCode))
+ .assertSuccess();
}
@Test
@@ -113,7 +158,7 @@
.addKeepClassAndMembersRules(Jdk8272564.Main.typeName())
.addOptionsModification(TestingOptions::allowExperimentClassFileVersion)
.run(parameters.getRuntime(), Jdk8272564.Main.typeName())
- .inspect(this::assertJdk8272564NotFixedCode)
+ .inspect(this::assertJdk8272564NotFixedCodeR8)
.assertSuccess();
}
}
