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
diff --git a/runtime/verifier/register_line.cc b/runtime/verifier/register_line.cc
index 02c93cf..f48b1e1 100644
--- a/runtime/verifier/register_line.cc
+++ b/runtime/verifier/register_line.cc
@@ -338,6 +338,8 @@
}
}
+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 @@
}
} 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 @@
}
} 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 @@
*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 0000000..8262f19
--- /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 d742b14..212c894 100644
--- a/test/088-monitor-verification/src/Main.java
+++ b/test/088-monitor-verification/src/Main.java
@@ -221,6 +221,8 @@
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) {
