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) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
