[lldbremote] Implement support for MultiBreakpoint packet

[felipepiovezan]

This is fairly straightforward, thanks to the helper functions created in the previous commit.

The following PRs are related to the MultiBreakpoint feature:

• [lldb] Propose MultiBreakpoint extension to GDB Remote #192910
• [debugserver] Implement MultiBreakpoint #192914
• [lldbremote][NFC] Factor out code handling breakpoint packets #192915
• [lldbremote] Implement support for MultiBreakpoint packet #192919
• [lldb][GDBRemote] Parse MultiBreakpoint+ capability #192962
• [lldb][NFC] Move BreakpointSite::IsEnabled/SetEnabled into Process #192964
• [lldb] Implement delayed breakpoints #192971
• [lldb] Override UpdateBreakpointSites in ProcessGDBRemote to use MultiBreakpoint #192988

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)

Changes

This is fairly straightfoward, thanks to the helper functions created in the previous commit.

#192910

---

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

6 Files Affected:

• (modified) lldb/include/lldb/Utility/StringExtractorGDBRemote.h (+1)
• (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py (+1)
• (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp (+59)
• (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h (+2)
• (modified) lldb/source/Utility/StringExtractorGDBRemote.cpp (+2)
• (modified) lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py (-1)

diff --git a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
index 439245fdc3083..1fdf913ccf3ae 100644
--- a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -162,6 +162,7 @@ class StringExtractorGDBRemote : public StringExtractor {
     eServerPacketType_X,
     eServerPacketType_Z,
     eServerPacketType_z,
+    eServerPacketType_MultiBreakpoint,
 
     eServerPacketType__M,
     eServerPacketType__m,
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
index 5ba642bbedf74..020649bd0023c 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -932,6 +932,7 @@ def add_qSupported_packets(self, client_features=[]):
         "SupportedWatchpointTypes",
         "SupportedCompressions",
         "MultiMemRead",
+        "MultiBreakpoint",
     ]
 
     def parse_qSupported_response(self, context):
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 4c77de9f34adc..8dc0656fc9de8 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -198,6 +198,9 @@ void GDBRemoteCommunicationServerLLGS::RegisterPacketHandlers() {
                                 &GDBRemoteCommunicationServerLLGS::Handle_Z);
   RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_z,
                                 &GDBRemoteCommunicationServerLLGS::Handle_z);
+  RegisterMemberFunctionHandler(
+      StringExtractorGDBRemote::eServerPacketType_MultiBreakpoint,
+      &GDBRemoteCommunicationServerLLGS::Handle_MultiBreakpoint);
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_QPassSignals,
       &GDBRemoteCommunicationServerLLGS::Handle_QPassSignals);
@@ -3123,6 +3126,61 @@ GDBRemoteCommunicationServerLLGS::Handle_z(StringExtractorGDBRemote &packet) {
                                 ExecuteRemoveBreakpoint(packet.GetStringRef()));
 }
 
+/// Split a MultiBreakpoint packet body into individual breakpoint requests. A
+/// ';' starts a new request only if it is followed by [Zz].
+static std::vector<llvm::StringRef>
+SplitBreakpointRequests(llvm::StringRef packet) {
+  std::vector<llvm::StringRef> requests;
+  size_t request_start = 0;
+  for (size_t i = 0; i + 1 < packet.size(); ++i) {
+    if (packet[i] != ';')
+      continue;
+    char next_char = packet[i + 1];
+    if (next_char == 'Z' || next_char == 'z') {
+      requests.push_back(packet.substr(request_start, i - request_start));
+      request_start = i + 1;
+    }
+  }
+  requests.push_back(packet.substr(request_start));
+  return requests;
+}
+
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_MultiBreakpoint(
+    StringExtractorGDBRemote &packet) {
+  llvm::StringRef packet_str = packet.GetStringRef();
+  if (!packet_str.consume_front("MultiBreakpoint:"))
+    return SendIllFormedResponse(packet,
+                                 "Invalid MultiBreakpoint packet prefix");
+
+  if (packet_str.empty())
+    return SendIllFormedResponse(packet, "MultiBreakpoint has no requests");
+
+  StreamString response;
+  bool first = true;
+  for (llvm::StringRef request : SplitBreakpointRequests(packet_str)) {
+    BreakpointResult result = request.starts_with("Z")
+                                  ? ExecuteSetBreakpoint(request)
+                                  : ExecuteRemoveBreakpoint(request);
+    if (!first)
+      response.PutChar(';');
+    switch (result.kind) {
+    case BreakpointResult::Kind::OK:
+      response.PutCString("OK");
+      break;
+    case BreakpointResult::Kind::Error:
+      response.Format("E{0:X-2}", result.error_code);
+      break;
+    case BreakpointResult::Kind::IllFormed:
+      response.PutCString("E03");
+      break;
+    }
+    first = false;
+  }
+
+  return SendPacketNoLock(response.GetString());
+}
+
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_s(StringExtractorGDBRemote &packet) {
   Log *log = GetLog(LLDBLog::Process | LLDBLog::Thread);
@@ -4338,6 +4396,7 @@ std::vector<std::string> GDBRemoteCommunicationServerLLGS::HandleFeatures(
                             "QListThreadsInStopReply+",
                             "qXfer:features:read+",
                             "QNonStop+",
+                            "MultiBreakpoint+",
                         });
 
   // report server-only features
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
index a663c0b949744..f375f2f7ca24f 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -216,6 +216,8 @@ class GDBRemoteCommunicationServerLLGS
 
   PacketResult Handle_z(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_MultiBreakpoint(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_s(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qXfer(StringExtractorGDBRemote &packet);
diff --git a/lldb/source/Utility/StringExtractorGDBRemote.cpp b/lldb/source/Utility/StringExtractorGDBRemote.cpp
index 40b5d037d0463..1c89545bf1616 100644
--- a/lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ b/lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -431,6 +431,8 @@ StringExtractorGDBRemote::GetServerPacketType() const {
     return eServerPacketType_m;
 
   case 'M':
+    if (PACKET_STARTS_WITH("MultiBreakpoint:"))
+      return eServerPacketType_MultiBreakpoint;
     return eServerPacketType_M;
 
   case 'p':
diff --git a/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py b/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py
index 0978a499c7112..e7f87efd0419e 100644
--- a/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py
+++ b/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py
@@ -9,7 +9,6 @@
 from lldbsuite.test import lldbutil
 
 
-@skipUnlessDarwin # Remove once lldbsever support is implemented.
 @skipIfOutOfTreeDebugserver
 class TestMultiBreakpoint(TestBase):
     def send_packet(self, packet_str):

[github-actions]

🐧 Linux x64 Test Results

• 33422 tests passed
• 532 tests skipped

✅ The build succeeded and all tests passed.

[felipepiovezan]

Linux bots aren't too happy with the tests, will investigate.

[felipepiovezan]

This keeps timing out ever since I made the response also be a dictionary...

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[DavidSpickett]

LGTM

And I do clearly see where we would easily be able to refactor this to support other packets of the same design pattern if that ever happens.

[DavidSpickett]

We need a skipIfWindows too because we debug in process on Windows. - https://lab.llvm.org/buildbot/#/builders/141/builds/17796

[slydiman]

The buildbot lldb-x86_64-win is broken too
https://lab.llvm.org/buildbot/#/builders/211/builds/7814

[felipepiovezan]

Ah, let me fix that now. Thank you for flagging this

[felipepiovezan]

#194627