guiandrade updated this revision to Diff 226958.
guiandrade edited the summary of this revision.
guiandrade added a comment.
Moving to a single config, use-g-packet-for-reading, that forces the use of 'g'
packets for reading, but does not force 'G' for writing. The latter only ends
up being used if 'p' packets aren't supported (it assumes that 'P' also will
not).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62931/new/
https://reviews.llvm.org/D62931
Files:
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
@@ -301,13 +301,14 @@
if (process_sp) {
ProcessGDBRemote *gdb_process =
static_cast<ProcessGDBRemote *>(process_sp.get());
- // read_all_registers_at_once will be true if 'p' packet is not
- // supported.
+ bool pSupported =
+ gdb_process->GetGDBRemote().GetpPacketSupported(GetID());
bool read_all_registers_at_once =
- !gdb_process->GetGDBRemote().GetpPacketSupported(GetID());
+ !pSupported || gdb_process->m_use_g_packet_for_reading;
+ bool write_all_registers_at_once = !pSupported;
reg_ctx_sp = std::make_shared<GDBRemoteRegisterContext>(
*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();
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
@@ -13,4 +13,8 @@
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.">;
}
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -284,6 +284,7 @@
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_replay_mode;
bool m_allow_flash_writes;
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -154,6 +154,11 @@
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);
+ }
};
typedef std::shared_ptr<PluginProperties> ProcessKDPPropertiesSP;
@@ -309,6 +314,9 @@
GetGlobalPluginProperties()->GetPacketTimeout();
if (timeout_seconds > 0)
m_gdb_comm.SetPacketTimeout(std::chrono::seconds(timeout_seconds));
+
+ m_use_g_packet_for_reading =
+ GetGlobalPluginProperties()->GetUseGPacketForReading();
}
// Destructor
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ 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<bool> m_reg_valid;
DataExtractor m_reg_data;
bool m_read_all_at_once;
+ bool m_write_all_at_once;
private:
// Helper function for ReadRegisterBytes().
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ 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);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -596,6 +596,8 @@
Status GetQXferMemoryMapRegionInfo(lldb::addr_t addr,
MemoryRegionInfo ®ion);
+ LazyBool GetThreadPacketSupported(lldb::tid_t tid, llvm::StringRef packetStr);
+
private:
DISALLOW_COPY_AND_ASSIGN(GDBRemoteCommunicationClient);
};
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -543,21 +543,24 @@
//
// 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;
}
+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;
Index: lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
@@ -115,7 +115,7 @@
return self.readRegister(int(regnum, 16))
if packet[0] == "P":
register, value = packet[1:].split("=")
- return self.readRegister(int(register, 16), value)
+ return self.writeRegister(int(register, 16), value)
if packet[0] == "m":
addr, length = [int(x, 16) for x in packet[1:].split(',')]
return self.readMemory(addr, length)
Index: lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -7,6 +7,18 @@
class TestGDBRemoteClient(GDBRemoteTestBase):
+ class gPacketResponder(MockGDBServerResponder):
+ def readRegisters(self):
+ return '0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000'
+
+ 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 +49,75 @@
error = lldb.SBError()
target.AttachToProcessWithID(lldb.SBListener(), 47, error)
self.assertEquals(error_msg, error.GetCString())
+
+ def test_read_registers_using_g_packets(self):
+ """Test reading registers using 'g' packets (default behavior)"""
+ self.server.responder = self.gPacketResponder()
+ target = self.createTarget("a.yaml")
+ process = self.connect(target)
+
+ self.assertEquals(1, self.server.responder.packetLog.count("g"))
+ self.server.responder.packetLog = []
+ self.read_registers(process)
+ # Reading registers should not cause any 'p' packets to be exchanged.
+ self.assertEquals(
+ 0, len([p for p in self.server.responder.packetLog if p.startswith("p")]))
+
+ def test_read_registers_using_p_packets(self):
+ """Test reading registers using 'p' packets"""
+ self.dbg.HandleCommand(
+ "settings set plugin.process.gdb-remote.use-g-packet-for-reading false")
+ target = self.createTarget("a.yaml")
+ process = self.connect(target)
+
+ self.read_registers(process)
+ self.assertFalse("g" in self.server.responder.packetLog)
+ self.assertGreater(
+ len([p for p in self.server.responder.packetLog if p.startswith("p")]), 0)
+
+ def test_write_registers_using_P_packets(self):
+ """Test writing registers using 'P' packets (default behavior)"""
+ self.server.responder = self.gPacketResponder()
+ target = self.createTarget("a.yaml")
+ process = self.connect(target)
+
+ self.write_registers(process)
+ self.assertEquals(0, len(
+ [p for p in self.server.responder.packetLog if p.startswith("G")]))
+ self.assertGreater(
+ len([p for p in self.server.responder.packetLog if p.startswith("P")]), 0)
+
+ def test_write_registers_using_G_packets(self):
+ """Test writing registers using 'G' packets"""
+
+ class MyResponder(self.gPacketResponder):
+ def readRegister(self, register):
+ # empty string means unsupported
+ return ""
+
+ self.server.responder = MyResponder()
+ target = self.createTarget("a.yaml")
+ process = self.connect(target)
+
+ self.write_registers(process)
+ self.assertEquals(0, len(
+ [p for p in self.server.responder.packetLog if p.startswith("P")]))
+ self.assertGreater(len(
+ [p for p in self.server.responder.packetLog if p.startswith("G")]), 0)
+
+ def read_registers(self, process):
+ self.for_each_gpr(
+ process, lambda r: self.assertEquals("0x00000000", r.GetValue()))
+
+ def write_registers(self, process):
+ self.for_each_gpr(
+ process, lambda r: r.SetValueFromCString("0x00000000"))
+
+ def for_each_gpr(self, process, operation):
+ registers = process.GetThreadAtIndex(0).GetFrameAtIndex(0).GetRegisters()
+ self.assertGreater(registers.GetSize(), 0)
+ regSet = registers[0]
+ numChildren = regSet.GetNumChildren()
+ self.assertGreater(numChildren, 0)
+ for i in range(numChildren):
+ operation(regSet.GetChildAtIndex(i))
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
