ART: Special form of lock aliasing

Check whether the instruction immediately before a monitor-enter
is a move-object, establishing previously untracked lock aliasing
in a low-overhead manner.

Bug: 20102779
Bug: 21169615
Bug: 21988678
Bug: 23300986
Change-Id: Ia10d6219357e7bce29f58134637b07d3f4857b2f

diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 1828b91..a506071 100644 (file)
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -1956,6 +1956,32 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
     }
     case Instruction::MONITOR_ENTER:
       work_line_->PushMonitor(this, inst->VRegA_11x(), work_insn_idx_);
+      // Check whether the previous instruction is a move-object with vAA as a source, creating
+      // untracked lock aliasing.
+      if (0 != work_insn_idx_ && !insn_flags_[work_insn_idx_].IsBranchTarget()) {
+        uint32_t prev_idx = work_insn_idx_ - 1;
+        while (0 != prev_idx && !insn_flags_[prev_idx].IsOpcode()) {
+          prev_idx--;
+        }
+        const Instruction* prev_inst = Instruction::At(code_item_->insns_ + prev_idx);
+        switch (prev_inst->Opcode()) {
+          case Instruction::MOVE_OBJECT:
+          case Instruction::MOVE_OBJECT_16:
+          case Instruction::MOVE_OBJECT_FROM16:
+            if (prev_inst->VRegB() == inst->VRegA_11x()) {
+              // Redo the copy. This won't change the register types, but update the lock status
+              // for the aliased register.
+              work_line_->CopyRegister1(this,
+                                        prev_inst->VRegA(),
+                                        prev_inst->VRegB(),
+                                        kTypeCategoryRef);
+            }
+            break;
+
+          default:  // Other instruction types ignored.
+            break;
+        }
+      }
       break;
     case Instruction::MONITOR_EXIT:
       /*

diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt
index 21a8171..6a452eb 100644 (file)
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -1,6 +1,5 @@
 PackedSwitch
 b/17790197
-b/17978759
 FloatBadArgReg
 negLong
 sameFieldNames
@@ -42,4 +41,5 @@ b/20843113
 b/23201502 (float)
 b/23201502 (double)
 b/23300986
+b/23300986 (2)
 Done!

diff --git a/test/800-smali/smali/b_17978759.smali b/test/800-smali/smali/b_17978759.smali
deleted file mode 100644 (file)
index 07bcae5..0000000
--- a/test/800-smali/smali/b_17978759.smali
+++ /dev/null
@@ -1,28 +0,0 @@
-.class public LB17978759;
-.super Ljava/lang/Object;
-
-  .method public constructor <init>()V
-    .registers 1
-    invoke-direct {p0}, Ljava/lang/Object;-><init>()V
-    return-void
-  .end method
-
-  .method public test()V
-    .registers 2
-
-    move-object   v0, p0
-    # v0 and p0 alias
-    monitor-enter p0
-    # monitor-enter on p0
-    monitor-exit  v0
-    # monitor-exit on v0, however, verifier doesn't track this and so this is
-    # a warning. Verifier will still think p0 is locked.
-
-    move-object   v0, p0
-    # v0 will now appear locked.
-    monitor-enter v0
-    # Attempt to lock v0 twice is a verifier failure.
-    monitor-exit  v0
-
-    return-void
-  .end method

diff --git a/test/800-smali/smali/b_23300986.smali b/test/800-smali/smali/b_23300986.smali
index 5ed8e5e..f008b92 100644 (file)
--- a/test/800-smali/smali/b_23300986.smali
+++ b/test/800-smali/smali/b_23300986.smali
@@ -11,3 +11,13 @@
    monitor-exit v1
    return-void
 .end method
+
+.method public static runAliasBeforeEnter(Ljava/lang/Object;)V
+   .registers 3
+   move-object v1, v2      # Copy parameter into v1, establishing an alias.
+   monitor-enter v2        # Lock on parameter
+   monitor-exit v1         # Unlock on alias
+   monitor-enter v2        # Do it again.
+   monitor-exit v1
+   return-void
+.end method

diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java
index a89b849..183958a 100644 (file)
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -53,8 +53,6 @@ public class Main {
                 new Object[]{123}, null, 123));
 
         testCases.add(new TestCase("b/17790197", "B17790197", "getInt", null, null, 100));
-        testCases.add(new TestCase("b/17978759", "B17978759", "test", null, new VerifyError(),
-                null));
         testCases.add(new TestCase("FloatBadArgReg", "FloatBadArgReg", "getInt",
                 new Object[]{100}, null, 100));
         testCases.add(new TestCase("negLong", "negLong", "negLong", null, null, 122142L));
@@ -129,6 +127,8 @@ public class Main {
                 new NullPointerException(), null));
         testCases.add(new TestCase("b/23300986", "B23300986", "runAliasAfterEnter",
                 new Object[] { new Object() }, null, null));
+        testCases.add(new TestCase("b/23300986 (2)", "B23300986", "runAliasBeforeEnter",
+                new Object[] { new Object() }, null, null));
     }
 
     public void runTests() {