[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 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 1/5] [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))

>From 1db7c14528bb709921a0d6607c2118477c8de500 Mon Sep 17 00:00:00 2001
From: Xusheng <xusheng at vector35.com>
Date: Mon, 12 Aug 2024 18:37:40 +0800
Subject: [PATCH 2/5] Fix format

---
 .../Process/gdb-remote/GDBRemoteCommunicationClient.cpp    | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index d8b17a8ff59baf..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+", "swbreak+", "hwbreak+"};
+                                       "multiprocess+",
+                                       "fork-events+",
+                                       "vfork-events+",
+                                       "swbreak+",
+                                       "hwbreak+"};
   StreamString packet;
   packet.PutCString("qSupported");
   for (uint32_t i = 0; i < features.size(); ++i) {

>From 2d085e46c50f1c909ce5b90dd34d6c47832564d0 Mon Sep 17 00:00:00 2001
From: Xusheng <xusheng at vector35.com>
Date: Mon, 12 Aug 2024 18:43:36 +0800
Subject: [PATCH 3/5] Put the condition as the last one and update comment

---
 .../source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index b8fe8fdc9b8742..e467577ab2b4f0 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2349,13 +2349,14 @@ 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))
           expedited_register_map[reg] = std::string(std::move(value));
+      } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
+        // 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)
       }
     }
 

>From d0851d4ff7e049d0c7537f5bd6b72bd66b29474a Mon Sep 17 00:00:00 2001
From: Xusheng <xusheng at vector35.com>
Date: Mon, 12 Aug 2024 22:05:59 +0800
Subject: [PATCH 4/5] Add a comment that lldb-server consumes the declared
 swbreak/hwbreak feature from the lldb

---
 lldb/tools/debugserver/source/RNBRemote.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
index f22d626c4af2c6..8e00fa8598f35b 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -3594,6 +3594,9 @@ rnb_err_t RNBRemote::HandlePacket_qSupported(const char *p) {
   reply << "SupportedWatchpointTypes=x86_64;";
 #endif
 
+  // 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
   return SendPacket(reply.str().c_str());
 }
 

>From d61ea562c82eab27ae2cc460b37f2f0d27961c1c Mon Sep 17 00:00:00 2001
From: Xusheng <xusheng at vector35.com>
Date: Mon, 12 Aug 2024 22:06:15 +0800
Subject: [PATCH 5/5] Remove the unnecessary if branch

---
 .../source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index e467577ab2b4f0..c7ce368ab41ce2 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2353,11 +2353,10 @@ StateType ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
         uint32_t reg = UINT32_MAX;
         if (!key.getAsInteger(16, reg))
           expedited_register_map[reg] = std::string(std::move(value));
-      } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
-        // 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)
       }
+      // 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) {