Cleanup and speedup NativeRegisterContextLinux_arm64

Summary:
This patch simplifies register accesses in NativeRegisterContextLinux_arm64
and also adds some bare minimum caching to avoid multiple calls to ptrace
during a stop.

Linux ptrace returns data in the form of structures containing GPR/FPR data.
This means that one single call is enough to read all GPRs or FPRs. We do
that once per stop and keep reading from or writing to the buffer that we
have in NativeRegisterContextLinux_arm64 class. Before a resume or detach we
write all buffers back.

This is tested on aarch64 thunder x1 with Ubuntu 18.04. Also tested
regressions on x86_64.

Reviewers: labath, clayborg

Reviewed By: labath

Subscribers: kristof.beyls, lldb-commits

Differential Revision: https://reviews.llvm.org/D69371

4 files changed, 130 insertions, 160 deletions

diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
index 3b98310c196..9a89a13f5f5 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
@@ -28,6 +28,9 @@ public:
   CreateHostNativeRegisterContextLinux(const ArchSpec &target_arch,
                                        NativeThreadProtocol &native_thread);
 
+  // Invalidates cached values in register context data structures
+  virtual void InvalidateAllRegisters(){}
+
 protected:
   lldb::ByteOrder GetByteOrder() const;
 
diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 15336d8749c..ea7337b632e 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -152,6 +152,9 @@ NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
   m_max_hwp_supported = 16;
   m_max_hbp_supported = 16;
   m_refresh_hwdebug_info = true;
