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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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; _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
