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

This fixes #56125 and
vadimcn/codelldb#666, as well as the
downstream issue in our binary ninja debugger:
Vector35/debugger#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

Author: xusheng6

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

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

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

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)