diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py b/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py --- a/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py +++ b/lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py @@ -7,6 +7,14 @@ class TestGDBRemoteClient(GDBRemoteTestBase): + def setUp(self): + super(TestGDBRemoteClient, self).setUp() + self._initial_platform = lldb.DBG.GetSelectedPlatform() + + def tearDown(self): + lldb.DBG.SetSelectedPlatform(self._initial_platform) + super(TestGDBRemoteClient, self).tearDown() + def test_connect(self): """Test connecting to a remote gdb server""" target = self.createTarget("a.yaml") @@ -37,3 +45,17 @@ error = lldb.SBError() target.AttachToProcessWithID(lldb.SBListener(), 47, error) self.assertEquals(error_msg, error.GetCString()) + + def test_get_registers_uses_g_packet(self): + """Test if the client uses a 'g' packet to read registers by default""" + class MyResponder(MockGDBServerResponder): + def readRegisters(self): + return '0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000' + + self.server.responder = MyResponder() + + target = self.createTarget("a.yaml") + process = self.connect(target) + registers = process.GetThreadAtIndex(0).GetFrameAtIndex(0).GetRegisters() + self.assertGreater(registers.GetSize(), 0) + self.assertPacketLogContains(["g"]) diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h --- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h +++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h @@ -60,6 +60,11 @@ virtual size_t GetFPRSize() = 0; + virtual uint32_t GetPtraceOffset(uint32_t gdb_remote_offset, + uint32_t reg_index) { + return gdb_remote_offset; + } + // The Do*** functions are executed on the privileged thread and can perform // ptrace // operations directly. diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp --- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp @@ -34,8 +34,8 @@ if (!reg_info) return Status("register %" PRIu32 " not found", reg_index); - return DoReadRegisterValue(reg_info->byte_offset, reg_info->name, - reg_info->byte_size, reg_value); + return DoReadRegisterValue(GetPtraceOffset(reg_info->byte_offset, reg_index), + reg_info->name, reg_info->byte_size, reg_value); } Status @@ -91,7 +91,8 @@ "for write register index %" PRIu32, __FUNCTION__, reg_to_write); - return DoWriteRegisterValue(reg_info->byte_offset, reg_info->name, reg_value); + return DoWriteRegisterValue(GetPtraceOffset(reg_info->byte_offset, reg_index), + reg_info->name, reg_value); } Status NativeRegisterContextLinux::ReadGPR() { diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h --- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h +++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h @@ -75,6 +75,9 @@ Status WriteFPR() override; + uint32_t GetPtraceOffset(uint32_t gdb_remote_offset, + uint32_t reg_index) override; + private: // Private member types. enum class XStateType { Invalid, FXSAVE, XSAVE }; diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp --- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp @@ -1213,4 +1213,11 @@ return 4; } +uint32_t +NativeRegisterContextLinux_x86_64::GetPtraceOffset(uint32_t gdb_remote_offset, + uint32_t reg_index) { + // If register is MPX, remove extra factor from gdb offset + return gdb_remote_offset - (IsMPX(reg_index) ? 128 : 0); +} + #endif // defined(__i386__) || defined(__x86_64__) diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h b/lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h --- a/lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h +++ b/lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h @@ -25,15 +25,18 @@ LLVM_EXTENSION offsetof(FPR, xsave) + \ LLVM_EXTENSION offsetof(XSAVE, ymmh[0]) + (32 * reg_index)) +// Guarantees BNDR/BNDC offsets do not overlap with YMM offsets. +#define GDB_REMOTE_OFFSET 128 + #define BNDR_OFFSET(reg_index) \ (LLVM_EXTENSION offsetof(UserArea, fpr) + \ LLVM_EXTENSION offsetof(FPR, xsave) + \ - LLVM_EXTENSION offsetof(XSAVE, mpxr[reg_index])) + LLVM_EXTENSION offsetof(XSAVE, mpxr[reg_index]) + GDB_REMOTE_OFFSET) #define BNDC_OFFSET(reg_index) \ (LLVM_EXTENSION offsetof(UserArea, fpr) + \ LLVM_EXTENSION offsetof(FPR, xsave) + \ - LLVM_EXTENSION offsetof(XSAVE, mpxc[reg_index])) + LLVM_EXTENSION offsetof(XSAVE, mpxc[reg_index]) + GDB_REMOTE_OFFSET) #ifdef DECLARE_REGISTER_INFOS_X86_64_STRUCT diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h @@ -234,6 +234,10 @@ bool GetpPacketSupported(lldb::tid_t tid); + bool GetgPacketSupported(lldb::tid_t tid); + + bool GetGPacketSupported(lldb::tid_t tid); + bool GetxPacketSupported(); bool GetVAttachOrWaitSupported(); @@ -516,6 +520,8 @@ LazyBool m_prepare_for_reg_writing_reply; LazyBool m_supports_p; LazyBool m_supports_x; + LazyBool m_supports_g; + LazyBool m_supports_G; LazyBool m_avoid_g_packets; LazyBool m_supports_QSaveRegisterState; LazyBool m_supports_qXfer_auxv_read; @@ -595,6 +601,8 @@ Status GetQXferMemoryMapRegionInfo(lldb::addr_t addr, MemoryRegionInfo ®ion); + LazyBool GetThreadPacketSupported(lldb::tid_t tid, llvm::StringRef packetStr); + private: DISALLOW_COPY_AND_ASSIGN(GDBRemoteCommunicationClient); }; diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -69,6 +69,7 @@ m_attach_or_wait_reply(eLazyBoolCalculate), m_prepare_for_reg_writing_reply(eLazyBoolCalculate), m_supports_p(eLazyBoolCalculate), m_supports_x(eLazyBoolCalculate), + m_supports_g(eLazyBoolCalculate), m_supports_G(eLazyBoolCalculate), m_avoid_g_packets(eLazyBoolCalculate), m_supports_QSaveRegisterState(eLazyBoolCalculate), m_supports_qXfer_auxv_read(eLazyBoolCalculate), @@ -543,21 +544,36 @@ // // Takes a valid thread ID because p needs to apply to a thread. bool GDBRemoteCommunicationClient::GetpPacketSupported(lldb::tid_t tid) { - if (m_supports_p == eLazyBoolCalculate) { - m_supports_p = eLazyBoolNo; - StreamString payload; - payload.PutCString("p0"); - StringExtractorGDBRemote response; - if (SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), - response, false) == - PacketResult::Success && - response.IsNormalResponse()) { - m_supports_p = eLazyBoolYes; - } - } + if (m_supports_p == eLazyBoolCalculate) + m_supports_p = GetThreadPacketSupported(tid, "p0"); return m_supports_p; } +bool GDBRemoteCommunicationClient::GetgPacketSupported(lldb::tid_t tid) { + if (m_supports_g == eLazyBoolCalculate) + m_supports_g = GetThreadPacketSupported(tid, "g"); + return m_supports_g; +} + +bool GDBRemoteCommunicationClient::GetGPacketSupported(lldb::tid_t tid) { + if (m_supports_G == eLazyBoolCalculate) + m_supports_G = GetThreadPacketSupported(tid, "G"); + return m_supports_G; +} + +LazyBool GDBRemoteCommunicationClient::GetThreadPacketSupported( + lldb::tid_t tid, llvm::StringRef packetStr) { + StreamString payload; + payload.PutCString(packetStr); + StringExtractorGDBRemote response; + if (SendThreadSpecificPacketAndWaitForResponse( + tid, std::move(payload), response, false) == PacketResult::Success && + response.IsNormalResponse()) { + return eLazyBoolYes; + } + return eLazyBoolNo; +} + StructuredData::ObjectSP GDBRemoteCommunicationClient::GetThreadsInfo() { // Get information on all threads at one using the "jThreadsInfo" packet StructuredData::ObjectSP object_sp; diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h @@ -41,7 +41,7 @@ public: GDBRemoteRegisterContext(ThreadGDBRemote &thread, uint32_t concrete_frame_idx, GDBRemoteDynamicRegisterInfo ®_info, - bool read_all_at_once); + bool read_all_at_once, bool write_all_at_once); ~GDBRemoteRegisterContext() override; @@ -114,6 +114,7 @@ std::vector m_reg_valid; DataExtractor m_reg_data; bool m_read_all_at_once; + bool m_write_all_at_once; private: // Helper function for ReadRegisterBytes(). diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp @@ -31,9 +31,11 @@ // GDBRemoteRegisterContext constructor GDBRemoteRegisterContext::GDBRemoteRegisterContext( ThreadGDBRemote &thread, uint32_t concrete_frame_idx, - GDBRemoteDynamicRegisterInfo ®_info, bool read_all_at_once) + GDBRemoteDynamicRegisterInfo ®_info, bool read_all_at_once, + bool write_all_at_once) : RegisterContext(thread, concrete_frame_idx), m_reg_info(reg_info), - m_reg_valid(), m_reg_data(), m_read_all_at_once(read_all_at_once) { + m_reg_valid(), m_reg_data(), m_read_all_at_once(read_all_at_once), + m_write_all_at_once(write_all_at_once) { // 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. @@ -333,7 +335,7 @@ { GDBRemoteClientBase::Lock lock(gdb_comm, false); if (lock) { - if (m_read_all_at_once) { + if (m_write_all_at_once) { // Invalidate all register values InvalidateIfNeeded(true); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -284,6 +284,8 @@ lldb::CommandObjectSP m_command_sp; int64_t m_breakpoint_pc_offset; lldb::tid_t m_initial_tid; // The initial thread ID, given by stub on attach + bool m_use_g_packet_for_reading; + bool m_use_g_packet_for_writing; bool m_replay_mode; bool m_allow_flash_writes; diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -154,6 +154,16 @@ nullptr, idx, g_processgdbremote_properties[idx].default_uint_value != 0); } + + bool GetUseGPacketForReading() const { + const uint32_t idx = ePropertyUseGPacketForReading; + return m_collection_sp->GetPropertyAtIndexAsBoolean(nullptr, idx, true); + } + + bool GetUseGPacketForWriting() const { + const uint32_t idx = ePropertyUseGPacketForWriting; + return m_collection_sp->GetPropertyAtIndexAsBoolean(nullptr, idx, true); + } }; typedef std::shared_ptr ProcessKDPPropertiesSP; @@ -309,6 +319,11 @@ GetGlobalPluginProperties()->GetPacketTimeout(); if (timeout_seconds > 0) m_gdb_comm.SetPacketTimeout(std::chrono::seconds(timeout_seconds)); + + m_use_g_packet_for_reading = + GetGlobalPluginProperties()->GetUseGPacketForReading(); + m_use_g_packet_for_writing = + GetGlobalPluginProperties()->GetUseGPacketForWriting(); } // Destructor diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td @@ -13,4 +13,12 @@ Global, DefaultFalse, Desc<"If true, the libraries-svr4 feature will be used to get a hold of the process's loaded modules.">; + def UseGPacketForReading: Property<"use-g-packet-for-reading", "Boolean">, + Global, + DefaultTrue, + Desc<"Specify if the server should use 'g' packets to read registers.">; + def UseGPacketForWriting: Property<"use-g-packet-for-writing", "Boolean">, + Global, + DefaultFalse, + Desc<"Specify if the server should use 'g' packets to write registers.">; } diff --git a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp --- a/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp @@ -301,13 +301,15 @@ if (process_sp) { ProcessGDBRemote *gdb_process = static_cast(process_sp.get()); - // read_all_registers_at_once will be true if 'p' packet is not - // supported. bool read_all_registers_at_once = - !gdb_process->GetGDBRemote().GetpPacketSupported(GetID()); + gdb_process->GetGDBRemote().GetgPacketSupported(GetID()) && + gdb_process->m_use_g_packet_for_reading; + bool write_all_registers_at_once = + gdb_process->GetGDBRemote().GetGPacketSupported(GetID()) && + gdb_process->m_use_g_packet_for_writing; reg_ctx_sp = std::make_shared( *this, concrete_frame_idx, gdb_process->m_register_info, - read_all_registers_at_once); + read_all_registers_at_once, write_all_registers_at_once); } } else { Unwind *unwinder = GetUnwinder(); diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp --- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp +++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp @@ -128,6 +128,11 @@ ASSERT_EQ(0, memcmp(buffer_sp->GetBytes(), one_register, sizeof one_register)); + async_result = std::async(std::launch::async, + [&] { return client.GetgPacketSupported(tid); }); + HandlePacket(server, "g;thread:0047;", all_registers_hex); + ASSERT_TRUE(async_result.get()); + read_result = std::async(std::launch::async, [&] { return client.ReadAllRegisters(tid); }); HandlePacket(server, "g;thread:0047;", all_registers_hex);