ART: Balanced locking
Change the verifier to check for balanced locking. When balanced
locking can't be guaranteed, use a new failure kind to punt to
the interpreter.
Add smali tests, with JNI code to check the balanced-locking result.
Bug: 23502994
Change-Id: Icd7db0be20ef2f69f0ac784de43dcba990035cd8
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index c006e62..89668f2 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -601,7 +601,7 @@
// Do not have failures that should punt to the interpreter.
!verified_method->HasRuntimeThrow() &&
(verified_method->GetEncounteredVerificationFailures() &
- verifier::VERIFY_ERROR_FORCE_INTERPRETER) == 0 &&
+ (verifier::VERIFY_ERROR_FORCE_INTERPRETER | verifier::VERIFY_ERROR_LOCKING)) == 0 &&
// Is eligable for compilation by methods-to-compile filter.
driver->IsMethodToCompile(method_ref);
if (compile) {
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 223268d..4f921bd 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -589,7 +589,7 @@
std::ostream& MethodVerifier::Fail(VerifyError error) {
// Mark the error type as encountered.
- encountered_failure_types_ |= (1U << static_cast<uint32_t>(error));
+ encountered_failure_types_ |= static_cast<uint32_t>(error);
switch (error) {
case VERIFY_ERROR_NO_CLASS:
@@ -601,6 +601,7 @@
case VERIFY_ERROR_INSTANTIATION:
case VERIFY_ERROR_CLASS_CHANGE:
case VERIFY_ERROR_FORCE_INTERPRETER:
+ case VERIFY_ERROR_LOCKING:
if (Runtime::Current()->IsAotCompiler() || !can_load_classes_) {
// If we're optimistically running verification at compile time, turn NO_xxx, ACCESS_xxx,
// class change and instantiation errors into soft verification errors so that we re-verify
@@ -631,12 +632,14 @@
}
}
break;
+
// Indication that verification should be retried at runtime.
case VERIFY_ERROR_BAD_CLASS_SOFT:
if (!allow_soft_failures_) {
have_pending_hard_failure_ = true;
}
break;
+
// Hard verification failures at compile time will still fail at runtime, so the class is
// marked as rejected to prevent it from being compiled.
case VERIFY_ERROR_BAD_CLASS_HARD: {
@@ -1657,6 +1660,33 @@
return DexFile::kDexNoIndex;
}
+// Setup a register line for the given return instruction.
+static void AdjustReturnLine(MethodVerifier* verifier,
+ const Instruction* ret_inst,
+ RegisterLine* line) {
+ Instruction::Code opcode = ret_inst->Opcode();
+
+ switch (opcode) {
+ case Instruction::RETURN_VOID:
+ case Instruction::RETURN_VOID_NO_BARRIER:
+ SafelyMarkAllRegistersAsConflicts(verifier, line);
+ break;
+
+ case Instruction::RETURN:
+ case Instruction::RETURN_OBJECT:
+ line->MarkAllRegistersAsConflictsExcept(verifier, ret_inst->VRegA_11x());
+ break;
+
+ case Instruction::RETURN_WIDE:
+ line->MarkAllRegistersAsConflictsExceptWide(verifier, ret_inst->VRegA_11x());
+ break;
+
+ default:
+ LOG(FATAL) << "Unknown return opcode " << opcode;
+ UNREACHABLE();
+ }
+}
+
bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
// If we're doing FindLocksAtDexPc, check whether we're at the dex pc we care about.
// We want the state _before_ the instruction, for the case where the dex pc we're
@@ -3078,10 +3108,9 @@
} else if (have_pending_runtime_throw_failure_) {
/* checking interpreter will throw, mark following code as unreachable */
opcode_flags = Instruction::kThrow;
- have_any_pending_runtime_throw_failure_ = true;
- // Reset the pending_runtime_throw flag. The flag is a global to decouple Fail and is per
- // instruction.
- have_pending_runtime_throw_failure_ = false;
+ // Note: the flag must be reset as it is only global to decouple Fail and is semantically per
+ // instruction. However, RETURN checking may throw LOCKING errors, so we clear at the
+ // very end.
}
/*
* If we didn't just set the result register, clear it out. This ensures that you can only use
@@ -3250,16 +3279,7 @@
if (insn_flags_[next_insn_idx].IsReturn()) {
// For returns we only care about the operand to the return, all other registers are dead.
const Instruction* ret_inst = Instruction::At(code_item_->insns_ + next_insn_idx);
- Instruction::Code opcode = ret_inst->Opcode();
- if (opcode == Instruction::RETURN_VOID || opcode == Instruction::RETURN_VOID_NO_BARRIER) {
- SafelyMarkAllRegistersAsConflicts(this, work_line_.get());
- } else {
- if (opcode == Instruction::RETURN_WIDE) {
- work_line_->MarkAllRegistersAsConflictsExceptWide(this, ret_inst->VRegA_11x());
- } else {
- work_line_->MarkAllRegistersAsConflictsExcept(this, ret_inst->VRegA_11x());
- }
- }
+ AdjustReturnLine(this, ret_inst, work_line_.get());
}
RegisterLine* next_line = reg_table_.GetLine(next_insn_idx);
if (next_line != nullptr) {
@@ -3280,9 +3300,7 @@
/* If we're returning from the method, make sure monitor stack is empty. */
if ((opcode_flags & Instruction::kReturn) != 0) {
- if (!work_line_->VerifyMonitorStackEmpty(this)) {
- return false;
- }
+ work_line_->VerifyMonitorStackEmpty(this);
}
/*
@@ -3302,6 +3320,12 @@
DCHECK_LT(*start_guess, code_item_->insns_size_in_code_units_);
DCHECK(insn_flags_[*start_guess].IsOpcode());
+ if (have_pending_runtime_throw_failure_) {
+ have_any_pending_runtime_throw_failure_ = true;
+ // Reset the pending_runtime_throw flag now.
+ have_pending_runtime_throw_failure_ = false;
+ }
+
return true;
} // NOLINT(readability/fn_size)
@@ -4425,31 +4449,15 @@
* there's nothing to "merge". Copy the registers over and mark it as changed. (This is the
* only way a register can transition out of "unknown", so this is not just an optimization.)
*/
- if (!insn_flags_[next_insn].IsReturn()) {
- target_line->CopyFromLine(merge_line);
- } else {
+ target_line->CopyFromLine(merge_line);
+ if (insn_flags_[next_insn].IsReturn()) {
// Verify that the monitor stack is empty on return.
- if (!merge_line->VerifyMonitorStackEmpty(this)) {
- return false;
- }
+ merge_line->VerifyMonitorStackEmpty(this);
+
// For returns we only care about the operand to the return, all other registers are dead.
// Initialize them as conflicts so they don't add to GC and deoptimization information.
const Instruction* ret_inst = Instruction::At(code_item_->insns_ + next_insn);
- Instruction::Code opcode = ret_inst->Opcode();
- if (opcode == Instruction::RETURN_VOID || opcode == Instruction::RETURN_VOID_NO_BARRIER) {
- // Explicitly copy the this-initialized flag from the merge-line, as we didn't copy its
- // state. Must be done before SafelyMarkAllRegistersAsConflicts as that will do the
- // super-constructor-call checking.
- target_line->CopyThisInitialized(*merge_line);
- SafelyMarkAllRegistersAsConflicts(this, target_line);
- } else {
- target_line->CopyFromLine(merge_line);
- if (opcode == Instruction::RETURN_WIDE) {
- target_line->MarkAllRegistersAsConflictsExceptWide(this, ret_inst->VRegA_11x());
- } else {
- target_line->MarkAllRegistersAsConflictsExcept(this, ret_inst->VRegA_11x());
- }
- }
+ AdjustReturnLine(this, ret_inst, target_line);
}
} else {
std::unique_ptr<RegisterLine> copy(gDebugVerify ?
diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h
index d0841f0..b57abf5 100644
--- a/runtime/verifier/method_verifier.h
+++ b/runtime/verifier/method_verifier.h
@@ -67,17 +67,17 @@
* to be rewritten to fail at runtime.
*/
enum VerifyError {
- VERIFY_ERROR_BAD_CLASS_HARD = 0, // VerifyError; hard error that skips compilation.
- VERIFY_ERROR_BAD_CLASS_SOFT = 1, // VerifyError; soft error that verifies again at runtime.
+ VERIFY_ERROR_BAD_CLASS_HARD = 1, // VerifyError; hard error that skips compilation.
+ VERIFY_ERROR_BAD_CLASS_SOFT = 2, // VerifyError; soft error that verifies again at runtime.
- VERIFY_ERROR_NO_CLASS = 2, // NoClassDefFoundError.
- VERIFY_ERROR_NO_FIELD = 3, // NoSuchFieldError.
- VERIFY_ERROR_NO_METHOD = 4, // NoSuchMethodError.
- VERIFY_ERROR_ACCESS_CLASS = 5, // IllegalAccessError.
- VERIFY_ERROR_ACCESS_FIELD = 6, // IllegalAccessError.
- VERIFY_ERROR_ACCESS_METHOD = 7, // IllegalAccessError.
- VERIFY_ERROR_CLASS_CHANGE = 8, // IncompatibleClassChangeError.
- VERIFY_ERROR_INSTANTIATION = 9, // InstantiationError.
+ VERIFY_ERROR_NO_CLASS = 4, // NoClassDefFoundError.
+ VERIFY_ERROR_NO_FIELD = 8, // NoSuchFieldError.
+ VERIFY_ERROR_NO_METHOD = 16, // NoSuchMethodError.
+ VERIFY_ERROR_ACCESS_CLASS = 32, // IllegalAccessError.
+ VERIFY_ERROR_ACCESS_FIELD = 64, // IllegalAccessError.
+ VERIFY_ERROR_ACCESS_METHOD = 128, // IllegalAccessError.
+ VERIFY_ERROR_CLASS_CHANGE = 256, // IncompatibleClassChangeError.
+ VERIFY_ERROR_INSTANTIATION = 512, // InstantiationError.
// For opcodes that don't have complete verifier support (such as lambda opcodes),
// we need a way to continue execution at runtime without attempting to re-verify
// (since we know it will fail no matter what). Instead, run as the interpreter
@@ -85,9 +85,11 @@
// on the fly.
//
// TODO: Once all new opcodes have implemented full verifier support, this can be removed.
- VERIFY_ERROR_FORCE_INTERPRETER = 10, // Skip the verification phase at runtime;
- // force the interpreter to do access checks.
- // (sets a soft fail at compile time).
+ VERIFY_ERROR_FORCE_INTERPRETER = 1024, // Skip the verification phase at runtime;
+ // force the interpreter to do access checks.
+ // (sets a soft fail at compile time).
+ VERIFY_ERROR_LOCKING = 2048, // Could not guarantee balanced locking. This should be
+ // punted to the interpreter with access checks.
};
std::ostream& operator<<(std::ostream& os, const VerifyError& rhs);
diff --git a/runtime/verifier/register_line-inl.h b/runtime/verifier/register_line-inl.h
index bee5834..1df2428 100644
--- a/runtime/verifier/register_line-inl.h
+++ b/runtime/verifier/register_line-inl.h
@@ -25,6 +25,10 @@
namespace art {
namespace verifier {
+// Should we dump a warning on failures to verify balanced locking? That would be an indication to
+// developers that their code will be slow.
+static constexpr bool kDumpLockFailures = true;
+
inline const RegType& RegisterLine::GetRegisterType(MethodVerifier* verifier, uint32_t vsrc) const {
// The register index was validated during the static pass, so we don't need to check it here.
DCHECK_LT(vsrc, num_regs_);
@@ -167,12 +171,14 @@
return true;
}
-inline bool RegisterLine::VerifyMonitorStackEmpty(MethodVerifier* verifier) const {
+inline void RegisterLine::VerifyMonitorStackEmpty(MethodVerifier* verifier) const {
if (MonitorStackDepth() != 0) {
- verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "expected empty monitor stack";
- return false;
- } else {
- return true;
+ verifier->Fail(VERIFY_ERROR_LOCKING);
+ if (kDumpLockFailures) {
+ LOG(WARNING) << "expected empty monitor stack in "
+ << PrettyMethod(verifier->GetMethodReference().dex_method_index,
+ *verifier->GetMethodReference().dex_file);
+ }
}
}
diff --git a/runtime/verifier/register_line.cc b/runtime/verifier/register_line.cc
index bb6df76..33c90e3 100644
--- a/runtime/verifier/register_line.cc
+++ b/runtime/verifier/register_line.cc
@@ -344,14 +344,22 @@
verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "monitor-enter on non-object ("
<< reg_type << ")";
} else if (monitors_.size() >= 32) {
- verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "monitor-enter stack overflow: "
- << monitors_.size();
+ verifier->Fail(VERIFY_ERROR_LOCKING);
+ if (kDumpLockFailures) {
+ LOG(WARNING) << "monitor-enter stack overflow while verifying "
+ << PrettyMethod(verifier->GetMethodReference().dex_method_index,
+ *verifier->GetMethodReference().dex_file);
+ }
} else {
if (SetRegToLockDepth(reg_idx, monitors_.size())) {
monitors_.push_back(insn_idx);
} else {
- verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "unexpected monitor-enter on register v" <<
- reg_idx;
+ verifier->Fail(VERIFY_ERROR_LOCKING);
+ if (kDumpLockFailures) {
+ LOG(WARNING) << "unexpected monitor-enter on register v" << reg_idx << " in "
+ << PrettyMethod(verifier->GetMethodReference().dex_method_index,
+ *verifier->GetMethodReference().dex_file);
+ }
}
}
}
@@ -361,16 +369,21 @@
if (!reg_type.IsReferenceTypes()) {
verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "monitor-exit on non-object (" << reg_type << ")";
} else if (monitors_.empty()) {
- verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "monitor-exit stack underflow";
+ verifier->Fail(VERIFY_ERROR_LOCKING);
+ if (kDumpLockFailures) {
+ LOG(WARNING) << "monitor-exit stack underflow while verifying "
+ << PrettyMethod(verifier->GetMethodReference().dex_method_index,
+ *verifier->GetMethodReference().dex_file);
+ }
} else {
monitors_.pop_back();
if (!IsSetLockDepth(reg_idx, monitors_.size())) {
- // Bug 3215458: Locks and unlocks are on objects, if that object is a literal then before
- // format "036" the constant collector may create unlocks on the same object but referenced
- // via different registers.
- ((verifier->DexFileVersion() >= 36) ? verifier->Fail(VERIFY_ERROR_BAD_CLASS_SOFT)
- : verifier->LogVerifyInfo())
- << "monitor-exit not unlocking the top of the monitor stack";
+ verifier->Fail(VERIFY_ERROR_LOCKING);
+ if (kDumpLockFailures) {
+ LOG(WARNING) << "monitor-exit not unlocking the top of the monitor stack while verifying "
+ << PrettyMethod(verifier->GetMethodReference().dex_method_index,
+ *verifier->GetMethodReference().dex_file);
+ }
} else {
// Record the register was unlocked
ClearRegToLockDepth(reg_idx, monitors_.size());
@@ -392,8 +405,13 @@
}
if (monitors_.size() > 0 || incoming_line->monitors_.size() > 0) {
if (monitors_.size() != incoming_line->monitors_.size()) {
- LOG(WARNING) << "mismatched stack depths (depth=" << MonitorStackDepth()
- << ", incoming depth=" << incoming_line->MonitorStackDepth() << ")";
+ verifier->Fail(VERIFY_ERROR_LOCKING);
+ if (kDumpLockFailures) {
+ LOG(WARNING) << "mismatched stack depths (depth=" << MonitorStackDepth()
+ << ", incoming depth=" << incoming_line->MonitorStackDepth() << ") in "
+ << PrettyMethod(verifier->GetMethodReference().dex_method_index,
+ *verifier->GetMethodReference().dex_file);
+ }
} else if (reg_to_lock_depths_ != incoming_line->reg_to_lock_depths_) {
for (uint32_t idx = 0; idx < num_regs_; idx++) {
size_t depths = reg_to_lock_depths_.count(idx);
@@ -402,14 +420,35 @@
if (depths == 0 || incoming_depths == 0) {
reg_to_lock_depths_.erase(idx);
} else {
- LOG(WARNING) << "mismatched stack depths for register v" << idx
- << ": " << depths << " != " << incoming_depths;
+ verifier->Fail(VERIFY_ERROR_LOCKING);
+ if (kDumpLockFailures) {
+ LOG(WARNING) << "mismatched stack depths for register v" << idx
+ << ": " << depths << " != " << incoming_depths << " in "
+ << PrettyMethod(verifier->GetMethodReference().dex_method_index,
+ *verifier->GetMethodReference().dex_file);
+ }
+ break;
+ }
+ } else if (depths > 0) {
+ // Check whether they're actually the same levels.
+ uint32_t locked_levels = reg_to_lock_depths_.find(idx)->second;
+ uint32_t incoming_locked_levels = incoming_line->reg_to_lock_depths_.find(idx)->second;
+ if (locked_levels != incoming_locked_levels) {
+ verifier->Fail(VERIFY_ERROR_LOCKING);
+ if (kDumpLockFailures) {
+ LOG(WARNING) << "mismatched lock levels for register v" << idx << ": "
+ << std::hex << locked_levels << std::dec << " != "
+ << std::hex << incoming_locked_levels << std::dec << " in "
+ << PrettyMethod(verifier->GetMethodReference().dex_method_index,
+ *verifier->GetMethodReference().dex_file);
+ }
break;
}
}
}
}
}
+
// Check whether "this" was initialized in both paths.
if (this_initialized_ && !incoming_line->this_initialized_) {
this_initialized_ = false;
diff --git a/runtime/verifier/register_line.h b/runtime/verifier/register_line.h
index 41f1e28..46db1c6 100644
--- a/runtime/verifier/register_line.h
+++ b/runtime/verifier/register_line.h
@@ -185,7 +185,9 @@
// Compare two register lines. Returns 0 if they match.
// Using this for a sort is unwise, since the value can change based on machine endianness.
int CompareLine(const RegisterLine* line2) const {
- DCHECK(monitors_ == line2->monitors_);
+ if (monitors_ != line2->monitors_) {
+ return 1;
+ }
// TODO: DCHECK(reg_to_lock_depths_ == line2->reg_to_lock_depths_);
return memcmp(&line_, &line2->line_, num_regs_ * sizeof(uint16_t));
}
@@ -298,8 +300,8 @@
}
// We expect no monitors to be held at certain points, such a method returns. Verify the stack
- // is empty, failing and returning false if not.
- bool VerifyMonitorStackEmpty(MethodVerifier* verifier) const;
+ // is empty, queueing a LOCKING error else.
+ void VerifyMonitorStackEmpty(MethodVerifier* verifier) const;
bool MergeRegisters(MethodVerifier* verifier, const RegisterLine* incoming_line)
SHARED_REQUIRES(Locks::mutator_lock_);
diff --git a/test/088-monitor-verification/expected.txt b/test/088-monitor-verification/expected.txt
index 07f5b0b..13b8c73 100644
--- a/test/088-monitor-verification/expected.txt
+++ b/test/088-monitor-verification/expected.txt
@@ -1,7 +1,12 @@
recursiveSync ok
nestedMayThrow ok
constantLock ok
-excessiveNesting ok
notNested ok
twoPath ok
triplet ok
+OK
+TooDeep
+NotStructuredOverUnlock
+NotStructuredUnderUnlock
+UnbalancedJoin
+UnbalancedStraight
diff --git a/test/088-monitor-verification/smali/NotStructuredOverUnlock.smali b/test/088-monitor-verification/smali/NotStructuredOverUnlock.smali
new file mode 100644
index 0000000..aa0c2d5
--- /dev/null
+++ b/test/088-monitor-verification/smali/NotStructuredOverUnlock.smali
@@ -0,0 +1,21 @@
+.class public LNotStructuredOverUnlock;
+
+.super Ljava/lang/Object;
+
+.method public static run(Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {}, LMain;->assertCallerIsInterpreted()V
+
+ # Lock twice, but unlock thrice.
+
+ monitor-enter v2 # 1
+ monitor-enter v2 # 2
+
+ monitor-exit v2 # 1
+ monitor-exit v2 # 2
+ monitor-exit v2 # 3
+
+ return-void
+
+.end method
diff --git a/test/088-monitor-verification/smali/NotStructuredUnderUnlock.smali b/test/088-monitor-verification/smali/NotStructuredUnderUnlock.smali
new file mode 100644
index 0000000..2c31fda
--- /dev/null
+++ b/test/088-monitor-verification/smali/NotStructuredUnderUnlock.smali
@@ -0,0 +1,21 @@
+.class public LNotStructuredUnderUnlock;
+
+.super Ljava/lang/Object;
+
+.method public static run(Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {}, LMain;->assertCallerIsInterpreted()V
+
+ # Lock thrice, but only unlock twice.
+
+ monitor-enter v2 # 1
+ monitor-enter v2 # 2
+ monitor-enter v2 # 3
+
+ monitor-exit v2 # 1
+ monitor-exit v2 # 2
+
+ return-void
+
+.end method
diff --git a/test/088-monitor-verification/smali/OK.smali b/test/088-monitor-verification/smali/OK.smali
new file mode 100644
index 0000000..596798d
--- /dev/null
+++ b/test/088-monitor-verification/smali/OK.smali
@@ -0,0 +1,68 @@
+.class public LOK;
+
+.super Ljava/lang/Object;
+
+.method public static run(Ljava/lang/Object;Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {v1, v2}, LOK;->runNoMonitors(Ljava/lang/Object;Ljava/lang/Object;)V
+
+ invoke-static {v1, v2}, LOK;->runStraightLine(Ljava/lang/Object;Ljava/lang/Object;)V
+
+ invoke-static {v1, v2}, LOK;->runBalancedJoin(Ljava/lang/Object;Ljava/lang/Object;)V
+
+ return-void
+
+.end method
+
+
+
+.method public static runNoMonitors(Ljava/lang/Object;Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {}, LMain;->assertCallerIsManaged()V
+
+ return-void
+
+.end method
+
+.method public static runStraightLine(Ljava/lang/Object;Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {}, LMain;->assertCallerIsManaged()V
+
+ monitor-enter v1 # 1
+ monitor-enter v2 # 2
+
+ monitor-exit v2 # 2
+ monitor-exit v1 # 1
+
+ return-void
+
+.end method
+
+.method public static runBalancedJoin(Ljava/lang/Object;Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {}, LMain;->assertCallerIsManaged()V
+
+ monitor-enter v1 # 1
+
+ if-eqz v2, :Lnull
+
+:LnotNull
+
+ monitor-enter v2 # 2
+ goto :Lend
+
+:Lnull
+ monitor-enter v2 # 2
+
+:Lend
+
+ monitor-exit v2 # 2
+ monitor-exit v1 # 1
+
+ return-void
+
+.end method
diff --git a/test/088-monitor-verification/smali/TooDeep.smali b/test/088-monitor-verification/smali/TooDeep.smali
new file mode 100644
index 0000000..1a8f2f0
--- /dev/null
+++ b/test/088-monitor-verification/smali/TooDeep.smali
@@ -0,0 +1,82 @@
+.class public LTooDeep;
+
+.super Ljava/lang/Object;
+
+.method public static run(Ljava/lang/Object;)V
+ .registers 3
+
+ # Lock depth is 33, which is more than the verifier supports. This should have been punted to
+ # the interpreter.
+ invoke-static {}, LMain;->assertCallerIsInterpreted()V
+
+ monitor-enter v2 # 1
+ monitor-enter v2 # 2
+ monitor-enter v2 # 3
+ monitor-enter v2 # 4
+ monitor-enter v2 # 5
+ monitor-enter v2 # 6
+ monitor-enter v2 # 7
+ monitor-enter v2 # 8
+ monitor-enter v2 # 9
+ monitor-enter v2 # 10
+ monitor-enter v2 # 11
+ monitor-enter v2 # 12
+ monitor-enter v2 # 13
+ monitor-enter v2 # 14
+ monitor-enter v2 # 15
+ monitor-enter v2 # 16
+ monitor-enter v2 # 17
+ monitor-enter v2 # 18
+ monitor-enter v2 # 19
+ monitor-enter v2 # 20
+ monitor-enter v2 # 21
+ monitor-enter v2 # 22
+ monitor-enter v2 # 23
+ monitor-enter v2 # 24
+ monitor-enter v2 # 25
+ monitor-enter v2 # 26
+ monitor-enter v2 # 27
+ monitor-enter v2 # 28
+ monitor-enter v2 # 29
+ monitor-enter v2 # 30
+ monitor-enter v2 # 31
+ monitor-enter v2 # 32
+ monitor-enter v2 # 33
+
+ monitor-exit v2 # 1
+ monitor-exit v2 # 2
+ monitor-exit v2 # 3
+ monitor-exit v2 # 4
+ monitor-exit v2 # 5
+ monitor-exit v2 # 6
+ monitor-exit v2 # 7
+ monitor-exit v2 # 8
+ monitor-exit v2 # 9
+ monitor-exit v2 # 10
+ monitor-exit v2 # 11
+ monitor-exit v2 # 12
+ monitor-exit v2 # 13
+ monitor-exit v2 # 14
+ monitor-exit v2 # 15
+ monitor-exit v2 # 16
+ monitor-exit v2 # 17
+ monitor-exit v2 # 18
+ monitor-exit v2 # 19
+ monitor-exit v2 # 20
+ monitor-exit v2 # 21
+ monitor-exit v2 # 22
+ monitor-exit v2 # 23
+ monitor-exit v2 # 24
+ monitor-exit v2 # 25
+ monitor-exit v2 # 26
+ monitor-exit v2 # 27
+ monitor-exit v2 # 28
+ monitor-exit v2 # 29
+ monitor-exit v2 # 30
+ monitor-exit v2 # 31
+ monitor-exit v2 # 32
+ monitor-exit v2 # 33
+
+ return-void
+
+.end method
diff --git a/test/088-monitor-verification/smali/UnbalancedJoin.smali b/test/088-monitor-verification/smali/UnbalancedJoin.smali
new file mode 100644
index 0000000..da8f773
--- /dev/null
+++ b/test/088-monitor-verification/smali/UnbalancedJoin.smali
@@ -0,0 +1,31 @@
+.class public LUnbalancedJoin;
+
+.super Ljava/lang/Object;
+
+.method public static run(Ljava/lang/Object;Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {}, LMain;->assertCallerIsInterpreted()V
+
+ if-eqz v2, :Lnull
+
+:LnotNull
+
+ monitor-enter v1 # 1
+ monitor-enter v2 # 2
+ goto :Lend
+
+:Lnull
+ monitor-enter v2 # 1
+ monitor-enter v1 # 2
+
+:Lend
+
+ # Lock levels are "opposite" for the joined flows.
+
+ monitor-exit v2 # 2
+ monitor-exit v1 # 1
+
+ return-void
+
+.end method
diff --git a/test/088-monitor-verification/smali/UnbalancedStraight.smali b/test/088-monitor-verification/smali/UnbalancedStraight.smali
new file mode 100644
index 0000000..68edb6c
--- /dev/null
+++ b/test/088-monitor-verification/smali/UnbalancedStraight.smali
@@ -0,0 +1,18 @@
+.class public LUnbalancedStraight;
+
+.super Ljava/lang/Object;
+
+.method public static run(Ljava/lang/Object;Ljava/lang/Object;)V
+ .registers 3
+
+ invoke-static {}, LMain;->assertCallerIsInterpreted()V
+
+ monitor-enter v1 # 1
+ monitor-enter v2 # 2
+
+ monitor-exit v1 # 1 Unbalanced unlock.
+ monitor-exit v2 # 2
+
+ return-void
+
+.end method
diff --git a/test/088-monitor-verification/src/Main.java b/test/088-monitor-verification/src/Main.java
index b60c71e..af1eaea 100644
--- a/test/088-monitor-verification/src/Main.java
+++ b/test/088-monitor-verification/src/Main.java
@@ -14,6 +14,9 @@
* limitations under the License.
*/
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
/*
* Entry point and tests that are expected to succeed.
@@ -38,11 +41,6 @@
System.out.println("constantLock ok");
m.notExcessiveNesting();
- try {
- TooDeep.excessiveNesting();
- System.err.println("excessiveNesting did not throw");
- } catch (VerifyError ve) {}
- System.out.println("excessiveNesting ok");
m.notNested();
System.out.println("notNested ok");
@@ -55,6 +53,9 @@
m.triplet(obj1, obj2, 0);
System.out.println("triplet ok");
+
+ System.loadLibrary("arttest");
+ runSmaliTests();
}
/**
@@ -216,4 +217,62 @@
doNothing(localObj);
}
+
+ // Smali testing code.
+ private static void runSmaliTests() {
+ runTest("OK", new Object[] { new Object(), new Object() }, null);
+ runTest("TooDeep", new Object[] { new Object() }, null);
+ runTest("NotStructuredOverUnlock", new Object[] { new Object() },
+ IllegalMonitorStateException.class);
+ runTest("NotStructuredUnderUnlock", new Object[] { new Object() }, null);
+ // TODO: new IllegalMonitorStateException());
+ runTest("UnbalancedJoin", new Object[] { new Object(), new Object() }, null);
+ runTest("UnbalancedStraight", new Object[] { new Object(), new Object() }, null);
+ }
+
+ private static void runTest(String className, Object[] parameters, Class<?> excType) {
+ System.out.println(className);
+ try {
+ Class<?> c = Class.forName(className);
+
+ Method[] methods = c.getDeclaredMethods();
+
+ // For simplicity we assume that test methods are not overloaded. So searching by name
+ // will give us the method we need to run.
+ Method method = null;
+ for (Method m : methods) {
+ if (m.getName().equals("run")) {
+ method = m;
+ break;
+ }
+ }
+
+ if (method == null) {
+ System.out.println("Could not find test method for " + className);
+ } else if (!Modifier.isStatic(method.getModifiers())) {
+ System.out.println("Test method for " + className + " is not static.");
+ } else {
+ method.invoke(null, parameters);
+ if (excType != null) {
+ System.out.println("Expected an exception in " + className);
+ }
+ }
+ } catch (Throwable exc) {
+ if (excType == null) {
+ System.out.println("Did not expect exception " + exc + " for " + className);
+ exc.printStackTrace(System.out);
+ } else if (exc instanceof InvocationTargetException && exc.getCause() != null &&
+ exc.getCause().getClass().equals(excType)) {
+ // Expected exception is wrapped in InvocationTargetException.
+ } else if (!excType.equals(exc.getClass())) {
+ System.out.println("Expected " + excType.getName() + ", but got " + exc.getClass());
+ } else {
+ // Expected exception, do nothing.
+ }
+ }
+ }
+
+ // Helpers for the smali code.
+ public static native void assertCallerIsInterpreted();
+ public static native void assertCallerIsManaged();
}
diff --git a/test/088-monitor-verification/src/TooDeep.java b/test/088-monitor-verification/src/TooDeep.java
deleted file mode 100644
index 76192e5..0000000
--- a/test/088-monitor-verification/src/TooDeep.java
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * Copyright (C) 2010 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-
-/**
- * The class has a method with too many levels of nested "synchronized"
- * blocks. The verifier will reject it.
- *
- * (It would be perfectly okay if the verifier *didn't* reject this.
- * The goal here is just to exercise the failure path. It also serves
- * as a check to see if the monitor checks are enabled.)
- */
-public class TooDeep {
-
- public static void excessiveNesting() {
- synchronized (TooDeep.class) { // 1
- synchronized (TooDeep.class) { // 2
- synchronized (TooDeep.class) { // 3
- synchronized (TooDeep.class) { // 4
- synchronized (TooDeep.class) { // 5
- synchronized (TooDeep.class) { // 6
- synchronized (TooDeep.class) { // 7
- synchronized (TooDeep.class) { // 8
- synchronized (TooDeep.class) { // 9
- synchronized (TooDeep.class) { // 10
- synchronized (TooDeep.class) { // 11
- synchronized (TooDeep.class) { // 12
- synchronized (TooDeep.class) { // 13
- synchronized (TooDeep.class) { // 14
- synchronized (TooDeep.class) { // 15
- synchronized (TooDeep.class) { // 16
- synchronized (TooDeep.class) { // 17
- synchronized (TooDeep.class) { // 18
- synchronized (TooDeep.class) { // 19
- synchronized (TooDeep.class) { // 20
- synchronized (TooDeep.class) { // 21
- synchronized (TooDeep.class) { // 22
- synchronized (TooDeep.class) { // 23
- synchronized (TooDeep.class) { // 24
- synchronized (TooDeep.class) { // 25
- synchronized (TooDeep.class) { // 26
- synchronized (TooDeep.class) { // 27
- synchronized (TooDeep.class) { // 28
- synchronized (TooDeep.class) { // 29
- synchronized (TooDeep.class) { // 30
- synchronized (TooDeep.class) { // 31
- synchronized (TooDeep.class) { // 32
- synchronized (TooDeep.class) { // 33
- }}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}
- }
-}
diff --git a/test/088-monitor-verification/stack_inspect.cc b/test/088-monitor-verification/stack_inspect.cc
new file mode 100644
index 0000000..e2899c3
--- /dev/null
+++ b/test/088-monitor-verification/stack_inspect.cc
@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "jni.h"
+
+#include "base/logging.h"
+#include "dex_file-inl.h"
+#include "mirror/class-inl.h"
+#include "nth_caller_visitor.h"
+#include "runtime.h"
+#include "scoped_thread_state_change.h"
+#include "stack.h"
+#include "thread-inl.h"
+
+namespace art {
+
+// public static native void assertCallerIsInterpreted();
+
+extern "C" JNIEXPORT void JNICALL Java_Main_assertCallerIsInterpreted(JNIEnv* env, jclass) {
+ LOG(INFO) << "assertCallerIsInterpreted";
+
+ ScopedObjectAccess soa(env);
+ NthCallerVisitor caller(soa.Self(), 1, false);
+ caller.WalkStack();
+ CHECK(caller.caller != nullptr);
+ LOG(INFO) << PrettyMethod(caller.caller);
+ CHECK(caller.GetCurrentShadowFrame() != nullptr);
+}
+
+// public static native void assertCallerIsManaged();
+
+extern "C" JNIEXPORT void JNICALL Java_Main_assertCallerIsManaged(JNIEnv* env, jclass cls) {
+ // Note: needs some smarts to not fail if there is no managed code, at all.
+ LOG(INFO) << "assertCallerIsManaged";
+
+ ScopedObjectAccess soa(env);
+
+ mirror::Class* klass = soa.Decode<mirror::Class*>(cls);
+ const DexFile& dex_file = klass->GetDexFile();
+ const OatFile::OatDexFile* oat_dex_file = dex_file.GetOatDexFile();
+ if (oat_dex_file == nullptr) {
+ // No oat file, this must be a test configuration that doesn't compile at all. Ignore that the
+ // result will be that we're running the interpreter.
+ return;
+ }
+
+ NthCallerVisitor caller(soa.Self(), 1, false);
+ caller.WalkStack();
+ CHECK(caller.caller != nullptr);
+ LOG(INFO) << PrettyMethod(caller.caller);
+
+ if (caller.GetCurrentShadowFrame() == nullptr) {
+ // Not a shadow frame, this looks good.
+ return;
+ }
+
+ // This could be an interpret-only or a verify-at-runtime compilation, or a read-barrier variant,
+ // or... It's not really safe to just reject now. Let's look at the access flags. If the method
+ // was successfully verified, its access flags should be set to mark it preverified, except when
+ // we're running soft-fail tests.
+ if (Runtime::Current()->IsVerificationSoftFail()) {
+ // Soft-fail config. Everything should be running with interpreter access checks, potentially.
+ return;
+ }
+ CHECK(caller.caller->IsPreverified());
+}
+
+} // namespace art
diff --git a/test/Android.libarttest.mk b/test/Android.libarttest.mk
index fcb9f8a..6150e87 100644
--- a/test/Android.libarttest.mk
+++ b/test/Android.libarttest.mk
@@ -25,6 +25,7 @@
004-StackWalk/stack_walk_jni.cc \
004-UnsafeTest/unsafe_test.cc \
051-thread/thread_test.cc \
+ 088-monitor-verification/stack_inspect.cc \
116-nodex2oat/nodex2oat.cc \
117-nopatchoat/nopatchoat.cc \
118-noimage-dex2oat/noimage-dex2oat.cc \