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

[via lldb-commits]

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

>From 51a356b384f28cce3b2cd874fbba139414b0e207 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

---
 .../gdb-remote/GDBRemoteCommunicationClient.cpp   |  7 +++++--
 .../GDBRemoteCommunicationServerLLGS.cpp          |  4 ++++
 .../Process/gdb-remote/ProcessGDBRemote.cpp       |  3 +++
 .../gdb_remote_client/TestStopPCs.py              | 15 +++++++++++----
 4 files changed, 23 insertions(+), 6 deletions(-)

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)