https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/217348
>From faa6c1308a8801a87dc771e082904493fb8015ce Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Thu, 13 Aug 2026 14:56:57 +0000 Subject: [PATCH 1/8] [LLDB][lldb-server][debugserver] Handle writes over software breakpoint sites Fixes #202672 lldb-server was not handling them at all, and I found 2 bugs in debugserver's implementation. Included unit tests will cover lldb-server, and a new API test covers both lldb-server and debugserver. When data is written over a software breakpoint location, it should not be written to memory but instead to the saved copy of the original data at that location. If this is not done, writes corrupt the software break instruction and it's no longer a valid breakpoint. This is how it should work: Memory contents: ----abcd---- Place a breakpoint: ----bkpt---- (saved bytes: abcd) Then we write over it: --abcdefgh-- The result: --abbkptgh-- (saved bytes: cdef) Remove the breakpoint: --abcdefgh-- When the breakpoint is removed, the saved bytes are written to memory and it looks just like you had written it all in the first place. This is what was happening with lldb-server: Memory contents: ----abcd---- Place a breakpoint: ----bkpt---- (saved bytes: abcd) Then we write over it: --abcdefgh-- The result: --abcdefgh-- (saved bytes: bkpt) (break no longer valid) Remove the breakpoint: Errors because it doesn't see the break instruction in memory. debugserver already has an algorithm to work out how to do this. It's in MachProcess::WriteMemory and what I've added to NativeProcessProtocol follows the same logic. I did have to do one fix to debugserver, but not within that code. I think debugserver already handled *most* cases correctly. The logic I have implemented is: * Assume breakpoints are sorted in ascending order of address (hence the change from std::unordered_map to std::map). * If the write address is before the first breakpoint, write up to that breakpoint. * If the write address is at or in a breakpoint, update that breakpoint's saved bytes until you reach the end of it, or the write is complete. * If there's write data left, move the write address forward to just beyond the breakpoint. * Repeat for all breakpoints or until the write is complete. * Finally, check for any remaining bytes to be written after the last breakpoint. This means a single write is split into multiple sometimes. The alternative is to make a copy of the write data, and patch the breakpoint instructions into that to create 1 write request. I decided not to do that because: * debugserver does not do this. * It is harder to reason about recovery if the single write fails. NativeProcessProtocol ReadMemory is no longer abstract, instead there is a lower level DoWriteMemory which sub-classes implement. DoWriteMemory is what WriteMemory used to be, so some places call it directly because they want to ignore breakpoint sites. I tried to avoid this extra abstraction layer, but I do not see a good alternative. debugserver's FindBreakpointsThatOverlapRange had 2 bugs which I think would have broken the tests: * The previous breakpoint check was actually recording the current breakpoint as overlapping. * Due to the use of lower_bound, if a write started inside of the last breakpoint, it would be missed. I tried to fix the second one minimally, but all options just made the loop even more complex. So I've rewritten the function to iterate all breakpoints., with a simpler early exit check. Tests have been added at the NativeProccess level for different layouts. The expected number of write calls is checked and breakpoint insertion is tested in 2 orders if there is > 1. There is an API test based on the steps reported in #216000. This test should run on all architectures and for both lldb-server and debugserver. I have had to take a small risk to get some dead code immediately before the breakpoint site. Short of using target specific assembly, I think what I've got is as safe as we can be. Limiting this test to AArch64 would not be a massive problem, as the same write splitting algorithm is used on all architectures. But just on principle I would like it to be as generic as it can be. --- .../lldb/Host/common/NativeProcessProtocol.h | 19 +- .../Host/common/NativeProcessProtocol.cpp | 73 ++++- .../Plugins/Process/AIX/NativeProcessAIX.cpp | 4 +- .../Plugins/Process/AIX/NativeProcessAIX.h | 4 +- .../Process/FreeBSD/NativeProcessFreeBSD.cpp | 4 +- .../Process/FreeBSD/NativeProcessFreeBSD.h | 4 +- .../Process/Linux/NativeProcessLinux.cpp | 15 +- .../Process/Linux/NativeProcessLinux.h | 4 +- .../Process/NetBSD/NativeProcessNetBSD.cpp | 4 +- .../Process/NetBSD/NativeProcessNetBSD.h | 4 +- .../Windows/Common/NativeProcessWindows.cpp | 4 +- .../Windows/Common/NativeProcessWindows.h | 4 +- .../write_over_software_breakpoint/Makefile | 3 + .../TestWriteOverSoftwareBreakpoint.py | 143 ++++++++++ .../write_over_software_breakpoint/main.c | 34 +++ .../Mock/ProcessMockAccelerator.cpp | 5 +- .../Accelerator/Mock/ProcessMockAccelerator.h | 4 +- .../Host/NativeProcessProtocolTest.cpp | 264 ++++++++++++++++++ .../Host/NativeProcessTestUtils.h | 22 +- 19 files changed, 573 insertions(+), 45 deletions(-) create mode 100644 lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/Makefile create mode 100644 lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py create mode 100644 lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c diff --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h b/lldb/include/lldb/Host/common/NativeProcessProtocol.h index 67206c4b55b79..2bb9e9a2532ad 100644 --- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h +++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h @@ -27,9 +27,9 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" #include "llvm/Support/MemoryBuffer.h" +#include <map> #include <mutex> #include <optional> -#include <unordered_map> #include <vector> namespace lldb_private { @@ -134,8 +134,10 @@ class NativeProcessProtocol { ReadCStringFromMemory(lldb::addr_t addr, char *buffer, size_t max_size, size_t &total_bytes_read); - virtual Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size, - size_t &bytes_written) = 0; + /// Write memory while not overwriting breakpoints in memory. The breakpoints' + /// saved bytes are updated with what would have been written. + Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size, + size_t &bytes_written); virtual llvm::Expected<lldb::addr_t> AllocateMemory(size_t size, uint32_t permissions) { @@ -457,7 +459,9 @@ class NativeProcessProtocol { llvm::ArrayRef<uint8_t> breakpoint_opcodes; }; - std::unordered_map<lldb::addr_t, SoftwareBreakpoint> m_software_breakpoints; + // Using std::map so that breakpoints are sorted in ascending address order. + // WriteMemory relies on this. + std::map<lldb::addr_t, SoftwareBreakpoint> m_software_breakpoints; lldb::pid_t m_pid; std::vector<std::unique_ptr<NativeThreadProtocol>> m_threads; @@ -482,6 +486,13 @@ class NativeProcessProtocol { // Extensions enabled per the last SetEnabledExtensions() call. Extension m_enabled_extensions; + // Write to memory, with no awareness of software breakpoint sites. Used to + // implement WriteMemory. May be called directly for use cases that should + // ignore software breakpoint sites, for example adding or removing those + // breakpoints. + virtual Status DoWriteMemory(lldb::addr_t addr, const void *buf, size_t size, + size_t &bytes_written) = 0; + // lldb_private::Host calls should be used to launch a process for debugging, // and then the process should be attached to. When attaching to a process // lldb_private::Host calls should be used to locate the process to attach diff --git a/lldb/source/Host/common/NativeProcessProtocol.cpp b/lldb/source/Host/common/NativeProcessProtocol.cpp index 8c5991e93aab3..029be5f3b0d47 100644 --- a/lldb/source/Host/common/NativeProcessProtocol.cpp +++ b/lldb/source/Host/common/NativeProcessProtocol.cpp @@ -396,7 +396,7 @@ Status NativeProcessProtocol::RemoveSoftwareBreakpoint(lldb::addr_t addr) { // We found a valid breakpoint opcode at this address, now restore the // saved opcode. size_t bytes_written = 0; - error = WriteMemory(addr, saved.data(), saved.size(), bytes_written); + error = DoWriteMemory(addr, saved.data(), saved.size(), bytes_written); if (error.Fail() || bytes_written < saved.size()) { return Status::FromErrorStringWithFormat( "addr=0x%" PRIx64 ": tried to write %zu bytes but only wrote %zu", @@ -454,8 +454,8 @@ NativeProcessProtocol::EnableSoftwareBreakpoint(lldb::addr_t addr, // Write a software breakpoint in place of the original opcode. size_t bytes_written = 0; - error = WriteMemory(addr, expected_trap->data(), expected_trap->size(), - bytes_written); + error = DoWriteMemory(addr, expected_trap->data(), expected_trap->size(), + bytes_written); if (error.Fail()) return error.ToError(); @@ -649,6 +649,73 @@ Status NativeProcessProtocol::RemoveBreakpoint(lldb::addr_t addr, return RemoveSoftwareBreakpoint(addr); } +Status NativeProcessProtocol::WriteMemory(lldb::addr_t addr, const void *buf, + size_t size, size_t &bytes_written) { + const uint8_t *byte_buf = static_cast<const uint8_t *>(buf); + bytes_written = 0; + Status error; + + if (!size) + return error; + + for (auto &[sbp_addr, sbp_data] : m_software_breakpoints) { + // If the address is before a breakpoint site, write up to the site, or to + // the end of the write. Whichever comes first. + if (addr < sbp_addr) { + size_t to_write = std::min(size, sbp_addr - addr); + size_t part_bytes_written = 0; + error = DoWriteMemory(addr, byte_buf, to_write, part_bytes_written); + bytes_written += part_bytes_written; + + if (error.Fail() || part_bytes_written < to_write) { + return Status::FromErrorStringWithFormat( + "addr=0x%" PRIx64 ": tried to write %zu bytes but only wrote %zu", + addr, to_write, part_bytes_written); + } + + byte_buf += to_write; + addr += to_write; + size -= to_write; + + if (!size) + break; + } + + // If the address is within a breakpoint site, update the saved opcodes + // for that site. + if ((addr >= sbp_addr) && + (addr < (sbp_addr + sbp_data.saved_opcodes.size()))) { + // Instead of writing this chunk, update the saved bytes in the + // breakpoint. + size_t idx = addr - sbp_addr; + size_t to_write = std::min(size, sbp_data.saved_opcodes.size() - idx); + for (size_t copied = 0; copied < to_write; + ++idx, ++bytes_written, ++byte_buf, ++addr, --size, ++copied) + sbp_data.saved_opcodes[idx] = *byte_buf; + } + + if (!size) + break; + } + + // If the write range extends beyond the last breakpoint site, write the + // remaining data. + if (size) { + // Write the remaining part after the last breakpoint, or the whole range + // in the case that there were no breakpoints. + size_t part_bytes_written = 0; + error = DoWriteMemory(addr, byte_buf, size, part_bytes_written); + bytes_written += part_bytes_written; + if (error.Fail() || part_bytes_written < size) { + return Status::FromErrorStringWithFormat( + "addr=0x%" PRIx64 ": tried to write %zu bytes but only wrote %zu", + addr, size, part_bytes_written); + } + } + + return Status(); +} + Status NativeProcessProtocol::ReadMemoryWithoutTrap(const ProcessAddress &process_addr, void *buf, size_t size, diff --git a/lldb/source/Plugins/Process/AIX/NativeProcessAIX.cpp b/lldb/source/Plugins/Process/AIX/NativeProcessAIX.cpp index de11641790655..6d99c22857549 100644 --- a/lldb/source/Plugins/Process/AIX/NativeProcessAIX.cpp +++ b/lldb/source/Plugins/Process/AIX/NativeProcessAIX.cpp @@ -244,8 +244,8 @@ Status NativeProcessAIX::ReadMemory(const ProcessAddress &process_addr, return Status("unsupported"); } -Status NativeProcessAIX::WriteMemory(lldb::addr_t addr, const void *buf, - size_t size, size_t &bytes_written) { +Status NativeProcessAIX::DoWriteMemory(lldb::addr_t addr, const void *buf, + size_t size, size_t &bytes_written) { return Status("unsupported"); } diff --git a/lldb/source/Plugins/Process/AIX/NativeProcessAIX.h b/lldb/source/Plugins/Process/AIX/NativeProcessAIX.h index 068b428e1c8a9..e862250055165 100644 --- a/lldb/source/Plugins/Process/AIX/NativeProcessAIX.h +++ b/lldb/source/Plugins/Process/AIX/NativeProcessAIX.h @@ -80,8 +80,8 @@ class NativeProcessAIX : public NativeProcessProtocol { Status ReadMemory(const ProcessAddress &addr, void *buf, size_t size, size_t &bytes_read) override; - Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size, - size_t &bytes_written) override; + Status DoWriteMemory(lldb::addr_t addr, const void *buf, size_t size, + size_t &bytes_written) override; size_t UpdateThreads() override; diff --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp index 74fdda1fd7934..1e8f37367eb7f 100644 --- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp +++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp @@ -900,8 +900,8 @@ Status NativeProcessFreeBSD::ReadMemory(const ProcessAddress &process_addr, return Status(); } -Status NativeProcessFreeBSD::WriteMemory(lldb::addr_t addr, const void *buf, - size_t size, size_t &bytes_written) { +Status NativeProcessFreeBSD::DoWriteMemory(lldb::addr_t addr, const void *buf, + size_t size, size_t &bytes_written) { const unsigned char *src = static_cast<const unsigned char *>(buf); Status error; struct ptrace_io_desc io; diff --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h index aecb7ab74d0f3..a22a385dfc6a4 100644 --- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h +++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h @@ -62,8 +62,8 @@ class NativeProcessFreeBSD : public NativeProcessELF { Status ReadMemory(const ProcessAddress &addr, void *buf, size_t size, size_t &bytes_read) override; - Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size, - size_t &bytes_written) override; + Status DoWriteMemory(lldb::addr_t addr, const void *buf, size_t size, + size_t &bytes_written) override; size_t UpdateThreads() override; diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp index d15907d7cedb7..758e46959965c 100644 --- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp @@ -1347,8 +1347,9 @@ NativeProcessLinux::Syscall(llvm::ArrayRef<uint64_t> args) { return std::move(Err); } - llvm::scope_exit restore_mem( - [&] { WriteMemory(exe_addr, memory.data(), memory.size(), bytes_read); }); + llvm::scope_exit restore_mem([&] { + DoWriteMemory(exe_addr, memory.data(), memory.size(), bytes_read); + }); if (llvm::Error Err = reg_ctx.SetPC(exe_addr).ToError()) return std::move(Err); @@ -1361,8 +1362,8 @@ NativeProcessLinux::Syscall(llvm::ArrayRef<uint64_t> args) { return std::move(Err); } } - if (llvm::Error Err = WriteMemory(exe_addr, syscall_data.Insn.data(), - syscall_data.Insn.size(), bytes_read) + if (llvm::Error Err = DoWriteMemory(exe_addr, syscall_data.Insn.data(), + syscall_data.Insn.size(), bytes_read) .ToError()) return std::move(Err); @@ -1667,8 +1668,8 @@ Status NativeProcessLinux::ReadMemory(const ProcessAddress &process_addr, return Status(); } -Status NativeProcessLinux::WriteMemory(lldb::addr_t addr, const void *buf, - size_t size, size_t &bytes_written) { +Status NativeProcessLinux::DoWriteMemory(lldb::addr_t addr, const void *buf, + size_t size, size_t &bytes_written) { const unsigned char *src = static_cast<const unsigned char *>(buf); size_t remainder; Status error; @@ -1699,7 +1700,7 @@ Status NativeProcessLinux::WriteMemory(lldb::addr_t addr, const void *buf, memcpy(buff, src, remainder); size_t bytes_written_rec; - error = WriteMemory(addr, buff, k_ptrace_word_size, bytes_written_rec); + error = DoWriteMemory(addr, buff, k_ptrace_word_size, bytes_written_rec); if (error.Fail()) return error; diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h index b45e5ff1546ef..23a0ccc3e4ffd 100644 --- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h +++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h @@ -99,8 +99,8 @@ class NativeProcessLinux : public NativeProcessELF, Status ReadMemory(const ProcessAddress &addr, void *buf, size_t size, size_t &bytes_read) override; - Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size, - size_t &bytes_written) override; + Status DoWriteMemory(lldb::addr_t addr, const void *buf, size_t size, + size_t &bytes_written) override; llvm::Expected<lldb::addr_t> AllocateMemory(size_t size, uint32_t permissions) override; diff --git a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp index 5b2b80866c98c..a69a33313923a 100644 --- a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp +++ b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp @@ -928,8 +928,8 @@ Status NativeProcessNetBSD::ReadMemory(const ProcessAddress &process_addr, return Status(); } -Status NativeProcessNetBSD::WriteMemory(lldb::addr_t addr, const void *buf, - size_t size, size_t &bytes_written) { +Status NativeProcessNetBSD::DoWriteMemory(lldb::addr_t addr, const void *buf, + size_t size, size_t &bytes_written) { const unsigned char *src = static_cast<const unsigned char *>(buf); Status error; struct ptrace_io_desc io; diff --git a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h index 599943290c163..ea9f8a219122b 100644 --- a/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h +++ b/lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h @@ -60,8 +60,8 @@ class NativeProcessNetBSD : public NativeProcessELF { Status ReadMemory(const ProcessAddress &addr, void *buf, size_t size, size_t &bytes_read) override; - Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size, - size_t &bytes_written) override; + Status DoWriteMemory(lldb::addr_t addr, const void *buf, size_t size, + size_t &bytes_written) override; lldb::addr_t GetSharedLibraryInfoAddress() override; diff --git a/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp b/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp index fc93a7e5747ed..fc00e3089e750 100644 --- a/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp +++ b/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp @@ -239,8 +239,8 @@ Status NativeProcessWindows::ReadMemory(const ProcessAddress &process_addr, return ProcessDebugger::ReadMemory(addr, buf, size, bytes_read); } -Status NativeProcessWindows::WriteMemory(lldb::addr_t addr, const void *buf, - size_t size, size_t &bytes_written) { +Status NativeProcessWindows::DoWriteMemory(lldb::addr_t addr, const void *buf, + size_t size, size_t &bytes_written) { return ProcessDebugger::WriteMemory(addr, buf, size, bytes_written); } diff --git a/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h b/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h index 7801d6febd28a..3074eea1bc932 100644 --- a/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h +++ b/lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h @@ -71,8 +71,8 @@ class NativeProcessWindows : public NativeProcessProtocol, Status ReadMemory(const ProcessAddress &addr, void *buf, size_t size, size_t &bytes_read) override; - Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size, - size_t &bytes_written) override; + Status DoWriteMemory(lldb::addr_t addr, const void *buf, size_t size, + size_t &bytes_written) override; llvm::Expected<lldb::addr_t> AllocateMemory(size_t size, uint32_t permissions) override; diff --git a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/Makefile b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/Makefile new file mode 100644 index 0000000000000..10495940055b6 --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/Makefile @@ -0,0 +1,3 @@ +C_SOURCES := main.c + +include Makefile.rules diff --git a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py new file mode 100644 index 0000000000000..8c24002af420e --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py @@ -0,0 +1,143 @@ +""" +Check that when data is written over a software breakpoint site, it does not +corrupt the breakpoint instruction, and is later written to memory when the +breakpoint is removed. +""" + +import lldb +from lldbsuite.test.lldbtest import * +import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.decorators import * + + +class WriteOverSoftwareBreakpoint(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + def test_write_over_breakpoint(self): + TestBase.setUp(self) + self.line = line_number("main.c", "// break here") + self.build() + exe = self.getBuildArtifact("a.out") + self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) + + lldbutil.run_break_set_by_file_and_line( + self, "main.c", self.line, num_expected_locations=1, loc_exact=True + ) + self.runCmd("run", RUN_SUCCEEDED) + self.expect( + "thread list", + STOPPED_DUE_TO_BREAKPOINT, + substrs=["stopped", "stop reason = breakpoint"], + ) + + target = self.dbg.GetSelectedTarget() + process = target.GetProcess() + + loop_start_breakpoint_addr = ( + target.breakpoints[0].GetLocationAtIndex(0).GetLoadAddress() + ) + + # Memory operations and breakpoint actions must be sent to the server + # right away instead of waiting for the next continue event. + self.runCmd("settings set target.process.disable-memory-cache on") + self.runCmd("settings set target.process.use-delayed-breakpoints false") + + # At this point we are stopped at the start of the for loop. + # We will set a further breakpoint in foo, and this is the one we will + # test by writing over it then continuing to it. + # We could use the same breakpoint, but arranging for dead code + # immediately before it is more risky. + + # lldb-server has its algorithm unit tested as part of + # NativeProcessProtocol, but debugserver does not use that. So we will + # do a few different types of overwrite here so we have coverage for both + # debug servers. + # + # We cannot be sure what the software break size will be, so I'm assuming + # 4 bytes because that's what Arm/AArch64 use. This means we may not be + # getting full coverage on Thumb or x86 but the test should still pass + # there. + # + # We assume that the instruction immediately before the breakpoint in + # foo is dead code, so we are allowed to corrupt it. + writes = [ + # Up to but not over breakpoint. + (-4, 4), + # Over start of breakpoint. + (-2, 4), + # Exactly over breakpoint. + (0, 4), + # Over end of breakpoint. + (2, 4), + # Immediately after breakpoint. + (4, 4), + # From before to after breakpoint. + (-4, 12), + ] + + for write_offset, write_size in writes: + # Place a breakpoint immediately after the dead code in foo. + bkpt = target.BreakpointCreateByName("place_break_here") + self.assertTrue(bkpt.IsValid()) + self.assertEqual(bkpt.GetNumLocations(), 1) + self.assertFalse(bkpt.IsHardware()) + + check_address = bkpt.GetLocationAtIndex(0).GetLoadAddress() + write_offset + + # Read around the breakpoint site. We assume that read subsitution + # is working, so the data here is the original contents of memory + # without the trap instruction. + err = lldb.SBError() + original_data = bytearray( + process.ReadMemory(check_address, write_size, err) + ) + self.assertSuccess(err) + self.assertEqual(len(original_data), write_size) + + # Write around/in/over the breakpoint site. + write_data = bytearray(range(write_size)) + wrote = process.WriteMemory(check_address, write_data, err) + self.assertSuccess(err) + self.assertEqual(wrote, write_size) + + # The data overlapping the breakpoint site should be in that breakpoint's + # saved data. So it will appear as if all the data was written to memory, + # even though it was not yet. + after_write = bytearray(process.ReadMemory(check_address, write_size, err)) + self.assertSuccess(err) + self.assertEqual(after_write, write_data) + + # The instruction in memory should still be intact so we can continue + # to the breakpoint. + process.Continue() + + thread = process.thread[0] + self.assertState(process.GetState(), lldb.eStateStopped) + self.assertStopReason(thread.GetStopReason(), lldb.eStopReasonBreakpoint) + # Should be stopped at the breakpoint we placed in foo. This proves that + # the breakpoint instruction was intact. + self.assertEqual( + bkpt.GetLocationAtIndex(0).GetLoadAddress(), + thread.selected_frame.GetPC(), + ) + + # When the breakpoint is removed the saved bytes will be written to + # memory. + self.assertTrue(target.BreakpointDelete(bkpt.GetID())) + + data = process.ReadMemory(check_address, write_size, err) + self.assertSuccess(err) + self.assertEqual(bytearray(data), write_data) + + # Restore the original instruction data. The dead code before the + # breakpoint is ok but the instructions after it must be put back + # so we can continue. + wrote = process.WriteMemory(check_address, original_data, err) + self.assertSuccess(err) + self.assertEqual(wrote, write_size) + + # Continue back to the start of the loop. + process.Continue() + self.assertState(process.GetState(), lldb.eStateStopped) + self.assertStopReason(thread.GetStopReason(), lldb.eStopReasonBreakpoint) + self.assertEqual(loop_start_breakpoint_addr, thread.selected_frame.GetPC()) diff --git a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c new file mode 100644 index 0000000000000..4c9100dda604a --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c @@ -0,0 +1,34 @@ +volatile int always_false; + +int foo() { + // This nop is deliberately dead code so that we have an instruction + // immediately prior to the breakpoint that we are allowed to corrupt. + if (always_false) { + // This dead code must be at least 4 bytes even on an architecture where + // nop is 1 byte. + asm volatile("nop\n" + "nop\n" + "nop\n" + "nop\n"); + } + // We are assuming that there are no instructions placed between the dead code + // above and the assembly below. This is not guaranteed but it's safer than + // assuming that this function will have no prologue instructions or breaking + // in the very first instruction of foo and hoping whatever comes before is + // not important. + asm volatile(".globl place_break_here\n" + "place_break_here:\n" + // The test will repeatedly add and remove a breakpoint here. + "nop"); + + return 0; +} + +int main() { + volatile int sum = 0; + // Pick a number of loops >= the number of write patterns being tested. + for (unsigned i = 0; i < 10; ++i) + sum += foo(); // break here + + return 0; +} diff --git a/lldb/tools/lldb-server/Plugins/Accelerator/Mock/ProcessMockAccelerator.cpp b/lldb/tools/lldb-server/Plugins/Accelerator/Mock/ProcessMockAccelerator.cpp index 8ee3510e67e0b..f179b9c6b8781 100644 --- a/lldb/tools/lldb-server/Plugins/Accelerator/Mock/ProcessMockAccelerator.cpp +++ b/lldb/tools/lldb-server/Plugins/Accelerator/Mock/ProcessMockAccelerator.cpp @@ -66,8 +66,9 @@ Status ProcessMockAccelerator::ReadMemory(const ProcessAddress &process_addr, return Status::FromErrorString("unimplemented"); } -Status ProcessMockAccelerator::WriteMemory(lldb::addr_t addr, const void *buf, - size_t size, size_t &bytes_written) { +Status ProcessMockAccelerator::DoWriteMemory(lldb::addr_t addr, const void *buf, + size_t size, + size_t &bytes_written) { bytes_written = 0; return Status::FromErrorString("unimplemented"); } diff --git a/lldb/tools/lldb-server/Plugins/Accelerator/Mock/ProcessMockAccelerator.h b/lldb/tools/lldb-server/Plugins/Accelerator/Mock/ProcessMockAccelerator.h index 6346773e40141..7245e4aeca5d5 100644 --- a/lldb/tools/lldb-server/Plugins/Accelerator/Mock/ProcessMockAccelerator.h +++ b/lldb/tools/lldb-server/Plugins/Accelerator/Mock/ProcessMockAccelerator.h @@ -42,8 +42,8 @@ class ProcessMockAccelerator : public NativeProcessProtocol { Status ReadMemory(const ProcessAddress &addr, void *buf, size_t size, size_t &bytes_read) override; - Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size, - size_t &bytes_written) override; + Status DoWriteMemory(lldb::addr_t addr, const void *buf, size_t size, + size_t &bytes_written) override; lldb::addr_t GetSharedLibraryInfoAddress() override; size_t UpdateThreads() override; diff --git a/lldb/unittests/Host/NativeProcessProtocolTest.cpp b/lldb/unittests/Host/NativeProcessProtocolTest.cpp index 91c4fd69d6e54..e5756d1db9f10 100644 --- a/lldb/unittests/Host/NativeProcessProtocolTest.cpp +++ b/lldb/unittests/Host/NativeProcessProtocolTest.cpp @@ -238,3 +238,267 @@ TEST(NativeProcessProtocolTest, ReadCStringFromMemory_CrossPageBoundary) { llvm::HasValue(llvm::StringRef("hello"))); EXPECT_EQ(bytes_read, 6UL); } + +void DoTestWriteMemoryPreservingTrap( + const std::vector<lldb::addr_t> &bp_addrs, + std::optional<lldb::addr_t> write_addr, + const std::vector<uint8_t> &write_data, uint32_t expected_number_of_writes, + const std::vector<uint8_t> expected_after_write_read_memory, + const std::vector<uint8_t> expected_after_write_read_memory_without_trap) { + NiceMock<MockDelegate> DummyDelegate; + MockProcess<NativeProcessProtocol> Process(DummyDelegate, + ArchSpec("aarch64-pc-linux")); + const std::vector<uint8_t> fake_memory{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}; + FakeMemory M{fake_memory}; + ON_CALL(Process, ReadMemory(_, _)) + .WillByDefault(Invoke(&M, &FakeMemory::Read)); + ON_CALL(Process, WriteMemory(_, _)) + .WillByDefault(Invoke(&M, &FakeMemory::Write)); + + for (auto bp_addr : bp_addrs) + EXPECT_THAT_ERROR( + Process.SetBreakpoint(bp_addr, 0, /*hardware=*/false).ToError(), + llvm::Succeeded()); + + if (expected_number_of_writes) + EXPECT_CALL(Process, WriteMemory(_, _)) + .Times(expected_number_of_writes) + .WillRepeatedly(DoDefault()); + else + EXPECT_CALL(Process, WriteMemory(_, _)).Times(0); + + if (write_addr) { + size_t bytes_written = 0; + Status err = Process.WriteMemory(*write_addr, write_data.data(), + write_data.size(), bytes_written); + EXPECT_THAT_ERROR(err.ToError(), llvm::Succeeded()); + EXPECT_EQ(bytes_written, write_data.size()); + } + + Mock::VerifyAndClearExpectations(&Process); + + // Anything written over a breakpoint should go into the saved bytes instead + // of into memory. + + // ReadMemory should show that the breakpoint instructions are unchanged. + auto memory_or_err = Process.ReadMemory(0, fake_memory.size()); + EXPECT_THAT_EXPECTED(memory_or_err, llvm::Succeeded()); + EXPECT_EQ(*memory_or_err, expected_after_write_read_memory); + + // ReadMemoryWithoutTrap should show that writes to the breakpoints have + // updated the saved data in the breakpoint. + memory_or_err = Process.ReadMemoryWithoutTrap(0, fake_memory.size()); + EXPECT_THAT_EXPECTED(memory_or_err, llvm::Succeeded()); + EXPECT_EQ(*memory_or_err, expected_after_write_read_memory_without_trap); + + // When the breakpoint is removed, the saved bytes are actually written + // to memory. + for (auto bp_addr : bp_addrs) + EXPECT_THAT_ERROR(Process.RemoveBreakpoint(bp_addr, false).ToError(), + llvm::Succeeded()); + + // The memory should contain the written data now. + memory_or_err = Process.ReadMemory(0, fake_memory.size()); + EXPECT_THAT_EXPECTED(memory_or_err, llvm::Succeeded()); + EXPECT_EQ(*memory_or_err, expected_after_write_read_memory_without_trap); + + // As there are no breakpoints, the result of ReadMemoryWithoutTrap should + // be the same. + memory_or_err = Process.ReadMemoryWithoutTrap(0, fake_memory.size()); + EXPECT_THAT_EXPECTED(memory_or_err, llvm::Succeeded()); + EXPECT_EQ(*memory_or_err, expected_after_write_read_memory_without_trap); +} + +void TestWriteMemoryPreservingTrap( + const std::vector<lldb::addr_t> &bp_addrs, + std::optional<lldb::addr_t> write_addr, + const std::vector<uint8_t> &write_data, uint32_t expected_number_of_writes, + const std::vector<uint8_t> expected_after_write_read_memory, + const std::vector<uint8_t> expected_after_write_read_memory_without_trap) { + auto bp_addrs_in_order = bp_addrs; + DoTestWriteMemoryPreservingTrap( + bp_addrs_in_order, write_addr, write_data, expected_number_of_writes, + expected_after_write_read_memory, + expected_after_write_read_memory_without_trap); + + // WriteMemoryPreservingTrap should not care in what order the breakpoints + // were inserted. + if (bp_addrs_in_order.size()) { + std::reverse(bp_addrs_in_order.begin(), bp_addrs_in_order.end()); + DoTestWriteMemoryPreservingTrap( + bp_addrs_in_order, write_addr, write_data, expected_number_of_writes, + expected_after_write_read_memory, + expected_after_write_read_memory_without_trap); + } +} + +TEST(NativeProcessProtocolTest, WriteMemoryPreservingTrap) { +// Software breakpoint instruction encoding for AArch64. +#define S__W__B__P 0x0, 0x0, 0x20, 0xd4 + + // In these tests, numbers are used for the initial memory contents and + // letters for the data being written. These variables are used for letters + // so that the inputs can be vertically aligned. + uint8_t a = 'a'; + uint8_t b = 'b'; + uint8_t c = 'c'; + uint8_t d = 'd'; + uint8_t e = 'e'; + uint8_t f = 'f'; + uint8_t g = 'g'; + uint8_t h = 'h'; + uint8_t i = 'i'; + uint8_t j = 'j'; + uint8_t k = 'k'; + + // Write nothing, set no breakpoints, nothing changes. + TestWriteMemoryPreservingTrap({}, std::nullopt, {}, 0, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}); + + // Write nothing, set a breakpoint. Breakpoint encoding should be visible in + // memory. + TestWriteMemoryPreservingTrap({0}, std::nullopt, {}, 0, + {S__W__B__P, 4, 5, 6, 7, 8, 9, 10}, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}); + + // 0 size write with no breakpoints set. + TestWriteMemoryPreservingTrap({}, 0, {}, 0, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}); + + // 0 size write not within a breakpoint. + TestWriteMemoryPreservingTrap({4}, 0, {}, 0, + {0, 1, 2, 3, S__W__B__P, 8, 9, 10}, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}); + + // 0 size write at start of a breakpoint. + TestWriteMemoryPreservingTrap({4}, 4, {}, 0, + {0, 1, 2, 3, S__W__B__P, 8, 9, 10}, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}); + + // 0 size write at end of a breakpoint. + TestWriteMemoryPreservingTrap({4}, 7, {}, 0, + {0, 1, 2, 3, S__W__B__P, 8, 9, 10}, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}); + + // 0 size write one beyond the end of a breakpoint. + TestWriteMemoryPreservingTrap({4}, 8, {}, 0, + {0, 1, 2, 3, S__W__B__P, 8, 9, 10}, + {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10}); + + // Write something but set no breakpoints. Data is written directly to memory. + TestWriteMemoryPreservingTrap({}, 2, {a, b}, 1, + {0, 1, a, b, 4, 5, 6, 7, 8, 9, 10}, + {0, 1, a, b, 4, 5, 6, 7, 8, 9, 10}); + + // Write before a breakpoint and do not overlap it. + TestWriteMemoryPreservingTrap({4}, 1, {a, b, c}, 1, + {0, a, b, c, S__W__B__P, 8, 9, 10}, + {0, a, b, c, 4, 5, 6, 7, 8, 9, 10}); + + // Write within a breakpoint, without writing outside of it. Data kept in + // saved bytes until removal. + TestWriteMemoryPreservingTrap({4}, 4, {a, b, c, d}, 0, + {0, 1, 2, 3, S__W__B__P, 8, 9, 10}, + {0, 1, 2, 3, a, b, c, d, 8, 9, 10}); + + // Write immediately after a breakpoint. Data is written directly to memory. + TestWriteMemoryPreservingTrap({2}, 6, {a, b, c}, 1, + {0, 1, S__W__B__P, a, b, c, 9, 10}, + {0, 1, 2, 3, 4, 5, a, b, c, 9, 10}); + + // Write a range overlapping the beginning of a breakpoint. First part will + // go direct to memory, the other to the saved bytes. + TestWriteMemoryPreservingTrap({2}, 0, {a, b, c, d}, 1, + {a, b, S__W__B__P, 6, 7, 8, 9, 10}, + {a, b, c, d, 4, 5, 6, 7, 8, 9, 10}); + + // Write overlapping end of breakpoint. First part goes to saved bytes, + // second part direct to memory. + TestWriteMemoryPreservingTrap({3}, 5, {a, b, c, d}, 1, + {0, 1, 2, S__W__B__P, c, d, 9, 10}, + {0, 1, 2, 3, 4, a, b, c, d, 9, 10}); + + // Write from before to after break. Ends go to memory, middle to saved bytes. + TestWriteMemoryPreservingTrap({2}, 0, {a, b, c, d, e, f, g, h}, 2, + {a, b, S__W__B__P, g, h, 8, 9, 10}, + {a, b, c, d, e, f, g, h, 8, 9, 10}); + + // Overlap a breakpoint at the very start of memory. + TestWriteMemoryPreservingTrap({0}, 0, {a, b, c, d, e, f}, 1, + {S__W__B__P, e, f, 6, 7, 8, 9, 10}, + {a, b, c, d, e, f, 6, 7, 8, 9, 10}); + + // Overlap a breakpoint at the very end of memory. + TestWriteMemoryPreservingTrap({7}, 5, {a, b, c, d, e, f}, 1, + {0, 1, 2, 3, 4, a, b, S__W__B__P}, + {0, 1, 2, 3, 4, a, b, c, d, e, f}); + + // Write up to a breakpoint. + TestWriteMemoryPreservingTrap({5}, 1, {a, b, c, d}, 1, + {0, a, b, c, d, S__W__B__P, 9, 10}, + {0, a, b, c, d, 5, 6, 7, 8, 9, 10}); + + // Write starting immediately after a breakpoint. + TestWriteMemoryPreservingTrap({2}, 6, {a, b, c, d}, 1, + {0, 1, S__W__B__P, a, b, c, d, 10}, + {0, 1, 2, 3, 4, 5, a, b, c, d, 10}); + + // Overlap 2 breakpoints, write extends before and after them. + TestWriteMemoryPreservingTrap({1, 6}, 0, {a, b, c, d, e, f, g, h, i, j, k}, 3, + {a, S__W__B__P, f, S__W__B__P, k}, + {a, b, c, d, e, f, g, h, i, j, k}); + + // Write starts within the first one, and ends after the second one. + TestWriteMemoryPreservingTrap({1, 6}, 2, {a, b, c, d, e, f, g, h, i}, 2, + {0, S__W__B__P, d, S__W__B__P, i}, + {0, 1, a, b, c, d, e, f, g, h, i}); + + // Write range from before first breakpoint to within second breakpoint. + TestWriteMemoryPreservingTrap({1, 6}, 0, {a, b, c, d, e, f, g, h, i}, 2, + {a, S__W__B__P, f, S__W__B__P, 10}, + {a, b, c, d, e, f, g, h, i, 9, 10}); + + // Write range from within first to beyond second. + TestWriteMemoryPreservingTrap({1, 6}, 2, {a, b, c, d, e, f, g, h, i}, 2, + {0, S__W__B__P, d, S__W__B__P, i}, + {0, 1, a, b, c, d, e, f, g, h, i}); + + // Write range from within first to within second. + TestWriteMemoryPreservingTrap({1, 6}, 2, {a, b, c, d, e, f, g}, 1, + {0, S__W__B__P, d, S__W__B__P, 10}, + {0, 1, a, b, c, d, e, f, g, 9, 10}); + + // Write in range between 2 breakpoints. + TestWriteMemoryPreservingTrap({0, 7}, 4, {a, b, c}, 1, + {S__W__B__P, a, b, c, S__W__B__P}, + {0, 1, 2, 3, a, b, c, 7, 8, 9, 10}); + + // 2 breakpoints with no gap between them, write overlaps the first one. + TestWriteMemoryPreservingTrap({2, 6}, 0, {a, b, c, d}, 1, + {a, b, S__W__B__P, S__W__B__P, 10}, + {a, b, c, d, 4, 5, 6, 7, 8, 9, 10}); + + // 2 breakpoints with no gap between them, write is within the first one. + TestWriteMemoryPreservingTrap({2, 6}, 3, {a, b}, 0, + {0, 1, S__W__B__P, S__W__B__P, 10}, + {0, 1, 2, a, b, 5, 6, 7, 8, 9, 10}); + + // 2 breakpoints with no gap between them, write is across both. + TestWriteMemoryPreservingTrap({2, 6}, 4, {a, b, c, d}, 0, + {0, 1, S__W__B__P, S__W__B__P, 10}, + {0, 1, 2, 3, a, b, c, d, 8, 9, 10}); + + // 2 breakpoints with no gap between them, write is within second one. + TestWriteMemoryPreservingTrap({2, 6}, 7, {a, b}, 0, + {0, 1, S__W__B__P, S__W__B__P, 10}, + {0, 1, 2, 3, 4, 5, 6, a, b, 9, 10}); + + // 2 breakpoints with no gap between them, write overlaps second one. + TestWriteMemoryPreservingTrap({2, 6}, 8, {a, b, c}, 1, + {0, 1, S__W__B__P, S__W__B__P, c}, + {0, 1, 2, 3, 4, 5, 6, 7, a, b, c}); + +#undef S__W__B__P +} diff --git a/lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h b/lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h index 087a6e7f92913..cbfebc292d867 100644 --- a/lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h +++ b/lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h @@ -84,10 +84,20 @@ template <typename T> class MockProcess : public T { return Status(); } - Status WriteMemory(addr_t Addr, const void *Buf, size_t Size, - size_t &BytesWritten) /*override*/ { + using T::WriteMemory; + + MOCK_METHOD2(ReadMemory, + llvm::Expected<std::vector<uint8_t>>(addr_t Addr, size_t Size)); + MOCK_METHOD2(WriteMemory, + llvm::Expected<size_t>(addr_t Addr, + llvm::ArrayRef<uint8_t> Data)); + + Status DoWriteMemory(addr_t Addr, const void *Buf, size_t Size, + size_t &BytesWritten) /*override*/ { auto ExpectedBytes = this->WriteMemory( - Addr, llvm::ArrayRef(static_cast<const uint8_t *>(Buf), Size)); + Addr, + llvm::ArrayRef(const_cast<uint8_t *>(static_cast<const uint8_t *>(Buf)), + Size)); if (!ExpectedBytes) { BytesWritten = 0; return Status::FromError(ExpectedBytes.takeError()); @@ -96,12 +106,6 @@ template <typename T> class MockProcess : public T { return Status(); } - MOCK_METHOD2(ReadMemory, - llvm::Expected<std::vector<uint8_t>>(addr_t Addr, size_t Size)); - MOCK_METHOD2(WriteMemory, - llvm::Expected<size_t>(addr_t Addr, - llvm::ArrayRef<uint8_t> Data)); - using T::GetSoftwareBreakpointTrapOpcode; llvm::Expected<std::vector<uint8_t>> ReadMemoryWithoutTrap(addr_t Addr, size_t Size) { >From b56c4dafeea084f902ec67ea3d0a7561cebf5625 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Wed, 19 Aug 2026 15:04:41 +0000 Subject: [PATCH 2/8] try to make label visible on windows --- .../write_over_software_breakpoint/main.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c index 4c9100dda604a..4a24cded476de 100644 --- a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c +++ b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c @@ -16,10 +16,15 @@ int foo() { // assuming that this function will have no prologue instructions or breaking // in the very first instruction of foo and hoping whatever comes before is // not important. - asm volatile(".globl place_break_here\n" - "place_break_here:\n" - // The test will repeatedly add and remove a breakpoint here. - "nop"); + asm volatile( +#ifdef _WIN32 + ".def place_break_here; .scl 2; .type 32; .endef;\n" +#else + ".globl place_break_here\n" +#endif + "place_break_here:\n" + // The test will repeatedly add and remove a breakpoint here. + "nop"); return 0; } >From 058265d64860dcbdf6cdf613280f3178e22ca8b2 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Thu, 20 Aug 2026 10:28:47 +0000 Subject: [PATCH 3/8] Use raw address for breakpoint --- .../TestWriteOverSoftwareBreakpoint.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py index 8c24002af420e..6a79cd44b038d 100644 --- a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py +++ b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py @@ -48,6 +48,15 @@ def test_write_over_breakpoint(self): # We could use the same breakpoint, but arranging for dead code # immediately before it is more risky. + # The breakpoint must be on an exact address, because we do not want + # lldb to adjust it based on the debug information for prologues + # and epilogues. + symbol_contexts = target.FindSymbols("place_break_here", lldb.eSymbolTypeCode) + self.assertEqual(1, len(symbol_contexts)) + label_symbol = symbol_contexts[0].GetSymbol() + self.assertTrue(label_symbol.IsValid()) + bkpt_address = label_symbol.GetStartAddress().GetLoadAddress(target) + # lldb-server has its algorithm unit tested as part of # NativeProcessProtocol, but debugserver does not use that. So we will # do a few different types of overwrite here so we have coverage for both @@ -77,12 +86,13 @@ def test_write_over_breakpoint(self): for write_offset, write_size in writes: # Place a breakpoint immediately after the dead code in foo. - bkpt = target.BreakpointCreateByName("place_break_here") + bkpt = target.BreakpointCreateByAddress(bkpt_address) self.assertTrue(bkpt.IsValid()) self.assertEqual(bkpt.GetNumLocations(), 1) self.assertFalse(bkpt.IsHardware()) + self.assertEqual(bkpt.GetLocationAtIndex(0).GetLoadAddress(), bkpt_address) - check_address = bkpt.GetLocationAtIndex(0).GetLoadAddress() + write_offset + check_address = bkpt_address + write_offset # Read around the breakpoint site. We assume that read subsitution # is working, so the data here is the original contents of memory @@ -117,7 +127,7 @@ def test_write_over_breakpoint(self): # Should be stopped at the breakpoint we placed in foo. This proves that # the breakpoint instruction was intact. self.assertEqual( - bkpt.GetLocationAtIndex(0).GetLoadAddress(), + bkpt_address, thread.selected_frame.GetPC(), ) >From 59cbe3a3d229192786b0e3d0900ee226176871cd Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Thu, 20 Aug 2026 10:38:35 +0000 Subject: [PATCH 4/8] remove windows experiment --- .../breakpoint/write_over_software_breakpoint/main.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c index 4a24cded476de..84e67c7c9701e 100644 --- a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c +++ b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c @@ -17,11 +17,7 @@ int foo() { // in the very first instruction of foo and hoping whatever comes before is // not important. asm volatile( -#ifdef _WIN32 - ".def place_break_here; .scl 2; .type 32; .endef;\n" -#else ".globl place_break_here\n" -#endif "place_break_here:\n" // The test will repeatedly add and remove a breakpoint here. "nop"); >From c3330a025ba7131a606fd95ee5712180ec6cd279 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Thu, 20 Aug 2026 10:44:25 +0000 Subject: [PATCH 5/8] look for any symbol --- .../TestWriteOverSoftwareBreakpoint.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py index 6a79cd44b038d..677404bfff094 100644 --- a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py +++ b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py @@ -51,7 +51,7 @@ def test_write_over_breakpoint(self): # The breakpoint must be on an exact address, because we do not want # lldb to adjust it based on the debug information for prologues # and epilogues. - symbol_contexts = target.FindSymbols("place_break_here", lldb.eSymbolTypeCode) + symbol_contexts = target.FindSymbols("place_break_here") self.assertEqual(1, len(symbol_contexts)) label_symbol = symbol_contexts[0].GetSymbol() self.assertTrue(label_symbol.IsValid()) >From 81e51dd64df67f396d20ad78b84bd5e25032e1eb Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Thu, 20 Aug 2026 11:36:39 +0000 Subject: [PATCH 6/8] different take on the windows workaround --- .../breakpoint/write_over_software_breakpoint/main.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c index 84e67c7c9701e..46eea01f834fc 100644 --- a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c +++ b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c @@ -18,6 +18,9 @@ int foo() { // not important. asm volatile( ".globl place_break_here\n" +#ifdef _WIN32 + ".def place_break_here; .scl 2; .type 32; .endef;\n" +#endif "place_break_here:\n" // The test will repeatedly add and remove a breakpoint here. "nop"); >From 97c439c0915e5497f8004e6274029ea5d325bc14 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Thu, 20 Aug 2026 11:57:13 +0000 Subject: [PATCH 7/8] formatting --- .../breakpoint/write_over_software_breakpoint/main.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c index 46eea01f834fc..05e64fecd0116 100644 --- a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c +++ b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/main.c @@ -16,14 +16,13 @@ int foo() { // assuming that this function will have no prologue instructions or breaking // in the very first instruction of foo and hoping whatever comes before is // not important. - asm volatile( - ".globl place_break_here\n" + asm volatile(".globl place_break_here\n" #ifdef _WIN32 - ".def place_break_here; .scl 2; .type 32; .endef;\n" + ".def place_break_here; .scl 2; .type 32; .endef;\n" #endif - "place_break_here:\n" - // The test will repeatedly add and remove a breakpoint here. - "nop"); + "place_break_here:\n" + // The test will repeatedly add and remove a breakpoint here. + "nop"); return 0; } >From dca0c8afc9bbe772d5a77de00f3ad2d283f9d8ec Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Thu, 20 Aug 2026 12:30:52 +0000 Subject: [PATCH 8/8] give up on Windows --- .../TestWriteOverSoftwareBreakpoint.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py index 677404bfff094..22900d915957d 100644 --- a/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py +++ b/lldb/test/API/functionalities/breakpoint/write_over_software_breakpoint/TestWriteOverSoftwareBreakpoint.py @@ -13,6 +13,8 @@ class WriteOverSoftwareBreakpoint(TestBase): NO_DEBUG_INFO_TESTCASE = True + # Could not find a way to make place_break_here visible to lldb on Windows. + @skipIfWindows def test_write_over_breakpoint(self): TestBase.setUp(self) self.line = line_number("main.c", "// break here") _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
