[Lldb-commits] [lldb] [debugserver][NFCI] Factor out logic handling breakpoint packets (PR #192912)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Apr 20 01:25:37 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Felipe de Azevedo Piovezan (felipepiovezan)
<details>
<summary>Changes</summary>
This creates a helper function that can be shared with the upcoming MultiBreakpoint packet.
It is largely NFC with one logging exception: where we were previously sending an "ILL_FORMED" error, we would identify the file/line in the location where the error was emitted. This has been lost, however the error messages are still unique enough that the line can be recovered from the error message, and this is a principle we've been trying to follow.
There was also a comment about gdb and refcounting of breakpoints. Such comment was removed as it did not seem applicable to any line of code.
---
Full diff: https://github.com/llvm/llvm-project/pull/192912.diff
2 Files Affected:
- (modified) lldb/tools/debugserver/source/RNBRemote.cpp (+41-28)
- (modified) lldb/tools/debugserver/source/RNBRemote.h (+24)
``````````diff
diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
index 57ffbc08d2590..7f419ae53bfbe 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -878,6 +878,13 @@ rnb_err_t RNBRemote::SendErrorPacket(std::string errcode,
return SendPacket(errcode);
}
+rnb_err_t RNBRemote::SendErrorPacket(uint32_t errcode,
+ const std::string &errmsg) {
+ char error_str[8];
+ snprintf(error_str, sizeof(error_str), "E%02x", errcode);
+ return SendErrorPacket(error_str, errmsg);
+}
+
/* Get a packet via gdb remote protocol.
Strip off the prefix/suffix, verify the checksum to make sure
a valid packet was received, send an ACK if they match. */
@@ -4077,38 +4084,35 @@ rnb_err_t RNBRemote::HandlePacket_T(const char *p) {
return SendPacket("OK");
}
-rnb_err_t RNBRemote::HandlePacket_z(const char *p) {
+RNBRemote::BreakpointResult RNBRemote::ExecuteBreakpointRequest(const char *p) {
if (p == NULL || *p == '\0')
- return HandlePacket_ILLFORMED(__FILE__, __LINE__, p,
- "No thread specified in z packet");
+ return BreakpointResult::CreateIllFormed("No thread specified in z packet");
if (!m_ctx.HasValidProcessID())
- return SendErrorPacket("E15");
+ return BreakpointResult::CreateError(0x15);
char packet_cmd = *p++;
char break_type = *p++;
if (*p++ != ',')
- return HandlePacket_ILLFORMED(__FILE__, __LINE__, p,
- "Comma separator missing in z packet");
+ return BreakpointResult::CreateIllFormed(
+ "Comma separator missing in z packet");
char *c = NULL;
nub_process_t pid = m_ctx.ProcessID();
errno = 0;
nub_addr_t addr = strtoull(p, &c, 16);
if (errno != 0 && addr == 0)
- return HandlePacket_ILLFORMED(__FILE__, __LINE__, p,
- "Invalid address in z packet");
+ return BreakpointResult::CreateIllFormed("Invalid address in z packet");
p = c;
if (*p++ != ',')
- return HandlePacket_ILLFORMED(__FILE__, __LINE__, p,
- "Comma separator missing in z packet");
+ return BreakpointResult::CreateIllFormed(
+ "Comma separator missing in z packet");
errno = 0;
auto byte_size = strtoul(p, &c, 16);
if (errno != 0 && byte_size == 0)
- return HandlePacket_ILLFORMED(__FILE__, __LINE__, p,
- "Invalid length in z packet");
+ return BreakpointResult::CreateIllFormed("Invalid length in z packet");
if (packet_cmd == 'Z') {
// set
@@ -4116,18 +4120,12 @@ rnb_err_t RNBRemote::HandlePacket_z(const char *p) {
case '0': // set software breakpoint
case '1': // set hardware breakpoint
{
- // gdb can send multiple Z packets for the same address and
- // these calls must be ref counted.
bool hardware = (break_type == '1');
if (DNBBreakpointSet(pid, addr, byte_size, hardware)) {
- // We successfully created a breakpoint, now lets full out
- // a ref count structure with the breakID and add it to our
- // map.
- return SendPacket("OK");
+ return BreakpointResult::CreateOK();
} else {
- // We failed to set the software breakpoint
- return SendErrorPacket("E09");
+ return BreakpointResult::CreateError(0x09);
}
} break;
@@ -4145,10 +4143,9 @@ rnb_err_t RNBRemote::HandlePacket_z(const char *p) {
watch_flags = WATCH_TYPE_READ | WATCH_TYPE_WRITE;
if (DNBWatchpointSet(pid, addr, byte_size, watch_flags, hardware)) {
- return SendPacket("OK");
+ return BreakpointResult::CreateOK();
} else {
- // We failed to set the watchpoint
- return SendErrorPacket("E09");
+ return BreakpointResult::CreateError(0x09);
}
} break;
@@ -4161,9 +4158,9 @@ rnb_err_t RNBRemote::HandlePacket_z(const char *p) {
case '0': // remove software breakpoint
case '1': // remove hardware breakpoint
if (DNBBreakpointClear(pid, addr)) {
- return SendPacket("OK");
+ return BreakpointResult::CreateOK();
} else {
- return SendErrorPacket("E08");
+ return BreakpointResult::CreateError(0x08);
}
break;
@@ -4171,9 +4168,9 @@ rnb_err_t RNBRemote::HandlePacket_z(const char *p) {
case '3': // remove read watchpoint
case '4': // remove access watchpoint
if (DNBWatchpointClear(pid, addr)) {
- return SendPacket("OK");
+ return BreakpointResult::CreateOK();
} else {
- return SendErrorPacket("E08");
+ return BreakpointResult::CreateError(0x08);
}
break;
@@ -4181,7 +4178,23 @@ rnb_err_t RNBRemote::HandlePacket_z(const char *p) {
break;
}
}
- return HandlePacket_UNIMPLEMENTED(p);
+ return BreakpointResult::CreateUnimplemented();
+}
+
+rnb_err_t RNBRemote::HandlePacket_z(const char *p) {
+ BreakpointResult result = ExecuteBreakpointRequest(p);
+ switch (result.kind) {
+ case BreakpointResult::Kind::OK:
+ return SendPacket("OK");
+ case BreakpointResult::Kind::Error:
+ return SendErrorPacket(result.error_code);
+ case BreakpointResult::Kind::IllFormed:
+ return HandlePacket_ILLFORMED(__FILE__, __LINE__, p,
+ result.message.c_str());
+ case BreakpointResult::Kind::Unimplemented:
+ return HandlePacket_UNIMPLEMENTED(p);
+ }
+ assert(false && "unhandled BreakpointResult kind");
}
// Extract the thread number from the thread suffix that might be appended to
diff --git a/lldb/tools/debugserver/source/RNBRemote.h b/lldb/tools/debugserver/source/RNBRemote.h
index b32c00adc8b23..5e306d889ba1c 100644
--- a/lldb/tools/debugserver/source/RNBRemote.h
+++ b/lldb/tools/debugserver/source/RNBRemote.h
@@ -266,6 +266,28 @@ class RNBRemote {
private:
RNBRemote(const RNBRemote &) = delete;
+ struct BreakpointResult {
+ enum class Kind { OK, Error, IllFormed, Unimplemented };
+
+ 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)};
+ }
+ static BreakpointResult CreateUnimplemented() {
+ return {Kind::Unimplemented, 0, {}};
+ }
+ };
+
+ /// Core logic for a Z/z breakpoint request.
+ BreakpointResult ExecuteBreakpointRequest(const char *p);
+
protected:
rnb_err_t GetCommData();
void CommDataReceived(const std::string &data);
@@ -360,6 +382,8 @@ class RNBRemote {
rnb_err_t SendPacket(const std::string &);
rnb_err_t SendErrorPacket(std::string errcode,
const std::string &errmsg = "");
+ rnb_err_t SendErrorPacket(uint32_t error_code,
+ const std::string &errmsg = "");
std::string CompressString(const std::string &);
void CreatePacketTable();
``````````
</details>
https://github.com/llvm/llvm-project/pull/192912
More information about the lldb-commits
mailing list