commit 4cbfadc29b4ba89b32fa6eb55a287db78d12a45c
author Nicolas Geoffray <ngeoffray@google.com> Wed Oct 10 16:09:43 2018 +0100
committer Nicolas Geoffray <ngeoffray@google.com> Fri Oct 12 10:07:41 2018 +0100
tree 70cd681c4326ab8cf034da6c0fb0fef222cd67b1
parent a48eb7e0690187618d2824a7d9b5601e7f5cdf80

Remove CHECK that GetThis() must always work.

Ensures GetVReg of a reference on compiled code checks that the
value is indeed a reference from the stack maps.

bug: 116683601
bug: 117452149
Test: 686-get-this
Change-Id: I3a370c45786a4892c7888491bec6d5ae64672f6c

runtime/interpreter/shadow_frame.h

diff --git a/runtime/interpreter/shadow_frame.h b/runtime/interpreter/shadow_frame.h
index 91371d1..88eb413 100644
--- a/runtime/interpreter/shadow_frame.h
+++ b/runtime/interpreter/shadow_frame.h
@@ -179,12 +179,8 @@
   mirror::Object* GetVRegReference(size_t i) const REQUIRES_SHARED(Locks::mutator_lock_) {
     DCHECK_LT(i, NumberOfVRegs());
     mirror::Object* ref;
-    if (HasReferenceArray()) {
-      ref = References()[i].AsMirrorPtr();
-    } else {
-      const uint32_t* vreg_ptr = &vregs_[i];
-      ref = reinterpret_cast<const StackReference<mirror::Object>*>(vreg_ptr)->AsMirrorPtr();
-    }
+    DCHECK(HasReferenceArray());
+    ref = References()[i].AsMirrorPtr();
     ReadBarrier::MaybeAssertToSpaceInvariant(ref);
     if (kVerifyFlags & kVerifyReads) {
       VerifyObject(ref);

runtime/stack.cc

diff --git a/runtime/stack.cc b/runtime/stack.cc
index eb9c661..25939d2 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -139,9 +139,9 @@
     } else {
       uint16_t reg = accessor.RegistersSize() - accessor.InsSize();
       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 " << ArtMethod::PrettyMethod(m);
+      if (!GetVReg(m, reg, kReferenceVReg, &value)) {
+        return nullptr;
+      }
       return reinterpret_cast<mirror::Object*>(value);
     }
   }
@@ -223,20 +223,39 @@
   switch (location_kind) {
     case DexRegisterLocation::Kind::kInStack: {
       const int32_t offset = dex_register_map[vreg].GetStackOffsetInBytes();
+      BitMemoryRegion stack_mask = code_info.GetStackMaskOf(stack_map);
+      if (kind == kReferenceVReg && !stack_mask.LoadBit(offset / kFrameSlotSize)) {
+        return false;
+      }
       const uint8_t* addr = reinterpret_cast<const uint8_t*>(cur_quick_frame_) + offset;
       *val = *reinterpret_cast<const uint32_t*>(addr);
       return true;
     }
-    case DexRegisterLocation::Kind::kInRegister:
+    case DexRegisterLocation::Kind::kInRegister: {
+      uint32_t register_mask = code_info.GetRegisterMaskOf(stack_map);
+      uint32_t reg = dex_register_map[vreg].GetMachineRegister();
+      if (kind == kReferenceVReg && !(register_mask & (1 << reg))) {
+        return false;
+      }
+      return GetRegisterIfAccessible(reg, kind, val);
+    }
     case DexRegisterLocation::Kind::kInRegisterHigh:
     case DexRegisterLocation::Kind::kInFpuRegister:
     case DexRegisterLocation::Kind::kInFpuRegisterHigh: {
+      if (kind == kReferenceVReg) {
+        return false;
+      }
       uint32_t reg = dex_register_map[vreg].GetMachineRegister();
       return GetRegisterIfAccessible(reg, kind, val);
     }
-    case DexRegisterLocation::Kind::kConstant:
-      *val = dex_register_map[vreg].GetConstant();
+    case DexRegisterLocation::Kind::kConstant: {
+      uint32_t result = dex_register_map[vreg].GetConstant();
+      if (kind == kReferenceVReg && result != 0) {
+        return false;
+      }
+      *val = result;
       return true;
+    }
     case DexRegisterLocation::Kind::kNone:
       return false;
     default: