Refactor register access from StackVisitor

Moves register access checking up to StackVisitor by adding methods
IsAccessibleGPR and IsAccessibleFPR in Context class. It allows to
simplify GetGPR/FPR and SetGPR/FPR methods in the Context class (and
its subclasses).

Also simplifies code in StackVisitor by adding IsAccessibleRegister,
GetRegister and SetRegister methods which then call either GPR or FPR
specific methods in Context depending on the nature of the accessed
register.

Bug: 18547544
Bug: 19106446
Change-Id: I6e707608d935a71571d0e975a6e766053de3763a

15 files changed, 253 insertions, 269 deletions

diff --git a/runtime/arch/arm/context_arm.cc b/runtime/arch/arm/context_arm.cc
index 9e8d282a87..c181e43132 100644
--- a/runtime/arch/arm/context_arm.cc
+++ b/runtime/arch/arm/context_arm.cc
@@ -67,26 +67,18 @@ void ArmContext::FillCalleeSaves(const StackVisitor& fr) {
   }
 }
 
-bool ArmContext::SetGPR(uint32_t reg, uintptr_t value) {
+void ArmContext::SetGPR(uint32_t reg, uintptr_t value) {
   DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfCoreRegisters));
+  DCHECK(IsAccessibleGPR(reg));
   DCHECK_NE(gprs_[reg], &gZero);  // Can't overwrite this static value since they are never reset.
-  if (gprs_[reg] != nullptr) {
-    *gprs_[reg] = value;
-    return true;
-  } else {
-    return false;
-  }
+  *gprs_[reg] = value;
 }
 
-bool ArmContext::SetFPR(uint32_t reg, uintptr_t value) {
+void ArmContext::SetFPR(uint32_t reg, uintptr_t value) {
   DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfSRegisters));
+  DCHECK(IsAccessibleFPR(reg));
   DCHECK_NE(fprs_[reg], &gZero);  // Can't overwrite this static value since they are never reset.
-  if (fprs_[reg] != nullptr) {
-    *fprs_[reg] = value;
-    return true;
-  } else {
-    return false;
-  }
+  *fprs_[reg] = value;
 }
 
 void ArmContext::SmashCallerSaves() {
diff --git a/runtime/arch/arm/context_arm.h b/runtime/arch/arm/context_arm.h
index e894f169d3..1ca973eb80 100644
--- a/runtime/arch/arm/context_arm.h
+++ b/runtime/arch/arm/context_arm.h
@@ -37,13 +37,16 @@ class ArmContext : public Context {
   void FillCalleeSaves(const StackVisitor& fr) OVERRIDE SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   void SetSP(uintptr_t new_sp) OVERRIDE {
-    bool success = SetGPR(SP, new_sp);
-    CHECK(success) << "Failed to set SP register";
+    SetGPR(SP, new_sp);
   }
 
   void SetPC(uintptr_t new_pc) OVERRIDE {
-    bool success = SetGPR(PC, new_pc);
-    CHECK(success) << "Failed to set PC register";
+    SetGPR(PC, new_pc);
+  }
+
+  bool IsAccessibleGPR(uint32_t reg) OVERRIDE {
+    DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfCoreRegisters));
+    return gprs_[reg] != nullptr;
   }
 
   uintptr_t* GetGPRAddress(uint32_t reg) OVERRIDE {
@@ -51,31 +54,26 @@ class ArmContext : public Context {
     return gprs_[reg];
   }
 
-  bool GetGPR(uint32_t reg, uintptr_t* val) OVERRIDE {
+  uintptr_t GetGPR(uint32_t reg) OVERRIDE {
     DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfCoreRegisters));
-    if (gprs_[reg] == nullptr) {
-      return false;
-    } else {
-      DCHECK(val != nullptr);
-      *val = *gprs_[reg];
-      return true;
-    }
+    DCHECK(IsAccessibleGPR(reg));
+    return *gprs_[reg];
   }
 
-  bool SetGPR(uint32_t reg, uintptr_t value) OVERRIDE;
+  void SetGPR(uint32_t reg, uintptr_t value) OVERRIDE;
+
+  bool IsAccessibleFPR(uint32_t reg) OVERRIDE {
+    DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfSRegisters));
+    return fprs_[reg] != nullptr;
+  }
 
-  bool GetFPR(uint32_t reg, uintptr_t* val) OVERRIDE {
+  uintptr_t GetFPR(uint32_t reg) OVERRIDE {
     DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfSRegisters));
-    if (fprs_[reg] == nullptr) {
-      return false;
-    } else {
-      DCHECK(val != nullptr);
-      *val = *fprs_[reg];
-      return true;
-    }
+    DCHECK(IsAccessibleFPR(reg));
+    return *fprs_[reg];
   }
 
-  bool SetFPR(uint32_t reg, uintptr_t value) OVERRIDE;
+  void SetFPR(uint32_t reg, uintptr_t value) OVERRIDE;
 
   void SmashCallerSaves() OVERRIDE;
   void DoLongJump() OVERRIDE;
diff --git a/runtime/arch/arm64/context_arm64.cc b/runtime/arch/arm64/context_arm64.cc
index 0a3148016a..7fc0555c5b 100644
--- a/runtime/arch/arm64/context_arm64.cc
+++ b/runtime/arch/arm64/context_arm64.cc
@@ -70,27 +70,19 @@ void Arm64Context::FillCalleeSaves(const StackVisitor& fr) {
   }
 }
 
