[llvm-branch-commits] [lldb] [lldbremote][NFC] Factor out code handling breakpoint packets (PR #192915)
Felipe de Azevedo Piovezan via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Apr 27 07:15:08 PDT 2026
https://github.com/felipepiovezan updated https://github.com/llvm/llvm-project/pull/192915
>From 3a367b3f71b7f6429cc9fe918ab79b57f1d12e06 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Fri, 20 Mar 2026 17:17:35 +0000
Subject: [PATCH 1/5] [lldb-server][NFC] Factor out code handling breakpoint
packets
This commit extracts the code handling breakpoint packets into a helper
function that can be used by a future implementation of the
MultiBreakpointPacket.
It is meant to be purely NFC.
There are two functions handling breakpoint packets (`handle_Z`
and `handle_z`) with a lot of repeated code. This commit did not attempt
to merge the two, as that would make the diff much larger due to subtle
differences in the error message produced by the two. The only
deduplication done is in the code processing a GDBStoppointType, where a
helper struct (`BreakpointKind`) and function (`std::optional<BreakpointKind> getBreakpointKind(GDBStoppointType stoppoint_type)`) was created.
https://github.com/llvm/llvm-project/pull/192910
---
.../GDBRemoteCommunicationServerLLGS.cpp | 254 ++++++++++--------
.../GDBRemoteCommunicationServerLLGS.h | 28 ++
2 files changed, 176 insertions(+), 106 deletions(-)
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 066a4cfcce8bb..dfa0755636d2a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2903,155 +2903,172 @@ GDBRemoteCommunicationServerLLGS::Handle_qMemoryRegionInfo(
return SendPacketNoLock(response.GetString());
}
-GDBRemoteCommunication::PacketResult
-GDBRemoteCommunicationServerLLGS::Handle_Z(StringExtractorGDBRemote &packet) {
+namespace {
+/// Helper struct to expand a GDBStoppointType into flags.
+struct BreakpointKind {
+ bool want_hardware;
+ bool want_breakpoint;
+ uint32_t watch_flags;
+
+ /// Invalid types must be handled prior to calling this.
+ BreakpointKind(GDBStoppointType stoppoint_type) {
+ switch (stoppoint_type) {
+ case eBreakpointSoftware:
+ want_hardware = false;
+ want_breakpoint = true;
+ break;
+ case eBreakpointHardware:
+ want_hardware = true;
+ want_breakpoint = true;
+ break;
+ case eWatchpointWrite:
+ watch_flags = 1;
+ want_hardware = true;
+ want_breakpoint = false;
+ break;
+ case eWatchpointRead:
+ watch_flags = 2;
+ want_hardware = true;
+ want_breakpoint = false;
+ break;
+ case eWatchpointReadWrite:
+ watch_flags = 3;
+ want_hardware = true;
+ want_breakpoint = false;
+ break;
+ default:
+ llvm_unreachable("unhandled GDBStoppointType");
+ }
+ }
+};
+
+/// If stoppoint_type is a valid type, create a BreakpointKind, otherwise
+/// returns nullopt.
+std::optional<BreakpointKind>
+getBreakpointKind(GDBStoppointType stoppoint_type) {
+ switch (stoppoint_type) {
+ case eBreakpointSoftware:
+ case eBreakpointHardware:
+ case eWatchpointWrite:
+ case eWatchpointRead:
+ case eWatchpointReadWrite:
+ return BreakpointKind(stoppoint_type);
+ case eStoppointInvalid:
+ break;
+ }
+ return std::nullopt;
+}
+} // namespace
+
+GDBRemoteCommunicationServerLLGS::BreakpointResult
+GDBRemoteCommunicationServerLLGS::ExecuteSetBreakpoint(
+ llvm::StringRef packet_str) {
// Ensure we have a process.
if (!m_current_process ||
(m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) {
Log *log = GetLog(LLDBLog::Process);
LLDB_LOG(log, "failed, no process available");
- return SendErrorResponse(0x15);
+ return BreakpointResult::CreateError(0x15);
}
+ StringExtractorGDBRemote packet(packet_str);
+
// Parse out software or hardware breakpoint or watchpoint requested.
packet.SetFilePos(strlen("Z"));
if (packet.GetBytesLeft() < 1)
- return SendIllFormedResponse(
- packet, "Too short Z packet, missing software/hardware specifier");
-
- bool want_breakpoint = true;
- bool want_hardware = false;
- uint32_t watch_flags = 0;
+ return BreakpointResult::CreateIllFormed(
+ "Too short Z packet, missing software/hardware specifier");
const GDBStoppointType stoppoint_type =
GDBStoppointType(packet.GetS32(eStoppointInvalid));
- switch (stoppoint_type) {
- case eBreakpointSoftware:
- want_hardware = false;
- want_breakpoint = true;
- break;
- case eBreakpointHardware:
- want_hardware = true;
- want_breakpoint = true;
- break;
- case eWatchpointWrite:
- watch_flags = 1;
- want_hardware = true;
- want_breakpoint = false;
- break;
- case eWatchpointRead:
- watch_flags = 2;
- want_hardware = true;
- want_breakpoint = false;
- break;
- case eWatchpointReadWrite:
- watch_flags = 3;
- want_hardware = true;
- want_breakpoint = false;
- break;
- case eStoppointInvalid:
- return SendIllFormedResponse(
- packet, "Z packet had invalid software/hardware specifier");
- }
+ std::optional<BreakpointKind> bp_kind = getBreakpointKind(stoppoint_type);
+ if (!bp_kind)
+ return BreakpointResult::CreateIllFormed(
+ "Z packet had invalid software/hardware specifier");
if ((packet.GetBytesLeft() < 1) || packet.GetChar() != ',')
- return SendIllFormedResponse(
- packet, "Malformed Z packet, expecting comma after stoppoint type");
+ return BreakpointResult::CreateIllFormed(
+ "Malformed Z packet, expecting comma after stoppoint type");
// Parse out the stoppoint address.
if (packet.GetBytesLeft() < 1)
- return SendIllFormedResponse(packet, "Too short Z packet, missing address");
+ return BreakpointResult::CreateIllFormed(
+ "Too short Z packet, missing address");
const lldb::addr_t addr = packet.GetHexMaxU64(false, 0);
if ((packet.GetBytesLeft() < 1) || packet.GetChar() != ',')
- return SendIllFormedResponse(
- packet, "Malformed Z packet, expecting comma after address");
+ return BreakpointResult::CreateIllFormed(
+ "Malformed Z packet, expecting comma after address");
// Parse out the stoppoint size (i.e. size hint for opcode size).
const uint32_t size =
packet.GetHexMaxU32(false, std::numeric_limits<uint32_t>::max());
if (size == std::numeric_limits<uint32_t>::max())
- return SendIllFormedResponse(
- packet, "Malformed Z packet, failed to parse size argument");
+ return BreakpointResult::CreateIllFormed(
+ "Malformed Z packet, failed to parse size argument");
- if (want_breakpoint) {
+ if (bp_kind->want_breakpoint) {
// Try to set the breakpoint.
const Status error =
- m_current_process->SetBreakpoint(addr, size, want_hardware);
+ m_current_process->SetBreakpoint(addr, size, bp_kind->want_hardware);
if (error.Success())
- return SendOKResponse();
+ return BreakpointResult::CreateOK();
Log *log = GetLog(LLDBLog::Breakpoints);
LLDB_LOG(log, "pid {0} failed to set breakpoint: {1}",
m_current_process->GetID(), error);
- return SendErrorResponse(0x09);
- } else {
- // Try to set the watchpoint.
- const Status error = m_current_process->SetWatchpoint(
- addr, size, watch_flags, want_hardware);
- if (error.Success())
- return SendOKResponse();
- Log *log = GetLog(LLDBLog::Watchpoints);
- LLDB_LOG(log, "pid {0} failed to set watchpoint: {1}",
- m_current_process->GetID(), error);
- return SendErrorResponse(0x09);
- }
+ return BreakpointResult::CreateError(0x09);
+ }
+
+ // Try to set the watchpoint.
+ const Status error = m_current_process->SetWatchpoint(
+ addr, size, bp_kind->watch_flags, bp_kind->want_hardware);
+ if (error.Success())
+ return BreakpointResult::CreateOK();
+ Log *log = GetLog(LLDBLog::Watchpoints);
+ LLDB_LOG(log, "pid {0} failed to set watchpoint: {1}",
+ m_current_process->GetID(), error);
+ return BreakpointResult::CreateError(0x09);
}
-GDBRemoteCommunication::PacketResult
-GDBRemoteCommunicationServerLLGS::Handle_z(StringExtractorGDBRemote &packet) {
+GDBRemoteCommunicationServerLLGS::BreakpointResult
+GDBRemoteCommunicationServerLLGS::ExecuteRemoveBreakpoint(
+ llvm::StringRef packet_str) {
// Ensure we have a process.
if (!m_current_process ||
(m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) {
Log *log = GetLog(LLDBLog::Process);
LLDB_LOG(log, "failed, no process available");
- return SendErrorResponse(0x15);
+ return BreakpointResult::CreateError(0x15);
}
+ StringExtractorGDBRemote packet(packet_str);
+
// Parse out software or hardware breakpoint or watchpoint requested.
packet.SetFilePos(strlen("z"));
if (packet.GetBytesLeft() < 1)
- return SendIllFormedResponse(
- packet, "Too short z packet, missing software/hardware specifier");
-
- bool want_breakpoint = true;
- bool want_hardware = false;
+ return BreakpointResult::CreateIllFormed(
+ "Too short z packet, missing software/hardware specifier");
const GDBStoppointType stoppoint_type =
GDBStoppointType(packet.GetS32(eStoppointInvalid));
- switch (stoppoint_type) {
- case eBreakpointHardware:
- want_breakpoint = true;
- want_hardware = true;
- break;
- case eBreakpointSoftware:
- want_breakpoint = true;
- break;
- case eWatchpointWrite:
- want_breakpoint = false;
- break;
- case eWatchpointRead:
- want_breakpoint = false;
- break;
- case eWatchpointReadWrite:
- want_breakpoint = false;
- break;
- default:
- return SendIllFormedResponse(
- packet, "z packet had invalid software/hardware specifier");
- }
+ std::optional<BreakpointKind> bp_kind = getBreakpointKind(stoppoint_type);
+ if (!bp_kind)
+ return BreakpointResult::CreateIllFormed(
+ "z packet had invalid software/hardware specifier");
if ((packet.GetBytesLeft() < 1) || packet.GetChar() != ',')
- return SendIllFormedResponse(
- packet, "Malformed z packet, expecting comma after stoppoint type");
+ return BreakpointResult::CreateIllFormed(
+ "Malformed z packet, expecting comma after stoppoint type");
// Parse out the stoppoint address.
if (packet.GetBytesLeft() < 1)
- return SendIllFormedResponse(packet, "Too short z packet, missing address");
+ return BreakpointResult::CreateIllFormed(
+ "Too short z packet, missing address");
const lldb::addr_t addr = packet.GetHexMaxU64(false, 0);
if ((packet.GetBytesLeft() < 1) || packet.GetChar() != ',')
- return SendIllFormedResponse(
- packet, "Malformed z packet, expecting comma after address");
+ return BreakpointResult::CreateIllFormed(
+ "Malformed z packet, expecting comma after address");
/*
// Parse out the stoppoint size (i.e. size hint for opcode size).
@@ -3062,26 +3079,51 @@ GDBRemoteCommunicationServerLLGS::Handle_z(StringExtractorGDBRemote &packet) {
size argument");
*/
- if (want_breakpoint) {
+ if (bp_kind->want_breakpoint) {
// Try to clear the breakpoint.
const Status error =
- m_current_process->RemoveBreakpoint(addr, want_hardware);
+ m_current_process->RemoveBreakpoint(addr, bp_kind->want_hardware);
if (error.Success())
- return SendOKResponse();
+ return BreakpointResult::CreateOK();
Log *log = GetLog(LLDBLog::Breakpoints);
LLDB_LOG(log, "pid {0} failed to remove breakpoint: {1}",
m_current_process->GetID(), error);
- return SendErrorResponse(0x09);
- } else {
- // Try to clear the watchpoint.
- const Status error = m_current_process->RemoveWatchpoint(addr);
- if (error.Success())
- return SendOKResponse();
- Log *log = GetLog(LLDBLog::Watchpoints);
- LLDB_LOG(log, "pid {0} failed to remove watchpoint: {1}",
- m_current_process->GetID(), error);
- return SendErrorResponse(0x09);
+ return BreakpointResult::CreateError(0x09);
+ }
+ // Try to clear the watchpoint.
+ const Status error = m_current_process->RemoveWatchpoint(addr);
+ if (error.Success())
+ return BreakpointResult::CreateOK();
+ Log *log = GetLog(LLDBLog::Watchpoints);
+ LLDB_LOG(log, "pid {0} failed to remove watchpoint: {1}",
+ m_current_process->GetID(), error);
+ return BreakpointResult::CreateError(0x09);
+}
+
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::SendBreakpointResponse(
+ StringExtractorGDBRemote &packet, const BreakpointResult &result) {
+ switch (result.kind) {
+ case BreakpointResult::Kind::OK:
+ return SendOKResponse();
+ case BreakpointResult::Kind::Error:
+ return SendErrorResponse(result.error_code);
+ case BreakpointResult::Kind::IllFormed:
+ return SendIllFormedResponse(packet, result.message.c_str());
}
+ llvm_unreachable("unhandled BreakpointResult kind");
+}
+
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_Z(StringExtractorGDBRemote &packet) {
+ return SendBreakpointResponse(packet,
+ ExecuteSetBreakpoint(packet.GetStringRef()));
+}
+
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_z(StringExtractorGDBRemote &packet) {
+ return SendBreakpointResponse(packet,
+ ExecuteRemoveBreakpoint(packet.GetStringRef()));
}
GDBRemoteCommunication::PacketResult
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
index b400cc2fc21cd..a663c0b949744 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -307,6 +307,34 @@ class GDBRemoteCommunicationServerLLGS
private:
llvm::Expected<std::unique_ptr<llvm::MemoryBuffer>> BuildTargetXml();
+ /// Helper struct for the Execute{Set,Remove}Breakpoint methods.
+ struct BreakpointResult {
+ enum class Kind { OK, Error, IllFormed };
+
+ Kind kind;
+ uint8_t error_code = 0; // Only meaningful when kind == Error.
+ std::string message; // Only meaningful when kind == IllFormed.
+
+ static BreakpointResult CreateOK() { return {Kind::OK, 0, {}}; }
+ static BreakpointResult CreateError(uint8_t code) {
+ return {Kind::Error, code, {}};
+ }
+ static BreakpointResult CreateIllFormed(std::string msg) {
+ return {Kind::IllFormed, 0, std::move(msg)};
+ }
+ };
+
+ /// Core logic for a Z (set breakpoint/watchpoint) request.
+ BreakpointResult ExecuteSetBreakpoint(llvm::StringRef packet_str);
+
+ /// Core logic for a z (remove breakpoint/watchpoint) request.
+ BreakpointResult ExecuteRemoveBreakpoint(llvm::StringRef packet_str);
+
+ /// Convert a BreakpointResult into a PacketResult, sending the appropriate
+ /// response.
+ PacketResult SendBreakpointResponse(StringExtractorGDBRemote &packet,
+ const BreakpointResult &result);
+
void HandleInferiorState_Exited(NativeProcessProtocol *process);
void HandleInferiorState_Stopped(NativeProcessProtocol *process);
>From 541748effb974c7d65579ea68932e1aa2f3e7d05 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Sat, 25 Apr 2026 09:59:46 +0100
Subject: [PATCH 2/5] fixup! uncontroversial review comments
---
.../GDBRemoteCommunicationServerLLGS.cpp | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index dfa0755636d2a..2bbdbc74a6d52 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2916,29 +2916,30 @@ struct BreakpointKind {
case eBreakpointSoftware:
want_hardware = false;
want_breakpoint = true;
- break;
+ return;
case eBreakpointHardware:
want_hardware = true;
want_breakpoint = true;
- break;
+ return;
case eWatchpointWrite:
watch_flags = 1;
want_hardware = true;
want_breakpoint = false;
- break;
+ return;
case eWatchpointRead:
watch_flags = 2;
want_hardware = true;
want_breakpoint = false;
- break;
+ return;
case eWatchpointReadWrite:
watch_flags = 3;
want_hardware = true;
want_breakpoint = false;
+ return;
+ case eStoppointInvalid:
break;
- default:
- llvm_unreachable("unhandled GDBStoppointType");
}
+ llvm_unreachable("unhandled GDBStoppointType");
}
};
@@ -2954,9 +2955,9 @@ getBreakpointKind(GDBStoppointType stoppoint_type) {
case eWatchpointReadWrite:
return BreakpointKind(stoppoint_type);
case eStoppointInvalid:
- break;
+ return std::nullopt;
}
- return std::nullopt;
+ llvm_unreachable("unhandled GDBStoppointType in getBreakpointKind");
}
} // namespace
>From 67f9649457f53c606fd3dee985615c0fd8a4fb76 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Sat, 25 Apr 2026 10:39:17 +0100
Subject: [PATCH 3/5] fixup! Replace BreakpointKind with a std::variant
---
.../GDBRemoteCommunicationServerLLGS.cpp | 80 +++++++------------
1 file changed, 28 insertions(+), 52 deletions(-)
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 2bbdbc74a6d52..aaa75b7233f77 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -15,6 +15,7 @@
#include <limits>
#include <optional>
#include <thread>
+#include <variant>
#include "GDBRemoteCommunicationServerLLGS.h"
#include "lldb/Host/ConnectionFileDescriptor.h"
@@ -2904,60 +2905,32 @@ GDBRemoteCommunicationServerLLGS::Handle_qMemoryRegionInfo(
}
namespace {
-/// Helper struct to expand a GDBStoppointType into flags.
-struct BreakpointKind {
+struct UseBreakpoint {
bool want_hardware;
- bool want_breakpoint;
- uint32_t watch_flags;
-
- /// Invalid types must be handled prior to calling this.
- BreakpointKind(GDBStoppointType stoppoint_type) {
- switch (stoppoint_type) {
- case eBreakpointSoftware:
- want_hardware = false;
- want_breakpoint = true;
- return;
- case eBreakpointHardware:
- want_hardware = true;
- want_breakpoint = true;
- return;
- case eWatchpointWrite:
- watch_flags = 1;
- want_hardware = true;
- want_breakpoint = false;
- return;
- case eWatchpointRead:
- watch_flags = 2;
- want_hardware = true;
- want_breakpoint = false;
- return;
- case eWatchpointReadWrite:
- watch_flags = 3;
- want_hardware = true;
- want_breakpoint = false;
- return;
- case eStoppointInvalid:
- break;
- }
- llvm_unreachable("unhandled GDBStoppointType");
- }
};
+struct UseWatchpoint {
+ uint32_t flags;
+ static constexpr bool want_hardware = true;
+};
+struct InvalidStoppoint {};
-/// If stoppoint_type is a valid type, create a BreakpointKind, otherwise
-/// returns nullopt.
-std::optional<BreakpointKind>
+std::variant<UseBreakpoint, UseWatchpoint, InvalidStoppoint>
getBreakpointKind(GDBStoppointType stoppoint_type) {
switch (stoppoint_type) {
case eBreakpointSoftware:
+ return UseBreakpoint{/*want_hardware*/ false};
case eBreakpointHardware:
+ return UseBreakpoint{/*want_hardware*/ true};
case eWatchpointWrite:
+ return UseWatchpoint{/*flags*/ 1};
case eWatchpointRead:
+ return UseWatchpoint{/*flags*/ 2};
case eWatchpointReadWrite:
- return BreakpointKind(stoppoint_type);
+ return UseWatchpoint{/*flags*/ 3};
case eStoppointInvalid:
- return std::nullopt;
+ return InvalidStoppoint();
}
- llvm_unreachable("unhandled GDBStoppointType in getBreakpointKind");
+ llvm_unreachable("unhandled GDBStoppointType");
}
} // namespace
@@ -2982,8 +2955,9 @@ GDBRemoteCommunicationServerLLGS::ExecuteSetBreakpoint(
const GDBStoppointType stoppoint_type =
GDBStoppointType(packet.GetS32(eStoppointInvalid));
- std::optional<BreakpointKind> bp_kind = getBreakpointKind(stoppoint_type);
- if (!bp_kind)
+ std::variant<UseBreakpoint, UseWatchpoint, InvalidStoppoint> bp_variant =
+ getBreakpointKind(stoppoint_type);
+ if (std::holds_alternative<InvalidStoppoint>(bp_variant))
return BreakpointResult::CreateIllFormed(
"Z packet had invalid software/hardware specifier");
@@ -3008,8 +2982,8 @@ GDBRemoteCommunicationServerLLGS::ExecuteSetBreakpoint(
return BreakpointResult::CreateIllFormed(
"Malformed Z packet, failed to parse size argument");
- if (bp_kind->want_breakpoint) {
- // Try to set the breakpoint.
+ // Try to set a breakpoint.
+ if (auto *bp_kind = std::get_if<UseBreakpoint>(&bp_variant)) {
const Status error =
m_current_process->SetBreakpoint(addr, size, bp_kind->want_hardware);
if (error.Success())
@@ -3020,9 +2994,10 @@ GDBRemoteCommunicationServerLLGS::ExecuteSetBreakpoint(
return BreakpointResult::CreateError(0x09);
}
- // Try to set the watchpoint.
+ // Try to set a watchpoint.
+ auto wp_kind = std::get<UseWatchpoint>(bp_variant);
const Status error = m_current_process->SetWatchpoint(
- addr, size, bp_kind->watch_flags, bp_kind->want_hardware);
+ addr, size, wp_kind.flags, wp_kind.want_hardware);
if (error.Success())
return BreakpointResult::CreateOK();
Log *log = GetLog(LLDBLog::Watchpoints);
@@ -3052,8 +3027,9 @@ GDBRemoteCommunicationServerLLGS::ExecuteRemoveBreakpoint(
const GDBStoppointType stoppoint_type =
GDBStoppointType(packet.GetS32(eStoppointInvalid));
- std::optional<BreakpointKind> bp_kind = getBreakpointKind(stoppoint_type);
- if (!bp_kind)
+ std::variant<UseBreakpoint, UseWatchpoint, InvalidStoppoint> bp_variant =
+ getBreakpointKind(stoppoint_type);
+ if (std::holds_alternative<InvalidStoppoint>(bp_variant))
return BreakpointResult::CreateIllFormed(
"z packet had invalid software/hardware specifier");
@@ -3080,8 +3056,8 @@ GDBRemoteCommunicationServerLLGS::ExecuteRemoveBreakpoint(
size argument");
*/
- if (bp_kind->want_breakpoint) {
- // Try to clear the breakpoint.
+ // Try to clear the breakpoint.
+ if (auto *bp_kind = std::get_if<UseBreakpoint>(&bp_variant)) {
const Status error =
m_current_process->RemoveBreakpoint(addr, bp_kind->want_hardware);
if (error.Success())
>From b22d4e466cb5bf64daeee8a08ab083475a1a1307 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Sat, 25 Apr 2026 11:12:55 +0100
Subject: [PATCH 4/5] fixup! Replace BreakpointResult with a std::variant
---
.../GDBRemoteCommunicationServerLLGS.cpp | 84 ++++++++++---------
.../GDBRemoteCommunicationServerLLGS.h | 24 ++----
2 files changed, 52 insertions(+), 56 deletions(-)
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index aaa75b7233f77..f16ca517e0561 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2942,7 +2942,7 @@ GDBRemoteCommunicationServerLLGS::ExecuteSetBreakpoint(
(m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) {
Log *log = GetLog(LLDBLog::Process);
LLDB_LOG(log, "failed, no process available");
- return BreakpointResult::CreateError(0x15);
+ return BreakpointError{0x15};
}
StringExtractorGDBRemote packet(packet_str);
@@ -2950,48 +2950,47 @@ GDBRemoteCommunicationServerLLGS::ExecuteSetBreakpoint(
// Parse out software or hardware breakpoint or watchpoint requested.
packet.SetFilePos(strlen("Z"));
if (packet.GetBytesLeft() < 1)
- return BreakpointResult::CreateIllFormed(
- "Too short Z packet, missing software/hardware specifier");
+ return BreakpointIllFormed{
+ "Too short Z packet, missing software/hardware specifier"};
const GDBStoppointType stoppoint_type =
GDBStoppointType(packet.GetS32(eStoppointInvalid));
std::variant<UseBreakpoint, UseWatchpoint, InvalidStoppoint> bp_variant =
getBreakpointKind(stoppoint_type);
if (std::holds_alternative<InvalidStoppoint>(bp_variant))
- return BreakpointResult::CreateIllFormed(
- "Z packet had invalid software/hardware specifier");
+ return BreakpointIllFormed{
+ "Z packet had invalid software/hardware specifier"};
if ((packet.GetBytesLeft() < 1) || packet.GetChar() != ',')
- return BreakpointResult::CreateIllFormed(
- "Malformed Z packet, expecting comma after stoppoint type");
+ return BreakpointIllFormed{
+ "Malformed Z packet, expecting comma after stoppoint type"};
// Parse out the stoppoint address.
if (packet.GetBytesLeft() < 1)
- return BreakpointResult::CreateIllFormed(
- "Too short Z packet, missing address");
+ return BreakpointIllFormed{"Too short Z packet, missing address"};
const lldb::addr_t addr = packet.GetHexMaxU64(false, 0);
if ((packet.GetBytesLeft() < 1) || packet.GetChar() != ',')
- return BreakpointResult::CreateIllFormed(
- "Malformed Z packet, expecting comma after address");
+ return BreakpointIllFormed{
+ "Malformed Z packet, expecting comma after address"};
// Parse out the stoppoint size (i.e. size hint for opcode size).
const uint32_t size =
packet.GetHexMaxU32(false, std::numeric_limits<uint32_t>::max());
if (size == std::numeric_limits<uint32_t>::max())
- return BreakpointResult::CreateIllFormed(
- "Malformed Z packet, failed to parse size argument");
+ return BreakpointIllFormed{
+ "Malformed Z packet, failed to parse size argument"};
// Try to set a breakpoint.
if (auto *bp_kind = std::get_if<UseBreakpoint>(&bp_variant)) {
const Status error =
m_current_process->SetBreakpoint(addr, size, bp_kind->want_hardware);
if (error.Success())
- return BreakpointResult::CreateOK();
+ return BreakpointOK();
Log *log = GetLog(LLDBLog::Breakpoints);
LLDB_LOG(log, "pid {0} failed to set breakpoint: {1}",
m_current_process->GetID(), error);
- return BreakpointResult::CreateError(0x09);
+ return BreakpointError{0x09};
}
// Try to set a watchpoint.
@@ -2999,11 +2998,11 @@ GDBRemoteCommunicationServerLLGS::ExecuteSetBreakpoint(
const Status error = m_current_process->SetWatchpoint(
addr, size, wp_kind.flags, wp_kind.want_hardware);
if (error.Success())
- return BreakpointResult::CreateOK();
+ return BreakpointOK();
Log *log = GetLog(LLDBLog::Watchpoints);
LLDB_LOG(log, "pid {0} failed to set watchpoint: {1}",
m_current_process->GetID(), error);
- return BreakpointResult::CreateError(0x09);
+ return BreakpointError{0x09};
}
GDBRemoteCommunicationServerLLGS::BreakpointResult
@@ -3014,7 +3013,7 @@ GDBRemoteCommunicationServerLLGS::ExecuteRemoveBreakpoint(
(m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) {
Log *log = GetLog(LLDBLog::Process);
LLDB_LOG(log, "failed, no process available");
- return BreakpointResult::CreateError(0x15);
+ return BreakpointError{0x15};
}
StringExtractorGDBRemote packet(packet_str);
@@ -3022,30 +3021,29 @@ GDBRemoteCommunicationServerLLGS::ExecuteRemoveBreakpoint(
// Parse out software or hardware breakpoint or watchpoint requested.
packet.SetFilePos(strlen("z"));
if (packet.GetBytesLeft() < 1)
- return BreakpointResult::CreateIllFormed(
- "Too short z packet, missing software/hardware specifier");
+ return BreakpointIllFormed{
+ "Too short z packet, missing software/hardware specifier"};
const GDBStoppointType stoppoint_type =
GDBStoppointType(packet.GetS32(eStoppointInvalid));
std::variant<UseBreakpoint, UseWatchpoint, InvalidStoppoint> bp_variant =
getBreakpointKind(stoppoint_type);
if (std::holds_alternative<InvalidStoppoint>(bp_variant))
- return BreakpointResult::CreateIllFormed(
- "z packet had invalid software/hardware specifier");
+ return BreakpointIllFormed{
+ "z packet had invalid software/hardware specifier"};
if ((packet.GetBytesLeft() < 1) || packet.GetChar() != ',')
- return BreakpointResult::CreateIllFormed(
- "Malformed z packet, expecting comma after stoppoint type");
+ return BreakpointIllFormed{
+ "Malformed z packet, expecting comma after stoppoint type"};
// Parse out the stoppoint address.
if (packet.GetBytesLeft() < 1)
- return BreakpointResult::CreateIllFormed(
- "Too short z packet, missing address");
+ return BreakpointIllFormed{"Too short z packet, missing address"};
const lldb::addr_t addr = packet.GetHexMaxU64(false, 0);
if ((packet.GetBytesLeft() < 1) || packet.GetChar() != ',')
- return BreakpointResult::CreateIllFormed(
- "Malformed z packet, expecting comma after address");
+ return BreakpointIllFormed{
+ "Malformed z packet, expecting comma after address"};
/*
// Parse out the stoppoint size (i.e. size hint for opcode size).
@@ -3061,34 +3059,38 @@ GDBRemoteCommunicationServerLLGS::ExecuteRemoveBreakpoint(
const Status error =
m_current_process->RemoveBreakpoint(addr, bp_kind->want_hardware);
if (error.Success())
- return BreakpointResult::CreateOK();
+ return BreakpointOK();
Log *log = GetLog(LLDBLog::Breakpoints);
LLDB_LOG(log, "pid {0} failed to remove breakpoint: {1}",
m_current_process->GetID(), error);
- return BreakpointResult::CreateError(0x09);
+ return BreakpointError{0x09};
}
// Try to clear the watchpoint.
const Status error = m_current_process->RemoveWatchpoint(addr);
if (error.Success())
- return BreakpointResult::CreateOK();
+ return BreakpointOK();
Log *log = GetLog(LLDBLog::Watchpoints);
LLDB_LOG(log, "pid {0} failed to remove watchpoint: {1}",
m_current_process->GetID(), error);
- return BreakpointResult::CreateError(0x09);
+ return BreakpointError{0x09};
}
GDBRemoteCommunication::PacketResult
GDBRemoteCommunicationServerLLGS::SendBreakpointResponse(
StringExtractorGDBRemote &packet, const BreakpointResult &result) {
- switch (result.kind) {
- case BreakpointResult::Kind::OK:
- return SendOKResponse();
- case BreakpointResult::Kind::Error:
- return SendErrorResponse(result.error_code);
- case BreakpointResult::Kind::IllFormed:
- return SendIllFormedResponse(packet, result.message.c_str());
- }
- llvm_unreachable("unhandled BreakpointResult kind");
+ return std::visit(
+ [&](auto &&arg) {
+ using T = std::decay_t<decltype(arg)>;
+ if constexpr (std::is_same_v<T, BreakpointOK>)
+ return SendOKResponse();
+ else if constexpr (std::is_same_v<T, BreakpointError>)
+ return SendErrorResponse(arg.error_code);
+ else if constexpr (std::is_same_v<T, BreakpointIllFormed>)
+ return SendIllFormedResponse(packet, arg.message.c_str());
+ else
+ static_assert(false, "non-exhaustive visitor!");
+ },
+ result);
}
GDBRemoteCommunication::PacketResult
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
index a663c0b949744..449f0ca9ce808 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -307,22 +307,16 @@ class GDBRemoteCommunicationServerLLGS
private:
llvm::Expected<std::unique_ptr<llvm::MemoryBuffer>> BuildTargetXml();
- /// Helper struct for the Execute{Set,Remove}Breakpoint methods.
- struct BreakpointResult {
- enum class Kind { OK, Error, IllFormed };
-
- Kind kind;
- uint8_t error_code = 0; // Only meaningful when kind == Error.
- std::string message; // Only meaningful when kind == IllFormed.
-
- static BreakpointResult CreateOK() { return {Kind::OK, 0, {}}; }
- static BreakpointResult CreateError(uint8_t code) {
- return {Kind::Error, code, {}};
- }
- static BreakpointResult CreateIllFormed(std::string msg) {
- return {Kind::IllFormed, 0, std::move(msg)};
- }
+ struct BreakpointOK {};
+ struct BreakpointIllFormed {
+ std::string message;
};
+ struct BreakpointError {
+ uint8_t error_code;
+ };
+
+ using BreakpointResult =
+ std::variant<BreakpointOK, BreakpointIllFormed, BreakpointError>;
/// Core logic for a Z (set breakpoint/watchpoint) request.
BreakpointResult ExecuteSetBreakpoint(llvm::StringRef packet_str);
>From 0635e83548d1026e80020508bf0c7d1da7695806 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Mon, 27 Apr 2026 15:14:21 +0100
Subject: [PATCH 5/5] fixup! add default value for want_hardware
---
.../Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index f16ca517e0561..b7570aea61db3 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2906,7 +2906,7 @@ GDBRemoteCommunicationServerLLGS::Handle_qMemoryRegionInfo(
namespace {
struct UseBreakpoint {
- bool want_hardware;
+ bool want_hardware = false;
};
struct UseWatchpoint {
uint32_t flags;
More information about the llvm-branch-commits
mailing list