[Lldb-commits] [lldb] [NFC][lldb] Extract Do{Dis}EnableBreakpoint into helper functions (PR #191136)
Felipe de Azevedo Piovezan via lldb-commits
lldb-commits at lists.llvm.org
Thu Apr 9 09:16:21 PDT 2026
https://github.com/felipepiovezan updated https://github.com/llvm/llvm-project/pull/191136
>From f5204c4fe7234693289b4ceb3363ea90992ccc40 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Thu, 2 Apr 2026 15:09:02 +0100
Subject: [PATCH 1/2] [NFC][lldb] Extract Do{Dis}EnableBreakpoint into helper
functions
Re-using this code will be important in an upcoming patch.
This commit also greatly simplifies the comments in the function.
---
.../Process/gdb-remote/ProcessGDBRemote.cpp | 220 ++++++++----------
1 file changed, 94 insertions(+), 126 deletions(-)
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index e264161c881f1..b4bce8755d7e6 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3267,6 +3267,97 @@ size_t ProcessGDBRemote::PutSTDIN(const char *src, size_t src_len,
return 0;
}
+/// Enable a single breakpoint site by trying Z0 (software), then Z1
+/// (hardware), then manual memory write as a last resort.
+static llvm::Error DoEnableBreakpointSite(ProcessGDBRemote &proc,
+ BreakpointSite &bp_site) {
+ Log *log = GetLog(GDBRLog::Breakpoints);
+ const addr_t addr = bp_site.GetLoadAddress();
+ const size_t bp_op_size = proc.GetSoftwareBreakpointTrapOpcode(&bp_site);
+ auto &gdb_comm = proc.GetGDBRemote();
+
+ // SupportsGDBStoppointPacket always returns true unless a previously sent
+ // packet failed. As such, query the function before AND after sending the
+ // packet.
+ if (gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware) &&
+ !bp_site.HardwareRequired()) {
+ uint8_t error_no = gdb_comm.SendGDBStoppointTypePacket(
+ eBreakpointSoftware, true, addr, bp_op_size,
+ proc.GetInterruptTimeout());
+ if (error_no == 0) {
+ bp_site.SetEnabled(true);
+ bp_site.SetType(BreakpointSite::eExternal);
+ return llvm::Error::success();
+ }
+ if (gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware)) {
+ if (error_no != UINT8_MAX)
+ return llvm::createStringErrorV(
+ "error: {0} sending the breakpoint request", error_no);
+ return llvm::createStringError("error sending the breakpoint request");
+ }
+ LLDB_LOGF(log, "Software breakpoints are unsupported");
+ }
+
+ if (gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) {
+ uint8_t error_no = gdb_comm.SendGDBStoppointTypePacket(
+ eBreakpointHardware, true, addr, bp_op_size,
+ proc.GetInterruptTimeout());
+ if (error_no == 0) {
+ bp_site.SetEnabled(true);
+ bp_site.SetType(BreakpointSite::eHardware);
+ return llvm::Error::success();
+ }
+ if (gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) {
+ if (error_no != UINT8_MAX)
+ return llvm::createStringErrorV(
+ "error: {0} sending the hardware breakpoint request "
+ "(hardware breakpoint resources might be exhausted or unavailable)",
+ error_no);
+ return llvm::createStringError(
+ "error sending the hardware breakpoint request "
+ "(hardware breakpoint resources might be exhausted or unavailable)");
+ }
+ LLDB_LOGF(log, "Hardware breakpoints are unsupported");
+ }
+
+ if (bp_site.HardwareRequired())
+ return llvm::createStringError("hardware breakpoints are not supported");
+
+ return proc.EnableSoftwareBreakpoint(&bp_site).takeError();
+}
+
+/// Disable a single breakpoint site directly by sending the appropriate
+/// z packet or restoring the original instruction.
+static llvm::Error DoDisableBreakpointSite(ProcessGDBRemote &proc,
+ BreakpointSite &bp_site) {
+ const addr_t addr = bp_site.GetLoadAddress();
+ const size_t bp_op_size = proc.GetSoftwareBreakpointTrapOpcode(&bp_site);
+ auto &gdb_comm = proc.GetGDBRemote();
+
+ switch (bp_site.GetType()) {
+ case BreakpointSite::eSoftware: {
+ Status error = proc.DisableSoftwareBreakpoint(&bp_site);
+ if (error.Fail())
+ return error.takeError();
+ break;
+ }
+ case BreakpointSite::eHardware:
+ if (gdb_comm.SendGDBStoppointTypePacket(eBreakpointHardware, false, addr,
+ bp_op_size,
+ proc.GetInterruptTimeout()))
+ return llvm::createStringError("unknown error");
+ break;
+ case BreakpointSite::eExternal:
+ if (gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, false, addr,
+ bp_op_size,
+ proc.GetInterruptTimeout()))
+ return llvm::createStringError("unknown error");
+ break;
+ }
+ bp_site.SetEnabled(false);
+ return llvm::Error::success();
+}
+
Status ProcessGDBRemote::EnableBreakpointSite(BreakpointSite *bp_site) {
Status error;
assert(bp_site != nullptr);
@@ -3293,103 +3384,7 @@ Status ProcessGDBRemote::EnableBreakpointSite(BreakpointSite *bp_site) {
return error;
}
- // Get the software breakpoint trap opcode size
- const size_t bp_op_size = GetSoftwareBreakpointTrapOpcode(bp_site);
-
- // SupportsGDBStoppointPacket() simply checks a boolean, indicating if this
- // breakpoint type is supported by the remote stub. These are set to true by
- // default, and later set to false only after we receive an unimplemented
- // response when sending a breakpoint packet. This means initially that
- // unless we were specifically instructed to use a hardware breakpoint, LLDB
- // will attempt to set a software breakpoint. HardwareRequired() also queries
- // a boolean variable which indicates if the user specifically asked for
- // hardware breakpoints. If true then we will skip over software
- // breakpoints.
- if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware) &&
- (!bp_site->HardwareRequired())) {
- // Try to send off a software breakpoint packet ($Z0)
- uint8_t error_no = m_gdb_comm.SendGDBStoppointTypePacket(
- eBreakpointSoftware, true, addr, bp_op_size, GetInterruptTimeout());
- if (error_no == 0) {
- // The breakpoint was placed successfully
- bp_site->SetEnabled(true);
- bp_site->SetType(BreakpointSite::eExternal);
- return error;
- }
-
- // SendGDBStoppointTypePacket() will return an error if it was unable to
- // set this breakpoint. We need to differentiate between a error specific
- // to placing this breakpoint or if we have learned that this breakpoint
- // type is unsupported. To do this, we must test the support boolean for
- // this breakpoint type to see if it now indicates that this breakpoint
- // type is unsupported. If they are still supported then we should return
- // with the error code. If they are now unsupported, then we would like to
- // fall through and try another form of breakpoint.
- if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware)) {
- if (error_no != UINT8_MAX)
- error = Status::FromErrorStringWithFormat(
- "error: %d sending the breakpoint request", error_no);
- else
- error = Status::FromErrorString("error sending the breakpoint request");
- return error;
- }
-
- // We reach here when software breakpoints have been found to be
- // unsupported. For future calls to set a breakpoint, we will not attempt
- // to set a breakpoint with a type that is known not to be supported.
- LLDB_LOGF(log, "Software breakpoints are unsupported");
-
- // So we will fall through and try a hardware breakpoint
- }
-
- // The process of setting a hardware breakpoint is much the same as above.
- // We check the supported boolean for this breakpoint type, and if it is
- // thought to be supported then we will try to set this breakpoint with a
- // hardware breakpoint.
- if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) {
- // Try to send off a hardware breakpoint packet ($Z1)
- uint8_t error_no = m_gdb_comm.SendGDBStoppointTypePacket(
- eBreakpointHardware, true, addr, bp_op_size, GetInterruptTimeout());
- if (error_no == 0) {
- // The breakpoint was placed successfully
- bp_site->SetEnabled(true);
- bp_site->SetType(BreakpointSite::eHardware);
- return error;
- }
-
- // Check if the error was something other then an unsupported breakpoint
- // type
- if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) {
- // Unable to set this hardware breakpoint
- if (error_no != UINT8_MAX)
- error = Status::FromErrorStringWithFormat(
- "error: %d sending the hardware breakpoint request "
- "(hardware breakpoint resources might be exhausted or unavailable)",
- error_no);
- else
- error = Status::FromErrorString(
- "error sending the hardware breakpoint request "
- "(hardware breakpoint resources "
- "might be exhausted or unavailable)");
- return error;
- }
-
- // We will reach here when the stub gives an unsupported response to a
- // hardware breakpoint
- LLDB_LOGF(log, "Hardware breakpoints are unsupported");
-
- // Finally we will falling through to a #trap style breakpoint
- }
-
- // Don't fall through when hardware breakpoints were specifically requested
- if (bp_site->HardwareRequired()) {
- error = Status::FromErrorString("hardware breakpoints are not supported");
- return error;
- }
-
- // As a last resort we want to place a manual breakpoint. An instruction is
- // placed into the process memory using memory write packets.
- return EnableSoftwareBreakpoint(bp_site);
+ return Status::FromError(DoEnableBreakpointSite(*this, *bp_site));
}
Status ProcessGDBRemote::DisableBreakpointSite(BreakpointSite *bp_site) {
@@ -3403,32 +3398,7 @@ Status ProcessGDBRemote::DisableBreakpointSite(BreakpointSite *bp_site) {
") addr = 0x%8.8" PRIx64,
site_id, (uint64_t)addr);
- if (bp_site->IsEnabled()) {
- const size_t bp_op_size = GetSoftwareBreakpointTrapOpcode(bp_site);
-
- BreakpointSite::Type bp_type = bp_site->GetType();
- switch (bp_type) {
- case BreakpointSite::eSoftware:
- error = DisableSoftwareBreakpoint(bp_site);
- break;
-
- case BreakpointSite::eHardware:
- if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointHardware, false,
- addr, bp_op_size,
- GetInterruptTimeout()))
- error = Status::FromErrorString("unknown error");
- break;
-
- case BreakpointSite::eExternal: {
- if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, false,
- addr, bp_op_size,
- GetInterruptTimeout()))
- error = Status::FromErrorString("unknown error");
- } break;
- }
- if (error.Success())
- bp_site->SetEnabled(false);
- } else {
+ if (!bp_site->IsEnabled()) {
LLDB_LOGF(log,
"ProcessGDBRemote::DisableBreakpointSite (site_id = %" PRIu64
") addr = 0x%8.8" PRIx64 " -- SUCCESS (already disabled)",
@@ -3436,9 +3406,7 @@ Status ProcessGDBRemote::DisableBreakpointSite(BreakpointSite *bp_site) {
return error;
}
- if (error.Success())
- error = Status::FromErrorString("unknown error");
- return error;
+ return Status::FromError(DoDisableBreakpointSite(*this, *bp_site));
}
// Pre-requisite: wp != NULL.
>From e2ea42a76909917cfa2705866355d3911a693828 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Thu, 9 Apr 2026 17:16:05 +0100
Subject: [PATCH 2/2] fixup! fix log usage
---
.../Plugins/Process/gdb-remote/ProcessGDBRemote.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index b4bce8755d7e6..29e52e75e96e2 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3292,10 +3292,10 @@ static llvm::Error DoEnableBreakpointSite(ProcessGDBRemote &proc,
if (gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware)) {
if (error_no != UINT8_MAX)
return llvm::createStringErrorV(
- "error: {0} sending the breakpoint request", error_no);
+ "error sending the breakpoint request: {0}", error_no);
return llvm::createStringError("error sending the breakpoint request");
}
- LLDB_LOGF(log, "Software breakpoints are unsupported");
+ LLDB_LOG(log, "Software breakpoints are unsupported");
}
if (gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) {
@@ -3310,14 +3310,14 @@ static llvm::Error DoEnableBreakpointSite(ProcessGDBRemote &proc,
if (gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) {
if (error_no != UINT8_MAX)
return llvm::createStringErrorV(
- "error: {0} sending the hardware breakpoint request "
+ "error sending the hardware breakpoint request: {0} "
"(hardware breakpoint resources might be exhausted or unavailable)",
error_no);
return llvm::createStringError(
"error sending the hardware breakpoint request "
"(hardware breakpoint resources might be exhausted or unavailable)");
}
- LLDB_LOGF(log, "Hardware breakpoints are unsupported");
+ LLDB_LOG(log, "Hardware breakpoints are unsupported");
}
if (bp_site.HardwareRequired())
More information about the lldb-commits
mailing list