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

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<mirror::Object*>(
-        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<mirror::Object*>(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<int32_t> 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<mirror::Object*>(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<mirror::Object*>(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<mirror::Object*>(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<mirror::Object*>(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<uint64_t*>(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<uint64_t*>(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<mirror::ArtMethod>* 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<mirror::ArtMethod>* 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<uint8_t*>(cur_quick_frame) + offset;
     return reinterpret_cast<uint32_t*>(vreg_addr);
@@ -582,9 +564,9 @@ class StackVisitor {
    *     | StackReference<ArtMethod>     |  ... (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<mirror::Object>* ref_addr =
-                    reinterpret_cast<StackReference<mirror::Object>*>(
-                        GetVRegAddr(cur_quick_frame, code_item, frame_info.CoreSpillMask(),
-                                    frame_info.FpSpillMask(), frame_info.FrameSizeInBytes(), reg));
+                    reinterpret_cast<StackReference<mirror::Object>*>(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;