https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/193894
>From d21d0f50f17238fa2203dfac94be27fad79ba18e Mon Sep 17 00:00:00 2001 From: Jason Molenda <[email protected]> Date: Thu, 23 Apr 2026 21:41:18 -0700 Subject: [PATCH 1/8] [lldb] New expedited register specfication for unavail regs When lldb-server/debugserver send a stop packet, they expedite the vaLues of many of the general purpose registers in the stop packet, so lldb doesn't need to fetch them separately. On Darwin systems using an M4 or newer SOC with SME, we need to fetch the streaming vector length (svl) register when in Streaming SVE Mode to correctly size the registers in lldb. On Darwin systems, when we are not in SSVE mode, svl is undefined -- it is not included in the expedited registers. However, lldb will still try to fetch the value, so we get a register-read packet at every stop on M4 and newer systems, trying to fetch the value. This patch adds a new format for the expedited registers. They are normally a `;` separated series of `{regnum}:{native endian bytes}`. This allows for `{regnum}:` alone, indicating that the register value for regnum cannot be fetched at this stop. It updates debugserver to always add svcr, tpidr2, and svl to the expedited registers, whether we can fetch them or not. Most of the changes happen in GDBRemoteRegisterContext. This class maintains a buffer sized for the register context bytes, and an array of bool to indicate whether we have fetched the value for a given register yet or not. If we have fetched the value, we can supply the bytes from the register context byte array. If it is not yet fetched, we read it. This PR changes this fetched-or-not bool array to a LazyBool with eLazyBoolYes meaning we have the bytes for the register value in the lldb buffer. eLazyBoolCalculate means we have not yet tried to read the register value. eLazyBoolNo means that the register is currently readable. The meaning of these 3 LazyBool states could be a little confusing as you read GDBRemoteRegisterContext, so all getting/setting of these is done through the methods SetAllRegistersValid() SetAllRegistersUnfetched() {Set,Get}RegisterIsValid() {Set,Get}RegisterIsUnavailable() {Set,Get}RegisterIsUnfetched() Whcih I think makes it a little easer to understand. The changes to GDBRemoteRegisterContext were the trickiest part of this PR, all the other changes are simple. rdar://161581129 --- lldb/docs/resources/lldbgdbremote.md | 14 +++ .../gdb-remote/GDBRemoteRegisterContext.cpp | 88 ++++++++++----- .../gdb-remote/GDBRemoteRegisterContext.h | 55 ++++++--- .../Process/gdb-remote/ProcessGDBRemote.cpp | 8 +- .../Process/gdb-remote/ThreadGDBRemote.cpp | 7 ++ .../Process/gdb-remote/ThreadGDBRemote.h | 2 + .../TestUnavailableRegisters.py | 105 ++++++++++++++++++ lldb/tools/debugserver/source/RNBRemote.cpp | 34 ++++-- 8 files changed, 256 insertions(+), 57 deletions(-) create mode 100644 lldb/test/API/functionalities/gdb_remote_client/TestUnavailableRegisters.py diff --git a/lldb/docs/resources/lldbgdbremote.md b/lldb/docs/resources/lldbgdbremote.md index 9aa7ad2259a6a..0d996237659e6 100644 --- a/lldb/docs/resources/lldbgdbremote.md +++ b/lldb/docs/resources/lldbgdbremote.md @@ -732,6 +732,11 @@ On macOS with debugserver, we expedite the frame pointer backchain for a thread the previous FP and PC), and follow the backchain. Most backtraces on macOS and iOS now don't require us to read any memory! +An expedited register may have an empty string as its value (`"21":""`) +which indicates that the register cannot be read at this current +stop point, and lldb should not try to read the register value with +a separate `p` read-register request, it will not succeed. + **Priority To Implement:** Low This is a performance optimization, which speeds up debugging by avoiding @@ -2090,6 +2095,9 @@ following forms: followed by a series of key/value pairs: * If key is a hex number, it is a register number and value is the hex value of the register in debuggee endian byte order. + An empty value indicates that the register cannot be fetched + at this stop point; lldb will not succeed if it sends a separate + read-register packet. * If key == "thread", then the value is the big endian hex thread-id of the stopped thread. * If key == "core", then value is a hex number of the core on @@ -2167,6 +2175,12 @@ following keys and values: be outside the watchpoint that was triggered, the remote stub should determine which watchpoint was triggered and report an address from within its range. + On an architecture like AArch64, the FAR address value + reported with the watchpoint exception may not be contained + within a watched memory address range, e.g. a 16-byte write + which only overlaps with a watched range near the end may + report the start of the 16-byte write. The stub is + responsible for rewriting this to the closest watched region. 2. Watchpoint hardware register index number. 3. Actual watchpoint trap address, which may be outside the range of any watched region of memory. On MIPS, an addr diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp index a513f53e57b7d..c338c96b829a0 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp @@ -41,11 +41,11 @@ GDBRemoteRegisterContext::GDBRemoteRegisterContext( // Resize our vector of bools to contain one bool for every register. We will // use these boolean values to know when a register value is valid in // m_reg_data. - m_reg_valid.resize(m_reg_info_sp->GetNumRegisters()); + m_reg_valid.resize(m_reg_info_sp->GetNumRegisters(), eLazyBoolCalculate); // Make a heap based buffer that is big enough to store all registers - DataBufferSP reg_data_sp( - new DataBufferHeap(m_reg_info_sp->GetRegisterDataByteSize(), 0)); + DataBufferSP reg_data_sp(new DataBufferHeap( + m_reg_info_sp->GetRegisterDataByteSize(), eLazyBoolCalculate)); m_reg_data.SetData(reg_data_sp); m_reg_data.SetByteOrder(thread.GetProcess()->GetByteOrder()); } @@ -53,15 +53,34 @@ GDBRemoteRegisterContext::GDBRemoteRegisterContext( // Destructor GDBRemoteRegisterContext::~GDBRemoteRegisterContext() = default; -void GDBRemoteRegisterContext::InvalidateAllRegisters() { - SetAllRegisterValid(false); +void GDBRemoteRegisterContext::SetAllRegistersValidState(LazyBool valid) { + if (valid == eLazyBoolYes) + m_gpacket_cached = true; + else + m_gpacket_cached = false; + std::vector<LazyBool>::iterator pos, end = m_reg_valid.end(); + for (pos = m_reg_valid.begin(); pos != end; ++pos) + *pos = valid; } -void GDBRemoteRegisterContext::SetAllRegisterValid(bool b) { - m_gpacket_cached = b; - std::vector<bool>::iterator pos, end = m_reg_valid.end(); - for (pos = m_reg_valid.begin(); pos != end; ++pos) - *pos = b; +LazyBool GDBRemoteRegisterContext::GetRegisterValidState(uint32_t reg) const { + assert(reg < m_reg_valid.size()); + if (reg < m_reg_valid.size()) + return m_reg_valid[reg]; + return eLazyBoolNo; +} + +void GDBRemoteRegisterContext::SetRegisterIsValidState( + const RegisterInfo *reg_info, LazyBool valid) { + if (reg_info) + SetRegisterIsValidState(reg_info->kinds[lldb::eRegisterKindLLDB], valid); +} + +void GDBRemoteRegisterContext::SetRegisterIsValidState(uint32_t reg, + LazyBool valid) { + assert(reg < m_reg_valid.size()); + if (reg < m_reg_valid.size()) + m_reg_valid[reg] = valid; } size_t GDBRemoteRegisterContext::GetRegisterCount() { @@ -86,7 +105,7 @@ bool GDBRemoteRegisterContext::ReadRegister(const RegisterInfo *reg_info, // Read the register if (ReadRegisterBytes(reg_info)) { const uint32_t reg = reg_info->kinds[eRegisterKindLLDB]; - if (m_reg_valid[reg] == false) + if (GetRegisterIsUnavailable(reg)) return false; if (reg_info->value_regs && reg_info->value_regs[0] != LLDB_INVALID_REGNUM && @@ -135,11 +154,11 @@ bool GDBRemoteRegisterContext::PrivateSetRegisterValue( data.data(), std::min(data.size(), reg_byte_size)); bool success = data.size() >= reg_byte_size; if (success) { - SetRegisterIsValid(reg, true); + SetRegisterIsValid(reg); } else if (data.size() > 0) { - // Only set register is valid to false if we copied some bytes, else leave + // Only set register to Unfetched if we copied some bytes, else leave // it as it was. - SetRegisterIsValid(reg, false); + SetRegisterIsUnfetched(reg); } return success; } @@ -184,7 +203,7 @@ bool GDBRemoteRegisterContext::PrivateSetRegisterValue(uint32_t reg, reg_info->byte_size, // dst length m_reg_data.GetByteOrder())) // dst byte order { - SetRegisterIsValid(reg, true); + SetRegisterIsValid(reg); return true; } return false; @@ -219,7 +238,7 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info) { const uint32_t reg = reg_info->kinds[eRegisterKindLLDB]; - if (!GetRegisterIsValid(reg)) { + if (GetRegisterIsUnfetched(reg)) { if (m_read_all_at_once && !m_gpacket_cached) { if (DataBufferSP buffer_sp = gdb_comm.ReadAllRegisters(m_thread.GetProtocolID())) { @@ -227,16 +246,18 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info) { buffer_sp->GetBytes(), std::min(buffer_sp->GetByteSize(), m_reg_data.GetByteSize())); if (buffer_sp->GetByteSize() >= m_reg_data.GetByteSize()) { - SetAllRegisterValid(true); + SetAllRegistersValid(); return true; } else if (buffer_sp->GetByteSize() > 0) { for (auto x : llvm::enumerate( m_reg_info_sp->registers< DynamicRegisterInfo::reg_collection_const_range>())) { const struct RegisterInfo ®info = x.value(); - m_reg_valid[x.index()] = - (reginfo.byte_offset + reginfo.byte_size <= - buffer_sp->GetByteSize()); + if (reginfo.byte_offset + reginfo.byte_size <= + buffer_sp->GetByteSize()) + SetRegisterIsValid(x.index()); + else + SetRegisterIsUnfetched(x.index()); } m_gpacket_cached = true; @@ -274,23 +295,31 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info) { success = false; else { // Read the containing register if it hasn't already been read - if (!GetRegisterIsValid(prim_reg)) + if (GetRegisterIsUnfetched(prim_reg)) success = GetPrimordialRegister(prim_reg_info, gdb_comm); + // Containing register cannot be read + else if (GetRegisterIsUnavailable(prim_reg)) + success = false; } } if (success) { // If we reach this point, all primordial register requests have // succeeded. Validate this composite register. - SetRegisterIsValid(reg_info, true); + SetRegisterIsValid(reg_info); + } else { + SetRegisterIsUnavailable(reg_info); } } else { // Get each register individually - GetPrimordialRegister(reg_info, gdb_comm); + if (GetPrimordialRegister(reg_info, gdb_comm)) + SetRegisterIsValid(reg_info); + else + SetRegisterIsUnavailable(reg_info); } // Make sure we got a valid register value after reading it - if (!GetRegisterIsValid(reg)) + if (GetRegisterIsUnfetched(reg) || GetRegisterIsUnavailable(reg)) return false; } @@ -342,7 +371,7 @@ bool GDBRemoteRegisterContext::SetPrimordialRegister( StringExtractorGDBRemote response; const uint32_t reg = reg_info->kinds[eRegisterKindLLDB]; // Invalidate just this register - SetRegisterIsValid(reg, false); + SetRegisterIsUnfetched(reg); return gdb_comm.WriteRegister( m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin], @@ -441,9 +470,8 @@ bool GDBRemoteRegisterContext::WriteRegisterBytes(const RegisterInfo *reg_info, for (uint32_t idx = 0, reg = reg_info->invalidate_regs[0]; reg != LLDB_INVALID_REGNUM; reg = reg_info->invalidate_regs[++idx]) - SetRegisterIsValid(ConvertRegisterKindToRegisterNumber( - eRegisterKindLLDB, reg), - false); + SetRegisterIsUnfetched( + ConvertRegisterKindToRegisterNumber(eRegisterKindLLDB, reg)); } if (success && should_reconfigure_registers && @@ -687,7 +715,7 @@ bool GDBRemoteRegisterContext::WriteAllRegisterValues( const uint8_t *restore_src = restore_data.PeekData(register_offset, reg_byte_size); if (restore_src) { - SetRegisterIsValid(reg, false); + SetRegisterIsUnfetched(reg); if (gdb_comm.WriteRegister( m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin], @@ -727,7 +755,7 @@ bool GDBRemoteRegisterContext::WriteAllRegisterValues( continue; } - SetRegisterIsValid(reg_info, false); + SetRegisterIsUnfetched(reg_info); if (gdb_comm.WriteRegister(m_thread.GetProtocolID(), reg_info->kinds[eRegisterKindProcessPlugin], {data_sp->GetBytes() + reg_info->byte_offset, diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h index 6a90f911353fe..f198d6c3ee445 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h @@ -50,7 +50,7 @@ class GDBRemoteRegisterContext : public RegisterContext { ~GDBRemoteRegisterContext() override; - void InvalidateAllRegisters() override; + void InvalidateAllRegisters() override { SetAllRegistersUnfetched(); } size_t GetRegisterCount() override; @@ -94,35 +94,58 @@ class GDBRemoteRegisterContext : public RegisterContext { bool PrivateSetRegisterValue(uint32_t reg, uint64_t val); - void SetAllRegisterValid(bool b); + void SetAllRegistersValid() { SetAllRegistersValidState(eLazyBoolYes); } + void SetAllRegistersUnfetched() { + SetAllRegistersValidState(eLazyBoolCalculate); + } bool GetRegisterIsValid(uint32_t reg) const { - assert(reg < m_reg_valid.size()); - if (reg < m_reg_valid.size()) - return m_reg_valid[reg]; - return false; + return GetRegisterValidState(reg) == eLazyBoolYes; } - - void SetRegisterIsValid(const RegisterInfo *reg_info, bool valid) { - if (reg_info) - return SetRegisterIsValid(reg_info->kinds[lldb::eRegisterKindLLDB], - valid); + bool GetRegisterIsUnavailable(uint32_t reg) const { + return GetRegisterValidState(reg) == eLazyBoolNo; + } + bool GetRegisterIsUnfetched(uint32_t reg) const { + return GetRegisterValidState(reg) == eLazyBoolCalculate; } - void SetRegisterIsValid(uint32_t reg, bool valid) { - assert(reg < m_reg_valid.size()); - if (reg < m_reg_valid.size()) - m_reg_valid[reg] = valid; + void SetRegisterIsValid(const RegisterInfo *reg_info) { + SetRegisterIsValidState(reg_info, eLazyBoolYes); + } + void SetRegisterIsUnavailable(const RegisterInfo *reg_info) { + SetRegisterIsValidState(reg_info, eLazyBoolNo); + } + void SetRegisterIsUnfetched(const RegisterInfo *reg_info) { + SetRegisterIsValidState(reg_info, eLazyBoolCalculate); + } + void SetRegisterIsValid(uint32_t reg) { + SetRegisterIsValidState(reg, eLazyBoolYes); + } + void SetRegisterIsUnavailable(uint32_t reg) { + SetRegisterIsValidState(reg, eLazyBoolNo); + } + void SetRegisterIsUnfetched(uint32_t reg) { + SetRegisterIsValidState(reg, eLazyBoolCalculate); } GDBRemoteDynamicRegisterInfoSP m_reg_info_sp; - std::vector<bool> m_reg_valid; + + /// eLazyBoolYes - we have the bytes for this register locally. + /// eLazyBoolCalculate - we have not yet tried to get the bytes. + /// eLazyBoolNo - the bytes are unreadable at this stop. + std::vector<LazyBool> m_reg_valid; + DataExtractor m_reg_data; bool m_read_all_at_once; bool m_write_all_at_once; bool m_gpacket_cached; private: + LazyBool GetRegisterValidState(uint32_t reg) const; + void SetAllRegistersValidState(LazyBool valid); + void SetRegisterIsValidState(const RegisterInfo *reg_info, LazyBool valid); + void SetRegisterIsValidState(uint32_t reg, LazyBool valid); + // Helper function for ReadRegisterBytes(). bool GetPrimordialRegister(const RegisterInfo *reg_info, GDBRemoteCommunicationClient &gdb_comm); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 07ef71917f771..1bdfa6f2fc436 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1752,12 +1752,16 @@ void ProcessGDBRemote::ParseExpeditedRegisters( RegisterContextSP gdb_reg_ctx_sp(gdb_thread->GetRegisterContext()); for (const auto &pair : expedited_register_map) { + uint32_t lldb_regnum = gdb_reg_ctx_sp->ConvertRegisterKindToRegisterNumber( + eRegisterKindProcessPlugin, pair.first); StringExtractor reg_value_extractor(pair.second); + if (reg_value_extractor.GetStringRef().empty()) { + gdb_thread->PrivateSetRegisterUnavailable(lldb_regnum); + continue; + } WritableDataBufferSP buffer_sp( new DataBufferHeap(reg_value_extractor.GetStringRef().size() / 2, 0)); reg_value_extractor.GetHexBytes(buffer_sp->GetData(), '\xcc'); - uint32_t lldb_regnum = gdb_reg_ctx_sp->ConvertRegisterKindToRegisterNumber( - eRegisterKindProcessPlugin, pair.first); gdb_thread->PrivateSetRegisterValue(lldb_regnum, buffer_sp->GetData()); } } diff --git a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp index 3d23c074c1be3..5d7c6f346e068 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp @@ -339,6 +339,13 @@ bool ThreadGDBRemote::PrivateSetRegisterValue(uint32_t reg, uint64_t regval) { return gdb_reg_ctx->PrivateSetRegisterValue(reg, regval); } +void ThreadGDBRemote::PrivateSetRegisterUnavailable(uint32_t reg) { + GDBRemoteRegisterContext *gdb_reg_ctx = + static_cast<GDBRemoteRegisterContext *>(GetRegisterContext().get()); + assert(gdb_reg_ctx); + gdb_reg_ctx->SetRegisterIsUnavailable(reg); +} + bool ThreadGDBRemote::CalculateStopInfo() { ProcessSP process_sp(GetProcess()); if (process_sp) diff --git a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h index 5bc90a3dedceb..714d2aaba5ac2 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h @@ -109,6 +109,8 @@ class ThreadGDBRemote : public Thread { bool PrivateSetRegisterValue(uint32_t reg, uint64_t regval); + void PrivateSetRegisterUnavailable(uint32_t reg); + bool CachedQueueInfoIsValid() const { return m_queue_kind != lldb::eQueueKindUnknown; } diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestUnavailableRegisters.py b/lldb/test/API/functionalities/gdb_remote_client/TestUnavailableRegisters.py new file mode 100644 index 0000000000000..92f48d1a1d554 --- /dev/null +++ b/lldb/test/API/functionalities/gdb_remote_client/TestUnavailableRegisters.py @@ -0,0 +1,105 @@ +import lldb +from lldbsuite.test.lldbtest import * +from lldbsuite.test.decorators import * +from lldbsuite.test.gdbclientutils import * +from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase + +# Test that lldb will cache when a register is unavailable, +# and not request the value multiple times at a stop event. +# +# Also test that a stop reply packet which indicates that +# the register cannot be read currently will keep lldb from +# trying to read it again. + + +class TestUnavailableRegisters(GDBRemoteTestBase): + @skipIfXmlSupportMissing + @skipIfLLVMTargetMissing("AArch64") + def test(self): + class MyResponder(MockGDBServerResponder): + def __init__(self): + super().__init__() + self.initial_x2_read = True + + def haltReason(self): + return "T02thread:1ff0d;threads:1ff0d;thread-pcs:000000010001bc00;00:0100000000000000;01:;03:0500000000000000;04:00bc010001000000;" + + def threadStopInfo(self, threadnum): + return "T02thread:1ff0d;threads:1ff0d;thread-pcs:000000010001bc00;00:0100000000000000;01:;03:0500000000000000;04:00bc010001000000;" + + def writeRegisters(self): + return "E02" + + def qHostInfo(self): + return "cputype:16777228;cpusubtype:2;endian:little;ptrsize:8;" + + def readRegisters(self): + return "E01" + + def readRegister(self, regnum): + # Register 1 was listed in the stop reply packet's expedited + # registers as unavailable. lldb should not send a "p1" to try + # to read it. Send back a value so we can detect if this happened. + if regnum == 1: + return "5555555555555555" + + # Register 2 was not included in the stop reply packet. Return an + # error when we try to read it the first time, return a value the + # second time we try to read it. We should never see the value. + if regnum == 2: + if self.initial_x2_read: + return "E20" + self.initial_x2_read = False + else: + return "8888888888888888" + + return "E03" + + def qXferRead(self, obj, annex, offset, length): + if annex == "target.xml": + return ( + """<?xml version="1.0"?> + <target version="1.0"> + <architecture>aarch64</architecture> + <feature name="org.gnu.gdb.aarch64.core"> + <reg name="x0" regnum="0" bitsize="64"/> + <reg name="x1" bitsize="64"/> + <reg name="x2" bitsize="64"/> + <reg name="x3" bitsize="64"/> + <reg name="x4" bitsize="64"/> + <reg name="pc" bitsize="64"/> + </feature> + </target>""", + False, + ) + else: + return None, False + + self.server.responder = MyResponder() + target = self.dbg.CreateTarget("") + if self.TraceOn(): + self.runCmd("log enable gdb-remote packets") + self.addTearDownHook(lambda: self.runCmd("log disable gdb-remote packets")) + process = self.connect(target) + + thread = process.GetThreadAtIndex(0) + frame = thread.GetFrameAtIndex(0) + + self.assertEqual(frame.FindRegister("pc").GetValueAsUnsigned(), 0x10001BC00) + + # value 1 from the stop reply packet's expedited registers + self.assertEqual(frame.FindRegister("x0").GetValueAsUnsigned(), 1) + + # Register is marked unavailable in the stop reply packet's + # expedited register list. Should not get a value by sending + # 'p1'. + self.assertFailure(frame.FindRegister("x1").GetError()) + + # x2 is _not_ in the expedited registers, and will reply with + # an error the first time it is requested. + self.assertFailure(frame.FindRegister("x2").GetError()) + + # x2 will reply with a value the second time it is requested. + # lldb should cache the error from the first attempt and not + # try again. + self.assertFailure(frame.FindRegister("x2").GetError()) diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp index 7f419ae53bfbe..1cde839b9c2b0 100644 --- a/lldb/tools/debugserver/source/RNBRemote.cpp +++ b/lldb/tools/debugserver/source/RNBRemote.cpp @@ -2909,11 +2909,12 @@ rnb_err_t RNBRemote::SendStopReplyPacketForThread(nub_thread_t tid) { if (!DNBThreadGetRegisterValueByID(pid, tid, regset, g_reg_entries[reg].nub_info.reg, reg_value.get())) - continue; - - debugserver_regnum_with_fixed_width_hex_register_value( - ostrm, pid, tid, &g_reg_entries[reg], reg_value.get(), - std::nullopt); + // base16(regnum):<no value>; means a register that cannot be fetched. + ostrm << RAWHEX8(g_reg_entries[reg].debugserver_regnum) << ":;"; + else + debugserver_regnum_with_fixed_width_hex_register_value( + ostrm, pid, tid, &g_reg_entries[reg], reg_value.get(), + std::nullopt); } } @@ -5716,15 +5717,30 @@ RNBRemote::GetJSONThreadsInfo(bool threads_with_valid_stop_info_only) { new JSONGenerator::Dictionary()); for (uint32_t reg = 0; reg < g_num_reg_entries; reg++) { + bool include_reg = false; // Expedite all registers in the first register set that aren't - // contained in other registers + // contained in other registers. if (g_reg_entries[reg].nub_info.set == 1 && - g_reg_entries[reg].nub_info.value_regs == NULL) { + g_reg_entries[reg].nub_info.value_regs == NULL) + include_reg = true; + // Include the SME state register values, whether we can + // fetch the value or not. + if (strcmp("svcr", g_reg_entries[reg].nub_info.name) == 0 || + strcmp("tpidr2", g_reg_entries[reg].nub_info.name) == 0 || + strcmp("svl", g_reg_entries[reg].nub_info.name) == 0) + include_reg = true; + + if (include_reg) { if (!DNBThreadGetRegisterValueByID( pid, tid, g_reg_entries[reg].nub_info.set, - g_reg_entries[reg].nub_info.reg, reg_value.get())) + g_reg_entries[reg].nub_info.reg, reg_value.get())) { + // Indicate unavailable registers as having an empty value + // string. + std::ostringstream reg_num; + reg_num << std::dec << g_reg_entries[reg].debugserver_regnum; + registers_dict_sp->AddStringItem(reg_num.str(), ""); continue; - + } std::ostringstream reg_num; reg_num << std::dec << g_reg_entries[reg].debugserver_regnum; // Encode native byte ordered bytes as hex ascii >From de6e38d860f4ef5eec43923a5ec52d79cb5b9c68 Mon Sep 17 00:00:00 2001 From: Jason Molenda <[email protected]> Date: Mon, 27 Apr 2026 13:52:38 -0700 Subject: [PATCH 2/8] try to add debug prints to x86-fp-write.test to debug x86 Linux PR test failure --- lldb/test/Shell/Register/x86-fp-write.test | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lldb/test/Shell/Register/x86-fp-write.test b/lldb/test/Shell/Register/x86-fp-write.test index 2040c2763307a..4db85e5d0ecc6 100644 --- a/lldb/test/Shell/Register/x86-fp-write.test +++ b/lldb/test/Shell/Register/x86-fp-write.test @@ -7,6 +7,8 @@ # RUN: %lldb -b -s %s %t | FileCheck %s process launch +reg read -a + register write fctrl 0x037b register write fstat 0x8884 # note: this needs to enable all registers for writes to be effective @@ -26,10 +28,14 @@ register write st5 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff}" register write st6 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff}" register write st7 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}" +reg read -a + process continue -# CHECK: process continue -# CHECK-DAG: fctrl = 0x037b +reg read -a + + +# CHECK: fctrl = 0x037b # CHECK-DAG: fstat = 0x8884 # CHECK-DAG: ftag = 0xa961 # CHECK-DAG: fop = 0x0033 >From abc44773838c1099fd9e9b96739dc316a6f74fe3 Mon Sep 17 00:00:00 2001 From: Jason Molenda <[email protected]> Date: Fri, 8 May 2026 14:15:06 -0700 Subject: [PATCH 3/8] Update lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp Co-authored-by: Jonas Devlieghere <[email protected]> --- .../Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp index c338c96b829a0..a2535132dc12c 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp @@ -54,10 +54,7 @@ GDBRemoteRegisterContext::GDBRemoteRegisterContext( GDBRemoteRegisterContext::~GDBRemoteRegisterContext() = default; void GDBRemoteRegisterContext::SetAllRegistersValidState(LazyBool valid) { - if (valid == eLazyBoolYes) - m_gpacket_cached = true; - else - m_gpacket_cached = false; + m_gpacket_cached = (valid == eLazyBoolYes); std::vector<LazyBool>::iterator pos, end = m_reg_valid.end(); for (pos = m_reg_valid.begin(); pos != end; ++pos) *pos = valid; >From 75eb48cb23487301ef78601a12b1970519ba8a35 Mon Sep 17 00:00:00 2001 From: Jason Molenda <[email protected]> Date: Fri, 8 May 2026 14:15:52 -0700 Subject: [PATCH 4/8] Update lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp Co-authored-by: Jonas Devlieghere <[email protected]> --- .../Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp index a2535132dc12c..71600015058aa 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp @@ -55,9 +55,8 @@ GDBRemoteRegisterContext::~GDBRemoteRegisterContext() = default; void GDBRemoteRegisterContext::SetAllRegistersValidState(LazyBool valid) { m_gpacket_cached = (valid == eLazyBoolYes); - std::vector<LazyBool>::iterator pos, end = m_reg_valid.end(); - for (pos = m_reg_valid.begin(); pos != end; ++pos) - *pos = valid; + for (auto& reg : m_reg_valid) + reg = valid; } LazyBool GDBRemoteRegisterContext::GetRegisterValidState(uint32_t reg) const { >From 5b090b2961c147d06b9613e168cabb4f816a4401 Mon Sep 17 00:00:00 2001 From: Jason Molenda <[email protected]> Date: Fri, 8 May 2026 14:16:45 -0700 Subject: [PATCH 5/8] Revert "try to add debug prints to x86-fp-write.test" This reverts commit de6e38d860f4ef5eec43923a5ec52d79cb5b9c68. --- lldb/test/Shell/Register/x86-fp-write.test | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lldb/test/Shell/Register/x86-fp-write.test b/lldb/test/Shell/Register/x86-fp-write.test index 4db85e5d0ecc6..2040c2763307a 100644 --- a/lldb/test/Shell/Register/x86-fp-write.test +++ b/lldb/test/Shell/Register/x86-fp-write.test @@ -7,8 +7,6 @@ # RUN: %lldb -b -s %s %t | FileCheck %s process launch -reg read -a - register write fctrl 0x037b register write fstat 0x8884 # note: this needs to enable all registers for writes to be effective @@ -28,14 +26,10 @@ register write st5 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x80 0xff 0xff}" register write st6 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xc0 0xff 0xff}" register write st7 "{0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00}" -reg read -a - process continue -reg read -a - - -# CHECK: fctrl = 0x037b +# CHECK: process continue +# CHECK-DAG: fctrl = 0x037b # CHECK-DAG: fstat = 0x8884 # CHECK-DAG: ftag = 0xa961 # CHECK-DAG: fop = 0x0033 >From 71996ff6583685acd339091801d579c0bb2e5e1d Mon Sep 17 00:00:00 2001 From: Jason Molenda <[email protected]> Date: Mon, 11 May 2026 00:28:12 -0700 Subject: [PATCH 6/8] Address some of Jonas' review comments. Make it clearer that the entire m_reg_valid set is being set to Calculate to start. Initialize a register byte data buffer to 0, not meaninglessly eLazyBoolCalculate. This will fix the x86-fp-write.test test fail on x86_64 Ubuntu, where writing to a slice register when the primordial hadn't been read yet (so the full register bytes aren't available). I'll deal with that misbehavior in a different PR. Also add a check in ProcessGDBRemote::ParseExpeditedRegisters that we are able to get the lldb register number for an expedited RegisterInfo. --- .../gdb-remote/GDBRemoteRegisterContext.cpp | 9 ++++++--- .../Process/gdb-remote/ProcessGDBRemote.cpp | 18 ++++++++++-------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp index 71600015058aa..e92b1f3347341 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp @@ -41,11 +41,12 @@ GDBRemoteRegisterContext::GDBRemoteRegisterContext( // Resize our vector of bools to contain one bool for every register. We will // use these boolean values to know when a register value is valid in // m_reg_data. - m_reg_valid.resize(m_reg_info_sp->GetNumRegisters(), eLazyBoolCalculate); + m_reg_valid.resize(m_reg_info_sp->GetNumRegisters()); + SetAllRegistersValidState(eLazyBoolCalculate); // Make a heap based buffer that is big enough to store all registers - DataBufferSP reg_data_sp(new DataBufferHeap( - m_reg_info_sp->GetRegisterDataByteSize(), eLazyBoolCalculate)); + DataBufferSP reg_data_sp( + new DataBufferHeap(m_reg_info_sp->GetRegisterDataByteSize(), 0)); m_reg_data.SetData(reg_data_sp); m_reg_data.SetByteOrder(thread.GetProcess()->GetByteOrder()); } @@ -281,6 +282,8 @@ bool GDBRemoteRegisterContext::ReadRegisterBytes(const RegisterInfo *reg_info) { bool success = true; for (uint32_t idx = 0; success; ++idx) { const uint32_t prim_reg = reg_info->value_regs[idx]; + // We've fetched all primordial registers that provide + // data for this reg. if (prim_reg == LLDB_INVALID_REGNUM) break; // We have a valid primordial register as our constituent. Grab the diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 1bdfa6f2fc436..cfa1061d7324f 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -1754,15 +1754,17 @@ void ProcessGDBRemote::ParseExpeditedRegisters( for (const auto &pair : expedited_register_map) { uint32_t lldb_regnum = gdb_reg_ctx_sp->ConvertRegisterKindToRegisterNumber( eRegisterKindProcessPlugin, pair.first); - StringExtractor reg_value_extractor(pair.second); - if (reg_value_extractor.GetStringRef().empty()) { - gdb_thread->PrivateSetRegisterUnavailable(lldb_regnum); - continue; + if (lldb_regnum != LLDB_INVALID_REGNUM) { + StringExtractor reg_value_extractor(pair.second); + if (reg_value_extractor.GetStringRef().empty()) { + gdb_thread->PrivateSetRegisterUnavailable(lldb_regnum); + continue; + } + WritableDataBufferSP buffer_sp( + new DataBufferHeap(reg_value_extractor.GetStringRef().size() / 2, 0)); + reg_value_extractor.GetHexBytes(buffer_sp->GetData(), '\xcc'); + gdb_thread->PrivateSetRegisterValue(lldb_regnum, buffer_sp->GetData()); } - WritableDataBufferSP buffer_sp( - new DataBufferHeap(reg_value_extractor.GetStringRef().size() / 2, 0)); - reg_value_extractor.GetHexBytes(buffer_sp->GetData(), '\xcc'); - gdb_thread->PrivateSetRegisterValue(lldb_regnum, buffer_sp->GetData()); } } >From 8bfadad996d26f8a41aabe6dadd6e6a5717c7c10 Mon Sep 17 00:00:00 2001 From: Jason Molenda <[email protected]> Date: Mon, 11 May 2026 00:34:11 -0700 Subject: [PATCH 7/8] ws fix --- .../Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp index e92b1f3347341..a81dbf060b4d6 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp @@ -56,8 +56,8 @@ GDBRemoteRegisterContext::~GDBRemoteRegisterContext() = default; void GDBRemoteRegisterContext::SetAllRegistersValidState(LazyBool valid) { m_gpacket_cached = (valid == eLazyBoolYes); - for (auto& reg : m_reg_valid) - reg = valid; + for (auto ® : m_reg_valid) + reg = valid; } LazyBool GDBRemoteRegisterContext::GetRegisterValidState(uint32_t reg) const { >From 6d8c43263a1aa0f71e1d91eebf29815fa7045b5f Mon Sep 17 00:00:00 2001 From: Jason Molenda <[email protected]> Date: Mon, 11 May 2026 00:40:58 -0700 Subject: [PATCH 8/8] ws fix plus --- .../Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp index a81dbf060b4d6..922edb23d3424 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp @@ -55,9 +55,9 @@ GDBRemoteRegisterContext::GDBRemoteRegisterContext( GDBRemoteRegisterContext::~GDBRemoteRegisterContext() = default; void GDBRemoteRegisterContext::SetAllRegistersValidState(LazyBool valid) { - m_gpacket_cached = (valid == eLazyBoolYes); - for (auto ® : m_reg_valid) - reg = valid; + m_gpacket_cached = (valid == eLazyBoolYes); + for (auto ® : m_reg_valid) + reg = valid; } LazyBool GDBRemoteRegisterContext::GetRegisterValidState(uint32_t reg) const { _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
