NativeProcessLinux: Remove some register context boilerplate

[labath]

Summary

This patch follows the spirit of D63594, and removes some null checks
for things which should be operating invariants. Specifically
{Read,Write}[GF]PR now no longer check whether the supplied buffers are
null, because they never are. After this, the Do*** versions of these
function no longer serve any purpose and are inlined into their callers.

Other cleanups are possible here too, but I am taking this one step at a
time because this involves a lot of architecture-specific code, which I
don't have the hardware to test on (I did do a build-test though).

[jankratochvil]

{Read,Write}[GF]PR now no longer check whether the supplied buffers are null, because they never are

But then there is NativeRegisterContextLinux_x86_64::GetFPRBuffer(). Which is never used so maybe together with NativeRegisterContextLinux_x86_64::GetFPRSize() they could be just assert(0);. It can be also considered as a different cleanup patch.

I did not test these patches but at least I am finally setting up now a (silent) buildbot for s390x+ppc64le (I haven't found a suitable ARM offer yet).

source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp

918 ↗

(On Diff #217153)

This keeps the existing bug in place as the size should be rather sizeof(ioVec). But then sizeof(ioVec) is also not much useful as it is used only for PtraceDisplayBytes. It is probably considered as a different cleanup patch so this patch does not touch it.

[labath]

In D66744#1646514, @jankratochvil wrote:

{Read,Write}[GF]PR now no longer check whether the supplied buffers are null, because they never are

But then there is NativeRegisterContextLinux_x86_64::GetFPRBuffer(). Which is never used so maybe together with NativeRegisterContextLinux_x86_64::GetFPRSize() they could be just assert(0);. It can be also considered as a different cleanup patch.

Hm.. I didn't notice that. I'll put that in separately. Ideally, I'd say these functions should be used, but that may require more cleanups in the x86 register context, such as detecting the register type earlier on instead of just when the operations fail. Overall, there's a lot of room for improvement in this area...

I did not test these patches but at least I am finally setting up now a (silent) buildbot for s390x+ppc64le (I haven't found a suitable ARM offer yet).

That would be awesome. Btw, we already have some arm (32&64) lldb bots set up by linaro, though they don't seem to be in a particularly good shape right now...

source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp

918 ↗

(On Diff #217153)

Yeah, I noticed that. I am keeping that for some other patch. Ideally, I'd like to refactor this even more so that one does not have to construct the iovec manually, and fix PtraceDisplayBytes to show the contents of the iovec, but I don't have a clear idea on the best way to do that.

[jankratochvil]

In D66744#1646572, @labath wrote:

In D66744#1646514, @jankratochvil wrote:

But then there is NativeRegisterContextLinux_x86_64::GetFPRBuffer(). Which is never used so maybe together with NativeRegisterContextLinux_x86_64::GetFPRSize() they could be just assert(0);. It can be also considered as a different cleanup patch.

Hm.. I didn't notice that. I'll put that in separately. Ideally, I'd say these functions should be used, but that may require more cleanups in the x86 register context, such as detecting the register type earlier on instead of just when the operations fail.

If it is left as runtime detected then the current default implementation of NativeRegisterContextLinux::ReadFPR() and NativeRegisterContextLinux::WriteFPR() could be also another class layer with that virtual void *GetFPRBuffer() = 0; virtual size_t GetFPRSize() = 0;.

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux.h

protected:
   virtual Status ReadGPR();
 
   virtual Status WriteGPR();
 
   virtual Status ReadFPR();
 
   virtual Status WriteFPR();
 
-  virtual void *GetGPRBuffer() { return nullptr; }
+  virtual void *GetGPRBuffer() = 0;
 
   virtual size_t GetGPRSize() {
     return GetRegisterInfoInterface().GetGPRSize();
   }
 
-  virtual void *GetFPRBuffer() { return nullptr; }
+  virtual void *GetFPRBuffer() = 0;
 
-  virtual size_t GetFPRSize() { return 0; }
+  virtual size_t GetFPRSize() = 0;
 
   // The Do*** functions are executed on the privileged thread and can perform
   // ptrace
   // operations directly.
   virtual Status DoReadRegisterValue(uint32_t offset, const char *reg_name,
                                      uint32_t size, RegisterValue &value);
 
   virtual Status DoWriteRegisterValue(uint32_t offset, const char *reg_name,
                                       const RegisterValue &value);
-
-  virtual Status DoReadGPR(void *buf, size_t buf_size);
-
-  virtual Status DoWriteGPR(void *buf, size_t buf_size);
-
-  virtual Status DoReadFPR(void *buf, size_t buf_size);
-
-  virtual Status DoWriteFPR(void *buf, size_t buf_size);
 };
 
 } // namespace process_linux
 } // namespace lldb_private
 
 #endif // #ifndef lldb_NativeRegisterContextLinux_h

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp

if (!register_to_write_info_p)
     return Status("NativeRegisterContextLinux::%s failed to get RegisterInfo "
                   "for write register index %" PRIu32,
                   __FUNCTION__, reg_to_write);
 
   return DoWriteRegisterValue(reg_info->byte_offset, reg_info->name, reg_value);
 }
 
 Status NativeRegisterContextLinux::ReadGPR() {
-  void *buf = GetGPRBuffer();
-  if (!buf)
-    return Status("GPR buffer is NULL");
-  size_t buf_size = GetGPRSize();
-
-  return DoReadGPR(buf, buf_size);
+  return NativeProcessLinux::PtraceWrapper(
+      PTRACE_GETREGS, m_thread.GetID(), nullptr, GetGPRBuffer(), GetGPRSize());
 }
 
 Status NativeRegisterContextLinux::WriteGPR() {
-  void *buf = GetGPRBuffer();
-  if (!buf)
-    return Status("GPR buffer is NULL");
-  size_t buf_size = GetGPRSize();
-
-  return DoWriteGPR(buf, buf_size);
+  return NativeProcessLinux::PtraceWrapper(
+      PTRACE_SETREGS, m_thread.GetID(), nullptr, GetGPRBuffer(), GetGPRSize());
 }
 
 Status NativeRegisterContextLinux::ReadFPR() {
-  void *buf = GetFPRBuffer();
-  if (!buf)
-    return Status("FPR buffer is NULL");
-  size_t buf_size = GetFPRSize();
-
-  return DoReadFPR(buf, buf_size);
+  return NativeProcessLinux::PtraceWrapper(PTRACE_GETFPREGS, m_thread.GetID(),
+                                           nullptr, GetFPRBuffer(),
+                                           GetFPRSize());
 }
 
 Status NativeRegisterContextLinux::WriteFPR() {
-  void *buf = GetFPRBuffer();
-  if (!buf)
-    return Status("FPR buffer is NULL");
-  size_t buf_size = GetFPRSize();
-
-  return DoWriteFPR(buf, buf_size);
+  return NativeProcessLinux::PtraceWrapper(PTRACE_SETFPREGS, m_thread.GetID(),
+                                           nullptr, GetFPRBuffer(),
+                                           GetFPRSize());
 }
 
 Status NativeRegisterContextLinux::ReadRegisterSet(void *buf, size_t buf_size,
                                                    unsigned int regset) {
   return NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET, m_thread.GetID(),
                                            static_cast<void *>(&regset), buf,
                                            buf_size);
 }