-bool Arm64Context::SetGPR(uint32_t reg, uintptr_t value) {
+void Arm64Context::SetGPR(uint32_t reg, uintptr_t value) {
   DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfXRegisters));
   DCHECK_NE(reg, static_cast<uint32_t>(XZR));
+  DCHECK(IsAccessibleGPR(reg));
   DCHECK_NE(gprs_[reg], &gZero);  // Can't overwrite this static value since they are never reset.
-  if (gprs_[reg] != nullptr) {
-    *gprs_[reg] = value;
-    return true;
-  } else {
-    return false;
-  }
+  *gprs_[reg] = value;
 }
 
-bool Arm64Context::SetFPR(uint32_t reg, uintptr_t value) {
+void Arm64Context::SetFPR(uint32_t reg, uintptr_t value) {
   DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfDRegisters));
+  DCHECK(IsAccessibleFPR(reg));
   DCHECK_NE(fprs_[reg], &gZero);  // Can't overwrite this static value since they are never reset.
-  if (fprs_[reg] != nullptr) {
-    *fprs_[reg] = value;
-    return true;
-  } else {
-    return false;
-  }
+  *fprs_[reg] = value;
 }
 
 void Arm64Context::SmashCallerSaves() {
diff --git a/runtime/arch/arm64/context_arm64.h b/runtime/arch/arm64/context_arm64.h
index d9a433bbce..6a4485b259 100644
--- a/runtime/arch/arm64/context_arm64.h
+++ b/runtime/arch/arm64/context_arm64.h
@@ -37,13 +37,16 @@ class Arm64Context : public Context {
   void FillCalleeSaves(const StackVisitor& fr) OVERRIDE SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   void SetSP(uintptr_t new_sp) OVERRIDE {
-    bool success = SetGPR(SP, new_sp);
-    CHECK(success) << "Failed to set SP register";
+    SetGPR(SP, new_sp);
   }
 
   void SetPC(uintptr_t new_lr) OVERRIDE {
-    bool success = SetGPR(LR, new_lr);
-    CHECK(success) << "Failed to set LR register";
+    SetGPR(LR, new_lr);
+  }
+
+  bool IsAccessibleGPR(uint32_t reg) OVERRIDE {
+    DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfXRegisters));
+    return gprs_[reg] != nullptr;
   }
 
   uintptr_t* GetGPRAddress(uint32_t reg) OVERRIDE {
@@ -51,31 +54,26 @@ class Arm64Context : public Context {
     return gprs_[reg];
   }
 
-  bool GetGPR(uint32_t reg, uintptr_t* val) OVERRIDE {
+  uintptr_t GetGPR(uint32_t reg) OVERRIDE {
     DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfXRegisters));
-    if (gprs_[reg] == nullptr) {
-      return false;
-    } else {
-      DCHECK(val != nullptr);
-      *val = *gprs_[reg];
-      return true;
-    }
+    DCHECK(IsAccessibleGPR(reg));
+    return *gprs_[reg];
   }
 
-  bool SetGPR(uint32_t reg, uintptr_t value) OVERRIDE;
+  void SetGPR(uint32_t reg, uintptr_t value) OVERRIDE;
+
+  bool IsAccessibleFPR(uint32_t reg) OVERRIDE {
+    DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfDRegisters));
+    return fprs_[reg] != nullptr;
+  }
 
-  bool GetFPR(uint32_t reg, uintptr_t* val) OVERRIDE {
+  uintptr_t GetFPR(uint32_t reg) OVERRIDE {
     DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfDRegisters));
-    if (fprs_[reg] == nullptr) {
-      return false;
-    } else {
-      DCHECK(val != nullptr);
-      *val = *fprs_[reg];
-      return true;
-    }
+    DCHECK(IsAccessibleFPR(reg));
+    return *fprs_[reg];
   }
 
-  bool SetFPR(uint32_t reg, uintptr_t value) OVERRIDE;
+  void SetFPR(uint32_t reg, uintptr_t value) OVERRIDE;
 
   void SmashCallerSaves() OVERRIDE;
   void DoLongJump() OVERRIDE;
diff --git a/runtime/arch/context.h b/runtime/arch/context.h
index 20a84dd902..ed8cab0cd2 100644
--- a/runtime/arch/context.h
+++ b/runtime/arch/context.h
@@ -49,24 +49,30 @@ class Context {
   // Sets the program counter value.
   virtual void SetPC(uintptr_t new_pc) = 0;
 
+  // Returns whether the given GPR is accessible (read or write).
+  virtual bool IsAccessibleGPR(uint32_t reg) = 0;
+
   // Gets the given GPRs address.
   virtual uintptr_t* GetGPRAddress(uint32_t reg) = 0;
 
-  // Reads the given GPR. Returns true if we successfully read the register and
-  // set its value into 'val', returns false otherwise.
-  virtual bool GetGPR(uint32_t reg, uintptr_t* val) = 0;
+  // Reads the given GPR. The caller is responsible for checking the register
+  // is accessible with IsAccessibleGPR.
+  virtual uintptr_t GetGPR(uint32_t reg) = 0;
+
+  // Sets the given GPR. The caller is responsible for checking the register
+  // is accessible with IsAccessibleGPR.
+  virtual void SetGPR(uint32_t reg, uintptr_t value) = 0;
 
-  // Sets the given GPR. Returns true if we successfully write the given value
-  // into the register, returns false otherwise.
-  virtual bool SetGPR(uint32_t reg, uintptr_t value) = 0;
+  // Returns whether the given FPR is accessible (read or write).
+  virtual bool IsAccessibleFPR(uint32_t reg) = 0;
 
-  // Reads the given FPR. Returns true if we successfully read the register and
-  // set its value into 'val', returns false otherwise.
-  virtual bool GetFPR(uint32_t reg, uintptr_t* val) = 0;
+  // Reads the given FPR. The caller is responsible for checking the register
+  // is accessible with IsAccessibleFPR.
+  virtual uintptr_t GetFPR(uint32_t reg) = 0;
 
-  // Sets the given FPR. Returns true if we successfully write the given value
-  // into the register, returns false otherwise.
-  virtual bool SetFPR(uint32_t reg, uintptr_t value) = 0;
+  // Sets the given FPR. The caller is responsible for checking the register
+  // is accessible with IsAccessibleFPR.
+  virtual void SetFPR(uint32_t reg, uintptr_t value) = 0;
 
   // Smashes the caller save registers. If we're throwing, we don't want to return bogus values.
   virtual void SmashCallerSaves() = 0;
diff --git a/runtime/arch/mips/context_mips.cc b/runtime/arch/mips/context_mips.cc
index e1f6c06d1b..6c0ab98947 100644
--- a/runtime/arch/mips/context_mips.cc
+++ b/runtime/arch/mips/context_mips.cc
@@ -67,26 +67,18 @@ void MipsContext::FillCalleeSaves(const StackVisitor& fr) {
   }
 }
 
