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

[via lldb-commits]

https://github.com/xusheng6 created https://github.com/llvm/llvm-project/pull/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, 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


>From 73c98df4baef99f96d9c67113ba2ed0d972e5a04 Mon Sep 17 00:00:00 2001
From: Xusheng <xusheng at vector35.com>
Date: Mon, 20 Mar 2023 20:24:11 +0800
Subject: [PATCH] [lldb] Claim to support swbreak and hwbreak packets when
 debugging a gdbremote

---
 .../Process/gdb-remote/GDBRemoteCommunicationClient.cpp        | 2 +-
 lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp    | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

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))