Status NativeRegisterContextLinux::DoWriteRegisterValue(
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_REGISTERS));
 
   void *buf = reinterpret_cast<void *>(value.GetAsUInt64());
   LLDB_LOG(log, "{0}: {1}", reg_name, buf);
 
   return NativeProcessLinux::PtraceWrapper(
       PTRACE_POKEUSER, m_thread.GetID(), reinterpret_cast<void *>(offset), buf);
 }
-
-Status NativeRegisterContextLinux::DoReadGPR(void *buf, size_t buf_size) {
-  return NativeProcessLinux::PtraceWrapper(PTRACE_GETREGS, m_thread.GetID(),
-                                           nullptr, buf, buf_size);
-}
-
-Status NativeRegisterContextLinux::DoWriteGPR(void *buf, size_t buf_size) {
-  return NativeProcessLinux::PtraceWrapper(PTRACE_SETREGS, m_thread.GetID(),
-                                           nullptr, buf, buf_size);
-}
-
-Status NativeRegisterContextLinux::DoReadFPR(void *buf, size_t buf_size) {
-  return NativeProcessLinux::PtraceWrapper(PTRACE_GETFPREGS, m_thread.GetID(),
-                                           nullptr, buf, buf_size);
-}
-
-Status NativeRegisterContextLinux::DoWriteFPR(void *buf, size_t buf_size) {
-  return NativeProcessLinux::PtraceWrapper(PTRACE_SETFPREGS, m_thread.GetID(),
-                                           nullptr, buf, buf_size);
-}

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h

 
 protected:
   Status DoReadRegisterValue(uint32_t offset, const char *reg_name,
                              uint32_t size, RegisterValue &value) override;
 
   Status DoWriteRegisterValue(uint32_t offset, const char *reg_name,
                               const RegisterValue &value) override;
 
-  Status DoReadGPR(void *buf, size_t buf_size) override;
+  Status ReadGPR() override;
 
