[Lldb-commits] [lldb] 5dbec8c - [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (#102873)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Aug 13 07:28:40 PDT 2024
Author: xusheng
Date: 2024-08-13T15:28:35+01:00
New Revision: 5dbec8c6ce473352cac2d89d6a5b81f65182df59
URL: https://github.com/llvm/llvm-project/commit/5dbec8c6ce473352cac2d89d6a5b81f65182df59
DIFF: https://github.com/llvm/llvm-project/commit/5dbec8c6ce473352cac2d89d6a5b81f65182df59.diff
LOG: [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (#102873)
This fixes https://github.com/llvm/llvm-project/issues/56125 and
https://github.com/vadimcn/codelldb/issues/666, as well as the
downstream issue in our binary ninja debugger:
https://github.com/Vector35/debugger/issues/535
Basically, lldb does not claim to support the `swbreak` packet so the
gdbserver would not use it. As a result, the gdbserver always sends the
unmodified program counter value which, on systems like x86, causes the
program counter to be off-by-one (or otherwise wrong). For reference,
the lldb-server always sends the modified program counter value so it
works perfectly with lldb.
https://sourceware.org/gdb/current/onlinedocs/gdb.html/Stop-Reply-Packets.html#swbreak-stop-reason
No new code is added to add support `swbreak`, since the way lldb works
already expects the remote to have adjusted the program counter. The
change just lets the gdbserver know that lldb supports it, so that it
will send the adjusted program counter.
To test this PR, you can use lldb to connect to a gdbserver running on
e.g., Ubuntu 22.04, and see the program counter is off-by-one without
the patch. With the patch, things work as expected
Added:
Modified:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 74e392249a94eb..83ba27783da471 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -352,8 +352,11 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
// build the qSupported packet
std::vector<std::string> features = {"xmlRegisters=i386,arm,mips,arc",
- "multiprocess+", "fork-events+",
- "vfork-events+"};
+ "multiprocess+",
+ "fork-events+",
+ "vfork-events+",
+ "swbreak+",
+ "hwbreak+"};
StreamString packet;
packet.PutCString("qSupported");
for (uint32_t i = 0; i < features.size(); ++i) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index a0b08a219ae147..345f5cd5de8491 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -4245,6 +4245,10 @@ std::vector<std::string> GDBRemoteCommunicationServerLLGS::HandleFeatures(
.Case("vfork-events+", Extension::vfork)
.Default({});
+ // We consume lldb's swbreak/hwbreak feature, but it doesn't change the
+ // behaviour of lldb-server. We always adjust the program counter for targets
+ // like x86
+
m_extensions_supported &= plugin_features;
// fork & vfork require multiprocess
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 6f9c2cc1e4b4e8..c7ce368ab41ce2 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2354,6 +2354,9 @@ StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
if (!key.getAsInteger(16, reg))
expedited_register_map[reg] = std::string(std::move(value));
}
+ // swbreak and hwbreak are also expected keys, but we don't need to
+ // change our behaviour for them because lldb always expects the remote
+ // to adjust the program counter (if relevant, e.g., for x86 targets)
}
if (stop_pid != LLDB_INVALID_PROCESS_ID && stop_pid != pid) {
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py b/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
index ef28cc95f7ad4b..3faae5fec38ba1 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestStopPCs.py
@@ -10,13 +10,17 @@ class TestStopPCs(GDBRemoteTestBase):
def test(self):
class MyResponder(MockGDBServerResponder):
def haltReason(self):
- return "T02thread:1ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;"
+ # lldb should treat the default halt reason, hwbreak and swbreak in the same way. Which is that it
+ # expects the stub to have corrected the PC already, so lldb should not modify it further.
+ return "T02thread:1ff0d;threads:1ff0d,2ff0d,3ff0d;thread-pcs:10001bc00,10002bc00,10003bc00;"
def threadStopInfo(self, threadnum):
if threadnum == 0x1FF0D:
- return "T02thread:1ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;"
+ return "T02thread:1ff0d;threads:1ff0d,2ff0d,3ff0d;thread-pcs:10001bc00,10002bc00,10003bc00;"
if threadnum == 0x2FF0D:
- return "T00thread:2ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;"
+ return "T00swbreak:;thread:2ff0d;threads:1ff0d,2ff0d,3ff0d;thread-pcs:10001bc00,10002bc00,10003bc00;"
+ if threadnum == 0x3FF0D:
+ return "T00hwbreak:;thread:3ff0d;threads:1ff0d,2ff0d,3ff0d;thread-pcs:10001bc00,10002bc00,10003bc00;"
def qXferRead(self, obj, annex, offset, length):
if annex == "target.xml":
@@ -40,10 +44,13 @@ def qXferRead(self, obj, annex, offset, length):
self.addTearDownHook(lambda: self.runCmd("log disable gdb-remote packets"))
process = self.connect(target)
- self.assertEqual(process.GetNumThreads(), 2)
+ self.assertEqual(process.GetNumThreads(), 3)
th0 = process.GetThreadAtIndex(0)
th1 = process.GetThreadAtIndex(1)
+ th2 = process.GetThreadAtIndex(2)
self.assertEqual(th0.GetThreadID(), 0x1FF0D)
self.assertEqual(th1.GetThreadID(), 0x2FF0D)
+ self.assertEqual(th2.GetThreadID(), 0x3FF0D)
self.assertEqual(th0.GetFrameAtIndex(0).GetPC(), 0x10001BC00)
self.assertEqual(th1.GetFrameAtIndex(0).GetPC(), 0x10002BC00)
+ self.assertEqual(th2.GetFrameAtIndex(0).GetPC(), 0x10003BC00)
More information about the lldb-commits
mailing list