[Lldb-commits] [lldb] Reapply "[lldb] Do not refcount breakpoints in lldb-server" (#195858) (PR #196891)
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Tue May 12 02:50:33 PDT 2026
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/196891
>From d670854de48f6b4565e955ca00be02f0e8c50cdc Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Mon, 11 May 2026 07:32:11 +0200
Subject: [PATCH 1/3] Reapply "[lldb] Do not refcount breakpoints in
lldb-server" (#196561)
This reverts commit ee44ba80855a91b42c4a66f33c41abaa7e79b099.
---
.../lldb/Host/common/NativeProcessProtocol.h | 1 -
lldb/source/Host/common/NativeProcessProtocol.cpp | 14 ++++----------
.../multi-breakpoint/TestMultiBreakpoint.py | 13 +++++++++++--
3 files changed, 15 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..a4d6351e05d65 100644
--- a/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py
+++ b/lldb/test/API/functionalities/multi-breakpoint/TestMultiBreakpoint.py
@@ -17,6 +17,8 @@
# 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 +62,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 +156,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"])
+ 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 +170,9 @@ 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 af6732a77058dc6c275667b651a82b2d3b6b61d4 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Mon, 11 May 2026 08:34:41 +0200
Subject: [PATCH 2/3] Don't set a "software single stepping" breakpoint if we
already have a breakpoint at that address.
---
.../lldb/Host/common/NativeProcessProtocol.h | 4 ++
.../Process/FreeBSD/NativeProcessFreeBSD.cpp | 17 +--------
.../Process/FreeBSD/NativeProcessFreeBSD.h | 3 +-
.../Process/Linux/NativeProcessLinux.cpp | 13 +++----
.../NativeProcessSoftwareSingleStep.cpp | 38 +++++++++----------
.../Utility/NativeProcessSoftwareSingleStep.h | 7 +++-
6 files changed, 34 insertions(+), 48 deletions(-)
diff --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
index 186a3d0f2f612..b9c0120016a2d 100644
--- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -158,6 +158,10 @@ class NativeProcessProtocol {
virtual Status RemoveBreakpoint(lldb::addr_t addr, bool hardware = false);
+ bool HasSoftwareBreakpoint(lldb::addr_t addr) {
+ return m_software_breakpoints.find(addr) != m_software_breakpoints.end();
+ }
+
// Hardware Breakpoint functions
virtual const HardwareBreakpointMap &GetHardwareBreakpointMap() const;
diff --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
index 46e9ac1cfd6fa..4853ab2827d9e 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -316,22 +316,7 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t pid) {
info.pl_siginfo.si_addr);
if (thread) {
- auto ®ctx = static_cast<NativeRegisterContextFreeBSD &>(
- thread->GetRegisterContext());
- auto thread_info =
- m_threads_stepping_with_breakpoint.find(thread->GetID());
- if (thread_info != m_threads_stepping_with_breakpoint.end() &&
- llvm::is_contained(thread_info->second, regctx.GetPC())) {
- thread->SetStoppedByTrace();
- for (auto &&bp_addr : thread_info->second) {
- Status brkpt_error = RemoveBreakpoint(bp_addr);
- if (brkpt_error.Fail())
- LLDB_LOG(log, "pid = {0} remove stepping breakpoint: {1}",
- thread_info->first, brkpt_error);
- }
- m_threads_stepping_with_breakpoint.erase(thread_info);
- } else
- thread->SetStoppedByBreakpoint();
+ thread->SetStoppedByBreakpoint();
FixupBreakpointPCAsNeeded(*thread);
SetCurrentThreadID(thread->GetID());
}
diff --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
index 4a3da9e987e3c..7e8bdc527f420 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
@@ -27,8 +27,7 @@ namespace process_freebsd {
/// for debugging.
///
/// Changes in the inferior process state are broadcasted.
-class NativeProcessFreeBSD : public NativeProcessELF,
- private NativeProcessSoftwareSingleStep {
+class NativeProcessFreeBSD : public NativeProcessELF {
public:
class Manager : public NativeProcessProtocol::Manager {
public:
diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 1ad57bd0c19e1..80f1b5662ba61 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -1931,14 +1931,13 @@ void NativeProcessLinux::SignalIfAllThreadsStopped() {
// Clear any temporary breakpoints we used to implement software single
// stepping.
- for (const auto &thread_info : m_threads_stepping_with_breakpoint) {
- for (auto &&bp_addr : thread_info.second) {
- Status error = RemoveBreakpoint(bp_addr);
- if (error.Fail())
- LLDB_LOG(log, "pid = {0} remove stepping breakpoint: {1}",
- thread_info.first, error);
- }
+ for (addr_t bp_addr : m_step_breakpoints) {
+ Status error = RemoveBreakpoint(bp_addr);
+ if (error.Fail())
+ LLDB_LOG(log, "pid = {0} remove stepping breakpoint: {1}", bp_addr,
+ error);
}
+ m_step_breakpoints.clear();
m_threads_stepping_with_breakpoint.clear();
// Notify the delegate about the stop
diff --git a/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp b/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
index eddf4b97babae..7069e4f8afaba 100644
--- a/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
+++ b/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
@@ -87,21 +87,6 @@ static size_t WriteMemoryCallback(EmulateInstruction *instruction, void *baton,
return length;
}
-static Status SetSoftwareBreakpoint(lldb::addr_t bp_addr, unsigned bp_size,
- NativeProcessProtocol &process) {
- Status error;
- error = process.SetBreakpoint(bp_addr, bp_size, /*hardware=*/false);
-
- // If setting the breakpoint fails because pc is out of the address
- // space, ignore it and let the debugee segfault.
- if (error.GetError() == EIO || error.GetError() == EFAULT)
- return Status();
- if (error.Fail())
- return error;
-
- return Status();
-}
-
Status NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping(
NativeThreadProtocol &thread) {
Status error;
@@ -122,24 +107,35 @@ Status NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping(
emulator_up->SetWriteMemCallback(&WriteMemoryCallback);
emulator_up->SetWriteRegCallback(&WriteRegisterCallback);
- auto bp_locaions_predictor =
+ auto bp_locations_predictor =
EmulateInstruction::CreateBreakpointLocationPredictor(
std::move(emulator_up));
- auto bp_locations = bp_locaions_predictor->GetBreakpointLocations(error);
+ BreakpointLocations candidates =
+ bp_locations_predictor->GetBreakpointLocations(error);
if (error.Fail())
return error;
- for (auto &&bp_addr : bp_locations) {
- auto bp_size = bp_locaions_predictor->GetBreakpointSize(bp_addr);
+ for (addr_t bp_addr : candidates) {
+ if (process.HasSoftwareBreakpoint(bp_addr))
+ continue;
+ auto bp_size = bp_locations_predictor->GetBreakpointSize(bp_addr);
if (auto err = bp_size.takeError())
return Status(toString(std::move(err)));
- error = SetSoftwareBreakpoint(bp_addr, *bp_size, process);
+ error = process.SetBreakpoint(bp_addr, *bp_size, /*hardware=*/false);
+
+ // If setting the breakpoint fails because pc is out of the address
+ // space, ignore it and let the debugee segfault.
+ if (error.GetError() == EIO || error.GetError() == EFAULT)
+ continue;
if (error.Fail())
return error;
+
+ m_step_breakpoints.emplace(bp_addr);
}
- m_threads_stepping_with_breakpoint.insert({thread.GetID(), bp_locations});
+ m_threads_stepping_with_breakpoint.emplace(thread.GetID(),
+ std::move(candidates));
return error;
}
diff --git a/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.h b/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.h
index 4e3fca30684fa..0e4d7c5656bb5 100644
--- a/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.h
+++ b/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.h
@@ -11,8 +11,8 @@
#include "lldb/Host/common/NativeProcessProtocol.h"
#include "lldb/Host/common/NativeThreadProtocol.h"
-
#include <map>
+#include <set>
namespace lldb_private {
@@ -22,9 +22,12 @@ class NativeProcessSoftwareSingleStep {
protected:
// List of thread ids stepping with a breakpoint with the address of
- // the relevan breakpoint
+ // next PC candidates.
std::map<lldb::tid_t, std::vector<lldb::addr_t>>
m_threads_stepping_with_breakpoint;
+
+ // The list of stepping breakpoints.
+ std::set<lldb::addr_t> m_step_breakpoints;
};
} // namespace lldb_private
>From 5383ae0f28a6e20d17885aeef46356ace3178f0c Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Tue, 12 May 2026 11:50:23 +0200
Subject: [PATCH 3/3] clear error when continuing
---
.../Process/Utility/NativeProcessSoftwareSingleStep.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp b/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
index 7069e4f8afaba..e962b5ae939b0 100644
--- a/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
+++ b/lldb/source/Plugins/Process/Utility/NativeProcessSoftwareSingleStep.cpp
@@ -127,8 +127,10 @@ Status NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping(
// If setting the breakpoint fails because pc is out of the address
// space, ignore it and let the debugee segfault.
- if (error.GetError() == EIO || error.GetError() == EFAULT)
+ if (error.GetError() == EIO || error.GetError() == EFAULT) {
+ error.Clear();
continue;
+ }
if (error.Fail())
return error;
More information about the lldb-commits
mailing list