[Lldb-commits] [lldb] b6f9d7b - Cleanup and speedup NativeRegisterContextLinux_arm64
Muhammad Omair Javaid via lldb-commits
lldb-commits at lists.llvm.org
Fri Dec 6 09:19:21 PST 2019
Author: Muhammad Omair Javaid
Date: 2019-12-06T22:18:57+05:00
New Revision: b6f9d7b8fb2eb6b78ac93ebd5ea4e36c04469285
URL: https://github.com/llvm/llvm-project/commit/b6f9d7b8fb2eb6b78ac93ebd5ea4e36c04469285
DIFF: https://github.com/llvm/llvm-project/commit/b6f9d7b8fb2eb6b78ac93ebd5ea4e36c04469285.diff
LOG: 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
Added:
Modified:
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
index 3b98310c1963..9a89a13f5f5b 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
@@ -28,6 +28,9 @@ class NativeRegisterContextLinux : public NativeRegisterContextRegisterInfo {
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 15336d8749ca..ea7337b632ee 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 ®_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 = ®s;
- ioVec.iov_len = sizeof regs;
- error = NativeProcessLinux::PtraceWrapper(
- PTRACE_GETREGSET, m_thread.GetID(), ®set, &ioVec, sizeof regs);
- if (error.Success()) {
- value.SetBytes((void *)(((unsigned char *)(®s)) + offset), 16,
- m_thread.GetProcess().GetByteOrder());
- }
- } else {
- elf_gregset_t regs;
- int regset = NT_PRSTATUS;
- struct iovec ioVec;
-
- ioVec.iov_base = ®s;
- ioVec.iov_len = sizeof regs;
- error = NativeProcessLinux::PtraceWrapper(
- PTRACE_GETREGSET, m_thread.GetID(), ®set, &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 = ®s;
- ioVec.iov_len = sizeof regs;
- error = NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET, tid, ®set,
- &ioVec, sizeof regs);
-
- if (error.Success()) {
- ::memcpy((void *)(((unsigned char *)(®s)) + offset), value.GetBytes(),
- 16);
- error = NativeProcessLinux::PtraceWrapper(PTRACE_SETREGSET, tid, ®set,
- &ioVec, sizeof regs);
- }
- } else {
- elf_gregset_t regs;
- int regset = NT_PRSTATUS;
- struct iovec ioVec;
-
- ioVec.iov_base = ®s;
- ioVec.iov_len = sizeof regs;
- error = NativeProcessLinux::PtraceWrapper(PTRACE_GETREGSET, tid, ®set,
- &ioVec, sizeof regs);
- if (error.Success()) {
- ::memcpy((void *)(((unsigned char *)(®s)) + offset), value.GetBytes(),
- 8);
- error = NativeProcessLinux::PtraceWrapper(PTRACE_SETREGSET, tid, ®set,
- &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 5bbe5ecbbbc7..f7c501a9fdf6 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
@@ -40,6 +40,8 @@ class NativeRegisterContextLinux_arm64 : public NativeRegisterContextLinux {
Status WriteAllRegisterValues(const lldb::DataBufferSP &data_sp) override;
+ void InvalidateAllRegisters() override;
+
// Hardware breakpoints/watchpoint management functions
uint32_t NumSupportedHardwareBreakpoints() override;
@@ -77,11 +79,6 @@ class NativeRegisterContextLinux_arm64 : public NativeRegisterContextLinux {
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 @@ class NativeRegisterContextLinux_arm64 : public NativeRegisterContextLinux {
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 677aee34a184..b2730e601294 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.
More information about the lldb-commits
mailing list