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());
+  }
+}