From 8567f4d4b9a79f041406026011fb8151b24b8c99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20G=C3=B3rny?= Date: Mon, 23 Aug 2021 15:29:45 +0200 Subject: [PATCH] [lldb] Support querying registers via generic names without alt_names Update GetRegisterInfoByName() methods to support getting registers by a generic name independently of alt_name entries in the register context. This makes it possible to use generic names when interacting with gdbserver (that does not supply alt_names). It also makes it possible to remove some of the duplicated information from register context declarations and/or use alt_names for another purpose. Differential Revision: https://reviews.llvm.org/D108554 --- lldb/include/lldb/Core/ValueObjectRegister.h | 9 +- lldb/source/API/SBFrame.cpp | 32 +--- lldb/source/Core/ValueObjectRegister.cpp | 23 ++- .../Host/common/NativeRegisterContext.cpp | 12 ++ lldb/source/Target/RegisterContext.cpp | 12 ++ .../TestGDBServerTargetXML.py | 152 ++++++++++++++++++ 6 files changed, 200 insertions(+), 40 deletions(-) create mode 100644 lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py diff --git a/lldb/include/lldb/Core/ValueObjectRegister.h b/lldb/include/lldb/Core/ValueObjectRegister.h index e210b36d2a45..20a7411b6fde 100644 --- a/lldb/include/lldb/Core/ValueObjectRegister.h +++ b/lldb/include/lldb/Core/ValueObjectRegister.h @@ -84,7 +84,7 @@ public: static lldb::ValueObjectSP Create(ExecutionContextScope *exe_scope, lldb::RegisterContextSP ®_ctx_sp, - uint32_t reg_num); + const RegisterInfo *reg_info); llvm::Optional GetByteSize() override; @@ -119,15 +119,16 @@ protected: CompilerType m_compiler_type; private: - void ConstructObject(uint32_t reg_num); + void ConstructObject(const RegisterInfo *reg_info); friend class ValueObjectRegisterSet; ValueObjectRegister(ValueObject &parent, lldb::RegisterContextSP ®_ctx_sp, - uint32_t reg_num); + const RegisterInfo *reg_info); ValueObjectRegister(ExecutionContextScope *exe_scope, ValueObjectManager &manager, - lldb::RegisterContextSP ®_ctx_sp, uint32_t reg_num); + lldb::RegisterContextSP ®_ctx_sp, + const RegisterInfo *reg_info); // For ValueObject only ValueObjectRegister(const ValueObjectRegister &) = delete; diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp index 8f9e426e066e..7107768ba884 100644 --- a/lldb/source/API/SBFrame.cpp +++ b/lldb/source/API/SBFrame.cpp @@ -633,18 +633,10 @@ SBValue SBFrame::FindValue(const char *name, ValueType value_type, { RegisterContextSP reg_ctx(frame->GetRegisterContext()); if (reg_ctx) { - const uint32_t num_regs = reg_ctx->GetRegisterCount(); - for (uint32_t reg_idx = 0; reg_idx < num_regs; ++reg_idx) { - const RegisterInfo *reg_info = - reg_ctx->GetRegisterInfoAtIndex(reg_idx); - if (reg_info && - ((reg_info->name && strcasecmp(reg_info->name, name) == 0) || - (reg_info->alt_name && - strcasecmp(reg_info->alt_name, name) == 0))) { - value_sp = ValueObjectRegister::Create(frame, reg_ctx, reg_idx); - sb_value.SetSP(value_sp); - break; - } + if (const RegisterInfo *reg_info = + reg_ctx->GetRegisterInfoByName(name)) { + value_sp = ValueObjectRegister::Create(frame, reg_ctx, reg_info); + sb_value.SetSP(value_sp); } } } break; @@ -953,18 +945,10 @@ SBValue SBFrame::FindRegister(const char *name) { if (frame) { RegisterContextSP reg_ctx(frame->GetRegisterContext()); if (reg_ctx) { - const uint32_t num_regs = reg_ctx->GetRegisterCount(); - for (uint32_t reg_idx = 0; reg_idx < num_regs; ++reg_idx) { - const RegisterInfo *reg_info = - reg_ctx->GetRegisterInfoAtIndex(reg_idx); - if (reg_info && - ((reg_info->name && strcasecmp(reg_info->name, name) == 0) || - (reg_info->alt_name && - strcasecmp(reg_info->alt_name, name) == 0))) { - value_sp = ValueObjectRegister::Create(frame, reg_ctx, reg_idx); - result.SetSP(value_sp); - break; - } + if (const RegisterInfo *reg_info = + reg_ctx->GetRegisterInfoByName(name)) { + value_sp = ValueObjectRegister::Create(frame, reg_ctx, reg_info); + result.SetSP(value_sp); } } } diff --git a/lldb/source/Core/ValueObjectRegister.cpp b/lldb/source/Core/ValueObjectRegister.cpp index 089fd7667080..743083a2d1ed 100644 --- a/lldb/source/Core/ValueObjectRegister.cpp +++ b/lldb/source/Core/ValueObjectRegister.cpp @@ -118,8 +118,9 @@ ValueObject *ValueObjectRegisterSet::CreateChildAtIndex( if (m_reg_ctx_sp && m_reg_set) { const size_t num_children = GetNumChildren(); if (idx < num_children) - valobj = new ValueObjectRegister(*this, m_reg_ctx_sp, - m_reg_set->registers[idx]); + valobj = new ValueObjectRegister( + *this, m_reg_ctx_sp, + m_reg_ctx_sp->GetRegisterInfoAtIndex(m_reg_set->registers[idx])); } return valobj; } @@ -132,8 +133,7 @@ ValueObjectRegisterSet::GetChildMemberWithName(ConstString name, const RegisterInfo *reg_info = m_reg_ctx_sp->GetRegisterInfoByName(name.GetStringRef()); if (reg_info != nullptr) - valobj = new ValueObjectRegister(*this, m_reg_ctx_sp, - reg_info->kinds[eRegisterKindLLDB]); + valobj = new ValueObjectRegister(*this, m_reg_ctx_sp, reg_info); } if (valobj) return valobj->GetSP(); @@ -155,8 +155,7 @@ ValueObjectRegisterSet::GetIndexOfChildWithName(ConstString name) { #pragma mark - #pragma mark ValueObjectRegister -void ValueObjectRegister::ConstructObject(uint32_t reg_num) { - const RegisterInfo *reg_info = m_reg_ctx_sp->GetRegisterInfoAtIndex(reg_num); +void ValueObjectRegister::ConstructObject(const RegisterInfo *reg_info) { if (reg_info) { m_reg_info = *reg_info; if (reg_info->name) @@ -168,29 +167,29 @@ void ValueObjectRegister::ConstructObject(uint32_t reg_num) { ValueObjectRegister::ValueObjectRegister(ValueObject &parent, lldb::RegisterContextSP ®_ctx_sp, - uint32_t reg_num) + const RegisterInfo *reg_info) : ValueObject(parent), m_reg_ctx_sp(reg_ctx_sp), m_reg_info(), m_reg_value(), m_type_name(), m_compiler_type() { assert(reg_ctx_sp.get()); - ConstructObject(reg_num); + ConstructObject(reg_info); } ValueObjectSP ValueObjectRegister::Create(ExecutionContextScope *exe_scope, lldb::RegisterContextSP ®_ctx_sp, - uint32_t reg_num) { + const RegisterInfo *reg_info) { auto manager_sp = ValueObjectManager::Create(); - return (new ValueObjectRegister(exe_scope, *manager_sp, reg_ctx_sp, reg_num)) + return (new ValueObjectRegister(exe_scope, *manager_sp, reg_ctx_sp, reg_info)) ->GetSP(); } ValueObjectRegister::ValueObjectRegister(ExecutionContextScope *exe_scope, ValueObjectManager &manager, lldb::RegisterContextSP ®_ctx, - uint32_t reg_num) + const RegisterInfo *reg_info) : ValueObject(exe_scope, manager), m_reg_ctx_sp(reg_ctx), m_reg_info(), m_reg_value(), m_type_name(), m_compiler_type() { assert(reg_ctx); - ConstructObject(reg_num); + ConstructObject(reg_info); } ValueObjectRegister::~ValueObjectRegister() = default; diff --git a/lldb/source/Host/common/NativeRegisterContext.cpp b/lldb/source/Host/common/NativeRegisterContext.cpp index 04d10aba4e63..d0afc2b47dac 100644 --- a/lldb/source/Host/common/NativeRegisterContext.cpp +++ b/lldb/source/Host/common/NativeRegisterContext.cpp @@ -56,6 +56,17 @@ NativeRegisterContext::GetRegisterInfoByName(llvm::StringRef reg_name, if (reg_name.empty()) return nullptr; + // Generic register names take precedence over specific register names. + // For example, on x86 we want "sp" to refer to the complete RSP/ESP register + // rather than the 16-bit SP pseudo-register. + uint32_t generic_reg = Args::StringToGenericRegister(reg_name); + if (generic_reg != LLDB_INVALID_REGNUM) { + const RegisterInfo *reg_info = + GetRegisterInfo(eRegisterKindGeneric, generic_reg); + if (reg_info) + return reg_info; + } + const uint32_t num_registers = GetRegisterCount(); for (uint32_t reg = start_idx; reg < num_registers; ++reg) { const RegisterInfo *reg_info = GetRegisterInfoAtIndex(reg); @@ -64,6 +75,7 @@ NativeRegisterContext::GetRegisterInfoByName(llvm::StringRef reg_name, reg_name.equals_insensitive(reg_info->alt_name)) return reg_info; } + return nullptr; } diff --git a/lldb/source/Target/RegisterContext.cpp b/lldb/source/Target/RegisterContext.cpp index bd50a9486ef3..0e50e650dd23 100644 --- a/lldb/source/Target/RegisterContext.cpp +++ b/lldb/source/Target/RegisterContext.cpp @@ -54,6 +54,17 @@ RegisterContext::GetRegisterInfoByName(llvm::StringRef reg_name, if (reg_name.empty()) return nullptr; + // Generic register names take precedence over specific register names. + // For example, on x86 we want "sp" to refer to the complete RSP/ESP register + // rather than the 16-bit SP pseudo-register. + uint32_t generic_reg = Args::StringToGenericRegister(reg_name); + if (generic_reg != LLDB_INVALID_REGNUM) { + const RegisterInfo *reg_info = + GetRegisterInfo(eRegisterKindGeneric, generic_reg); + if (reg_info) + return reg_info; + } + const uint32_t num_registers = GetRegisterCount(); for (uint32_t reg = start_idx; reg < num_registers; ++reg) { const RegisterInfo *reg_info = GetRegisterInfoAtIndex(reg); @@ -62,6 +73,7 @@ RegisterContext::GetRegisterInfoByName(llvm::StringRef reg_name, reg_name.equals_insensitive(reg_info->alt_name)) return reg_info; } + return nullptr; } diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py b/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py new file mode 100644 index 000000000000..255e36a3104a --- /dev/null +++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py @@ -0,0 +1,152 @@ +from __future__ import print_function +import lldb +from lldbsuite.test.lldbtest import * +from lldbsuite.test.decorators import * +from gdbclientutils import * + + +class TestGDBServerTargetXML(GDBRemoteTestBase): + + @skipIfXmlSupportMissing + @skipIfRemote + @skipIfLLVMTargetMissing("X86") + def test_x86_64_regs(self): + """Test grabbing various x86_64 registers from gdbserver.""" + reg_data = [ + "0102030405060708", # rcx + "1112131415161718", # rdx + "2122232425262728", # rsi + "3132333435363738", # rdi + "4142434445464748", # rbp + "5152535455565758", # rsp + "6162636465666768", # r8 + "7172737475767778", # r9 + "8182838485868788", # rip + "91929394", # eflags + "0102030405060708090a", # st0 + "1112131415161718191a", # st1 + ] + 6 * [ + "2122232425262728292a" # st2..st7 + ] + [ + "8182838485868788898a8b8c8d8e8f90", # xmm0 + "9192939495969798999a9b9c9d9e9fa0", # xmm1 + ] + 14 * [ + "a1a2a3a4a5a6a7a8a9aaabacadaeafb0", # xmm2..xmm15 + ] + [ + "00000000", # mxcsr + ] + [ + "b1b2b3b4b5b6b7b8b9babbbcbdbebfc0", # ymm0h + "c1c2c3c4c5c6c7c8c9cacbcccdcecfd0", # ymm1h + ] + 14 * [ + "d1d2d3d4d5d6d7d8d9dadbdcdddedfe0", # ymm2h..ymm15h + ] + + class MyResponder(MockGDBServerResponder): + def qXferRead(self, obj, annex, offset, length): + if annex == "target.xml": + return """ + + + i386:x86-64 + GNU/Linux + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + """, False + else: + return None, False + + def readRegister(self, regnum): + return "" + + def readRegisters(self): + return "".join(reg_data) + + def writeRegisters(self, reg_hex): + return "OK" + + def haltReason(self): + return "T02thread:1ff0d;threads:1ff0d;thread-pcs:000000010001bc00;07:0102030405060708;10:1112131415161718;" + + self.server.responder = MyResponder() + + target = self.createTarget("basic_eh_frame.yaml") + process = self.connect(target) + lldbutil.expect_state_changes(self, self.dbg.GetListener(), process, + [lldb.eStateStopped]) + + # test generic aliases + self.match("register read arg4", + ["rcx = 0x0807060504030201"]) + self.match("register read arg3", + ["rdx = 0x1817161514131211"]) + self.match("register read arg2", + ["rsi = 0x2827262524232221"]) + self.match("register read arg1", + ["rdi = 0x3837363534333231"]) + self.match("register read fp", + ["rbp = 0x4847464544434241"]) + self.match("register read sp", + ["rsp = 0x5857565554535251"]) + self.match("register read arg5", + ["r8 = 0x6867666564636261"]) + self.match("register read arg6", + ["r9 = 0x7877767574737271"]) + self.match("register read pc", + ["rip = 0x8887868584838281"]) + self.match("register read flags", + ["eflags = 0x94939291"])