Merge "Remove end-marker before removing chained check-cast."
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 30244bf..02c06da 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
@@ -1651,7 +1651,7 @@
&& outValue.isUsed()
&& outValue.numberOfPhiUsers() == 0
&& outValue.uniqueUsers().stream().allMatch(isCheckcastToSubtype)) {
- removeOrReplaceByDebugLocalWrite(it, inValue, outValue);
+ removeOrReplaceByDebugLocalWrite(checkCast, it, inValue, outValue);
continue;
}
@@ -1671,7 +1671,7 @@
if (TypeLatticeElement.lessThanOrEqual(appInfo, inTypeLattice, castTypeLattice)) {
assert outTypeLattice.equals(inTypeLattice);
needToRemoveTrivialPhis = needToRemoveTrivialPhis || outValue.numberOfPhiUsers() != 0;
- removeOrReplaceByDebugLocalWrite(it, inValue, outValue);
+ removeOrReplaceByDebugLocalWrite(checkCast, it, inValue, outValue);
continue;
}
// Otherwise, keep the checkcast to preserve verification errors. E.g., down-cast:
@@ -1691,16 +1691,20 @@
if (needToRemoveTrivialPhis) {
code.removeAllTrivialPhis();
}
- it = code.instructionIterator();
assert code.isConsistentSSA();
}
private void removeOrReplaceByDebugLocalWrite(
- InstructionIterator it, Value inValue, Value outValue) {
- if (outValue.getLocalInfo() != inValue.getLocalInfo() && outValue.hasLocalInfo()) {
+ Instruction currentInstruction, InstructionIterator it, Value inValue, Value outValue) {
+ if (outValue.hasLocalInfo() && outValue.getLocalInfo() != inValue.getLocalInfo()) {
DebugLocalWrite debugLocalWrite = new DebugLocalWrite(outValue, inValue);
it.replaceCurrentInstruction(debugLocalWrite);
} else {
+ if (outValue.hasLocalInfo()) {
+ assert outValue.getLocalInfo() == inValue.getLocalInfo();
+ // Should remove the end-marker before replacing the current instruction.
+ currentInstruction.removeDebugValue(outValue.getLocalInfo());
+ }
outValue.replaceUsers(inValue);
it.removeOrReplaceByDebugLocalRead();
}
diff --git a/src/test/java/com/android/tools/r8/TestBase.java b/src/test/java/com/android/tools/r8/TestBase.java
index 403ee70..fa35d0d 100644
--- a/src/test/java/com/android/tools/r8/TestBase.java
+++ b/src/test/java/com/android/tools/r8/TestBase.java
@@ -12,8 +12,11 @@
import com.android.tools.r8.ToolHelper.ArtCommandBuilder;
import com.android.tools.r8.ToolHelper.DexVm;
import com.android.tools.r8.ToolHelper.ProcessResult;
+import com.android.tools.r8.cf.code.CfInstruction;
import com.android.tools.r8.code.Instruction;
import com.android.tools.r8.errors.Unreachable;
+import com.android.tools.r8.graph.CfCode;
+import com.android.tools.r8.graph.Code;
import com.android.tools.r8.graph.DexCode;
import com.android.tools.r8.graph.DexEncodedMethod;
import com.android.tools.r8.graph.SmaliWriter;
@@ -827,6 +830,13 @@
.map(kind::cast);
}
+ protected Stream<CfInstruction> filterInstructionKind(
+ CfCode code, Class<? extends CfInstruction> kind) {
+ return code.getInstructions().stream()
+ .filter(kind::isInstance)
+ .map(kind::cast);
+ }
+
public enum MinifyMode {
NONE,
JAVA,
diff --git a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
index 99222d0..b6cdb48 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
@@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.
package com.android.tools.r8.debug;
+import com.android.tools.r8.TestBase;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.ToolHelper.ArtCommandBuilder;
import com.android.tools.r8.ToolHelper.DexVm;
@@ -79,7 +80,7 @@
* The protocol messages are described here:
* https://docs.oracle.com/javase/8/docs/platform/jpda/jdwp/jdwp-protocol.html
*/
-public abstract class DebugTestBase {
+public abstract class DebugTestBase extends TestBase {
// Set to true to enable verbose logs
private static final boolean DEBUG_TESTS = false;
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/checkcast/CheckCastDebugTest.java b/src/test/java/com/android/tools/r8/ir/optimize/checkcast/CheckCastDebugTest.java
new file mode 100644
index 0000000..6d6b4c7
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/checkcast/CheckCastDebugTest.java
@@ -0,0 +1,55 @@
+// 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.ir.optimize.checkcast;
+
+import com.android.tools.r8.NeverInline;
+
+class A {
+ @NeverInline
+ @Override
+ public String toString() {
+ return "A";
+ }
+}
+
+class B extends A {
+ @NeverInline
+ @Override
+ public String toString() {
+ return super.toString() + "B";
+ }
+}
+
+class C extends B {
+ @NeverInline
+ @Override
+ public String toString() {
+ return super.toString() + "C";
+ }
+}
+
+class CheckCastDebugTest {
+ @NeverInline
+ static void differentLocals() {
+ Object obj = new C();
+ A a = (A) obj;
+ B b = (B) a;
+ C c = (C) b;
+ System.out.println(c.toString());
+ }
+
+ @NeverInline
+ static void sameLocal() {
+ Object obj = new C();
+ obj = (A) obj;
+ obj = (B) obj;
+ obj = (C) obj;
+ System.out.println(obj.toString());
+ }
+
+ public static void main(String[] args) {
+ differentLocals();
+ sameLocal();
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/checkcast/CheckCastDebugTestRunner.java b/src/test/java/com/android/tools/r8/ir/optimize/checkcast/CheckCastDebugTestRunner.java
new file mode 100644
index 0000000..44720ed
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/ir/optimize/checkcast/CheckCastDebugTestRunner.java
@@ -0,0 +1,178 @@
+// 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.ir.optimize.checkcast;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+
+import com.android.tools.r8.ClassFileConsumer;
+import com.android.tools.r8.CompilationMode;
+import com.android.tools.r8.DexIndexedConsumer;
+import com.android.tools.r8.NeverInline;
+import com.android.tools.r8.R8Command;
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.cf.code.CfCheckCast;
+import com.android.tools.r8.debug.CfDebugTestConfig;
+import com.android.tools.r8.debug.DebugTestBase;
+import com.android.tools.r8.debug.DebugTestConfig;
+import com.android.tools.r8.debug.DexDebugTestConfig;
+import com.android.tools.r8.graph.Code;
+import com.android.tools.r8.code.CheckCast;
+import com.android.tools.r8.origin.Origin;
+import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import com.android.tools.r8.utils.codeinspector.CodeInspector;
+import com.android.tools.r8.utils.codeinspector.MethodSubject;
+import com.google.common.collect.ImmutableList;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.Collection;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class CheckCastDebugTestRunner extends DebugTestBase {
+ private static final Class<?> MAIN = CheckCastDebugTest.class;
+ private final Backend backend;
+
+ private Path r8Out;
+ private CodeInspector inspector;
+
+ @Parameterized.Parameters(name = "Backend: {0}")
+ public static Collection<Backend> data() {
+ return Arrays.asList(Backend.values());
+ }
+
+ public CheckCastDebugTestRunner(Backend backend) {
+ this.backend = backend;
+ }
+
+ private static int testCounter = 0;
+
+ @Before
+ public void setUp() throws Exception {
+ AndroidApp app = readClasses(NeverInline.class, A.class, B.class, C.class, MAIN);
+ r8Out = temp.newFile(String.format("r8Out-%s-%d.zip", backend, testCounter++)).toPath();
+ R8Command.Builder builder = ToolHelper.prepareR8CommandBuilder(app);
+ if (backend == Backend.DEX) {
+ builder.setProgramConsumer(new DexIndexedConsumer.ArchiveConsumer(r8Out));
+ } else {
+ assert backend == Backend.CF;
+ builder.setProgramConsumer(new ClassFileConsumer.ArchiveConsumer(r8Out));
+ }
+ builder.addProguardConfiguration(
+ ImmutableList.of(
+ "-dontobfuscate",
+ keepMainProguardConfigurationWithInliningAnnotation(MAIN)),
+ Origin.unknown());
+ builder.setMode(CompilationMode.DEBUG);
+ ToolHelper.allowTestProguardOptions(builder);
+ ToolHelper.runR8(builder.build(), o -> o.enableVerticalClassMerging = false);
+ inspector = new CodeInspector(r8Out);
+ ClassSubject classSubject = inspector.clazz(MAIN);
+ assertThat(classSubject, isPresent());
+ }
+
+ @Test
+ public void test_differentLocals() throws Throwable {
+ ClassSubject classSubject = inspector.clazz(MAIN);
+ MethodSubject method = classSubject.method("void", "differentLocals", ImmutableList.of());
+ assertThat(method, isPresent());
+ long count = countCheckCast(method.getMethod().getCode());
+ assertEquals(1, count);
+
+ DebugTestConfig config = backend == Backend.CF
+ ? new CfDebugTestConfig()
+ : new DexDebugTestConfig();
+ config.addPaths(r8Out);
+ runDebugTest(config, MAIN.getCanonicalName(),
+ // Object obj = new C();
+ breakpoint(MAIN.getCanonicalName(), "differentLocals", "()V", 35),
+ run(),
+ checkNoLocal("obj"),
+ checkNoLocal("a"),
+ checkNoLocal("b"),
+ checkNoLocal("c"),
+ // A a = (A) obj;
+ breakpoint(MAIN.getCanonicalName(), "differentLocals", "()V", 36),
+ run(),
+ checkLocal("obj"),
+ checkNoLocal("a"),
+ checkNoLocal("b"),
+ checkNoLocal("c"),
+ // B b = (B) a;
+ breakpoint(MAIN.getCanonicalName(), "differentLocals", "()V", 37),
+ run(),
+ checkLocal("obj"),
+ checkLocal("a"),
+ checkNoLocal("b"),
+ checkNoLocal("c"),
+ // C c = (C) b;
+ breakpoint(MAIN.getCanonicalName(), "differentLocals", "()V", 38),
+ run(),
+ checkLocal("obj"),
+ checkLocal("a"),
+ checkLocal("b"),
+ checkNoLocal("c"),
+ // System.out.println(c.toString());
+ breakpoint(MAIN.getCanonicalName(), "differentLocals", "()V", 39),
+ run(),
+ checkLocal("obj"),
+ checkLocal("a"),
+ checkLocal("b"),
+ checkLocal("c"),
+ run()
+ );
+ }
+
+ @Test
+ public void test_sameLocal() throws Throwable {
+ ClassSubject classSubject = inspector.clazz(MAIN);
+ MethodSubject method = classSubject.method("void", "sameLocal", ImmutableList.of());
+ assertThat(method, isPresent());
+ long count = countCheckCast(method.getMethod().getCode());
+ assertEquals(1, count);
+
+ DebugTestConfig config = backend == Backend.CF
+ ? new CfDebugTestConfig()
+ : new DexDebugTestConfig();
+ config.addPaths(r8Out);
+ runDebugTest(config, MAIN.getCanonicalName(),
+ // Object obj = new C();
+ breakpoint(MAIN.getCanonicalName(), "sameLocal", "()V", 44),
+ run(),
+ checkNoLocal("obj"),
+ // obj = (A) obj;
+ breakpoint(MAIN.getCanonicalName(), "sameLocal", "()V", 45),
+ run(),
+ checkLocal("obj"),
+ // obj = (B) obj;
+ breakpoint(MAIN.getCanonicalName(), "sameLocal", "()V", 46),
+ run(),
+ checkLocal("obj"),
+ // obj = (C) obj;
+ breakpoint(MAIN.getCanonicalName(), "sameLocal", "()V", 47),
+ run(),
+ checkLocal("obj"),
+ // System.out.println(obj.toString());
+ breakpoint(MAIN.getCanonicalName(), "sameLocal", "()V", 48),
+ run(),
+ checkLocal("obj"),
+ run()
+ );
+ }
+
+ private long countCheckCast(Code code) {
+ if (backend == Backend.DEX) {
+ return filterInstructionKind(code.asDexCode(), CheckCast.class).count();
+ } else {
+ assert backend == Backend.CF;
+ return filterInstructionKind(code.asCfCode(), CfCheckCast.class).count();
+ }
+ }
+
+}
diff --git a/src/test/java/com/android/tools/r8/ir/optimize/checkcast/CheckCastRemovalTest.java b/src/test/java/com/android/tools/r8/ir/optimize/checkcast/CheckCastRemovalTest.java
index b141955..c953515 100644
--- a/src/test/java/com/android/tools/r8/ir/optimize/checkcast/CheckCastRemovalTest.java
+++ b/src/test/java/com/android/tools/r8/ir/optimize/checkcast/CheckCastRemovalTest.java
@@ -99,7 +99,7 @@
}
@Test
- public void downCasts() throws Exception {
+ public void downCasts_noLocal() throws Exception {
JasminBuilder builder = new JasminBuilder();
// C < B < A
ClassBuilder a = builder.addClass("A");
@@ -133,6 +133,91 @@
}
@Test
+ public void downCasts_differentLocals() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+ // C < B < A
+ ClassBuilder a = builder.addClass("A");
+ a.addDefaultConstructor();
+ ClassBuilder b = builder.addClass("B", "A");
+ b.addDefaultConstructor();
+ ClassBuilder c = builder.addClass("C", "B");
+ c.addDefaultConstructor();
+ ClassBuilder classBuilder = builder.addClass(CLASS_NAME);
+ MethodSignature main = classBuilder.addMainMethod(
+ ".limit stack 3",
+ ".limit locals 3",
+ ".var 0 is a LA; from Label1 to Label2",
+ ".var 1 is b LB; from Label1 to Label2",
+ ".var 2 is c LC; from Label1 to Label2",
+ "Label1:",
+ "new A",
+ "dup",
+ "invokespecial A/<init>()V",
+ "astore_0",
+ "aload_0",
+ "checkcast B", // Gone
+ "astore_1",
+ "aload_1",
+ "checkcast C", // Should be kept to preserve cast exception
+ "astore_2",
+ "Label2:",
+ "return");
+
+ List<String> pgConfigs = ImmutableList.of(
+ "-keep class " + CLASS_NAME + " { *; }",
+ "-keep class A { *; }",
+ "-keep class B { *; }",
+ "-keep class C { *; }",
+ "-dontoptimize",
+ "-dontshrink");
+ AndroidApp app = compileWithR8InDebugMode(builder, pgConfigs, null, backend);
+
+ checkCheckCasts(app, main, "C");
+ checkRuntimeException(builder, app, CLASS_NAME, "ClassCastException");
+ }
+
+ @Test
+ public void downCasts_sameLocal() throws Exception {
+ JasminBuilder builder = new JasminBuilder();
+ // C < B < A
+ ClassBuilder a = builder.addClass("A");
+ a.addDefaultConstructor();
+ ClassBuilder b = builder.addClass("B", "A");
+ b.addDefaultConstructor();
+ ClassBuilder c = builder.addClass("C", "B");
+ c.addDefaultConstructor();
+ ClassBuilder classBuilder = builder.addClass(CLASS_NAME);
+ MethodSignature main = classBuilder.addMainMethod(
+ ".limit stack 3",
+ ".limit locals 1",
+ ".var 0 is a LA; from Label1 to Label2",
+ "Label1:",
+ "new A",
+ "dup",
+ "invokespecial A/<init>()V",
+ "astore_0",
+ "aload_0",
+ "checkcast B", // Gone
+ "astore_0",
+ "aload_0",
+ "checkcast C", // Should be kept to preserve cast exception
+ "Label2:",
+ "return");
+
+ List<String> pgConfigs = ImmutableList.of(
+ "-keep class " + CLASS_NAME + " { *; }",
+ "-keep class A { *; }",
+ "-keep class B { *; }",
+ "-keep class C { *; }",
+ "-dontoptimize",
+ "-dontshrink");
+ AndroidApp app = compileWithR8InDebugMode(builder, pgConfigs, null, backend);
+
+ checkCheckCasts(app, main, "C");
+ checkRuntimeException(builder, app, CLASS_NAME, "ClassCastException");
+ }
+
+ @Test
public void bothUpAndDowncast() throws Exception {
JasminBuilder builder = new JasminBuilder();
ClassBuilder classBuilder = builder.addClass(CLASS_NAME);
@@ -180,7 +265,7 @@
"-dontshrink");
AndroidApp app = compileWithR8(builder, pgConfigs, null, backend);
- checkCheckCasts(app, main, "Example");
+ checkCheckCasts(app, main, null);
checkRuntimeException(builder, app, CLASS_NAME, "NullPointerException");
}
@@ -201,6 +286,7 @@
assertTrue(!found && instruction.isCheckCast(maybeType));
found = true;
}
+ assertTrue(found);
}
private void checkRuntime(JasminBuilder builder, AndroidApp app, String className)
diff --git a/src/test/java/com/android/tools/r8/jasmin/JasminTestBase.java b/src/test/java/com/android/tools/r8/jasmin/JasminTestBase.java
index f106793..cfc49fe 100644
--- a/src/test/java/com/android/tools/r8/jasmin/JasminTestBase.java
+++ b/src/test/java/com/android/tools/r8/jasmin/JasminTestBase.java
@@ -5,6 +5,7 @@
import static org.junit.Assert.fail;
+import com.android.tools.r8.CompilationMode;
import com.android.tools.r8.OutputMode;
import com.android.tools.r8.R8Command;
import com.android.tools.r8.TestBase;
@@ -106,6 +107,21 @@
return ToolHelper.runR8(builder.build(), optionsConsumer);
}
+ protected AndroidApp compileWithR8InDebugMode(
+ JasminBuilder builder,
+ List<String> proguardConfigs,
+ Consumer<InternalOptions> optionsConsumer,
+ Backend backend)
+ throws Exception {
+ R8Command command =
+ ToolHelper.prepareR8CommandBuilder(builder.build(), emptyConsumer(backend))
+ .addLibraryFiles(runtimeJar(backend))
+ .addProguardConfiguration(proguardConfigs, Origin.unknown())
+ .setMode(CompilationMode.DEBUG)
+ .build();
+ return ToolHelper.runR8(command, optionsConsumer);
+ }
+
protected AndroidApp compileWithR8(
JasminBuilder builder,
List<String> proguardConfigs,
