diff options
author | 2025-01-31 13:21:09 +0000 | |
---|---|---|
committer | 2025-02-11 09:31:52 -0800 | |
commit | 01a7d316730f2ed7dac7fa85c9e707ab0241ef82 (patch) | |
tree | 43e9631020f3f71cdf640b8ded1eaa91daf896dc | |
parent | 74072380fe75b9ee7ab90aaa64080af015769cbe (diff) |
verifier: Keep locking info for no-op move-object.
Refactor `move{,-wide,-object}` verification. Make sure we
keep the locking info for no-op `move-object vN, vN`.
Refactor `monitor-enter` and `monitor-exit` verification as
well because the former used the same helper functions as
`move-object`. Bail out on a hard failure - previously,
we'd keep processing previous instructions and use helper
functions that can add an additional hard failure.
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Change-Id: I2d76812ecf1b8232b4a5da0426461d6f572da13b
-rw-r--r-- | runtime/verifier/method_verifier.cc | 158 | ||||
-rw-r--r-- | runtime/verifier/reg_type.h | 34 | ||||
-rw-r--r-- | runtime/verifier/register_line-inl.h | 47 | ||||
-rw-r--r-- | runtime/verifier/register_line.cc | 28 | ||||
-rw-r--r-- | runtime/verifier/register_line.h | 20 | ||||
-rw-r--r-- | test/088-monitor-verification/smali/OK.smali | 20 | ||||
-rw-r--r-- | test/088-monitor-verification/src/Main.java | 1 |
7 files changed, 207 insertions, 101 deletions
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index 1de1e6bce8..b0e9d440ee 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -836,6 +836,25 @@ class MethodVerifierImpl : public ::art::verifier::MethodVerifier { << ", target index " << target_index; } + NO_INLINE void FailForCopyReference(uint32_t vdst, uint32_t vsrc, const RegType& type) + REQUIRES_SHARED(Locks::mutator_lock_) { + Fail(VERIFY_ERROR_BAD_CLASS_HARD) + << "copy-reference v" << vdst << "<-v" << vsrc << " type=" << type; + } + + NO_INLINE void FailForCopyCat1(uint32_t vdst, uint32_t vsrc, const RegType& type) + REQUIRES_SHARED(Locks::mutator_lock_) { + Fail(VERIFY_ERROR_BAD_CLASS_HARD) + << "copy-cat1 v" << vdst << "<-v" << vsrc << " type=" << type; + } + + NO_INLINE void FailForCopyCat2( + uint32_t vdst, uint32_t vsrc, const RegType& type_l, const RegType& type_h) + REQUIRES_SHARED(Locks::mutator_lock_) { + Fail(VERIFY_ERROR_BAD_CLASS_HARD) + << "copy-cat2 v" << vdst << "<-v" << vsrc << " type=" << type_l << "/" << type_h; + } + NO_INLINE void FailForRegisterType(uint32_t vsrc, const RegType& check_type, const RegType& src_type, @@ -869,6 +888,60 @@ class MethodVerifierImpl : public ::art::verifier::MethodVerifier { vsrc, reg_types_.GetFromId(src_type_id), reg_types_.GetFromId(src_type_id_h)); } + ALWAYS_INLINE inline bool VerifyCopyReference(uint32_t vdst, uint32_t vsrc) + REQUIRES_SHARED(Locks::mutator_lock_) { + const RegType& type = work_line_->GetRegisterType(this, vsrc); + // Allow conflicts to be copied around. + if (UNLIKELY(!type.IsConflict() && !type.IsReferenceTypes())) { + FailForCopyReference(vdst, vsrc, type); + return false; + } + work_line_->CopyReference(vdst, vsrc, type); + return true; + } + + ALWAYS_INLINE inline bool VerifyCopyCat1(uint32_t vdst, uint32_t vsrc) + REQUIRES_SHARED(Locks::mutator_lock_) { + uint16_t src_type_id = work_line_->GetRegisterTypeId(vsrc); + if (UNLIKELY(src_type_id >= RegTypeCache::NumberOfRegKindCacheIds()) || + UNLIKELY(RegTypeCache::RegKindForId(src_type_id) != RegType::kConflict && + !RegType::IsCategory1Types(RegTypeCache::RegKindForId(src_type_id)))) { + const RegType& type = reg_types_.GetFromId(src_type_id); + DCHECK(!type.IsConflict() && !type.IsCategory1Types()) << type; + FailForCopyCat1(vdst, vsrc, type); + return false; + } + RegType::Kind kind = RegTypeCache::RegKindForId(src_type_id); + DCHECK(kind == RegType::kConflict || RegType::IsCategory1Types(kind)) << kind; + work_line_->SetRegisterType(vdst, kind); + return true; + } + + ALWAYS_INLINE inline bool VerifyCopyCat2(uint32_t vdst, uint32_t vsrc) + REQUIRES_SHARED(Locks::mutator_lock_) { + uint16_t src_type_id_l = work_line_->GetRegisterTypeId(vsrc); + uint16_t src_type_id_h = work_line_->GetRegisterTypeId(vsrc + 1); + auto to_high_id = [](uint16_t low_id) ALWAYS_INLINE { + RegType::Kind low_kind = RegTypeCache::RegKindForId(low_id); + DCHECK(RegType::IsLowHalf(low_kind)); + return RegTypeCache::IdForRegKind(RegType::ToHighHalf(low_kind)); + }; + if (UNLIKELY(src_type_id_l >= RegTypeCache::NumberOfRegKindCacheIds()) || + UNLIKELY(!RegType::IsLowHalf(RegTypeCache::RegKindForId(src_type_id_l))) || + UNLIKELY(src_type_id_h != to_high_id(src_type_id_l))) { + const RegType& type_l = reg_types_.GetFromId(src_type_id_l); + const RegType& type_h = reg_types_.GetFromId(src_type_id_h); + DCHECK(!type_l.CheckWidePair(type_h)); + FailForCopyCat2(vdst, vsrc, type_l, type_h); + return false; + } + DCHECK(reg_types_.GetFromId(src_type_id_l).CheckWidePair(reg_types_.GetFromId(src_type_id_h))); + work_line_->SetRegisterTypeWide(vdst, + RegTypeCache::RegKindForId(src_type_id_l), + RegTypeCache::RegKindForId(src_type_id_h)); + return true; + } + ALWAYS_INLINE inline bool VerifyRegisterType(uint32_t vsrc, const RegType& check_type) REQUIRES_SHARED(Locks::mutator_lock_) { // Verify the src register type against the check type refining the type of the register @@ -2664,35 +2737,49 @@ bool MethodVerifier<kVerifierDebug>::CodeFlowVerifyInstruction(uint32_t* start_g break; case Instruction::MOVE: - work_line_->CopyRegister1( - this, inst->VRegA_12x(inst_data), inst->VRegB_12x(inst_data), kTypeCategory1nr); + if (!VerifyCopyCat1(inst->VRegA_12x(inst_data), inst->VRegB_12x(inst_data))) { + return false; + } break; case Instruction::MOVE_FROM16: - work_line_->CopyRegister1( - this, inst->VRegA_22x(inst_data), inst->VRegB_22x(), kTypeCategory1nr); + if (!VerifyCopyCat1(inst->VRegA_22x(inst_data), inst->VRegB_22x())) { + return false; + } break; case Instruction::MOVE_16: - work_line_->CopyRegister1(this, inst->VRegA_32x(), inst->VRegB_32x(), kTypeCategory1nr); + if (!VerifyCopyCat1(inst->VRegA_32x(), inst->VRegB_32x())) { + return false; + } break; case Instruction::MOVE_WIDE: - work_line_->CopyRegister2(this, inst->VRegA_12x(inst_data), inst->VRegB_12x(inst_data)); + if (!VerifyCopyCat2(inst->VRegA_12x(inst_data), inst->VRegB_12x(inst_data))) { + return false; + } break; case Instruction::MOVE_WIDE_FROM16: - work_line_->CopyRegister2(this, inst->VRegA_22x(inst_data), inst->VRegB_22x()); + if (!VerifyCopyCat2(inst->VRegA_22x(inst_data), inst->VRegB_22x())) { + return false; + } break; case Instruction::MOVE_WIDE_16: - work_line_->CopyRegister2(this, inst->VRegA_32x(), inst->VRegB_32x()); + if (!VerifyCopyCat2(inst->VRegA_32x(), inst->VRegB_32x())) { + return false; + } break; case Instruction::MOVE_OBJECT: - work_line_->CopyRegister1( - this, inst->VRegA_12x(inst_data), inst->VRegB_12x(inst_data), kTypeCategoryRef); + if (!VerifyCopyReference(inst->VRegA_12x(inst_data), inst->VRegB_12x(inst_data))) { + return false; + } break; case Instruction::MOVE_OBJECT_FROM16: - work_line_->CopyRegister1( - this, inst->VRegA_22x(inst_data), inst->VRegB_22x(), kTypeCategoryRef); + if (!VerifyCopyReference(inst->VRegA_22x(inst_data), inst->VRegB_22x())) { + return false; + } break; case Instruction::MOVE_OBJECT_16: - work_line_->CopyRegister1(this, inst->VRegA_32x(), inst->VRegB_32x(), kTypeCategoryRef); + if (!VerifyCopyReference(inst->VRegA_32x(), inst->VRegB_32x())) { + return false; + } break; /* @@ -2882,8 +2969,14 @@ bool MethodVerifier<kVerifierDebug>::CodeFlowVerifyInstruction(uint32_t* start_g work_line_->SetRegisterType<LockOp::kClear>( inst->VRegA_21c(inst_data), reg_types_.JavaLangInvokeMethodType()); break; - case Instruction::MONITOR_ENTER: - work_line_->PushMonitor(this, inst->VRegA_11x(inst_data), work_insn_idx_); + case Instruction::MONITOR_ENTER: { + uint32_t vreg = inst->VRegA_11x(inst_data); + const RegType& reg_type = work_line_->GetRegisterType(this, vreg); + if (!reg_type.IsReferenceTypes()) { + Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "monitor-enter on non-object (" << reg_type << ")"; + return false; + } + work_line_->PushMonitor(this, vreg, reg_type, 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_ && !GetInstructionFlags(work_insn_idx_).IsBranchTarget()) { @@ -2896,13 +2989,10 @@ bool MethodVerifier<kVerifierDebug>::CodeFlowVerifyInstruction(uint32_t* start_g case Instruction::MOVE_OBJECT: case Instruction::MOVE_OBJECT_16: case Instruction::MOVE_OBJECT_FROM16: - if (prev_inst.VRegB() == inst->VRegA_11x(inst_data)) { + if (static_cast<uint32_t>(prev_inst.VRegB()) == vreg) { // 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); + work_line_->CopyReference(prev_inst.VRegA(), vreg, reg_type); } break; @@ -2931,16 +3021,12 @@ bool MethodVerifier<kVerifierDebug>::CodeFlowVerifyInstruction(uint32_t* start_g } // Update the lock status for the aliased register. - if (prev_inst.VRegA() == inst->VRegA_11x(inst_data)) { - work_line_->CopyRegister1(this, - prev2_inst.VRegA(), - inst->VRegA_11x(inst_data), - kTypeCategoryRef); - } else if (prev2_inst.VRegA() == inst->VRegA_11x(inst_data)) { - work_line_->CopyRegister1(this, - prev_inst.VRegA(), - inst->VRegA_11x(inst_data), - kTypeCategoryRef); + uint32_t prev_inst_vregA = prev_inst.VRegA_21c(prev_inst.Fetch16(0)); + uint32_t prev2_inst_vregA = prev2_inst.VRegA_21c(prev2_inst.Fetch16(0)); + if (prev_inst_vregA == vreg) { + work_line_->CopyReference(prev2_inst_vregA, vreg, reg_type); + } else if (prev2_inst_vregA == vreg) { + work_line_->CopyReference(prev_inst_vregA, vreg, reg_type); } break; } @@ -2950,7 +3036,8 @@ bool MethodVerifier<kVerifierDebug>::CodeFlowVerifyInstruction(uint32_t* start_g } } break; - case Instruction::MONITOR_EXIT: + } + case Instruction::MONITOR_EXIT: { /* * monitor-exit instructions are odd. They can throw exceptions, * but when they do they act as if they succeeded and the PC is @@ -2972,8 +3059,15 @@ bool MethodVerifier<kVerifierDebug>::CodeFlowVerifyInstruction(uint32_t* start_g * "live" so we still need to check it. */ opcode_flags &= ~Instruction::kThrow; - work_line_->PopMonitor(this, inst->VRegA_11x(inst_data)); + uint32_t vreg = inst->VRegA_11x(inst_data); + const RegType& reg_type = work_line_->GetRegisterType(this, vreg); + if (!reg_type.IsReferenceTypes()) { + Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "monitor-exit on non-object (" << reg_type << ")"; + return false; + } + work_line_->PopMonitor(this, vreg, reg_type); break; + } case Instruction::CHECK_CAST: case Instruction::INSTANCE_OF: { /* diff --git a/runtime/verifier/reg_type.h b/runtime/verifier/reg_type.h index a8f31c523c..58dd88215d 100644 --- a/runtime/verifier/reg_type.h +++ b/runtime/verifier/reg_type.h @@ -170,21 +170,36 @@ class RegType { constexpr bool IsZeroOrNull() const { return IsZero() || IsNull(); } - bool IsCategory1Types() const { - return IsIntegralTypes() || IsFloat(); + static constexpr bool IsCategory1Types(Kind kind) { + return IsIntegralTypes(kind) || kind == kFloat; } - bool IsCategory2Types() const { + constexpr bool IsCategory1Types() const { + return IsCategory1Types(GetKind()); + } + constexpr bool IsCategory2Types() const { return IsLowHalf(); // Don't expect explicit testing of high halves } static constexpr bool IsBooleanTypes(Kind kind) { return kind == Kind::kBoolean || kind == Kind::kZero || kind == Kind::kBooleanConstant; } constexpr bool IsBooleanTypes() const { return IsBooleanTypes(GetKind()); } + static constexpr bool IsByteTypes(Kind kind) { + return kind == kByte || + kind == kPositiveByteConstant || + kind == kByteConstant || + IsBooleanTypes(kind); + } constexpr bool IsByteTypes() const { - return IsByte() || IsPositiveByteConstant() || IsByteConstant() || IsBooleanTypes(); + return IsByteTypes(GetKind()); + } + static constexpr bool IsShortTypes(Kind kind) { + return kind == kShort || + kind == kPositiveShortConstant || + kind == kShortConstant || + IsByteTypes(kind); } constexpr bool IsShortTypes() const { - return IsShort() || IsPositiveShortConstant() || IsShortConstant() || IsByteTypes(); + return IsShortTypes(GetKind()); } constexpr bool IsCharTypes() const { return IsChar() || @@ -193,8 +208,15 @@ class RegType { IsPositiveByteConstant() || IsBooleanTypes(); } + static constexpr bool IsIntegralTypes(Kind kind) { + return kind == kInteger || + kind == kIntegerConstant || + kind == kChar || + kind == kCharConstant || + IsShortTypes(kind); + } constexpr bool IsIntegralTypes() const { - return IsInteger() || IsIntegerConstant() || IsChar() || IsCharConstant() || IsShortTypes(); + return IsIntegralTypes(GetKind()); } // Give the constant value encoded, but this shouldn't be called in the // general case. diff --git a/runtime/verifier/register_line-inl.h b/runtime/verifier/register_line-inl.h index 9511fce3fe..2bf48596f3 100644 --- a/runtime/verifier/register_line-inl.h +++ b/runtime/verifier/register_line-inl.h @@ -66,8 +66,8 @@ template <LockOp kLockOp> inline void RegisterLine::SetRegisterType(uint32_t vdst, const RegType& new_type) { DCHECK(!new_type.IsLowHalf()); DCHECK(!new_type.IsHighHalf()); - // Should only keep locks for reference types. - DCHECK_IMPLIES(kLockOp == LockOp::kKeep, new_type.IsReferenceTypes()); + // Should only keep locks for reference types, or when copying a conflict with `move-object`. + DCHECK_IMPLIES(kLockOp == LockOp::kKeep, new_type.IsReferenceTypes() || new_type.IsConflict()); SetRegisterTypeImpl<kLockOp>(vdst, new_type.GetId()); } @@ -126,41 +126,14 @@ inline void RegisterLine::SetRegisterTypeForNewInstance(uint32_t vdst, allocation_dex_pcs_[vdst] = dex_pc; } -inline void RegisterLine::CopyRegister1(MethodVerifier* verifier, uint32_t vdst, uint32_t vsrc, - TypeCategory cat) { - DCHECK(cat == kTypeCategory1nr || cat == kTypeCategoryRef); - const RegType& type = GetRegisterType(verifier, vsrc); - if (type.IsLowHalf() || type.IsHighHalf()) { - verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "Expected category1 register type not '" - << type << "'"; - return; - } - // FIXME: If `vdst == vsrc`, we clear locking information before we try to copy it below. Adding - // `move-object v1, v1` to the middle of `OK.runStraightLine()` in run-test 088 makes it fail. - SetRegisterType<LockOp::kClear>(vdst, type); - if (!type.IsConflict() && // Allow conflicts to be copied around. - ((cat == kTypeCategory1nr && !type.IsCategory1Types()) || - (cat == kTypeCategoryRef && !type.IsReferenceTypes()))) { - verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "copy1 v" << vdst << "<-v" << vsrc << " type=" << type - << " cat=" << static_cast<int>(cat); - } else if (cat == kTypeCategoryRef) { - CopyRegToLockDepth(vdst, vsrc); - if (allocation_dex_pcs_ != nullptr) { - // Copy allocation dex pc for uninitialized types. (Copy unused value for other types.) - allocation_dex_pcs_[vdst] = allocation_dex_pcs_[vsrc]; - } - } -} - -inline void RegisterLine::CopyRegister2(MethodVerifier* verifier, uint32_t vdst, uint32_t vsrc) { - const RegType& type_l = GetRegisterType(verifier, vsrc); - const RegType& type_h = GetRegisterType(verifier, vsrc + 1); - - if (!type_l.CheckWidePair(type_h)) { - verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "copy2 v" << vdst << "<-v" << vsrc - << " type=" << type_l << "/" << type_h; - } else { - SetRegisterTypeWide(vdst, type_l, type_h); +inline void RegisterLine::CopyReference(uint32_t vdst, uint32_t vsrc, const RegType& type) { + DCHECK_EQ(type.GetId(), GetRegisterTypeId(vsrc)); + DCHECK(type.IsConflict() || type.IsReferenceTypes()); + SetRegisterType<LockOp::kKeep>(vdst, type); + CopyRegToLockDepth(vdst, vsrc); + if (allocation_dex_pcs_ != nullptr) { + // Copy allocation dex pc for uninitialized types. (Copy unused value for other types.) + allocation_dex_pcs_[vdst] = allocation_dex_pcs_[vsrc]; } } diff --git a/runtime/verifier/register_line.cc b/runtime/verifier/register_line.cc index 4466077a74..bcf1341048 100644 --- a/runtime/verifier/register_line.cc +++ b/runtime/verifier/register_line.cc @@ -142,19 +142,17 @@ void RegisterLine::CopyResultRegister2(MethodVerifier* verifier, uint32_t vdst) 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()) { - verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "monitor-enter on non-object (" - << reg_type << ")"; - } else if (monitors_.size() >= kMaxMonitorStackDepth) { +void RegisterLine::PushMonitor( + MethodVerifier* verifier, uint32_t vreg, const RegType& reg_type, int32_t insn_idx) { + DCHECK_EQ(reg_type.GetId(), GetRegisterTypeId(vreg)); + if (monitors_.size() >= kMaxMonitorStackDepth) { verifier->Fail(VERIFY_ERROR_LOCKING); if (kDumpLockFailures) { VLOG(verifier) << "monitor-enter stack overflow while verifying " << verifier->GetMethodReference().PrettyMethod(); } } else { - if (SetRegToLockDepth(reg_idx, monitors_.size())) { + if (SetRegToLockDepth(vreg, 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()) { @@ -165,18 +163,16 @@ void RegisterLine::PushMonitor(MethodVerifier* verifier, uint32_t reg_idx, int32 } else { verifier->Fail(VERIFY_ERROR_LOCKING); if (kDumpLockFailures) { - VLOG(verifier) << "unexpected monitor-enter on register v" << reg_idx << " in " + VLOG(verifier) << "unexpected monitor-enter on register v" << vreg << " in " << verifier->GetMethodReference().PrettyMethod(); } } } } -void RegisterLine::PopMonitor(MethodVerifier* verifier, uint32_t reg_idx) { - const RegType& reg_type = GetRegisterType(verifier, reg_idx); - if (!reg_type.IsReferenceTypes()) { - verifier->Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "monitor-exit on non-object (" << reg_type << ")"; - } else if (monitors_.empty()) { +void RegisterLine::PopMonitor(MethodVerifier* verifier, uint32_t vreg, const RegType& reg_type) { + DCHECK_EQ(reg_type.GetId(), GetRegisterTypeId(vreg)); + if (monitors_.empty()) { verifier->Fail(VERIFY_ERROR_LOCKING); if (kDumpLockFailures) { VLOG(verifier) << "monitor-exit stack underflow while verifying " @@ -185,14 +181,14 @@ void RegisterLine::PopMonitor(MethodVerifier* verifier, uint32_t reg_idx) { } else { monitors_.pop_back(); - bool success = IsSetLockDepth(reg_idx, monitors_.size()); + bool success = IsSetLockDepth(vreg, 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; + vreg = kVirtualNullRegister; } } @@ -205,7 +201,7 @@ void RegisterLine::PopMonitor(MethodVerifier* verifier, uint32_t reg_idx) { } else { // Record the register was unlocked. This clears all aliases, thus it will also clear the // null lock, if necessary. - ClearRegToLockDepth(reg_idx, monitors_.size()); + ClearRegToLockDepth(vreg, monitors_.size()); } } } diff --git a/runtime/verifier/register_line.h b/runtime/verifier/register_line.h index da3baabeaa..c1aceaead4 100644 --- a/runtime/verifier/register_line.h +++ b/runtime/verifier/register_line.h @@ -78,13 +78,8 @@ class RegisterLine { // Create a register line of num_regs registers. static RegisterLine* Create(size_t num_regs, ArenaAllocator& allocator); - // Implement category-1 "move" instructions. Copy a 32-bit value from "vsrc" to "vdst". - void CopyRegister1(MethodVerifier* verifier, uint32_t vdst, uint32_t vsrc, TypeCategory cat) - REQUIRES_SHARED(Locks::mutator_lock_); - - // Implement category-2 "move" instructions. Copy a 64-bit value from "vsrc" to "vdst". This - // copies both halves of the register. - void CopyRegister2(MethodVerifier* verifier, uint32_t vdst, uint32_t vsrc) + // Copy reference (or conflict) register. + void CopyReference(uint32_t vdst, uint32_t vsrc, const RegType& type) REQUIRES_SHARED(Locks::mutator_lock_); // Implement "move-result". Copy the category-1 value from the result register to another @@ -205,11 +200,12 @@ class RegisterLine { ALWAYS_INLINE static size_t ComputeSize(size_t num_regs); // Verify/push monitor onto the monitor stack, locking the value in reg_idx at location insn_idx. - void PushMonitor(MethodVerifier* verifier, uint32_t reg_idx, int32_t insn_idx) + void PushMonitor( + MethodVerifier* verifier, uint32_t vreg, const RegType& reg_type, int32_t insn_idx) REQUIRES_SHARED(Locks::mutator_lock_); // Verify/pop monitor from monitor stack ensuring that we believe the monitor is locked - void PopMonitor(MethodVerifier* verifier, uint32_t reg_idx) + void PopMonitor(MethodVerifier* verifier, uint32_t vreg, const RegType& reg_type) REQUIRES_SHARED(Locks::mutator_lock_); // Stack of currently held monitors and where they were locked @@ -263,9 +259,13 @@ class RegisterLine { REQUIRES_SHARED(Locks::mutator_lock_); void CopyRegToLockDepth(size_t dst, size_t src) { + // Note: We do not clear the entry for `dst` before copying, so we need to `Overwrite()` + // or `erase()`. This preserves the lock depths in the unlikely case that `dst == src`. auto it = reg_to_lock_depths_.find(src); if (it != reg_to_lock_depths_.end()) { - reg_to_lock_depths_.Put(dst, it->second); + reg_to_lock_depths_.Overwrite(dst, it->second); + } else { + reg_to_lock_depths_.erase(dst); } } diff --git a/test/088-monitor-verification/smali/OK.smali b/test/088-monitor-verification/smali/OK.smali index a43ecb0704..042ea37d7f 100644 --- a/test/088-monitor-verification/smali/OK.smali +++ b/test/088-monitor-verification/smali/OK.smali @@ -9,6 +9,8 @@ invoke-static {v1, v2}, LOK;->runStraightLine(Ljava/lang/Object;Ljava/lang/Object;)V + invoke-static {v1, v2}, LOK;->runStraightLine2(Ljava/lang/Object;Ljava/lang/Object;)V + invoke-static {v1, v2}, LOK;->runBalancedJoin(Ljava/lang/Object;Ljava/lang/Object;)V return-void @@ -41,6 +43,24 @@ .end method +.method public static runStraightLine2(Ljava/lang/Object;Ljava/lang/Object;)V + .registers 3 + + invoke-static {}, LMain;->assertIsManaged()V + + monitor-enter v1 # 1 + monitor-enter v2 # 2 + + # No-op move should not invalidate locking information. + move-object v2, v2 + + 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 diff --git a/test/088-monitor-verification/src/Main.java b/test/088-monitor-verification/src/Main.java index 3ed1939a08..a7c24960b4 100644 --- a/test/088-monitor-verification/src/Main.java +++ b/test/088-monitor-verification/src/Main.java @@ -42,6 +42,7 @@ public class Main { ensureJitCompiled(TwoPath.class, "twoPath"); ensureJitCompiled(Class.forName("OK"), "runNoMonitors"); ensureJitCompiled(Class.forName("OK"), "runStraightLine"); + ensureJitCompiled(Class.forName("OK"), "runStraightLine2"); ensureJitCompiled(Class.forName("OK"), "runBalancedJoin"); ensureJitCompiled(Class.forName("NullLocks"), "run"); |