Version 1.3.47.

Merge: Workaround Art instance-of verifier bug.
CL: https://r8-review.googlesource.com/c/r8/+/32222

R=zerny@google.com

Bug: 120985556
Change-Id: Ia0fdb2c71c67a1c04b1734546badb1d400d60015
diff --git a/src/main/java/com/android/tools/r8/Version.java b/src/main/java/com/android/tools/r8/Version.java
index 3d5acb9..62b346a 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.46";
+  public static final String LABEL = "1.3.47";
 
   private Version() {
   }
diff --git a/src/main/java/com/android/tools/r8/ir/code/InstanceOf.java b/src/main/java/com/android/tools/r8/ir/code/InstanceOf.java
index c5dc5e4..acce406 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InstanceOf.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InstanceOf.java
@@ -41,7 +41,7 @@
   public void buildDex(DexBuilder builder) {
     int dest = builder.allocatedRegister(dest(), getNumber());
     int value = builder.allocatedRegister(value(), getNumber());
-    builder.add(this, new com.android.tools.r8.code.InstanceOf(dest, value, type));
+    builder.addInstanceOf(this, new com.android.tools.r8.code.InstanceOf(dest, value, type));
   }
 
   @Override
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java b/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
index 4e11690..8927476 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/DexBuilder.java
@@ -21,6 +21,7 @@
 import com.android.tools.r8.code.IfLtz;
 import com.android.tools.r8.code.IfNe;
 import com.android.tools.r8.code.IfNez;
+import com.android.tools.r8.code.InstanceOf;
 import com.android.tools.r8.code.Instruction;
 import com.android.tools.r8.code.Move16;
 import com.android.tools.r8.code.MoveFrom16;
@@ -103,6 +104,9 @@
   // getInfo/setInfo methods to access the mapping.
   private Info[] instructionToInfo;
 
+  // Keeps track of the previous non-fallthrough info added to the dex builder.
+  private Info previousNonFallthroughInfo;
+
   // The number of ingoing and outgoing argument registers for the code.
   private int inRegisterCount = 0;
   private int outRegisterCount = 0;
@@ -418,6 +422,38 @@
     }
   }
 
+  private boolean needsNopBetweenMoveAndInstanceOf(InstanceOf instanceOf) {
+    if (!options.canHaveArtInstanceOfVerifierBug()) {
+      return false;
+    }
+    if (previousNonFallthroughInfo instanceof MoveInfo) {
+      MoveInfo moveInfo = (MoveInfo) previousNonFallthroughInfo;
+      int moveSrcRegister = moveInfo.srcRegister(this);
+      int moveDestRegister = moveInfo.destRegister(this);
+      // If the previous move materializes as a move and we have the pattern:
+      //
+      //  move vA, vB
+      //  instance-of vB, vA, Type
+      //
+      // we insert a nop between the move and the instance-of instruction to make sure
+      // that we do not trigger a verifier bug in Art. See b/120985556.
+      if (moveSrcRegister != moveDestRegister
+          && moveSrcRegister == instanceOf.A
+          && moveDestRegister == instanceOf.B) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  public void addInstanceOf(com.android.tools.r8.ir.code.InstanceOf ir, InstanceOf instanceOf) {
+    if (needsNopBetweenMoveAndInstanceOf(instanceOf)) {
+      add(ir, new Nop(), instanceOf);
+    } else {
+      add(ir, instanceOf);
+    }
+  }
+
   public void addIf(If branch) {
     assert nextBlock == branch.fallthroughBlock();
     add(branch, new IfInfo(branch));
@@ -512,6 +548,9 @@
   }
 
   private void setInfo(com.android.tools.r8.ir.code.Instruction instruction, Info info) {
+    if (!(info instanceof FallThroughInfo)) {
+      previousNonFallthroughInfo = info;
+    }
     instructionToInfo[instructionNumberToIndex(instruction.getNumber())] = info;
   }
 
@@ -1158,11 +1197,18 @@
       return (Move) getIR();
     }
 
+    public int srcRegister(DexBuilder builder) {
+      return builder.argumentOrAllocateRegister(getMove().src(), getMove().getNumber());
+    }
+
+    public int destRegister(DexBuilder builder) {
+      return builder.allocatedRegister(getMove().dest(), getMove().getNumber());
+    }
+
     @Override
     public int computeSize(DexBuilder builder) {
-      Move move = getMove();
-      int srcRegister = builder.argumentOrAllocateRegister(move.src(), move.getNumber());
-      int destRegister = builder.allocatedRegister(move.dest(), move.getNumber());
+      int srcRegister = srcRegister(builder);
+      int destRegister = destRegister(builder);
       if (srcRegister == destRegister) {
         size = 1;
       } else if (srcRegister <= Constants.U4BIT_MAX && destRegister <= Constants.U4BIT_MAX) {
@@ -1179,8 +1225,8 @@
     public void addInstructions(DexBuilder builder, List<Instruction> instructions) {
       Move move = getMove();
       MoveType moveType = MoveType.fromValueType(move.outType());
-      int src = builder.argumentOrAllocateRegister(move.src(), move.getNumber());
-      int dest = builder.allocatedRegister(move.dest(), move.getNumber());
+      int src = srcRegister(builder);
+      int dest = destRegister(builder);
       Instruction instruction = null;
       switch (size) {
         case 1:
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 2612d0b..1299091 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -752,4 +752,32 @@
   public boolean canHaveTracingPastInstructionsStreamBug() {
     return minApiLevel < AndroidApiLevel.L.getLevel();
   }
+
+  // The art verifier incorrectly propagates type information for the following pattern:
+  //
+  //  move vA, vB
+  //  instance-of vB, vA, Type
+  //  if-eqz/nez vB
+  //
+  // In that case it will assume that vB has object type after the if. Therefore, if the
+  // result of the instance-of operation is reused in a boolean context the verifier will
+  // fail with a type conflict.
+  //
+  // In order to make sure that cannot happen, we insert a nop between the move and
+  // the instance-of instruction so that this pattern in the art verifier does not
+  // match.
+  //
+  //  move vA, vB
+  //  nop
+  //  instance-of vB, vA, Type
+  //  if-eqz/nez vB
+  //
+  // This happens rarely, but it can happen in debug mode where the move
+  // put a value into a new register which has associated locals information.
+  //
+  // See b/120985556.
+  public boolean canHaveArtInstanceOfVerifierBug() {
+    // TODO(ager): Update this with an actual bound when the issue has been fixed.
+    return true;
+  }
 }