[Lldb-commits] [lldb] [lldb] Do not refcount breakpoints in lldb-server (PR #195858)
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Tue May 5 07:06:49 PDT 2026
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/195858
>From 0bc7469136334aa92849aa83013b938edb936b3f Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Tue, 5 May 2026 15:48:08 +0200
Subject: [PATCH 1/2] [lldb] Do not refcount breakpoints in lldb-server
We did not say so explictly, but I'd argue that via #195815, we are
supporting stubs which do not refcount breakpoints. In these stubs the
set/clear breakpoint packets are idempotent:
- setting a breakpoint for the second time is a no-op (returns OK)
- clearing a breakpoint clears it, regardless of how many times it has
been set
- clearing a non-existent breakpoint (either because it was already
cleared, or because it was never set) returns an error
This makes lldb-server one of those stubs, which makes the code slightly
simpler, but more importantly, ensures we do not regress this behavior.
---
.../lldb/Host/common/NativeProcessProtocol.h | 1 -
lldb/source/Host/common/NativeProcessProtocol.cpp | 14 ++++----------
.../multi-breakpoint/TestMultiBreakpoint.py | 10 ++++++++--
3 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
index 06b36c2cc9eb5..186a3d0f2f612 100644
--- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -419,7 +419,6 @@ class NativeProcessProtocol {
protected:
struct SoftwareBreakpoint {
- uint32_t ref_count;
llvm::SmallVector<uint8_t, 4> saved_opcodes;
llvm::ArrayRef<uint8_t> breakpoint_opcodes;
};
diff --git a/lldb/source/Host/common/NativeProcessProtocol.cpp b/lldb/source/Host/common/NativeProcessProtocol.cpp
index 196f54b93538d..dbffdc619ef42 100644
--- a/lldb/source/Host/common/NativeProcessProtocol.cpp
+++ b/lldb/source/Host/common/NativeProcessProtocol.cpp
@@ -344,10 +344,8 @@ Status NativeProcessProtocol::SetSoftwareBreakpoint(lldb::addr_t addr,
LLDB_LOG(log, "addr = {0:x}, size_hint = {1}", addr, size_hint);
auto it = m_software_breakpoints.find(addr);
- if (it != m_software_breakpoints.end()) {
- ++it->second.ref_count;
+ if (it != m_software_breakpoints.end())
return Status();
- }
auto expected_bkpt = EnableSoftwareBreakpoint(addr, size_hint);
if (!expected_bkpt)
return Status::FromError(expected_bkpt.takeError());
@@ -362,14 +360,10 @@ Status NativeProcessProtocol::RemoveSoftwareBreakpoint(lldb::addr_t addr) {
auto it = m_software_breakpoints.find(addr);
if (it == m_software_breakpoints.end())
return Status::FromErrorString("Breakpoint not found.");
- assert(it->second.ref_count > 0);
- if (--it->second.ref_count > 0)
- return Status();
// Remove the entry from m_software_breakpoints rightaway, so that we don't
- // leave behind an entry with ref_count == 0 in case one of the following
- // conditions returns an error. The breakpoint is moved so that it can be
- // accessed below.
+ // leave behind an entry in case one of the following conditions returns an
+ // error. The breakpoint is moved so that it can be accessed below.
SoftwareBreakpoint bkpt = std::move(it->second);
m_software_breakpoints.erase(it);
@@ -503,7 +497,7 @@ NativeProcessProtocol::EnableSoftwareBreakpoint(lldb::addr_t addr,
}
LLDB_LOG(log, "addr = {0:x}: SUCCESS", addr);
- return SoftwareBreakpoint{1, saved_opcode_bytes, *expected_trap};
+ return SoftwareBreakpoint{saved_opcode_bytes, *expected_trap};
}
llvm::Expected<llvm::ArrayRef<uint8_t>>
diff --git a/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py b/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py
index eb9e2952d5a49..8e21884ea93b9 100644
--- a/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py
+++ b/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py
@@ -17,6 +17,9 @@
# Runs on systems where we can always predict the software break size
@skipIf(archs=no_match(["x86_64", "arm64", "aarch64"]))
class TestMultiBreakpoint(TestBase):
+
+ NO_DEBUG_INFO_TESTCASE = True
+
def check_invalid_packet(self, packet_str):
reply = lldbutil.send_packet_get_reply(self, packet_str)
if reply.startswith("E"):
@@ -60,6 +63,9 @@ def get_function_address(self, name):
return f"{addr:x}"
def test_multi_breakpoint(self):
+ # Debugserver uses refcounted breakpoints
+ breakpoints_are_refcounted = self.platformIsDarwin()
+
self.build()
source_file = lldb.SBFileSpec("main.c")
self.target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(
@@ -151,7 +157,7 @@ def make_packet(array):
# Clean up both.
array = [f"z0,{addr_a},{bp_kind}", f"z0,{addr_a},{bp_kind}"]
reply = self.send_packet(make_packet(array))
- self.assertMultiResponse(reply, ["OK", "OK"])
+ self.assertMultiResponse(reply, ["OK", "OK" if breakpoints_are_refcounted else "error"])
# --- Set the same breakpoint twice, but remove it thrice.
array = [f"Z0,{addr_a},{bp_kind}", f"Z0,{addr_a},{bp_kind}"]
@@ -163,7 +169,7 @@ def make_packet(array):
f"z0,{addr_a},{bp_kind}",
]
reply = self.send_packet(make_packet(array))
- self.assertMultiResponse(reply, ["OK", "OK", "error"])
+ self.assertMultiResponse(reply, ["OK", "OK" if breakpoints_are_refcounted else "error", "error"])
# --- Set and remove the same address in a single packet ---
# The spec requires requests to be executed in order, so the set
>From 5cdcbe4d9339b2495e2b305906e390a65e37931a Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Tue, 5 May 2026 16:06:44 +0200
Subject: [PATCH 2/2] format
---
.../multi-breakpoint/TestMultiBreakpoint.py | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py b/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py
index 8e21884ea93b9..01f13e2e5a992 100644
--- a/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py
+++ b/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py
@@ -157,7 +157,9 @@ def make_packet(array):
# Clean up both.
array = [f"z0,{addr_a},{bp_kind}", f"z0,{addr_a},{bp_kind}"]
reply = self.send_packet(make_packet(array))
- self.assertMultiResponse(reply, ["OK", "OK" if breakpoints_are_refcounted else "error"])
+ self.assertMultiResponse(
+ reply, ["OK", "OK" if breakpoints_are_refcounted else "error"]
+ )
# --- Set the same breakpoint twice, but remove it thrice.
array = [f"Z0,{addr_a},{bp_kind}", f"Z0,{addr_a},{bp_kind}"]
@@ -169,7 +171,9 @@ def make_packet(array):
f"z0,{addr_a},{bp_kind}",
]
reply = self.send_packet(make_packet(array))
- self.assertMultiResponse(reply, ["OK", "OK" if breakpoints_are_refcounted else "error", "error"])
+ self.assertMultiResponse(
+ reply, ["OK", "OK" if breakpoints_are_refcounted else "error", "error"]
+ )
# --- Set and remove the same address in a single packet ---
# The spec requires requests to be executed in order, so the set
More information about the lldb-commits
mailing list