+
+  m_gpr_is_valid = false;
+  m_fpu_is_valid = false;
 }
 
 uint32_t NativeRegisterContextLinux_arm64::GetRegisterSetCount() const {
@@ -185,41 +188,39 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info,
 
   const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
 
-  if (IsFPR(reg)) {
-    error = ReadFPR();
-    if (error.Fail())
-      return error;
-  } else {
-    uint32_t full_reg = reg;
-    bool is_subreg = reg_info->invalidate_regs &&
-                     (reg_info->invalidate_regs[0] != LLDB_INVALID_REGNUM);
+  if (reg == LLDB_INVALID_REGNUM)
+    return Status("no lldb regnum for %s", reg_info && reg_info->name
+                                               ? reg_info->name
+                                               : "<unknown register>");
+
+  uint8_t *src;
+  uint32_t offset;
 
-    if (is_subreg) {
-      // Read the full aligned 64-bit register.
-      full_reg = reg_info->invalidate_regs[0];
+  if (IsGPR(reg)) {
+    if (!m_gpr_is_valid) {
+      error = ReadGPR();
+      if (error.Fail())
+        return error;
     }
 
-    error = ReadRegisterRaw(full_reg, reg_value);
+    offset = reg_info->byte_offset;
+    assert(offset < GetGPRSize());
+    src = (uint8_t *)GetGPRBuffer() + offset;
 
-    if (error.Success()) {
-      // If our read was not aligned (for ah,bh,ch,dh), shift our returned
-      // value one byte to the right.
-      if (is_subreg && (reg_info->byte_offset & 0x1))
-        reg_value.SetUInt64(reg_value.GetAsUInt64() >> 8);
+  } else if (IsFPR(reg)) {
+    if (!m_fpu_is_valid) {
 
-      // If our return byte size was greater than the return value reg size,
-      // then use the type specified by reg_info rather than the uint64_t
-      // default
-      if (reg_value.GetByteSize() > reg_info->byte_size)
-        reg_value.SetType(reg_info);
+      error = ReadFPR();
+      if (error.Fail())
+        return error;
     }
-    return error;
-  }
+    offset = CalculateFprOffset(reg_info);
+    assert(offset < GetFPRSize());
+    src = (uint8_t *)GetFPRBuffer() + offset;
+  } else
+    return Status("failed - register wasn't recognized to be a GPR or an FPR, "
+                  "write strategy unknown");
 
-  // Get pointer to m_fpr variable and set the data from it.
-  uint32_t fpr_offset = CalculateFprOffset(reg_info);
-  assert(fpr_offset < sizeof m_fpr);
-  uint8_t *src = (uint8_t *)&m_fpr + fpr_offset;
   reg_value.SetFromMemoryData(reg_info, src, reg_info->byte_size,
                               eByteOrderLittle, error);
 
@@ -228,48 +229,51 @@ NativeRegisterContextLinux_arm64::ReadRegister(const RegisterInfo *reg_info,
 
 Status NativeRegisterContextLinux_arm64::WriteRegister(
     const RegisterInfo *reg_info, const RegisterValue &reg_value) {
+  Status error;
+
   if (!reg_info)
     return Status("reg_info NULL");
 
-  const uint32_t reg_index = reg_info->kinds[lldb::eRegisterKindLLDB];
-  if (reg_index == LLDB_INVALID_REGNUM)
+  const uint32_t reg = reg_info->kinds[lldb::eRegisterKindLLDB];
+
+  if (reg == LLDB_INVALID_REGNUM)
     return Status("no lldb regnum for %s", reg_info && reg_info->name
                                                ? reg_info->name
                                                : "<unknown register>");
 
-  if (IsGPR(reg_index))
-    return WriteRegisterRaw(reg_index, reg_value);
-
-  if (IsFPR(reg_index)) {
-    // Get pointer to m_fpr variable and set the data to it.
-    uint32_t fpr_offset = CalculateFprOffset(reg_info);
-    assert(fpr_offset < sizeof m_fpr);
-    uint8_t *dst = (uint8_t *)&m_fpr + fpr_offset;
-    switch (reg_info->byte_size) {
-    case 2:
-      *(uint16_t *)dst = reg_value.GetAsUInt16();
-      break;
-    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 register data size %" PRIu32,
-                    reg_info->byte_size);
+  uint8_t *dst;
+  uint32_t offset;
+
+  if (IsGPR(reg)) {
+    if (!m_gpr_is_valid) {
+      error = ReadGPR();
+      if (error.Fail())
+        return error;
     }
 
-    Status error = WriteFPR();
-    if (error.Fail())
-      return error;
+    offset = reg_info->byte_offset;
+    assert(offset < GetGPRSize());
+    dst = (uint8_t *)GetGPRBuffer() + offset;
 
-    return Status();
+    ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
+
+    return WriteGPR();
+  } else if (IsFPR(reg)) {
+    if (!m_fpu_is_valid) {
+      error = ReadFPR();
+      if (error.Fail())
+        return error;
+    }
+    offset = CalculateFprOffset(reg_info);
+    assert(offset < GetFPRSize());
+    dst = (uint8_t *)GetFPRBuffer() + offset;
+
+    ::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
+
+    return WriteFPR();
   }
 
-  return Status("failed - register wasn't recognized to be a GPR or an FPR, "
-                "write strategy unknown");
+  return error;
 }
 
 Status NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
@@ -277,18 +281,21 @@ Status NativeRegisterContextLinux_arm64::ReadAllRegisterValues(
   Status error;
 
   data_sp.reset(new DataBufferHeap(REG_CONTEXT_SIZE, 0));
-  error = ReadGPR();
-  if (error.Fail())
-    return error;
-
-  error = ReadFPR();
-  if (error.Fail())
-    return error;
+  if (!m_gpr_is_valid) {
+    error = ReadGPR();
+    if (error.Fail())
+      return error;
+  }
 
+  if (!m_fpu_is_valid) {
+    error = ReadFPR();
+    if (error.Fail())
+      return error;
+  }
   uint8_t *dst = data_sp->GetBytes();
-  ::memcpy(dst, &m_gpr_arm64, GetGPRSize());
+  ::memcpy(dst, GetGPRBuffer(), GetGPRSize());
   dst += GetGPRSize();
-  ::memcpy(dst, &m_fpr, sizeof(m_fpr));
+  ::memcpy(dst, GetFPRBuffer(), GetFPRSize());
 
   return error;
 }
@@ -320,14 +327,14 @@ Status NativeRegisterContextLinux_arm64::WriteAllRegisterValues(
                                    __FUNCTION__);
     return error;
   }
-  ::memcpy(&m_gpr_arm64, src, GetRegisterInfoInterface().GetGPRSize());
+  ::memcpy(GetGPRBuffer(), src, GetRegisterInfoInterface().GetGPRSize());
 
   error = WriteGPR();
   if (error.Fail())
     return error;
 
   src += GetRegisterInfoInterface().GetGPRSize();
-  ::memcpy(&m_fpr, src, sizeof(m_fpr));
+  ::memcpy(GetFPRBuffer(), src, GetFPRSize());
 
   error = WriteFPR();
   if (error.Fail())
@@ -832,117 +839,65 @@ Status NativeRegisterContextLinux_arm64::WriteHardwareDebugRegs(int hwbType) {
                                            &hwbType, &ioVec, ioVec.iov_len);
 }
 
-Status NativeRegisterContextLinux_arm64::DoReadRegisterValue(
-    uint32_t offset, const char *reg_name, uint32_t size,
-    RegisterValue &value) {
-  Status error;
-  if (offset > sizeof(struct user_pt_regs)) {
-    offset -= sizeof(struct user_pt_regs);
-    if (offset > sizeof(struct user_fpsimd_state)) {
-      error.SetErrorString("invalid offset value");
-      return error;
-    }
-    elf_fpregset_t regs;
-    int regset = NT_FPREGSET;
-    struct iovec ioVec;
-
-    ioVec.iov_base = &regs;
-    ioVec.iov_len = sizeof regs;
-    error = NativeProcessLinux::PtraceWrapper(
-        PTRACE_GETREGSET, m_thread.GetID(), &regset, &ioVec, sizeof regs);
-    if (error.Success()) {
-      value.SetBytes((void *)(((unsigned char *)(&regs)) + offset), 16,
-                     m_thread.GetProcess().GetByteOrder());
-    }
-  } else {
-    elf_gregset_t regs;
-    int regset = NT_PRSTATUS;
-    struct iovec ioVec;
-
-    ioVec.iov_base = &regs;
-    ioVec.iov_len = sizeof regs;
-    error = NativeProcessLinux::PtraceWrapper(
-        PTRACE_GETREGSET, m_thread.GetID(), &regset, &ioVec, sizeof regs);
-    if (error.Success()) {
-      value.SetBytes((void *)(((unsigned char *)(regs)) + offset), 8,
-                     m_thread.GetProcess().GetByteOrder());
-    }
-  }
-  return error;
-}
-
-Status NativeRegisterContextLinux_arm64::DoWriteRegisterValue(
-    uint32_t offset, const char *reg_name, const RegisterValue &value) {
+Status NativeRegisterContextLinux_arm64::ReadGPR() {
   Status error;
-  ::pid_t tid = m_thread.GetID();
-  if (offset > sizeof(struct user_pt_regs)) {
-    offset -= sizeof(struct user_pt_regs);
-    if (offset > sizeof(struct user_fpsimd_state)) {
-      error.SetErrorString("invalid offset value");
-      return error;
-    }
-    elf_fpregset_t regs;
-    int regset = NT_FPREGSET;
-    struct iovec ioVec;
-
-    ioVec.iov_base = &regs;
-    ioVec.iov_len = sizeof regs;
-    error = NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET, tid, &regset,
-                                              &ioVec, sizeof regs);
-
-    if (error.Success()) {
-      ::memcpy((void *)(((unsigned char *)(&regs)) + offset), value.GetBytes(),
-               16);
-      error = NativeProcessLinux::PtraceWrapper(PTRACE_SETREGSET, tid, &regset,
-                                                &ioVec, sizeof regs);
-    }
-  } else {
-    elf_gregset_t regs;
-    int regset = NT_PRSTATUS;
-    struct iovec ioVec;
-
-    ioVec.iov_base = &regs;
-    ioVec.iov_len = sizeof regs;
-    error = NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET, tid, &regset,
-                                              &ioVec, sizeof regs);
-    if (error.Success()) {
-      ::memcpy((void *)(((unsigned char *)(&regs)) + offset), value.GetBytes(),
-               8);
-      error = NativeProcessLinux::PtraceWrapper(PTRACE_SETREGSET, tid, &regset,
-                                                &ioVec, sizeof regs);
-    }
-  }
-  return error;
-}
 
-Status NativeRegisterContextLinux_arm64::ReadGPR() {
   struct iovec ioVec;
+
   ioVec.iov_base = GetGPRBuffer();
   ioVec.iov_len = GetGPRSize();
-  return ReadRegisterSet(&ioVec, GetGPRSize(), NT_PRSTATUS);
+
+  error = ReadRegisterSet(&ioVec, GetGPRSize(), NT_PRSTATUS);
+
+  if (error.Success())
+    m_gpr_is_valid = true;
+
+  return error;
 }
 
 Status NativeRegisterContextLinux_arm64::WriteGPR() {
   struct iovec ioVec;
+
+  m_gpr_is_valid = false;
+
   ioVec.iov_base = GetGPRBuffer();
   ioVec.iov_len = GetGPRSize();
+
   return WriteRegisterSet(&ioVec, GetGPRSize(), NT_PRSTATUS);
 }
 
 Status NativeRegisterContextLinux_arm64::ReadFPR() {
+  Status error;
+
   struct iovec ioVec;
+
   ioVec.iov_base = GetFPRBuffer();
   ioVec.iov_len = GetFPRSize();
-  return ReadRegisterSet(&ioVec, GetFPRSize(), NT_FPREGSET);
+
+  error = ReadRegisterSet(&ioVec, GetFPRSize(), NT_FPREGSET);
+
+  if (error.Success())
+    m_fpu_is_valid = true;
+
+  return error;
 }
 
 Status NativeRegisterContextLinux_arm64::WriteFPR() {
   struct iovec ioVec;
+
+  m_fpu_is_valid = false;
+
   ioVec.iov_base = GetFPRBuffer();
   ioVec.iov_len = GetFPRSize();
+
   return WriteRegisterSet(&ioVec, GetFPRSize(), NT_FPREGSET);
 }
 
+void NativeRegisterContextLinux_arm64::InvalidateAllRegisters() {
+  m_gpr_is_valid = false;
+  m_fpu_is_valid = false;
+}
+
 uint32_t NativeRegisterContextLinux_arm64::CalculateFprOffset(
     const RegisterInfo *reg_info) const {
   return reg_info->byte_offset -
diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
index 5bbe5ecbbbc..f7c501a9fdf 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -40,6 +40,8 @@ public:
 
   Status WriteAllRegisterValues(const lldb::DataBufferSP &data_sp) override;
 
+  void InvalidateAllRegisters() override;
+
   // Hardware breakpoints/watchpoint management functions
 
   uint32_t NumSupportedHardwareBreakpoints() override;
@@ -77,11 +79,6 @@ public:
   enum DREGType { eDREGTypeWATCH = 0, eDREGTypeBREAK };
 
 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 ReadGPR() override;
 
@@ -125,8 +122,17 @@ private:
     uint32_t fpcr;
   };
 
-  uint64_t m_gpr_arm64[k_num_gpr_registers_arm64]; // 64-bit general purpose
-                                                   // registers.
+  struct GPR {
+    uint64_t x[31];
+    uint64_t sp;
+    uint64_t pc;
+    uint64_t pstate;
+  };
+
+  bool m_gpr_is_valid;
+  bool m_fpu_is_valid;
+
+  GPR m_gpr_arm64; // 64-bit general purpose registers.
   RegInfo m_reg_info;
   FPU m_fpr; // floating-point registers including extended register sets.
 
diff --git a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
index 677aee34a18..b2730e60129 100644
--- a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -241,6 +241,9 @@ Status NativeThreadLinux::Resume(uint32_t signo) {
   if (signo != LLDB_INVALID_SIGNAL_NUMBER)
     data = signo;
 
+  // Before thread resumes, clear any cached register data structures
+  GetRegisterContext().InvalidateAllRegisters();
+
   return NativeProcessLinux::PtraceWrapper(PTRACE_CONT, GetID(), nullptr,
                                            reinterpret_cast<void *>(data));
 }
@@ -262,6 +265,9 @@ Status NativeThreadLinux::SingleStep(uint32_t signo) {
   if (signo != LLDB_INVALID_SIGNAL_NUMBER)
     data = signo;
 
+  // Before thread resumes, clear any cached register data structures
+  GetRegisterContext().InvalidateAllRegisters();
+
   // If hardware single-stepping is not supported, we just do a continue. The
   // breakpoint on the next instruction has been setup in
   // NativeProcessLinux::Resume.