ART: Add simple null alias tracking for lock counting

Null is the only literal for objects, and one may lock and unlock
on different registers containing null, which is still balanced.

Bug: 23502994
Change-Id: Ibfbf1b8c2aa7d1409e3e426988d2d15efe1f2d0d

3 files changed, 53 insertions, 2 deletions

diff --git a/runtime/verifier/register_line.cc b/runtime/verifier/register_line.cc
index 02c93cf864..f48b1e1212 100644
--- a/runtime/verifier/register_line.cc
+++ b/runtime/verifier/register_line.cc
@@ -338,6 +338,8 @@ void RegisterLine::CheckLiteralOp(MethodVerifier* verifier, const Instruction* i
   }
 }
 
+static constexpr uint32_t kVirtualNullRegister = std::numeric_limits<uint32_t>::max();
+
 void RegisterLine::PushMonitor(MethodVerifier* verifier, uint32_t reg_idx, int32_t insn_idx) {
   const RegType& reg_type = GetRegisterType(verifier, reg_idx);
   if (!reg_type.IsReferenceTypes()) {
@@ -352,6 +354,12 @@ void RegisterLine::PushMonitor(MethodVerifier* verifier, uint32_t reg_idx, int32
     }
   } else {
     if (SetRegToLockDepth(reg_idx, monitors_.size())) {
+      // Null literals can establish aliases that we can't easily track. As such, handle the zero
+      // case as the 2^32-1 register (which isn't available in dex bytecode).
+      if (reg_type.IsZero()) {
+        SetRegToLockDepth(kVirtualNullRegister, monitors_.size());
+      }
+
       monitors_.push_back(insn_idx);
     } else {
       verifier->Fail(VERIFY_ERROR_LOCKING);
@@ -377,7 +385,19 @@ void RegisterLine::PopMonitor(MethodVerifier* verifier, uint32_t reg_idx) {
     }
   } else {
     monitors_.pop_back();
-    if (!IsSetLockDepth(reg_idx, monitors_.size())) {
+
+    bool success = IsSetLockDepth(reg_idx, monitors_.size());
+
+    if (!success && reg_type.IsZero()) {
+      // Null literals can establish aliases that we can't easily track. As such, handle the zero
+      // case as the 2^32-1 register (which isn't available in dex bytecode).
+      success = IsSetLockDepth(kVirtualNullRegister, monitors_.size());
+      if (success) {
+        reg_idx = kVirtualNullRegister;
+      }
+    }
+
+    if (!success) {
       verifier->Fail(VERIFY_ERROR_LOCKING);
       if (kDumpLockFailures) {
         LOG(WARNING) << "monitor-exit not unlocking the top of the monitor stack while verifying "
@@ -385,7 +405,8 @@ void RegisterLine::PopMonitor(MethodVerifier* verifier, uint32_t reg_idx) {
                                      *verifier->GetMethodReference().dex_file);
       }
     } else {
-      // Record the register was unlocked
+      // Record the register was unlocked. This clears all aliases, thus it will also clear the
+      // null lock, if necessary.
       ClearRegToLockDepth(reg_idx, monitors_.size());
     }
   }
diff --git a/test/088-monitor-verification/smali/NullLocks.smali b/test/088-monitor-verification/smali/NullLocks.smali
new file mode 100644
index 0000000000..8262f19e22
--- /dev/null
+++ b/test/088-monitor-verification/smali/NullLocks.smali
@@ -0,0 +1,28 @@
+.class public LNullLocks;
+
+.super Ljava/lang/Object;
+
+.method public static run(Z)V
+   .registers 3
+
+   invoke-static {}, LMain;->assertIsManaged()V
+
+   if-eqz v2, :Lfalse
+
+   const v0, 0           # Null.
+   monitor-enter v0
+   const v1, 0           # Another null. This should be detected as an alias, such that the exit
+                         # will not fail verification.
+   monitor-exit v1
+
+   monitor-enter v0
+   monitor-exit v1
+
+   monitor-enter v1
+   monitor-exit v0
+
+:Lfalse
+
+   return-void
+
+.end method
diff --git a/test/088-monitor-verification/src/Main.java b/test/088-monitor-verification/src/Main.java
index d742b1410a..212c894bd5 100644
--- a/test/088-monitor-verification/src/Main.java
+++ b/test/088-monitor-verification/src/Main.java
@@ -221,6 +221,8 @@ public class Main {
                 IllegalMonitorStateException.class);
         runTest("UnbalancedJoin", new Object[] { new Object(), new Object() }, null);
         runTest("UnbalancedStraight", new Object[] { new Object(), new Object() }, null);
+        runTest("NullLocks", new Object[] { false }, null);
+        runTest("NullLocks", new Object[] { true }, NullPointerException.class);
     }
 
     private static void runTest(String className, Object[] parameters, Class<?> excType) {