-  Status DoWriteGPR(void *buf, size_t buf_size) override;
+  Status WriteGPR() override;
 
-  Status DoReadFPR(void *buf, size_t buf_size) override;
+  Status ReadFPR() override;
 
-  Status DoWriteFPR(void *buf, size_t buf_size) override;
+  Status WriteFPR() override;
 
   void *GetGPRBuffer() override { return &m_gpr_arm; }
 
   void *GetFPRBuffer() override { return &m_fpr; }
 
   size_t GetFPRSize() override { return sizeof(m_fpr); }
 
 private:

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp

Status NativeRegisterContextLinux_arm::DoReadRegisterValue(
   // devices (always return "Bad address"). To avoid using PTRACE_PEEKUSER we
   // read out the full GPR register set instead. This approach is about 4 times
   // slower but the performance overhead is negligible in comparision to
   // processing time in lldb-server.
   assert(offset % 4 == 0 && "Try to write a register with unaligned offset");
   if (offset + sizeof(uint32_t) > sizeof(m_gpr_arm))
     return Status("Register isn't fit into the size of the GPR area");
 
-  Status error = DoReadGPR(m_gpr_arm, sizeof(m_gpr_arm));
+  Status error = ReadGPR();
   if (error.Fail())
     return error;
 
   value.SetUInt32(m_gpr_arm[offset / sizeof(uint32_t)]);
   return Status();
 }
 
 Status NativeRegisterContextLinux_arm::DoWriteRegisterValue(
     uint32_t offset, const char *reg_name, const RegisterValue &value) {
   // PTRACE_POKEUSER don't work in the aarch64 linux kernel used on android
   // devices (always return "Bad address"). To avoid using PTRACE_POKEUSER we
   // read out the full GPR register set, modify the requested register and
   // write it back. This approach is about 4 times slower but the performance
   // overhead is negligible in comparision to processing time in lldb-server.
   assert(offset % 4 == 0 && "Try to write a register with unaligned offset");
   if (offset + sizeof(uint32_t) > sizeof(m_gpr_arm))
     return Status("Register isn't fit into the size of the GPR area");
 
-  Status error = DoReadGPR(m_gpr_arm, sizeof(m_gpr_arm));
+  Status error = ReadGPR();
   if (error.Fail())
     return error;
 
   uint32_t reg_value = value.GetAsUInt32();
   // As precaution for an undefined behavior encountered while setting PC we
   // will clear thumb bit of new PC if we are already in thumb mode; that is
   // CPSR thumb mode bit is set.
   if (offset / sizeof(uint32_t) == gpr_pc_arm) {
     // Check if we are already in thumb mode and thumb bit of current PC is
     // read out to be zero and thumb bit of next PC is read out to be one.
     if ((m_gpr_arm[gpr_cpsr_arm] & 0x20) && !(m_gpr_arm[gpr_pc_arm] & 0x01) &&
         (value.GetAsUInt32() & 0x01)) {
       reg_value &= (~1ull);
     }
   }
 
   m_gpr_arm[offset / sizeof(uint32_t)] = reg_value;
-  return DoWriteGPR(m_gpr_arm, sizeof(m_gpr_arm));
+  return WriteGPR();
 }
 
