https://github.com/royitaqi updated
https://github.com/llvm/llvm-project/pull/148738
>From 890eb98ceada6495f6a089f7f0a4823b7369d2ec Mon Sep 17 00:00:00 2001
From: royshi <roy...@devvm24317.cln0.facebook.com>
Date: Mon, 14 Jul 2025 13:45:13 -0700
Subject: [PATCH 1/2] [lldb] Fix a crash in lldb-server during
RemoveSoftwareBreakpoint()
---
.../Host/common/NativeProcessProtocol.cpp | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Host/common/NativeProcessProtocol.cpp
b/lldb/source/Host/common/NativeProcessProtocol.cpp
index 405acbb5662d6..196f54b93538d 100644
--- a/lldb/source/Host/common/NativeProcessProtocol.cpp
+++ b/lldb/source/Host/common/NativeProcessProtocol.cpp
@@ -366,12 +366,19 @@ Status
NativeProcessProtocol::RemoveSoftwareBreakpoint(lldb::addr_t addr) {
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.
+ SoftwareBreakpoint bkpt = std::move(it->second);
+ m_software_breakpoints.erase(it);
+
// This is the last reference. Let's remove the breakpoint.
Status error;
// Clear a software breakpoint instruction
- llvm::SmallVector<uint8_t, 4> curr_break_op(
- it->second.breakpoint_opcodes.size(), 0);
+ llvm::SmallVector<uint8_t, 4> curr_break_op(bkpt.breakpoint_opcodes.size(),
+ 0);
// Read the breakpoint opcode
size_t bytes_read = 0;
@@ -382,10 +389,10 @@ Status
NativeProcessProtocol::RemoveSoftwareBreakpoint(lldb::addr_t addr) {
"addr=0x%" PRIx64 ": tried to read %zu bytes but only read %zu", addr,
curr_break_op.size(), bytes_read);
}
- const auto &saved = it->second.saved_opcodes;
+ const auto &saved = bkpt.saved_opcodes;
// Make sure the breakpoint opcode exists at this address
- if (llvm::ArrayRef(curr_break_op) != it->second.breakpoint_opcodes) {
- if (curr_break_op != it->second.saved_opcodes)
+ if (llvm::ArrayRef(curr_break_op) != bkpt.breakpoint_opcodes) {
+ if (curr_break_op != bkpt.saved_opcodes)
return Status::FromErrorString(
"Original breakpoint trap is no longer in memory.");
LLDB_LOG(log,
@@ -418,7 +425,6 @@ Status
NativeProcessProtocol::RemoveSoftwareBreakpoint(lldb::addr_t addr) {
llvm::make_range(saved.begin(), saved.end()));
}
- m_software_breakpoints.erase(it);
return Status();
}
>From d15f269446dbd8abcaa4cf365b03dc647824dc90 Mon Sep 17 00:00:00 2001
From: royshi <roy...@devvm24317.cln0.facebook.com>
Date: Tue, 15 Jul 2025 14:58:49 -0700
Subject: [PATCH 2/2] Add unit test
---
.../Host/NativeProcessProtocolTest.cpp | 93 ++++++++++++++++++-
1 file changed, 92 insertions(+), 1 deletion(-)
diff --git a/lldb/unittests/Host/NativeProcessProtocolTest.cpp
b/lldb/unittests/Host/NativeProcessProtocolTest.cpp
index a48e67c9213da..91c4fd69d6e54 100644
--- a/lldb/unittests/Host/NativeProcessProtocolTest.cpp
+++ b/lldb/unittests/Host/NativeProcessProtocolTest.cpp
@@ -73,6 +73,97 @@ TEST(NativeProcessProtocolTest, SetBreakpointFailVerify) {
llvm::Failed());
}
+TEST(NativeProcessProtocolTest, RemoveSoftwareBreakpoint) {
+ NiceMock<MockDelegate> DummyDelegate;
+ MockProcess<NativeProcessProtocol> Process(DummyDelegate,
+ ArchSpec("x86_64-pc-linux"));
+ auto Trap = cantFail(Process.GetSoftwareBreakpointTrapOpcode(1));
+ auto Original = std::vector<uint8_t>{0xbb};
+
+ // Set up a breakpoint.
+ {
+ InSequence S;
+ EXPECT_CALL(Process, ReadMemory(0x47, 1))
+ .WillOnce(Return(ByMove(Original)));
+ EXPECT_CALL(Process, WriteMemory(0x47, Trap)).WillOnce(Return(ByMove(1)));
+ EXPECT_CALL(Process, ReadMemory(0x47, 1)).WillOnce(Return(ByMove(Trap)));
+ EXPECT_THAT_ERROR(Process.SetBreakpoint(0x47, 0, false).ToError(),
+ llvm::Succeeded());
+ }
+
+ // Remove the breakpoint for the first time. This should remove the
breakpoint
+ // from m_software_breakpoints.
+ //
+ // Should succeed.
+ {
+ InSequence S;
+ EXPECT_CALL(Process, ReadMemory(0x47, 1)).WillOnce(Return(ByMove(Trap)));
+ EXPECT_CALL(Process, WriteMemory(0x47, llvm::ArrayRef(Original)))
+ .WillOnce(Return(ByMove(1)));
+ EXPECT_CALL(Process, ReadMemory(0x47, 1))
+ .WillOnce(Return(ByMove(Original)));
+ EXPECT_THAT_ERROR(Process.RemoveBreakpoint(0x47, false).ToError(),
+ llvm::Succeeded());
+ }
+
+ // Remove the breakpoint for the second time.
+ //
+ // Should fail. None of the ReadMemory() or WriteMemory() should be called,
+ // because the function should early return when seeing that the breakpoint
+ // isn't in m_software_breakpoints.
+ {
+ EXPECT_CALL(Process, ReadMemory(_, _)).Times(0);
+ EXPECT_CALL(Process, WriteMemory(_, _)).Times(0);
+ EXPECT_THAT_ERROR(Process.RemoveBreakpoint(0x47, false).ToError(),
+ llvm::Failed());
+ }
+}
+
+TEST(NativeProcessProtocolTest, RemoveSoftwareBreakpointMemoryError) {
+ NiceMock<MockDelegate> DummyDelegate;
+ MockProcess<NativeProcessProtocol> Process(DummyDelegate,
+ ArchSpec("x86_64-pc-linux"));
+ auto Trap = cantFail(Process.GetSoftwareBreakpointTrapOpcode(1));
+ auto Original = std::vector<uint8_t>{0xbb};
+ auto SomethingElse = std::vector<uint8_t>{0xaa};
+
+ // Set up a breakpoint.
+ {
+ InSequence S;
+ EXPECT_CALL(Process, ReadMemory(0x47, 1))
+ .WillOnce(Return(ByMove(Original)));
+ EXPECT_CALL(Process, WriteMemory(0x47, Trap)).WillOnce(Return(ByMove(1)));
+ EXPECT_CALL(Process, ReadMemory(0x47, 1)).WillOnce(Return(ByMove(Trap)));
+ EXPECT_THAT_ERROR(Process.SetBreakpoint(0x47, 0, false).ToError(),
+ llvm::Succeeded());
+ }
+
+ // Remove the breakpoint for the first time, with an unexpected value read by
+ // the first ReadMemory(). This should cause an early return, with the
+ // breakpoint removed from m_software_breakpoints.
+ //
+ // Should fail.
+ {
+ InSequence S;
+ EXPECT_CALL(Process, ReadMemory(0x47, 1))
+ .WillOnce(Return(ByMove(SomethingElse)));
+ EXPECT_THAT_ERROR(Process.RemoveBreakpoint(0x47, false).ToError(),
+ llvm::Failed());
+ }
+
+ // Remove the breakpoint for the second time.
+ //
+ // Should fail. None of the ReadMemory() or WriteMemory() should be called,
+ // because the function should early return when seeing that the breakpoint
+ // isn't in m_software_breakpoints.
+ {
+ EXPECT_CALL(Process, ReadMemory(_, _)).Times(0);
+ EXPECT_CALL(Process, WriteMemory(_, _)).Times(0);
+ EXPECT_THAT_ERROR(Process.RemoveBreakpoint(0x47, false).ToError(),
+ llvm::Failed());
+ }
+}
+
TEST(NativeProcessProtocolTest, ReadMemoryWithoutTrap) {
NiceMock<MockDelegate> DummyDelegate;
MockProcess<NativeProcessProtocol> Process(DummyDelegate,
@@ -146,4 +237,4 @@ TEST(NativeProcessProtocolTest,
ReadCStringFromMemory_CrossPageBoundary) {
bytes_read),
llvm::HasValue(llvm::StringRef("hello")));
EXPECT_EQ(bytes_read, 6UL);
-}
\ No newline at end of file
+}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
