Version 1.3.23.
Merge: Extend the liveness of the receiver to the entire method.
CL: https://r8-review.googlesource.com/27565
Merge: Add regression test for crash during debugging in certain Art VMs.
CL: https://r8-review.googlesource.com/27863
R=sgjesse@google.com, zerny@google.com
Bug: 116837585
Change-Id: Ie18020fbac6ddfb0cc91f68e58bc9c65944c70f2
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index a412fb5..d265881 100644
--- a/src/main/java/com/android/tools/r8/Version.java
+++ b/src/main/java/com/android/tools/r8/Version.java
@@ -11,7 +11,7 @@
// This field is accessed from release scripts using simple pattern matching.
// Therefore, changing this field could break our release scripts.
- public static final String LABEL = "1.3.22";
+ public static final String LABEL = "1.3.23";
private Version() {
}
diff --git a/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java b/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
index 75bf9c0..b4995c9 100644
--- a/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
+++ b/src/main/java/com/android/tools/r8/ir/regalloc/LinearScanRegisterAllocator.java
@@ -2465,7 +2465,8 @@
// This assumption is used during verification. Allowing the receiver register to be
// overwritten can therefore lead to verification errors. If we could be targeting one of these
// VMs we block the receiver register throughout the method.
- if (options.canHaveThisTypeVerifierBug() && !code.method.accessFlags.isStatic()) {
+ if ((options.canHaveThisTypeVerifierBug() || options.canHaveThisJitCodeDebuggingBug())
+ && !code.method.accessFlags.isStatic()) {
for (Instruction instruction : code.blocks.get(0).getInstructions()) {
if (instruction.isArgument() && instruction.outValue().isThis()) {
Value thisValue = instruction.outValue();
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 c7d6561..e0e4227 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -577,6 +577,17 @@
return minApiLevel < AndroidApiLevel.M.getLevel();
}
+ // Art crashes if we do dead reference elimination of the receiver in release mode and Art
+ // is asked for the |this| object over a JDWP connection at a point where the receiver
+ // register has been clobbered.
+ //
+ // See b/116683601 and b/116837585.
+ public boolean canHaveThisJitCodeDebuggingBug() {
+ // TODO(b/116841249): Make this an actual min-sdk guard once we know that Art no longer crashes
+ // on these accesses.
+ return true;
+ }
+
// The dalvik jit had a bug where the long operations add, sub, or, xor and and would write
// the first part of the result long before reading the second part of the input longs.
public boolean canHaveOverlappingLongRegisterBug() {
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 17e49a5..99222d0 100644
--- a/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
+++ b/src/test/java/com/android/tools/r8/debug/DebugTestBase.java
@@ -313,6 +313,12 @@
return new JUnit3Wrapper.Command.BreakpointCommand(className, methodName, methodSignature, line);
}
+ protected final JUnit3Wrapper.Command breakOnException(String className, String methodName,
+ boolean caught, boolean uncaught) {
+ return new JUnit3Wrapper.Command.BreakOnExceptionCommand(
+ className, methodName, caught, uncaught);
+ }
+
protected final JUnit3Wrapper.Command stepOver() {
return stepOver(DEFAULT_FILTER);
}
@@ -1607,6 +1613,39 @@
}
}
+ // Break on exceptions thrown in className.methodName.
+ class BreakOnExceptionCommand implements Command {
+ private static final int ALL_EXCEPTIONS = 0;
+ private final String className;
+ private final String methodName;
+ private final boolean caught;
+ private final boolean uncaught;
+
+ public BreakOnExceptionCommand(
+ String className, String methodName, boolean caught, boolean uncaught) {
+ this.className = className;
+ this.methodName = methodName;
+ this.caught = caught;
+ this.uncaught = uncaught;
+ }
+
+ @Override
+ public void perform(JUnit3Wrapper testBase) {
+ ReplyPacket replyPacket =
+ testBase.getMirror().setException(ALL_EXCEPTIONS, caught, uncaught);
+ assert replyPacket.getErrorCode() == Error.NONE;
+ int breakpointId = replyPacket.getNextValueAsInt();
+ testBase.events.put(
+ Integer.valueOf(breakpointId),
+ new BreakOnExceptionHandler(className, methodName));
+ }
+
+ @Override
+ public String toString() {
+ return "breakOnException";
+ }
+ }
+
class BreakpointCommand implements Command {
private final String className;
@@ -1813,6 +1852,29 @@
}
}
+ private static class BreakOnExceptionHandler extends DefaultEventHandler {
+ private final String className;
+ private final String methodName;
+
+ BreakOnExceptionHandler(String className, String methodName) {
+ this.className = className;
+ this.methodName = methodName;
+ }
+
+ @Override
+ public void handle(JUnit3Wrapper testBase) {
+ boolean inMethod =
+ testBase.getDebuggeeState().getTopFrame().getMethodName().equals(methodName);
+ boolean inClass =
+ testBase.getDebuggeeState().getTopFrame().getClassName().equals(className);
+ if (!(inClass && inMethod)) {
+ // Not the right place, continue until the next exception.
+ testBase.enqueueCommandFirst(new JUnit3Wrapper.Command.RunCommand());
+ }
+ testBase.setState(State.ProcessCommand);
+ }
+ }
+
private static class StepEventHandler extends DefaultEventHandler {
private final JUnit3Wrapper.Command.StepCommand stepCommand;
diff --git a/src/test/java/com/android/tools/r8/debug/DoNotCrashOnAccessToThis.java b/src/test/java/com/android/tools/r8/debug/DoNotCrashOnAccessToThis.java
new file mode 100644
index 0000000..0b26f37
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/debug/DoNotCrashOnAccessToThis.java
@@ -0,0 +1,36 @@
+// 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.debug;
+
+public class DoNotCrashOnAccessToThis {
+ private String[] array = new String[] { "asdf" };
+
+ public int length() {
+ return array.length;
+ }
+
+ public void mightClobberThis(int i) {
+ // Make a local copy of the receiver so that we can let the receiver be clobbered in
+ // release mode.
+ String[] localArray = array;
+ // Keep receiver alive so that localArray is not what is clobbering the receiver register.
+ // Attempt to get an integer into the receiver register.
+ int index = length();
+ String s = localArray[index + i];
+ }
+
+ public static void main(String[] args) {
+ DoNotCrashOnAccessToThis instance = new DoNotCrashOnAccessToThis();
+ for (int i = 0; i < 100000; i++) {
+ instance.mightClobberThis(-1);
+ }
+ try {
+ instance.mightClobberThis(0);
+ } catch (ArrayIndexOutOfBoundsException e) {
+ System.out.println("Caught ArrayIndexOutOfBoundsException");
+ }
+
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/debug/DoNotCrashOnAccessToThisRunner.java b/src/test/java/com/android/tools/r8/debug/DoNotCrashOnAccessToThisRunner.java
new file mode 100644
index 0000000..3e562cf
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/debug/DoNotCrashOnAccessToThisRunner.java
@@ -0,0 +1,60 @@
+// 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.debug;
+
+import com.android.tools.r8.ToolHelper;
+import com.android.tools.r8.ToolHelper.DexVm;
+import com.android.tools.r8.utils.AndroidApiLevel;
+import com.google.common.collect.ImmutableList;
+import java.util.Collection;
+import org.junit.Assume;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class DoNotCrashOnAccessToThisRunner extends DebugTestBase {
+
+ private static final Class CLASS = DoNotCrashOnAccessToThis.class;
+ private static final String FILE = CLASS.getSimpleName() + ".java";
+ private static final String NAME = CLASS.getCanonicalName();
+
+ private final DebugTestConfig config;
+
+ @Parameterized.Parameters(name = "{0}")
+ public static Collection<Object[]> setup() {
+ DelayedDebugTestConfig cf =
+ temp -> new CfDebugTestConfig().addPaths(ToolHelper.getClassPathForTests());
+ DelayedDebugTestConfig d8 =
+ temp -> new D8DebugTestConfig().compileAndAdd(
+ temp,
+ ImmutableList.of(ToolHelper.getClassFileForTestClass(CLASS)),
+ options -> {
+ // Release mode so receiver can be clobbered.
+ options.debug = false;
+ // Api level M so that the workarounds for Lollipop verifier doesn't
+ // block the receiver register. We want to check b/116683601 which
+ // happens on at least 7.0.0.
+ options.minApiLevel = AndroidApiLevel.M.getLevel();
+ });
+ return ImmutableList.of(new Object[]{"CF", cf}, new Object[]{"D8", d8});
+ }
+
+ public DoNotCrashOnAccessToThisRunner(String name, DelayedDebugTestConfig config) {
+ this.config = config.getConfig(temp);
+ }
+
+ @Test
+ public void doNotCrash() throws Throwable {
+ Assume.assumeFalse(ToolHelper.getDexVm().isOlderThanOrEqual(DexVm.ART_6_0_1_HOST));
+ runDebugTest(
+ config,
+ NAME,
+ breakOnException(NAME, "mightClobberThis", true, false),
+ run(),
+ checkLine(FILE, 21),
+ checkLocal("this"),
+ run());
+ }
+}