-Status NativeRegisterContextLinux_arm::DoReadGPR(void *buf, size_t buf_size) {
+Status NativeRegisterContextLinux_arm::ReadGPR() {
 #ifdef __arm__
-  return NativeRegisterContextLinux::DoReadGPR(buf, buf_size);
+  return NativeRegisterContextLinux::ReadGPR();
 #else  // __aarch64__
   struct iovec ioVec;
-  ioVec.iov_base = buf;
-  ioVec.iov_len = buf_size;
+  ioVec.iov_base = GetGPRBuffer();
+  ioVec.iov_len = GetGPRSize();
 
-  return ReadRegisterSet(&ioVec, buf_size, NT_PRSTATUS);
+  return ReadRegisterSet(&ioVec, GetGPRSize(), NT_PRSTATUS);
 #endif // __arm__
 }
 
-Status NativeRegisterContextLinux_arm::DoWriteGPR(void *buf, size_t buf_size) {
+Status NativeRegisterContextLinux_arm::WriteGPR() {
 #ifdef __arm__
-  return NativeRegisterContextLinux::DoWriteGPR(buf, buf_size);
+  return NativeRegisterContextLinux::WriteGPR();
 #else  // __aarch64__
   struct iovec ioVec;
-  ioVec.iov_base = buf;
-  ioVec.iov_len = buf_size;
+  ioVec.iov_base = GetGPRBuffer();
+  ioVec.iov_len = GetGPRSize();
 
-  return WriteRegisterSet(&ioVec, buf_size, NT_PRSTATUS);
+  return WriteRegisterSet(&ioVec, GetGPRSize(), NT_PRSTATUS);
 #endif // __arm__
 }
 
-Status NativeRegisterContextLinux_arm::DoReadFPR(void *buf, size_t buf_size) {
+Status NativeRegisterContextLinux_arm::ReadFPR() {
 #ifdef __arm__
   return NativeProcessLinux::PtraceWrapper(PTRACE_GETVFPREGS, m_thread.GetID(),
-                                           nullptr, buf, buf_size);
+                                           nullptr, GetFPRBuffer(),
+                                           GetFPRSize());
 #else  // __aarch64__
   struct iovec ioVec;
-  ioVec.iov_base = buf;
-  ioVec.iov_len = buf_size;
+  ioVec.iov_base = GetFPRBuffer();
+  ioVec.iov_len = GetFPRSize();
 
-  return ReadRegisterSet(&ioVec, buf_size, NT_ARM_VFP);
+  return ReadRegisterSet(&ioVec, GetFPRSize(), NT_ARM_VFP);
 #endif // __arm__
 }
 
-Status NativeRegisterContextLinux_arm::DoWriteFPR(void *buf, size_t buf_size) {
+Status NativeRegisterContextLinux_arm::WriteFPR() {
 #ifdef __arm__
   return NativeProcessLinux::PtraceWrapper(PTRACE_SETVFPREGS, m_thread.GetID(),
-                                           nullptr, buf, buf_size);
+                                           nullptr, GetFPRBuffer(),
+                                           GetFPRSize());
 #else  // __aarch64__
   struct iovec ioVec;
-  ioVec.iov_base = buf;
-  ioVec.iov_len = buf_size;
+  ioVec.iov_base = GetFPRBuffer();
+  ioVec.iov_len = GetFPRSize();
 
-  return WriteRegisterSet(&ioVec, buf_size, NT_ARM_VFP);
+  return WriteRegisterSet(&ioVec, GetFPRSize(), NT_ARM_VFP);
 #endif // __arm__
 }
 
 #endif // defined(__arm__) || defined(__arm64__) || defined(__aarch64__)

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h

 
 protected:
   Status DoReadRegisterValue(uint32_t offset, const char *reg_name,
                              uint32_t size, RegisterValue &value) override;
 
   Status DoWriteRegisterValue(uint32_t offset, const char *reg_name,
                               const RegisterValue &value) override;
 
-  Status DoReadGPR(void *buf, size_t buf_size) override;
+  Status ReadGPR() override;
 
-  Status DoWriteGPR(void *buf, size_t buf_size) override;
+  Status WriteGPR() override;
 
-  Status DoReadFPR(void *buf, size_t buf_size) override;
+  Status ReadFPR() override;
 
-  Status DoWriteFPR(void *buf, size_t buf_size) override;
+  Status WriteFPR() override;
 
   void *GetGPRBuffer() override { return &m_gpr_arm64; }
 
   void *GetFPRBuffer() override { return &m_fpr; }
 
   size_t GetFPRSize() override { return sizeof(m_fpr); }
 
 private:

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp

if (error.Success()) {
                8);
       error = NativeProcessLinux::PtraceWrapper(PTRACE_SETREGSET, tid, &regset,
                                                 &ioVec, sizeof regs);
     }
   }
   return error;
 }
 
-Status NativeRegisterContextLinux_arm64::DoReadGPR(void *buf, size_t buf_size) {
-  int regset = NT_PRSTATUS;
+Status NativeRegisterContextLinux_arm64::ReadGPR() {
   struct iovec ioVec;
-  Status error;
-
-  ioVec.iov_base = buf;
-  ioVec.iov_len = buf_size;
-  return NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET, m_thread.GetID(),
-                                           &regset, &ioVec, buf_size);
+  ioVec.iov_base = GetGPRBuffer();
+  ioVec.iov_len = GetGPRSize();
+  return ReadRegisterSet(&ioVec, GetGPRSize(), NT_PRSTATUS);
 }
 
