From 15b9d5274399736ac09705f0507df24fac4f00c1 Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Thu, 12 Mar 2015 15:05:13 +0000 Subject: API change in StackVisitor::GetVReg*. - Remove GetVReg() and SetVReg() that were expecting to always succeed. - Change Quick-only methods to take a FromQuickCode suffix. - Change deopt to use dead values when GetVReg does not succeed: the optimizing compiler will not have a location for uninitialized Dex registers and potentially dead registers. Change-Id: Ida05773a97aff8aa69e0caf42ea961f80f854b77 --- compiler/dex/quick/ralloc_util.cc | 6 ++-- oatdump/oatdump.cc | 22 +++++++----- runtime/monitor.cc | 7 ++-- runtime/quick_exception_handler.cc | 71 +++++++++++++++++++++++++++++++------- runtime/stack.cc | 25 +++++++++----- runtime/stack.h | 36 +++++-------------- runtime/thread.cc | 6 ++-- 7 files changed, 108 insertions(+), 65 deletions(-) diff --git a/compiler/dex/quick/ralloc_util.cc b/compiler/dex/quick/ralloc_util.cc index 682fa281ac..741657bc69 100644 --- a/compiler/dex/quick/ralloc_util.cc +++ b/compiler/dex/quick/ralloc_util.cc @@ -1322,9 +1322,9 @@ void Mir2Lir::DoPromotion() { /* Returns sp-relative offset in bytes for a VReg */ int Mir2Lir::VRegOffset(int v_reg) { const DexFile::CodeItem* code_item = mir_graph_->GetCurrentDexCompilationUnit()->GetCodeItem(); - return StackVisitor::GetVRegOffset(code_item, core_spill_mask_, - fp_spill_mask_, frame_size_, v_reg, - cu_->instruction_set); + return StackVisitor::GetVRegOffsetFromQuickCode(code_item, core_spill_mask_, + fp_spill_mask_, frame_size_, v_reg, + cu_->instruction_set); } /* Returns sp-relative offset in bytes for a SReg */ diff --git a/oatdump/oatdump.cc b/oatdump/oatdump.cc index 9ae3b79f62..9512376f39 100644 --- a/oatdump/oatdump.cc +++ b/oatdump/oatdump.cc @@ -1138,10 +1138,13 @@ class OatDumper { os << "\n\tlocals:"; } - uint32_t offset = StackVisitor::GetVRegOffset(code_item, oat_method.GetCoreSpillMask(), - oat_method.GetFpSpillMask(), - oat_method.GetFrameSizeInBytes(), reg, - GetInstructionSet()); + uint32_t offset = StackVisitor::GetVRegOffsetFromQuickCode( + code_item, + oat_method.GetCoreSpillMask(), + oat_method.GetFpSpillMask(), + oat_method.GetFrameSizeInBytes(), + reg, + GetInstructionSet()); os << " v" << reg << "[sp + #" << offset << "]"; } @@ -1170,10 +1173,13 @@ class OatDumper { : oat_method.GetCoreSpillMask(); os << (is_float ? "fr" : "r") << vmap_table.ComputeRegister(spill_mask, vmap_offset, kind); } else { - uint32_t offset = StackVisitor::GetVRegOffset(code_item, oat_method.GetCoreSpillMask(), - oat_method.GetFpSpillMask(), - oat_method.GetFrameSizeInBytes(), reg, - GetInstructionSet()); + uint32_t offset = StackVisitor::GetVRegOffsetFromQuickCode( + code_item, + oat_method.GetCoreSpillMask(), + oat_method.GetFpSpillMask(), + oat_method.GetFrameSizeInBytes(), + reg, + GetInstructionSet()); os << "[sp + #" << offset << "]"; } } diff --git a/runtime/monitor.cc b/runtime/monitor.cc index d41d37e40a..1a80ded13d 100644 --- a/runtime/monitor.cc +++ b/runtime/monitor.cc @@ -1043,8 +1043,11 @@ void Monitor::VisitLocks(StackVisitor* stack_visitor, void (*callback)(mirror::O } uint16_t monitor_register = ((monitor_enter_instruction >> 8) & 0xff); - mirror::Object* o = reinterpret_cast( - stack_visitor->GetVReg(m, monitor_register, kReferenceVReg)); + uint32_t value; + bool success = stack_visitor->GetVReg(m, monitor_register, kReferenceVReg, &value); + CHECK(success) << "Failed to read v" << monitor_register << " of kind " + << kReferenceVReg << " in method " << PrettyMethod(m); + mirror::Object* o = reinterpret_cast(value); callback(o, callback_context); } } diff --git a/runtime/quick_exception_handler.cc b/runtime/quick_exception_handler.cc index 0eb8eca7d2..243260345e 100644 --- a/runtime/quick_exception_handler.cc +++ b/runtime/quick_exception_handler.cc @@ -205,52 +205,97 @@ class DeoptimizeStackVisitor FINAL : public StackVisitor { ShadowFrame* new_frame = ShadowFrame::Create(num_regs, nullptr, h_method.Get(), dex_pc); self_->SetShadowFrameUnderConstruction(new_frame); const std::vector kinds(verifier.DescribeVRegs(dex_pc)); + + // Markers for dead values, used when the verifier knows a Dex register is undefined, + // or when the compiler knows the register has not been initialized, or is not used + // anymore in the method. + static constexpr uint32_t kDeadValue = 0xEBADDE09; + static constexpr uint64_t kLongDeadValue = 0xEBADDE09EBADDE09; for (uint16_t reg = 0; reg < num_regs; ++reg) { VRegKind kind = GetVRegKind(reg, kinds); switch (kind) { case kUndefined: - new_frame->SetVReg(reg, 0xEBADDE09); + new_frame->SetVReg(reg, kDeadValue); break; case kConstant: new_frame->SetVReg(reg, kinds.at((reg * 2) + 1)); break; - case kReferenceVReg: - new_frame->SetVRegReference(reg, - reinterpret_cast(GetVReg(h_method.Get(), - reg, kind))); + case kReferenceVReg: { + uint32_t value = 0; + if (GetVReg(h_method.Get(), reg, kind, &value)) { + new_frame->SetVRegReference(reg, reinterpret_cast(value)); + } else { + new_frame->SetVReg(reg, kDeadValue); + } break; + } case kLongLoVReg: if (GetVRegKind(reg + 1, kinds) == kLongHiVReg) { // Treat it as a "long" register pair. - new_frame->SetVRegLong(reg, GetVRegPair(h_method.Get(), reg, kLongLoVReg, kLongHiVReg)); + uint64_t value = 0; + if (GetVRegPair(h_method.Get(), reg, kLongLoVReg, kLongHiVReg, &value)) { + new_frame->SetVRegLong(reg, value); + } else { + new_frame->SetVRegLong(reg, kLongDeadValue); + } } else { - new_frame->SetVReg(reg, GetVReg(h_method.Get(), reg, kind)); + uint32_t value = 0; + if (GetVReg(h_method.Get(), reg, kind, &value)) { + new_frame->SetVReg(reg, value); + } else { + new_frame->SetVReg(reg, kDeadValue); + } } break; case kLongHiVReg: if (GetVRegKind(reg - 1, kinds) == kLongLoVReg) { // Nothing to do: we treated it as a "long" register pair. } else { - new_frame->SetVReg(reg, GetVReg(h_method.Get(), reg, kind)); + uint32_t value = 0; + if (GetVReg(h_method.Get(), reg, kind, &value)) { + new_frame->SetVReg(reg, value); + } else { + new_frame->SetVReg(reg, kDeadValue); + } } break; case kDoubleLoVReg: if (GetVRegKind(reg + 1, kinds) == kDoubleHiVReg) { - // Treat it as a "double" register pair. - new_frame->SetVRegLong(reg, GetVRegPair(h_method.Get(), reg, kDoubleLoVReg, kDoubleHiVReg)); + uint64_t value = 0; + if (GetVRegPair(h_method.Get(), reg, kDoubleLoVReg, kDoubleHiVReg, &value)) { + // Treat it as a "double" register pair. + new_frame->SetVRegLong(reg, value); + } else { + new_frame->SetVRegLong(reg, kLongDeadValue); + } } else { - new_frame->SetVReg(reg, GetVReg(h_method.Get(), reg, kind)); + uint32_t value = 0; + if (GetVReg(h_method.Get(), reg, kind, &value)) { + new_frame->SetVReg(reg, value); + } else { + new_frame->SetVReg(reg, kDeadValue); + } } break; case kDoubleHiVReg: if (GetVRegKind(reg - 1, kinds) == kDoubleLoVReg) { // Nothing to do: we treated it as a "double" register pair. } else { - new_frame->SetVReg(reg, GetVReg(h_method.Get(), reg, kind)); + uint32_t value = 0; + if (GetVReg(h_method.Get(), reg, kind, &value)) { + new_frame->SetVReg(reg, value); + } else { + new_frame->SetVReg(reg, kDeadValue); + } } break; default: - new_frame->SetVReg(reg, GetVReg(h_method.Get(), reg, kind)); + uint32_t value = 0; + if (GetVReg(h_method.Get(), reg, kind, &value)) { + new_frame->SetVReg(reg, value); + } else { + new_frame->SetVReg(reg, kDeadValue); + } break; } } diff --git a/runtime/stack.cc b/runtime/stack.cc index b8ca21e867..47b85ad345 100644 --- a/runtime/stack.cc +++ b/runtime/stack.cc @@ -137,7 +137,11 @@ mirror::Object* StackVisitor::GetThisObject() const { return nullptr; } else { uint16_t reg = code_item->registers_size_ - code_item->ins_size_; - return reinterpret_cast(GetVReg(m, reg, kReferenceVReg)); + uint32_t value = 0; + bool success = GetVReg(m, reg, kReferenceVReg, &value); + // We currently always guarantee the `this` object is live throughout the method. + CHECK(success) << "Failed to read the this object in " << PrettyMethod(m); + return reinterpret_cast(value); } } } @@ -181,8 +185,8 @@ bool StackVisitor::GetVRegFromQuickCode(mirror::ArtMethod* m, uint16_t vreg, VRe const DexFile::CodeItem* code_item = m->GetCodeItem(); DCHECK(code_item != nullptr) << PrettyMethod(m); // Can't be NULL or how would we compile // its instructions? - *val = *GetVRegAddr(cur_quick_frame_, code_item, frame_info.CoreSpillMask(), - frame_info.FpSpillMask(), frame_info.FrameSizeInBytes(), vreg); + *val = *GetVRegAddrFromQuickCode(cur_quick_frame_, code_item, frame_info.CoreSpillMask(), + frame_info.FpSpillMask(), frame_info.FrameSizeInBytes(), vreg); return true; } } @@ -291,8 +295,9 @@ bool StackVisitor::GetVRegPairFromQuickCode(mirror::ArtMethod* m, uint16_t vreg, const DexFile::CodeItem* code_item = m->GetCodeItem(); DCHECK(code_item != nullptr) << PrettyMethod(m); // Can't be NULL or how would we compile // its instructions? - uint32_t* addr = GetVRegAddr(cur_quick_frame_, code_item, frame_info.CoreSpillMask(), - frame_info.FpSpillMask(), frame_info.FrameSizeInBytes(), vreg); + uint32_t* addr = GetVRegAddrFromQuickCode( + cur_quick_frame_, code_item, frame_info.CoreSpillMask(), + frame_info.FpSpillMask(), frame_info.FrameSizeInBytes(), vreg); *val = *reinterpret_cast(addr); return true; } @@ -365,8 +370,9 @@ bool StackVisitor::SetVRegFromQuickCode(mirror::ArtMethod* m, uint16_t vreg, uin const DexFile::CodeItem* code_item = m->GetCodeItem(); DCHECK(code_item != nullptr) << PrettyMethod(m); // Can't be NULL or how would we compile // its instructions? - uint32_t* addr = GetVRegAddr(cur_quick_frame_, code_item, frame_info.CoreSpillMask(), - frame_info.FpSpillMask(), frame_info.FrameSizeInBytes(), vreg); + uint32_t* addr = GetVRegAddrFromQuickCode( + cur_quick_frame_, code_item, frame_info.CoreSpillMask(), + frame_info.FpSpillMask(), frame_info.FrameSizeInBytes(), vreg); *addr = new_value; return true; } @@ -494,8 +500,9 @@ bool StackVisitor::SetVRegPairFromQuickCode(mirror::ArtMethod* m, uint16_t vreg, const DexFile::CodeItem* code_item = m->GetCodeItem(); DCHECK(code_item != nullptr) << PrettyMethod(m); // Can't be NULL or how would we compile // its instructions? - uint32_t* addr = GetVRegAddr(cur_quick_frame_, code_item, frame_info.CoreSpillMask(), - frame_info.FpSpillMask(), frame_info.FrameSizeInBytes(), vreg); + uint32_t* addr = GetVRegAddrFromQuickCode( + cur_quick_frame_, code_item, frame_info.CoreSpillMask(), + frame_info.FpSpillMask(), frame_info.FrameSizeInBytes(), vreg); *reinterpret_cast(addr) = new_value; return true; } diff --git a/runtime/stack.h b/runtime/stack.h index 13bd47fa9d..aab54ba628 100644 --- a/runtime/stack.h +++ b/runtime/stack.h @@ -486,29 +486,10 @@ class StackVisitor { bool GetVReg(mirror::ArtMethod* m, uint16_t vreg, VRegKind kind, uint32_t* val) const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); - uint32_t GetVReg(mirror::ArtMethod* m, uint16_t vreg, VRegKind kind) const - SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { - uint32_t val; - bool success = GetVReg(m, vreg, kind, &val); - CHECK(success) << "Failed to read v" << vreg << " of kind " << kind << " in method " - << PrettyMethod(m); - return val; - } - bool GetVRegPair(mirror::ArtMethod* m, uint16_t vreg, VRegKind kind_lo, VRegKind kind_hi, uint64_t* val) const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); - uint64_t GetVRegPair(mirror::ArtMethod* m, uint16_t vreg, VRegKind kind_lo, - VRegKind kind_hi) const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) { - uint64_t val; - bool success = GetVRegPair(m, vreg, kind_lo, kind_hi, &val); - CHECK(success) << "Failed to read vreg pair " << vreg - << " of kind [" << kind_lo << "," << kind_hi << "] in method " - << PrettyMethod(m); - return val; - } - bool SetVReg(mirror::ArtMethod* m, uint16_t vreg, uint32_t new_value, VRegKind kind) SHARED_LOCKS_REQUIRED(Locks::mutator_lock_); @@ -519,11 +500,12 @@ class StackVisitor { uintptr_t* GetGPRAddress(uint32_t reg) const; // This is a fast-path for getting/setting values in a quick frame. - uint32_t* GetVRegAddr(StackReference* cur_quick_frame, - const DexFile::CodeItem* code_item, - uint32_t core_spills, uint32_t fp_spills, size_t frame_size, - uint16_t vreg) const { - int offset = GetVRegOffset(code_item, core_spills, fp_spills, frame_size, vreg, kRuntimeISA); + uint32_t* GetVRegAddrFromQuickCode(StackReference* cur_quick_frame, + const DexFile::CodeItem* code_item, + uint32_t core_spills, uint32_t fp_spills, size_t frame_size, + uint16_t vreg) const { + int offset = GetVRegOffsetFromQuickCode( + code_item, core_spills, fp_spills, frame_size, vreg, kRuntimeISA); DCHECK_EQ(cur_quick_frame, GetCurrentQuickFrame()); uint8_t* vreg_addr = reinterpret_cast(cur_quick_frame) + offset; return reinterpret_cast(vreg_addr); @@ -582,9 +564,9 @@ class StackVisitor { * | StackReference | ... (reg == num_total_code_regs == special_temp_value) <<== sp, 16-byte aligned * +===============================+ */ - static int GetVRegOffset(const DexFile::CodeItem* code_item, - uint32_t core_spills, uint32_t fp_spills, - size_t frame_size, int reg, InstructionSet isa) { + static int GetVRegOffsetFromQuickCode(const DexFile::CodeItem* code_item, + uint32_t core_spills, uint32_t fp_spills, + size_t frame_size, int reg, InstructionSet isa) { DCHECK_EQ(frame_size & (kStackAlignment - 1), 0U); DCHECK_NE(reg, -1); int spill_size = POPCOUNT(core_spills) * GetBytesPerGprSpillLocation(isa) diff --git a/runtime/thread.cc b/runtime/thread.cc index e8e93555ac..8e98d530a9 100644 --- a/runtime/thread.cc +++ b/runtime/thread.cc @@ -2216,9 +2216,9 @@ class ReferenceMapVisitor : public StackVisitor { } } else { StackReference* ref_addr = - reinterpret_cast*>( - GetVRegAddr(cur_quick_frame, code_item, frame_info.CoreSpillMask(), - frame_info.FpSpillMask(), frame_info.FrameSizeInBytes(), reg)); + reinterpret_cast*>(GetVRegAddrFromQuickCode( + cur_quick_frame, code_item, frame_info.CoreSpillMask(), + frame_info.FpSpillMask(), frame_info.FrameSizeInBytes(), reg)); mirror::Object* ref = ref_addr->AsMirrorPtr(); if (ref != nullptr) { mirror::Object* new_ref = ref; -- cgit v1.2.3