-bool MipsContext::SetGPR(uint32_t reg, uintptr_t value) {
+void MipsContext::SetGPR(uint32_t reg, uintptr_t value) {
   CHECK_LT(reg, static_cast<uint32_t>(kNumberOfCoreRegisters));
+  DCHECK(IsAccessibleGPR(reg));
   CHECK_NE(gprs_[reg], &gZero);  // Can't overwrite this static value since they are never reset.
-  if (gprs_[reg] != nullptr) {
-    *gprs_[reg] = value;
-    return true;
-  } else {
-    return false;
-  }
+  *gprs_[reg] = value;
 }
 
-bool MipsContext::SetFPR(uint32_t reg, uintptr_t value) {
+void MipsContext::SetFPR(uint32_t reg, uintptr_t value) {
   CHECK_LT(reg, static_cast<uint32_t>(kNumberOfFRegisters));
+  DCHECK(IsAccessibleFPR(reg));
   CHECK_NE(fprs_[reg], &gZero);  // Can't overwrite this static value since they are never reset.
-  if (fprs_[reg] != nullptr) {
-    *fprs_[reg] = value;
-    return true;
-  } else {
-    return false;
-  }
+  *fprs_[reg] = value;
 }
 
 void MipsContext::SmashCallerSaves() {
diff --git a/runtime/arch/mips/context_mips.h b/runtime/arch/mips/context_mips.h
index f2ee335d48..d8a0b673c8 100644
--- a/runtime/arch/mips/context_mips.h
+++ b/runtime/arch/mips/context_mips.h
@@ -36,13 +36,16 @@ class MipsContext : public Context {
   void FillCalleeSaves(const StackVisitor& fr) OVERRIDE SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   void SetSP(uintptr_t new_sp) OVERRIDE {
-    bool success = SetGPR(SP, new_sp);
-    CHECK(success) << "Failed to set SP register";
+    SetGPR(SP, new_sp);
   }
 
   void SetPC(uintptr_t new_pc) OVERRIDE {
-    bool success = SetGPR(RA, new_pc);
-    CHECK(success) << "Failed to set RA register";
+    SetGPR(RA, new_pc);
+  }
+
+  bool IsAccessibleGPR(uint32_t reg) OVERRIDE {
+    CHECK_LT(reg, static_cast<uint32_t>(kNumberOfCoreRegisters));
+    return gprs_[reg] != nullptr;
   }
 
   uintptr_t* GetGPRAddress(uint32_t reg) OVERRIDE {
@@ -50,31 +53,26 @@ class MipsContext : public Context {
     return gprs_[reg];
   }
 
-  bool GetGPR(uint32_t reg, uintptr_t* val) OVERRIDE {
+  uintptr_t GetGPR(uint32_t reg) OVERRIDE {
     CHECK_LT(reg, static_cast<uint32_t>(kNumberOfCoreRegisters));
-    if (gprs_[reg] == nullptr) {
-      return false;
-    } else {
-      DCHECK(val != nullptr);
-      *val = *gprs_[reg];
-      return true;
-    }
+    DCHECK(IsAccessibleGPR(reg));
+    return *gprs_[reg];
   }
 
-  bool SetGPR(uint32_t reg, uintptr_t value) OVERRIDE;
+  void SetGPR(uint32_t reg, uintptr_t value) OVERRIDE;
+
+  bool IsAccessibleFPR(uint32_t reg) OVERRIDE {
+    CHECK_LT(reg, static_cast<uint32_t>(kNumberOfFRegisters));
+    return fprs_[reg] != nullptr;
+  }
 
-  bool GetFPR(uint32_t reg, uintptr_t* val) OVERRIDE {
+  uintptr_t GetFPR(uint32_t reg) OVERRIDE {
     CHECK_LT(reg, static_cast<uint32_t>(kNumberOfFRegisters));
-    if (fprs_[reg] == nullptr) {
-      return false;
-    } else {
-      DCHECK(val != nullptr);
-      *val = *fprs_[reg];
-      return true;
-    }
+    DCHECK(IsAccessibleFPR(reg));
+    return *fprs_[reg];
   }
 
-  bool SetFPR(uint32_t reg, uintptr_t value) OVERRIDE;
+  void SetFPR(uint32_t reg, uintptr_t value) OVERRIDE;
 
   void SmashCallerSaves() OVERRIDE;
   void DoLongJump() OVERRIDE;
diff --git a/runtime/arch/mips64/context_mips64.cc b/runtime/arch/mips64/context_mips64.cc
index 7523adee31..1c96bd43ab 100644
--- a/runtime/arch/mips64/context_mips64.cc
+++ b/runtime/arch/mips64/context_mips64.cc
@@ -67,26 +67,18 @@ void Mips64Context::FillCalleeSaves(const StackVisitor& fr) {
   }
 }
 
-bool Mips64Context::SetGPR(uint32_t reg, uintptr_t value) {
+void Mips64Context::SetGPR(uint32_t reg, uintptr_t value) {
   CHECK_LT(reg, static_cast<uint32_t>(kNumberOfGpuRegisters));
+  DCHECK(IsAccessibleGPR(reg));
   CHECK_NE(gprs_[reg], &gZero);  // Can't overwrite this static value since they are never reset.
-  if (gprs_[reg] != nullptr) {
-    *gprs_[reg] = value;
-    return true;
-  } else {
-    return false;
-  }
+  *gprs_[reg] = value;
 }
 
-bool Mips64Context::SetFPR(uint32_t reg, uintptr_t value) {
+void Mips64Context::SetFPR(uint32_t reg, uintptr_t value) {
   CHECK_LT(reg, static_cast<uint32_t>(kNumberOfFpuRegisters));
+  DCHECK(IsAccessibleFPR(reg));
   CHECK_NE(fprs_[reg], &gZero);  // Can't overwrite this static value since they are never reset.
-  if (fprs_[reg] != nullptr) {
-    *fprs_[reg] = value;
-    return true;
-  } else {
-    return false;
-  }
+  *fprs_[reg] = value;
 }
 
 void Mips64Context::SmashCallerSaves() {
diff --git a/runtime/arch/mips64/context_mips64.h b/runtime/arch/mips64/context_mips64.h
index 4ba5f13b14..104672348b 100644
--- a/runtime/arch/mips64/context_mips64.h
+++ b/runtime/arch/mips64/context_mips64.h
@@ -36,13 +36,16 @@ class Mips64Context : public Context {
   void FillCalleeSaves(const StackVisitor& fr) OVERRIDE SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   void SetSP(uintptr_t new_sp) OVERRIDE {
-    bool success = SetGPR(SP, new_sp);
-    CHECK(success) << "Failed to set SP register";
+    SetGPR(SP, new_sp);
   }
 
   void SetPC(uintptr_t new_pc) OVERRIDE {
-    bool success = SetGPR(RA, new_pc);
-    CHECK(success) << "Failed to set RA register";
+    SetGPR(RA, new_pc);
+  }
+
+  bool IsAccessibleGPR(uint32_t reg) OVERRIDE {
+    DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfGpuRegisters));
+    return gprs_[reg] != nullptr;
   }
 
   uintptr_t* GetGPRAddress(uint32_t reg) OVERRIDE {
@@ -50,31 +53,26 @@ class Mips64Context : public Context {
     return gprs_[reg];
   }
 
-  bool GetGPR(uint32_t reg, uintptr_t* val) OVERRIDE {
+  uintptr_t GetGPR(uint32_t reg) OVERRIDE {
     CHECK_LT(reg, static_cast<uint32_t>(kNumberOfGpuRegisters));
-    if (gprs_[reg] == nullptr) {
-      return false;
-    } else {
-      DCHECK(val != nullptr);
-      *val = *gprs_[reg];
-      return true;
-    }
+    DCHECK(IsAccessibleGPR(reg));
+    return *gprs_[reg];
   }
 
-  bool SetGPR(uint32_t reg, uintptr_t value) OVERRIDE;
+  void SetGPR(uint32_t reg, uintptr_t value) OVERRIDE;
+
+  bool IsAccessibleFPR(uint32_t reg) OVERRIDE {
+    CHECK_LT(reg, static_cast<uint32_t>(kNumberOfFpuRegisters));
+    return fprs_[reg] != nullptr;
+  }
 
-  bool GetFPR(uint32_t reg, uintptr_t* val) OVERRIDE {
+  uintptr_t GetFPR(uint32_t reg) OVERRIDE {
     CHECK_LT(reg, static_cast<uint32_t>(kNumberOfFpuRegisters));
-    if (fprs_[reg] == nullptr) {
-      return false;
-    } else {
-      DCHECK(val != nullptr);
-      *val = *fprs_[reg];
-      return true;
-    }
+    DCHECK(IsAccessibleFPR(reg));
+    return *fprs_[reg];
   }
 
-  bool SetFPR(uint32_t reg, uintptr_t value) OVERRIDE;
+  void SetFPR(uint32_t reg, uintptr_t value) OVERRIDE;
 
   void SmashCallerSaves() OVERRIDE;
   void DoLongJump() OVERRIDE;
diff --git a/runtime/arch/x86/context_x86.cc b/runtime/arch/x86/context_x86.cc
index d7cc70488d..4ea4684f06 100644
--- a/runtime/arch/x86/context_x86.cc
+++ b/runtime/arch/x86/context_x86.cc
@@ -83,26 +83,18 @@ void X86Context::SmashCallerSaves() {
   memset(&fprs_[0], '\0', sizeof(fprs_));
 }
 
-bool X86Context::SetGPR(uint32_t reg, uintptr_t value) {
+void X86Context::SetGPR(uint32_t reg, uintptr_t value) {
   CHECK_LT(reg, static_cast<uint32_t>(kNumberOfCpuRegisters));
+  DCHECK(IsAccessibleGPR(reg));
   CHECK_NE(gprs_[reg], &gZero);
-  if (gprs_[reg] != nullptr) {
-    *gprs_[reg] = value;
-    return true;
-  } else {
-    return false;
-  }
+  *gprs_[reg] = value;
 }
 
-bool X86Context::SetFPR(uint32_t reg, uintptr_t value) {
+void X86Context::SetFPR(uint32_t reg, uintptr_t value) {
   CHECK_LT(reg, static_cast<uint32_t>(kNumberOfFloatRegisters));
+  DCHECK(IsAccessibleFPR(reg));
   CHECK_NE(fprs_[reg], reinterpret_cast<const uint32_t*>(&gZero));
-  if (fprs_[reg] != nullptr) {
-    *fprs_[reg] = value;
-    return true;
-  } else {
-    return false;
-  }
+  *fprs_[reg] = value;
 }
 
 void X86Context::DoLongJump() {
diff --git a/runtime/arch/x86/context_x86.h b/runtime/arch/x86/context_x86.h
index d18be54fee..f73fa517af 100644
--- a/runtime/arch/x86/context_x86.h
+++ b/runtime/arch/x86/context_x86.h
@@ -36,44 +36,43 @@ class X86Context : public Context {
   void FillCalleeSaves(const StackVisitor& fr) OVERRIDE SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   void SetSP(uintptr_t new_sp) OVERRIDE {
-    bool success = SetGPR(ESP, new_sp);
-    CHECK(success) << "Failed to set ESP register";
+    SetGPR(ESP, new_sp);
   }
 
   void SetPC(uintptr_t new_pc) OVERRIDE {
     eip_ = new_pc;
   }
 
+  bool IsAccessibleGPR(uint32_t reg) OVERRIDE {
+    DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfCpuRegisters));
+    return gprs_[reg] != nullptr;
+  }
+
   uintptr_t* GetGPRAddress(uint32_t reg) OVERRIDE {
     DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfCpuRegisters));
     return gprs_[reg];
   }
 
-  bool GetGPR(uint32_t reg, uintptr_t* val) OVERRIDE {
+  uintptr_t GetGPR(uint32_t reg) OVERRIDE {
     DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfCpuRegisters));
-    if (gprs_[reg] == nullptr) {
-      return false;
-    } else {
-      DCHECK(val != nullptr);
-      *val = *gprs_[reg];
-      return true;
-    }
+    DCHECK(IsAccessibleGPR(reg));
+    return *gprs_[reg];
   }
 
-  bool SetGPR(uint32_t reg, uintptr_t value) OVERRIDE;
+  void SetGPR(uint32_t reg, uintptr_t value) OVERRIDE;
+
+  bool IsAccessibleFPR(uint32_t reg) OVERRIDE {
+    DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfFloatRegisters));
+    return fprs_[reg] != nullptr;
+  }
 
-  bool GetFPR(uint32_t reg, uintptr_t* val) OVERRIDE {
+  uintptr_t GetFPR(uint32_t reg) OVERRIDE {
     DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfFloatRegisters));
-    if (fprs_[reg] == nullptr) {
-      return false;
-    } else {
-      DCHECK(val != nullptr);
-      *val = *fprs_[reg];
-      return true;
-    }
+    DCHECK(IsAccessibleFPR(reg));
+    return *fprs_[reg];
   }
 
-  bool SetFPR(uint32_t reg, uintptr_t value) OVERRIDE;
+  void SetFPR(uint32_t reg, uintptr_t value) OVERRIDE;
 
   void SmashCallerSaves() OVERRIDE;
   void DoLongJump() OVERRIDE;
diff --git a/runtime/arch/x86_64/context_x86_64.cc b/runtime/arch/x86_64/context_x86_64.cc
index 6e9b99c2f7..cdc2ec71da 100644
--- a/runtime/arch/x86_64/context_x86_64.cc
+++ b/runtime/arch/x86_64/context_x86_64.cc
@@ -91,26 +91,18 @@ void X86_64Context::SmashCallerSaves() {
   fprs_[XMM11] = nullptr;
 }
 
-bool X86_64Context::SetGPR(uint32_t reg, uintptr_t value) {
+void X86_64Context::SetGPR(uint32_t reg, uintptr_t value) {
   CHECK_LT(reg, static_cast<uint32_t>(kNumberOfCpuRegisters));
+  DCHECK(IsAccessibleGPR(reg));
   CHECK_NE(gprs_[reg], &gZero);
-  if (gprs_[reg] != nullptr) {
-    *gprs_[reg] = value;
-    return true;
-  } else {
-    return false;
-  }
+  *gprs_[reg] = value;
 }
 
-bool X86_64Context::SetFPR(uint32_t reg, uintptr_t value) {
+void X86_64Context::SetFPR(uint32_t reg, uintptr_t value) {
   CHECK_LT(reg, static_cast<uint32_t>(kNumberOfFloatRegisters));
+  DCHECK(IsAccessibleFPR(reg));
   CHECK_NE(fprs_[reg], reinterpret_cast<const uint64_t*>(&gZero));
-  if (fprs_[reg] != nullptr) {
-    *fprs_[reg] = value;
-    return true;
-  } else {
-    return false;
-  }
+  *fprs_[reg] = value;
 }
 
 extern "C" void art_quick_do_long_jump(uintptr_t*, uintptr_t*);
diff --git a/runtime/arch/x86_64/context_x86_64.h b/runtime/arch/x86_64/context_x86_64.h
index 902c3b9876..0dda06ec4a 100644
--- a/runtime/arch/x86_64/context_x86_64.h
+++ b/runtime/arch/x86_64/context_x86_64.h
@@ -36,44 +36,43 @@ class X86_64Context : public Context {
   void FillCalleeSaves(const StackVisitor& fr) OVERRIDE SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
   void SetSP(uintptr_t new_sp) OVERRIDE {
-    bool success = SetGPR(RSP, new_sp);
-    CHECK(success) << "Failed to set RSP register";
+    SetGPR(RSP, new_sp);
   }
 
   void SetPC(uintptr_t new_pc) OVERRIDE {
     rip_ = new_pc;
   }
 
+  bool IsAccessibleGPR(uint32_t reg) OVERRIDE {
+    DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfCpuRegisters));
+    return gprs_[reg] != nullptr;
+  }
+
   uintptr_t* GetGPRAddress(uint32_t reg) OVERRIDE {
     DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfCpuRegisters));
     return gprs_[reg];
   }
 
-  bool GetGPR(uint32_t reg, uintptr_t* val) OVERRIDE {
+  uintptr_t GetGPR(uint32_t reg) OVERRIDE {
     DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfCpuRegisters));
-    if (gprs_[reg] == nullptr) {
-      return false;
-    } else {
-      DCHECK(val != nullptr);
-      *val = *gprs_[reg];
-      return true;
-    }
+    DCHECK(IsAccessibleGPR(reg));
+    return *gprs_[reg];
   }
 
-  bool SetGPR(uint32_t reg, uintptr_t value) OVERRIDE;
+  void SetGPR(uint32_t reg, uintptr_t value) OVERRIDE;
+
+  bool IsAccessibleFPR(uint32_t reg) OVERRIDE {
+    DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfFloatRegisters));
+    return fprs_[reg] != nullptr;
+  }
 
-  bool GetFPR(uint32_t reg, uintptr_t* val) OVERRIDE {
+  uintptr_t GetFPR(uint32_t reg) OVERRIDE {
     DCHECK_LT(reg, static_cast<uint32_t>(kNumberOfFloatRegisters));
-    if (fprs_[reg] == nullptr) {
-      return false;
-    } else {
-      DCHECK(val != nullptr);
-      *val = *fprs_[reg];
-      return true;
-    }
+    DCHECK(IsAccessibleFPR(reg));
+    return *fprs_[reg];
   }
 
-  bool SetFPR(uint32_t reg, uintptr_t value) OVERRIDE;
+  void SetFPR(uint32_t reg, uintptr_t value) OVERRIDE;
 
   void SmashCallerSaves() OVERRIDE;
   void DoLongJump() OVERRIDE;
diff --git a/runtime/stack.cc b/runtime/stack.cc
index 3165898e8c..b771aa71a1 100644
--- a/runtime/stack.cc
+++ b/runtime/stack.cc
@@ -172,11 +172,10 @@ bool StackVisitor::GetVReg(mirror::ArtMethod* m, uint16_t vreg, VRegKind kind,
       bool is_float = (kind == kFloatVReg) || (kind == kDoubleLoVReg) || (kind == kDoubleHiVReg);
       uint32_t spill_mask = is_float ? frame_info.FpSpillMask() : frame_info.CoreSpillMask();
       uint32_t reg = vmap_table.ComputeRegister(spill_mask, vmap_offset, kind);
-      uintptr_t ptr_val;
-      bool success = is_float ? GetFPR(reg, &ptr_val) : GetGPR(reg, &ptr_val);
-      if (!success) {
+      if (!IsAccessibleRegister(reg, is_float)) {
         return false;
       }
+      uintptr_t ptr_val = GetRegister(reg, is_float);
       bool target64 = Is64BitInstructionSet(kRuntimeISA);
       if (target64) {
         bool wide_lo = (kind == kLongLoVReg) || (kind == kDoubleLoVReg);
@@ -194,11 +193,14 @@ bool StackVisitor::GetVReg(mirror::ArtMethod* m, uint16_t vreg, VRegKind kind,
       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);
+      uint32_t* addr = GetVRegAddr(cur_quick_frame_, code_item, frame_info.CoreSpillMask(),
+                                   frame_info.FpSpillMask(), frame_info.FrameSizeInBytes(), vreg);
+      DCHECK(addr != nullptr);
+      *val = *addr;
       return true;
     }
   } else {
+    DCHECK(cur_shadow_frame_ != nullptr);
     *val = cur_shadow_frame_->GetVReg(vreg);
     return true;
   }
@@ -228,12 +230,11 @@ bool StackVisitor::GetVRegPair(mirror::ArtMethod* m, uint16_t vreg, VRegKind kin
       uint32_t spill_mask = is_float ? frame_info.FpSpillMask() : frame_info.CoreSpillMask();
       uint32_t reg_lo = vmap_table.ComputeRegister(spill_mask, vmap_offset_lo, kind_lo);
       uint32_t reg_hi = vmap_table.ComputeRegister(spill_mask, vmap_offset_hi, kind_hi);
-      uintptr_t ptr_val_lo, ptr_val_hi;
-      bool success = is_float ? GetFPR(reg_lo, &ptr_val_lo) : GetGPR(reg_lo, &ptr_val_lo);
-      success &= is_float ? GetFPR(reg_hi, &ptr_val_hi) : GetGPR(reg_hi, &ptr_val_hi);
-      if (!success) {
+      if (!IsAccessibleRegister(reg_lo, is_float) || !IsAccessibleRegister(reg_hi, is_float)) {
         return false;
       }
+      uintptr_t ptr_val_lo = GetRegister(reg_lo, is_float);
+      uintptr_t ptr_val_hi = GetRegister(reg_hi, is_float);
       bool target64 = Is64BitInstructionSet(kRuntimeISA);
       if (target64) {
         int64_t value_long_lo = static_cast<int64_t>(ptr_val_lo);
@@ -249,10 +250,12 @@ bool StackVisitor::GetVRegPair(mirror::ArtMethod* m, uint16_t vreg, VRegKind kin
                                                         // its instructions?
       uint32_t* addr = GetVRegAddr(cur_quick_frame_, code_item, frame_info.CoreSpillMask(),
                                    frame_info.FpSpillMask(), frame_info.FrameSizeInBytes(), vreg);
+      DCHECK(addr != nullptr);
       *val = *reinterpret_cast<uint64_t*>(addr);
       return true;
     }
   } else {
+    DCHECK(cur_shadow_frame_ != nullptr);
     *val = cur_shadow_frame_->GetVRegLong(vreg);
     return true;
   }
@@ -273,17 +276,16 @@ bool StackVisitor::SetVReg(mirror::ArtMethod* m, uint16_t vreg, uint32_t new_val
       bool is_float = (kind == kFloatVReg) || (kind == kDoubleLoVReg) || (kind == kDoubleHiVReg);
       uint32_t spill_mask = is_float ? frame_info.FpSpillMask() : frame_info.CoreSpillMask();
       const uint32_t reg = vmap_table.ComputeRegister(spill_mask, vmap_offset, kind);
+      if (!IsAccessibleRegister(reg, is_float)) {
+        return false;
+      }
       bool target64 = Is64BitInstructionSet(kRuntimeISA);
       // Deal with 32 or 64-bit wide registers in a way that builds on all targets.
       if (target64) {
         bool wide_lo = (kind == kLongLoVReg) || (kind == kDoubleLoVReg);
         bool wide_hi = (kind == kLongHiVReg) || (kind == kDoubleHiVReg);
         if (wide_lo || wide_hi) {
-          uintptr_t old_reg_val;
-          bool success = is_float ? GetFPR(reg, &old_reg_val) : GetGPR(reg, &old_reg_val);
-          if (!success) {
-            return false;
-          }
+          uintptr_t old_reg_val = GetRegister(reg, is_float);
           uint64_t new_vreg_portion = static_cast<uint64_t>(new_value);
           uint64_t old_reg_val_as_wide = static_cast<uint64_t>(old_reg_val);
           uint64_t mask = 0xffffffff;
@@ -295,21 +297,20 @@ bool StackVisitor::SetVReg(mirror::ArtMethod* m, uint16_t vreg, uint32_t new_val
           new_value = static_cast<uintptr_t>((old_reg_val_as_wide & mask) | new_vreg_portion);
         }
       }
-      if (is_float) {
-        return SetFPR(reg, new_value);
-      } else {
-        return SetGPR(reg, new_value);
-      }
+      SetRegister(reg, new_value, is_float);
+      return true;
     } else {
       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);
+      DCHECK(addr != nullptr);
       *addr = new_value;
       return true;
     }
   } else {
+    DCHECK(cur_shadow_frame_ != nullptr);
     cur_shadow_frame_->SetVReg(vreg, new_value);
     return true;
   }
@@ -339,17 +340,16 @@ bool StackVisitor::SetVRegPair(mirror::ArtMethod* m, uint16_t vreg, uint64_t new
       uint32_t spill_mask = is_float ? frame_info.FpSpillMask() : frame_info.CoreSpillMask();
       uint32_t reg_lo = vmap_table.ComputeRegister(spill_mask, vmap_offset_lo, kind_lo);
       uint32_t reg_hi = vmap_table.ComputeRegister(spill_mask, vmap_offset_hi, kind_hi);
+      if (!IsAccessibleRegister(reg_lo, is_float) || !IsAccessibleRegister(reg_hi, is_float)) {
+        return false;
+      }
       uintptr_t new_value_lo = static_cast<uintptr_t>(new_value & 0xFFFFFFFF);
       uintptr_t new_value_hi = static_cast<uintptr_t>(new_value >> 32);
       bool target64 = Is64BitInstructionSet(kRuntimeISA);
       // Deal with 32 or 64-bit wide registers in a way that builds on all targets.
       if (target64) {
-        uintptr_t old_reg_val_lo, old_reg_val_hi;
-        bool success = is_float ? GetFPR(reg_lo, &old_reg_val_lo) : GetGPR(reg_lo, &old_reg_val_lo);
-        success &= is_float ? GetFPR(reg_hi, &old_reg_val_hi) : GetGPR(reg_hi, &old_reg_val_hi);
-        if (!success) {
-          return false;
-        }
+        uintptr_t old_reg_val_lo = GetRegister(reg_lo, is_float);
+        uintptr_t old_reg_val_hi = GetRegister(reg_hi, is_float);
         uint64_t new_vreg_portion_lo = static_cast<uint64_t>(new_value_lo);
         uint64_t new_vreg_portion_hi = static_cast<uint64_t>(new_value_hi) << 32;
         uint64_t old_reg_val_lo_as_wide = static_cast<uint64_t>(old_reg_val_lo);
@@ -359,47 +359,64 @@ bool StackVisitor::SetVRegPair(mirror::ArtMethod* m, uint16_t vreg, uint64_t new
         new_value_lo = static_cast<uintptr_t>((old_reg_val_lo_as_wide & mask_lo) | new_vreg_portion_lo);
         new_value_hi = static_cast<uintptr_t>((old_reg_val_hi_as_wide & mask_hi) | new_vreg_portion_hi);
       }
-      bool success = is_float ? SetFPR(reg_lo, new_value_lo) : SetGPR(reg_lo, new_value_lo);
-      success &= is_float ? SetFPR(reg_hi, new_value_hi) : SetGPR(reg_hi, new_value_hi);
-      return success;
+      SetRegister(reg_lo, new_value_lo, is_float);
+      SetRegister(reg_hi, new_value_hi, is_float);
+      return true;
     } else {
       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);
+      DCHECK(addr != nullptr);
       *reinterpret_cast<uint64_t*>(addr) = new_value;
       return true;
     }
   } else {
+    DCHECK(cur_shadow_frame_ != nullptr);
     cur_shadow_frame_->SetVRegLong(vreg, new_value);
     return true;
   }
 }
 
+bool StackVisitor::IsAccessibleGPR(uint32_t reg) const {
+  DCHECK(context_ != nullptr);
+  return context_->IsAccessibleGPR(reg);
+}
+
 uintptr_t* StackVisitor::GetGPRAddress(uint32_t reg) const {
-  DCHECK(cur_quick_frame_ != NULL) << "This is a quick frame routine";
+  DCHECK(cur_quick_frame_ != nullptr) << "This is a quick frame routine";
+  DCHECK(context_ != nullptr);
   return context_->GetGPRAddress(reg);
 }
 
-bool StackVisitor::GetGPR(uint32_t reg, uintptr_t* val) const {
-  DCHECK(cur_quick_frame_ != NULL) << "This is a quick frame routine";
-  return context_->GetGPR(reg, val);
+uintptr_t StackVisitor::GetGPR(uint32_t reg) const {
+  DCHECK(cur_quick_frame_ != nullptr) << "This is a quick frame routine";
+  DCHECK(context_ != nullptr);
+  return context_->GetGPR(reg);
+}
+
+void StackVisitor::SetGPR(uint32_t reg, uintptr_t value) {
+  DCHECK(cur_quick_frame_ != nullptr) << "This is a quick frame routine";
+  DCHECK(context_ != nullptr);
+  context_->SetGPR(reg, value);
 }
 
-bool StackVisitor::SetGPR(uint32_t reg, uintptr_t value) {
-  DCHECK(cur_quick_frame_ != NULL) << "This is a quick frame routine";
-  return context_->SetGPR(reg, value);
+bool StackVisitor::IsAccessibleFPR(uint32_t reg) const {
+  DCHECK(context_ != nullptr);
+  return context_->IsAccessibleFPR(reg);
 }
 
-bool StackVisitor::GetFPR(uint32_t reg, uintptr_t* val) const {
-  DCHECK(cur_quick_frame_ != NULL) << "This is a quick frame routine";
-  return context_->GetFPR(reg, val);
+uintptr_t StackVisitor::GetFPR(uint32_t reg) const {
+  DCHECK(cur_quick_frame_ != nullptr) << "This is a quick frame routine";
+  DCHECK(context_ != nullptr);
+  return context_->GetFPR(reg);
 }
 
-bool StackVisitor::SetFPR(uint32_t reg, uintptr_t value) {
-  DCHECK(cur_quick_frame_ != NULL) << "This is a quick frame routine";
-  return context_->SetFPR(reg, value);
+void StackVisitor::SetFPR(uint32_t reg, uintptr_t value) {
+  DCHECK(cur_quick_frame_ != nullptr) << "This is a quick frame routine";
+  DCHECK(context_ != nullptr);
+  context_->SetFPR(reg, value);
 }
 
 uintptr_t StackVisitor::GetReturnPc() const {
diff --git a/runtime/stack.h b/runtime/stack.h
index b2b207273a..5a86ca1b3b 100644
--- a/runtime/stack.h
+++ b/runtime/stack.h
@@ -649,10 +649,29 @@ class StackVisitor {
   StackVisitor(Thread* thread, Context* context, size_t num_frames)
       SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
 
-  bool GetGPR(uint32_t reg, uintptr_t* val) const;
-  bool SetGPR(uint32_t reg, uintptr_t value);
-  bool GetFPR(uint32_t reg, uintptr_t* val) const;
-  bool SetFPR(uint32_t reg, uintptr_t value);
+  bool IsAccessibleRegister(uint32_t reg, bool is_float) const {
+    return is_float ? IsAccessibleFPR(reg) : IsAccessibleGPR(reg);
+  }
+  uintptr_t GetRegister(uint32_t reg, bool is_float) const {
+    DCHECK(IsAccessibleRegister(reg, is_float));
+    return is_float ? GetFPR(reg) : GetGPR(reg);
+  }
+  void SetRegister(uint32_t reg, uintptr_t value, bool is_float) {
+    DCHECK(IsAccessibleRegister(reg, is_float));
+    if (is_float) {
+      SetFPR(reg, value);
+    } else {
+      SetGPR(reg, value);
+    }
+  }
+
+  bool IsAccessibleGPR(uint32_t reg) const;
+  uintptr_t GetGPR(uint32_t reg) const;
+  void SetGPR(uint32_t reg, uintptr_t value);
+
+  bool IsAccessibleFPR(uint32_t reg) const;
+  uintptr_t GetFPR(uint32_t reg) const;
+  void SetFPR(uint32_t reg, uintptr_t value);
 
   void SanityCheckFrame() const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);