-Status NativeRegisterContextLinux_arm64::DoWriteGPR(void *buf,
-                                                    size_t buf_size) {
-  int regset = NT_PRSTATUS;
+Status NativeRegisterContextLinux_arm64::WriteGPR() {
   struct iovec ioVec;
-  Status error;
-
-  ioVec.iov_base = buf;
-  ioVec.iov_len = buf_size;
-  return NativeProcessLinux::PtraceWrapper(PTRACE_SETREGSET, m_thread.GetID(),
-                                           &regset, &ioVec, buf_size);
+  ioVec.iov_base = GetGPRBuffer();
+  ioVec.iov_len = GetGPRSize();
+  return WriteRegisterSet(&ioVec, GetGPRSize(), NT_PRSTATUS);
 }
 
-Status NativeRegisterContextLinux_arm64::DoReadFPR(void *buf, size_t buf_size) {
-  int regset = NT_FPREGSET;
+Status NativeRegisterContextLinux_arm64::ReadFPR() {
   struct iovec ioVec;
-  Status error;
-
-  ioVec.iov_base = buf;
-  ioVec.iov_len = buf_size;
-  return NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET, m_thread.GetID(),
-                                           &regset, &ioVec, buf_size);
+  ioVec.iov_base = GetFPRBuffer();
+  ioVec.iov_len = GetFPRSize();
+  return ReadRegisterSet(&ioVec, GetFPRSize(), NT_FPREGSET);
 }
 
-Status NativeRegisterContextLinux_arm64::DoWriteFPR(void *buf,
-                                                    size_t buf_size) {
-  int regset = NT_FPREGSET;
+Status NativeRegisterContextLinux_arm64::WriteFPR() {
   struct iovec ioVec;
-  Status error;
-
-  ioVec.iov_base = buf;
-  ioVec.iov_len = buf_size;
-  return NativeProcessLinux::PtraceWrapper(PTRACE_SETREGSET, m_thread.GetID(),
-                                           &regset, &ioVec, buf_size);
+  ioVec.iov_base = GetFPRBuffer();
+  ioVec.iov_len = GetFPRSize();
+  return WriteRegisterSet(&ioVec, GetFPRSize(), NT_FPREGSET);
 }
 
 uint32_t NativeRegisterContextLinux_arm64::CalculateFprOffset(
     const RegisterInfo *reg_info) const {
   return reg_info->byte_offset -
          GetRegisterInfoAtIndex(m_reg_info.first_fpr)->byte_offset;
 }
 
 #endif // defined (__arm64__) || defined (__aarch64__)

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h

public:
 
   lldb::addr_t GetWatchpointAddress(uint32_t wp_index) override;
 
   uint32_t GetWatchpointSize(uint32_t wp_index);
 
   bool WatchpointIsEnabled(uint32_t wp_index);
 
 protected:
-  Status DoReadGPR(void *buf, size_t buf_size) override;
-
-  Status DoWriteGPR(void *buf, size_t buf_size) override;
-
-  Status DoReadFPR(void *buf, size_t buf_size) override;
-
-  Status DoWriteFPR(void *buf, size_t buf_size) override;
-
   bool IsVMX(unsigned reg);
 
   bool IsVSX(unsigned reg);
 
   Status ReadVMX();
 
   Status WriteVMX();
 

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp

 bool NativeRegisterContextLinux_ppc64le::IsGPR(unsigned reg) const {
   return reg <= k_last_gpr_ppc64le; // GPR's come first.
 }
 
 bool NativeRegisterContextLinux_ppc64le::IsFPR(unsigned reg) const {
   return (k_first_fpr_ppc64le <= reg && reg <= k_last_fpr_ppc64le);
 }
 
-Status NativeRegisterContextLinux_ppc64le::DoReadGPR(
-    void *buf, size_t buf_size) {
-  int regset = NT_PRSTATUS;
-  return NativeProcessLinux::PtraceWrapper(PTRACE_GETREGS, m_thread.GetID(),
-                                           &regset, buf, buf_size);
-}
-
-Status NativeRegisterContextLinux_ppc64le::DoWriteGPR(
-    void *buf, size_t buf_size) {
-  int regset = NT_PRSTATUS;
-  return NativeProcessLinux::PtraceWrapper(PTRACE_SETREGS, m_thread.GetID(),
-                                           &regset, buf, buf_size);
-}
-
-Status NativeRegisterContextLinux_ppc64le::DoReadFPR(void *buf,
-                                                     size_t buf_size) {
-  int regset = NT_FPREGSET;
-  return NativeProcessLinux::PtraceWrapper(PTRACE_GETFPREGS, m_thread.GetID(),
-                                           &regset, buf, buf_size);
-}
-
-Status NativeRegisterContextLinux_ppc64le::DoWriteFPR(void *buf,
-                                                      size_t buf_size) {
-  int regset = NT_FPREGSET;
-  return NativeProcessLinux::PtraceWrapper(PTRACE_SETFPREGS, m_thread.GetID(),
-                                           &regset, buf, buf_size);
-}
-
 uint32_t NativeRegisterContextLinux_ppc64le::CalculateFprOffset(
     const RegisterInfo *reg_info) const {
   return reg_info->byte_offset -
          GetRegisterInfoAtIndex(k_first_fpr_ppc64le)->byte_offset;
 }
 
 uint32_t NativeRegisterContextLinux_ppc64le::CalculateVmxOffset(
     const RegisterInfo *reg_info) const {

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h

 //===-- NativeRegisterContextLinux_s390x.h ----------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #if defined(__s390x__) && defined(__linux__)
 
 #ifndef lldb_NativeRegisterContextLinux_s390x_h
 #define lldb_NativeRegisterContextLinux_s390x_h
 
 #include "Plugins/Process/Linux/NativeRegisterContextLinux.h"
 #include "Plugins/Process/Utility/RegisterContext_s390x.h"
 #include "Plugins/Process/Utility/lldb-s390x-register-enums.h"
+#include <asm/ptrace.h>
 
 namespace lldb_private {
 namespace process_linux {
 
 class NativeProcessLinux;
 
 class NativeRegisterContextLinux_s390x : public NativeRegisterContextLinux {
 public:

 
 protected:
   Status DoReadRegisterValue(uint32_t offset, const char *reg_name,
                              uint32_t size, RegisterValue &value) override;
 
   Status DoWriteRegisterValue(uint32_t offset, const char *reg_name,
                               const RegisterValue &value) override;
 
-  Status DoReadGPR(void *buf, size_t buf_size) override;
+  Status ReadGPR() override;
 
-  Status DoWriteGPR(void *buf, size_t buf_size) override;
+  Status WriteGPR() override;
 
-  Status DoReadFPR(void *buf, size_t buf_size) override;
+  Status ReadFPR() override;
 
-  Status DoWriteFPR(void *buf, size_t buf_size) override;
+  Status WriteFPR() override;
+
+  void *GetGPRBuffer() override { return &m_regs; }
+  size_t GetGPRSize() override { return sizeof(m_regs); }
+  void *GetFPRBuffer() override { return &m_fp_regs; }
+  size_t GetFPRSize() override { return sizeof(m_fp_regs); }
 
 private:
   // Info about register ranges.
   struct RegInfo {
     uint32_t num_registers;
     uint32_t num_gpr_registers;
     uint32_t num_fpr_registers;
 
     uint32_t last_gpr;
     uint32_t first_fpr;
     uint32_t last_fpr;
   };
 
   // Private member variables.
   RegInfo m_reg_info;
   lldb::addr_t m_watchpoint_addr;
 
+  s390_regs m_regs;
+  s390_fp_regs m_fp_regs;
+
   // Private member methods.
   bool IsRegisterSetAvailable(uint32_t set_index) const;
 
   bool IsGPR(uint32_t reg_index) const;
 
   bool IsFPR(uint32_t reg_index) const;
 
   Status PeekUserArea(uint32_t offset, void *buf, size_t buf_size);

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp

 #include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/Status.h"
 
 #include "Plugins/Process/Utility/RegisterContextLinux_s390x.h"
 
-#include <asm/ptrace.h>
 #include <linux/uio.h>
 #include <sys/ptrace.h>
 
 using namespace lldb_private;
 using namespace lldb_private::process_linux;
 
 // Private namespace.
 
NativeRegisterContextLinux_s390x::ReadRegister(const RegisterInfo *reg_info,
 
   const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
   if (reg == LLDB_INVALID_REGNUM)
     return Status("register \"%s\" is an internal-only lldb register, cannot "
                   "read directly",
                   reg_info->name);
 
   if (IsGPR(reg)) {
-    s390_regs regs;
-    Status error = DoReadGPR(&regs, sizeof(regs));
+    Status error = ReadGPR();
     if (error.Fail())
       return error;
 
-    uint8_t *src = (uint8_t *)&regs + reg_info->byte_offset;
-    assert(reg_info->byte_offset + reg_info->byte_size <= sizeof(regs));
+    uint8_t *src = (uint8_t *)&m_regs + reg_info->byte_offset;
+    assert(reg_info->byte_offset + reg_info->byte_size <= sizeof(m_regs));
     switch (reg_info->byte_size) {
     case 4:
       reg_value.SetUInt32(*(uint32_t *)src);
       break;
     case 8:
       reg_value.SetUInt64(*(uint64_t *)src);
       break;
     default:
       assert(false && "Unhandled data size.");
       return Status("unhandled byte size: %" PRIu32, reg_info->byte_size);
     }
     return Status();
   }
 
   if (IsFPR(reg)) {
-    s390_fp_regs fp_regs;
-    Status error = DoReadFPR(&fp_regs, sizeof(fp_regs));
+    Status error = ReadFPR();
     if (error.Fail())
       return error;
 
     // byte_offset is just the offset within FPR, not the whole user area.
-    uint8_t *src = (uint8_t *)&fp_regs + reg_info->byte_offset;
-    assert(reg_info->byte_offset + reg_info->byte_size <= sizeof(fp_regs));
+    uint8_t *src = (uint8_t *)&m_fp_regs + reg_info->byte_offset;
+    assert(reg_info->byte_offset + reg_info->byte_size <= sizeof(m_fp_regs));
     switch (reg_info->byte_size) {
     case 4:
       reg_value.SetUInt32(*(uint32_t *)src);
       break;
     case 8:
       reg_value.SetUInt64(*(uint64_t *)src);
       break;
     default:
Status NativeRegisterContextLinux_s390x::WriteRegister(
 
   const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
   if (reg == LLDB_INVALID_REGNUM)
     return Status("register \"%s\" is an internal-only lldb register, cannot "
                   "write directly",
                   reg_info->name);
 
   if (IsGPR(reg)) {
-    s390_regs regs;
-    Status error = DoReadGPR(&regs, sizeof(regs));
+    Status error = ReadGPR();
     if (error.Fail())
       return error;
 
-    uint8_t *dst = (uint8_t *)&regs + reg_info->byte_offset;
-    assert(reg_info->byte_offset + reg_info->byte_size <= sizeof(regs));
+    uint8_t *dst = (uint8_t *)&m_regs + reg_info->byte_offset;
+    assert(reg_info->byte_offset + reg_info->byte_size <= sizeof(m_regs));
     switch (reg_info->byte_size) {
     case 4:
       *(uint32_t *)dst = reg_value.GetAsUInt32();
       break;
     case 8:
       *(uint64_t *)dst = reg_value.GetAsUInt64();
       break;
     default:
       assert(false && "Unhandled data size.");
       return Status("unhandled byte size: %" PRIu32, reg_info->byte_size);
     }
-    return DoWriteGPR(&regs, sizeof(regs));
+    return WriteGPR();
   }
 
   if (IsFPR(reg)) {
-    s390_fp_regs fp_regs;
-    Status error = DoReadFPR(&fp_regs, sizeof(fp_regs));
+    Status error = ReadFPR();
     if (error.Fail())
       return error;
 
     // byte_offset is just the offset within fp_regs, not the whole user area.
-    uint8_t *dst = (uint8_t *)&fp_regs + reg_info->byte_offset;
-    assert(reg_info->byte_offset + reg_info->byte_size <= sizeof(fp_regs));
+    uint8_t *dst = (uint8_t *)&m_fp_regs + reg_info->byte_offset;
+    assert(reg_info->byte_offset + reg_info->byte_size <= sizeof(m_fp_regs));
     switch (reg_info->byte_size) {
     case 4:
       *(uint32_t *)dst = reg_value.GetAsUInt32();
       break;
     case 8:
       *(uint64_t *)dst = reg_value.GetAsUInt64();
       break;
     default:
       assert(false && "Unhandled data size.");
       return Status("unhandled byte size: %" PRIu32, reg_info->byte_size);
     }
-    return DoWriteFPR(&fp_regs, sizeof(fp_regs));
+    return WriteFPR();
   }
 
   if (reg == lldb_last_break_s390x) {
     return Status("The last break address is read-only");
   }
 
   if (reg == lldb_system_call_s390x) {
     uint32_t system_call = reg_value.GetAsUInt32();
     return DoWriteRegisterSet(NT_S390_SYSTEM_CALL, &system_call, 4);
   }
 
   return Status("failed - register wasn't recognized");
 }
 
 Status NativeRegisterContextLinux_s390x::ReadAllRegisterValues(
     lldb::DataBufferSP &data_sp) {
   Status error;
 
   data_sp.reset(new DataBufferHeap(REG_CONTEXT_SIZE, 0));
   uint8_t *dst = data_sp->GetBytes();
-  error = DoReadGPR(dst, sizeof(s390_regs));
-  dst += sizeof(s390_regs);
+  error = ReadGPR();
   if (error.Fail())
     return error;
+  memcpy(dst, GetGPRBuffer(), GetGPRSize());
+  dst += GetGPRSize();
 
-  error = DoReadFPR(dst, sizeof(s390_fp_regs));
-  dst += sizeof(s390_fp_regs);
+  error = ReadFPR();
   if (error.Fail())
     return error;
+  memcpy(dst, GetFPRBuffer(), GetFPRSize());
+  dst += GetFPRSize();
 
   // Ignore errors if the regset is unsupported (happens on older kernels).
   DoReadRegisterSet(NT_S390_SYSTEM_CALL, dst, 4);
   dst += 4;
 
   // To enable inferior function calls while the process is stopped in an
   // interrupted system call, we need to clear the system call flag. It will be
   // restored to its original value by WriteAllRegisterValues. Again we ignore
Status NativeRegisterContextLinux_s390x::WriteAllRegisterValues(
   if (data_sp->GetByteSize() != REG_CONTEXT_SIZE) {
     error.SetErrorStringWithFormat(
         "NativeRegisterContextLinux_s390x::%s data_sp contained mismatched "
         "data size, expected %" PRIu64 ", actual %" PRIu64,
         __FUNCTION__, REG_CONTEXT_SIZE, data_sp->GetByteSize());
     return error;
   }
 
-  uint8_t *src = data_sp->GetBytes();
+  const uint8_t *src = data_sp->GetBytes();
   if (src == nullptr) {
     error.SetErrorStringWithFormat("NativeRegisterContextLinux_s390x::%s "
                                    "DataBuffer::GetBytes() returned a null "
                                    "pointer",
                                    __FUNCTION__);
     return error;
   }
 
-  error = DoWriteGPR(src, sizeof(s390_regs));
-  src += sizeof(s390_regs);
+  memcpy(GetGPRBuffer(), src, GetGPRSize());
+  src += GetGPRSize();
+  error = WriteGPR();
   if (error.Fail())
     return error;
 
-  error = DoWriteFPR(src, sizeof(s390_fp_regs));
-  src += sizeof(s390_fp_regs);
+  memcpy(GetFPRBuffer(), src, GetFPRSize());
+  src += GetFPRSize();
+  error = WriteFPR();
   if (error.Fail())
     return error;
 
   // Ignore errors if the regset is unsupported (happens on older kernels).
   DoWriteRegisterSet(NT_S390_SYSTEM_CALL, src, 4);
   src += 4;
 
   return error;
Status NativeRegisterContextLinux_s390x::PokeUserArea(uint32_t offset,
   parea.len = buf_size;
   parea.process_addr = (addr_t)buf;
   parea.kernel_addr = offset;
 
   return NativeProcessLinux::PtraceWrapper(PTRACE_POKEUSR_AREA,
                                            m_thread.GetID(), &parea);
 }
 
-Status NativeRegisterContextLinux_s390x::DoReadGPR(void *buf, size_t buf_size) {
-  assert(buf_size == sizeof(s390_regs));
-  return PeekUserArea(offsetof(user_regs_struct, psw), buf, buf_size);
+Status NativeRegisterContextLinux_s390x::ReadGPR() {
+  return PeekUserArea(offsetof(user_regs_struct, psw), GetGPRBuffer(),
+                      GetGPRSize());
 }
 
-Status NativeRegisterContextLinux_s390x::DoWriteGPR(void *buf,
-                                                    size_t buf_size) {
-  assert(buf_size == sizeof(s390_regs));
-  return PokeUserArea(offsetof(user_regs_struct, psw), buf, buf_size);
+Status NativeRegisterContextLinux_s390x::WriteGPR() {
+  return PokeUserArea(offsetof(user_regs_struct, psw), GetGPRBuffer(),
+                      GetGPRSize());
 }
 
-Status NativeRegisterContextLinux_s390x::DoReadFPR(void *buf, size_t buf_size) {
-  assert(buf_size == sizeof(s390_fp_regs));
-  return PeekUserArea(offsetof(user_regs_struct, fp_regs), buf, buf_size);
+Status NativeRegisterContextLinux_s390x::ReadFPR() {
+  return PeekUserArea(offsetof(user_regs_struct, fp_regs), GetGPRBuffer(),
+                      GetGPRSize());
 }
 
-Status NativeRegisterContextLinux_s390x::DoWriteFPR(void *buf,
-                                                    size_t buf_size) {
-  assert(buf_size == sizeof(s390_fp_regs));
-  return PokeUserArea(offsetof(user_regs_struct, fp_regs), buf, buf_size);
+Status NativeRegisterContextLinux_s390x::WriteFPR() {
+  return PokeUserArea(offsetof(user_regs_struct, fp_regs), GetGPRBuffer(),
+                      GetGPRSize());
 }
 
 Status NativeRegisterContextLinux_s390x::DoReadRegisterSet(uint32_t regset,
                                                            void *buf,
                                                            size_t buf_size) {
   struct iovec iov;
   iov.iov_base = buf;
   iov.iov_len = buf_size;