[Lldb-commits] [lldb] [lldb] Claim to support swbreak and hwbreak packets when debugging a gdbremote (PR #102873)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: xusheng (xusheng6)

<details>
<summary>Changes</summary>

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, it always sends the unmodified program counter value which, on systems like x86, causes the program counter to be off-by-off (or otherwise wrong).

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, and it will send the adjusted program counter. 

I am unable to add a unit test for this. I browsed the gdbserver unit test code and it seems to be testing against the gdbserver in lldb, which also does not use the `swbreak` packet. But as discussed in the issue, there is no reason to add support for sending swbreak in lldb-server.

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


---
Full diff: https://github.com/llvm/llvm-project/pull/102873.diff


2 Files Affected:

- (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+1-1) 
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+3) 


``````````diff
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 74e392249a94eb..d8b17a8ff59baf 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -353,7 +353,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   // build the qSupported packet
   std::vector<std::string> features = {"xmlRegisters=i386,arm,mips,arc",
                                        "multiprocess+", "fork-events+",
-                                       "vfork-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/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 6f9c2cc1e4b4e8..b8fe8fdc9b8742 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2349,6 +2349,9 @@ StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
         if (!value.getAsInteger(0, addressing_bits)) {
           addressable_bits.SetHighmemAddressableBits(addressing_bits);
         }
+      } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
+        // There is nothing needs to be done for swbreak or hwbreak since
+        // the value is expected to be empty
       } else if (key.size() == 2 && ::isxdigit(key[0]) && ::isxdigit(key[1])) {
         uint32_t reg = UINT32_MAX;
         if (!key.getAsInteger(16, reg))

``````````

</details>


https://github.com/llvm/llvm-project/pull/102873