Merge "ClassInliner should not remove IncompatibleClassChangeError"
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 ae57f48..e610d20 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
@@ -762,6 +762,11 @@
return;
}
+ DexClass clazz = appInfo.definitionFor(method.method.holder);
+ if (clazz == null) {
+ return;
+ }
+
boolean receiverUsedAsReturnValue = false;
boolean seenSuperInitCall = false;
for (Instruction insn : receiver.uniqueUsers()) {
@@ -773,7 +778,7 @@
if (insn.isInstanceGet() ||
(insn.isInstancePut() && insn.asInstancePut().object() == receiver)) {
DexField field = insn.asFieldInstruction().getField();
- if (field.clazz == method.method.holder) {
+ if (field.clazz == clazz.type && clazz.lookupInstanceField(field) != null) {
// Since class inliner currently only supports classes directly extending
// java.lang.Object, we don't need to worry about fields defined in superclasses.
continue;
diff --git a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
index 7bdff33..d7a57df 100644
--- a/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
+++ b/src/main/java/com/android/tools/r8/ir/optimize/classinliner/ClassInliner.java
@@ -169,14 +169,16 @@
Map<InvokeMethodWithReceiver, InliningInfo> methodCalls = new IdentityHashMap<>();
+ DexClass definition = appInfo.definitionFor(clazz);
+
for (Instruction user : receiver.uniqueUsers()) {
// Field read/write.
if (user.isInstanceGet() ||
(user.isInstancePut() && user.asInstancePut().value() != receiver)) {
- if (user.asFieldInstruction().getField().clazz == newInstanceInsn.clazz) {
- // Eligible field read or write. Note: as long as we consider only classes eligible
- // if they directly extend java.lang.Object we don't need to check if the field
- // really exists in the class.
+ DexField field = user.asFieldInstruction().getField();
+ if (field.clazz == newInstanceInsn.clazz && definition.lookupInstanceField(field) != null) {
+ // Since class inliner currently only supports classes directly extending
+ // java.lang.Object, we don't need to worry about fields defined in superclasses.
continue;
}
return null; // Not eligible.
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
index 5df1cee..38768cd 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/classinliner/ClassInlinerTest.java
@@ -4,9 +4,11 @@
package com.android.tools.r8.ir.optimize.classinliner;
+import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
import com.android.tools.r8.OutputMode;
@@ -32,10 +34,13 @@
import com.android.tools.r8.ir.optimize.classinliner.trivial.Iface2Impl;
import com.android.tools.r8.ir.optimize.classinliner.trivial.ReferencedFields;
import com.android.tools.r8.ir.optimize.classinliner.trivial.TrivialTestClass;
+import com.android.tools.r8.jasmin.JasminBuilder;
+import com.android.tools.r8.jasmin.JasminBuilder.ClassBuilder;
import com.android.tools.r8.naming.MemberNaming.MethodSignature;
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.DexInspector;
import com.android.tools.r8.utils.DexInspector.ClassSubject;
+import com.android.tools.r8.utils.InternalOptions;
import com.google.common.collect.Sets;
import java.nio.file.Path;
import java.util.Collections;
@@ -178,6 +183,39 @@
assertFalse(inspector.clazz(BuildersTestClass.Pos.class).isPresent());
}
+ @Test
+ public void testErroneousInput() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+
+ ClassBuilder testClass = builder.addClass("A");
+ testClass.addStaticFinalField("f", "I", "123");
+ testClass.addDefaultConstructor();
+
+ ClassBuilder mainClass = builder.addClass("Main");
+ mainClass.addMainMethod(
+ ".limit stack 3",
+ ".limit locals 1",
+ " getstatic java/lang/System/out Ljava/io/PrintStream;",
+ " new A",
+ " dup",
+ " invokespecial A/<init>()V",
+ " getfield A/f I",
+ " invokevirtual java/io/PrintStream/print(I)V",
+ " return");
+
+ AndroidApp compiled =
+ compileWithR8(builder.build(), getProguardConfig(mainClass.name), this::configure);
+
+ // Check that the code fails with an IncompatibleClassChangeError with Java.
+ ProcessResult javaResult =
+ runOnJavaRaw(mainClass.name, builder.buildClasses().toArray(new byte[2][]));
+ assertThat(javaResult.stderr, containsString("IncompatibleClassChangeError"));
+
+ // Check that the code fails with an IncompatibleClassChangeError with ART.
+ ProcessResult artResult = runOnArtRaw(compiled, mainClass.name);
+ assertThat(artResult.stderr, containsString("IncompatibleClassChangeError"));
+ }
+
private Set<String> collectNewInstanceTypes(
ClassSubject clazz, String methodName, String... params) {
assertNotNull(clazz);
@@ -189,15 +227,8 @@
}
private AndroidApp runR8(AndroidApp app, Class mainClass) throws Exception {
- String config = keepMainProguardConfiguration(mainClass) + "\n"
- + "-dontobfuscate\n"
- + "-allowaccessmodification";
-
- AndroidApp compiled = compileWithR8(app, config,
- o -> {
- o.enableClassInlining = true;
- o.classInliningInstructionLimit = 10000;
- });
+ AndroidApp compiled =
+ compileWithR8(app, getProguardConfig(mainClass.getCanonicalName()), this::configure);
// Materialize file for execution.
Path generatedDexFile = temp.getRoot().toPath().resolve("classes.jar");
@@ -220,4 +251,16 @@
return compiled;
}
+
+ private String getProguardConfig(String main) {
+ return keepMainProguardConfiguration(main)
+ + "\n"
+ + "-dontobfuscate\n"
+ + "-allowaccessmodification";
+ }
+
+ private void configure(InternalOptions options) {
+ options.enableClassInlining = true;
+ options.classInliningInstructionLimit = 10000;
+ }
}
