[Lldb-commits] [lldb] r343076 - Fix a memory read bug in lldb-server
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Wed Sep 26 00:31:41 PDT 2018
Author: labath
Date: Wed Sep 26 00:31:41 2018
New Revision: 343076
URL: http://llvm.org/viewvc/llvm-project?rev=343076&view=rev
Log:
Fix a memory read bug in lldb-server
NativeProcessProtocol::ReadMemoryWithoutTrap had a bug, where it failed
to properly remove inserted breakpoint opcodes if the memory read
partially overlapped the trap opcode. This could not happen on x86
because it has a one-byte breakpoint instruction, but it could happen on
arm, which has a 4-byte breakpoint instruction (in arm mode).
Since triggerring this condition would only be possible on an arm
machine (and even then it would be a bit tricky). I test this using a
NativeProcessProtocol unit test.
Modified:
lldb/trunk/source/Host/common/NativeBreakpointList.cpp
lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp
Modified: lldb/trunk/source/Host/common/NativeBreakpointList.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/NativeBreakpointList.cpp?rev=343076&r1=343075&r2=343076&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/NativeBreakpointList.cpp (original)
+++ lldb/trunk/source/Host/common/NativeBreakpointList.cpp Wed Sep 26 00:31:41 2018
@@ -210,20 +210,31 @@ Status NativeBreakpointList::GetBreakpoi
Status NativeBreakpointList::RemoveTrapsFromBuffer(lldb::addr_t addr, void *buf,
size_t size) const {
+ auto data = llvm::makeMutableArrayRef(static_cast<uint8_t *>(buf), size);
for (const auto &map : m_breakpoints) {
- lldb::addr_t bp_addr = map.first;
- // Breapoint not in range, ignore
- if (bp_addr < addr || addr + size <= bp_addr)
- continue;
const auto &bp_sp = map.second;
// Not software breakpoint, ignore
if (!bp_sp->IsSoftwareBreakpoint())
continue;
auto software_bp_sp = std::static_pointer_cast<SoftwareBreakpoint>(bp_sp);
- auto opcode_addr = static_cast<char *>(buf) + bp_addr - addr;
- auto saved_opcodes = software_bp_sp->m_saved_opcodes;
+
+ lldb::addr_t bp_addr = map.first;
auto opcode_size = software_bp_sp->m_opcode_size;
- ::memcpy(opcode_addr, saved_opcodes, opcode_size);
+
+ // Breapoint not in range, ignore
+ if (bp_addr + opcode_size < addr || addr + size <= bp_addr)
+ continue;
+
+ auto saved_opcodes =
+ llvm::makeArrayRef(software_bp_sp->m_saved_opcodes, opcode_size);
+ if (bp_addr < addr) {
+ saved_opcodes = saved_opcodes.drop_front(addr - bp_addr);
+ bp_addr = addr;
+ }
+ auto bp_data = data.drop_front(bp_addr - addr);
+ std::copy_n(saved_opcodes.begin(),
+ std::min(saved_opcodes.size(), bp_data.size()),
+ bp_data.begin());
}
return Status();
}
Modified: lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp?rev=343076&r1=343075&r2=343076&view=diff
==============================================================================
--- lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp (original)
+++ lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp Wed Sep 26 00:31:41 2018
@@ -70,10 +70,22 @@ public:
llvm::ArrayRef<uint8_t> Data));
using NativeProcessProtocol::GetSoftwareBreakpointTrapOpcode;
+ llvm::Expected<std::vector<uint8_t>> ReadMemoryWithoutTrap(addr_t Addr,
+ size_t Size);
private:
ArchSpec Arch;
};
+
+class FakeMemory {
+public:
+ FakeMemory(llvm::ArrayRef<uint8_t> Data) : Data(Data) {}
+ llvm::Expected<std::vector<uint8_t>> Read(addr_t Addr, size_t Size);
+ llvm::Expected<size_t> Write(addr_t Addr, llvm::ArrayRef<uint8_t> Chunk);
+
+private:
+ std::vector<uint8_t> Data;
+};
} // namespace
Status MockProcess::ReadMemory(addr_t Addr, void *Buf, size_t Size,
@@ -101,6 +113,37 @@ Status MockProcess::WriteMemory(addr_t A
return Status();
}
+llvm::Expected<std::vector<uint8_t>>
+MockProcess::ReadMemoryWithoutTrap(addr_t Addr, size_t Size) {
+ std::vector<uint8_t> Data(Size, 0);
+ size_t BytesRead;
+ Status ST = NativeProcessProtocol::ReadMemoryWithoutTrap(
+ Addr, Data.data(), Data.size(), BytesRead);
+ if (ST.Fail())
+ return ST.ToError();
+ Data.resize(BytesRead);
+ return std::move(Data);
+}
+
+llvm::Expected<std::vector<uint8_t>> FakeMemory::Read(addr_t Addr,
+ size_t Size) {
+ if (Addr >= Data.size())
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Address out of range.");
+ Size = std::min(Size, Data.size() - Addr);
+ return std::vector<uint8_t>(&Data[Addr], &Data[Addr + Size]);
+}
+
+llvm::Expected<size_t> FakeMemory::Write(addr_t Addr,
+ llvm::ArrayRef<uint8_t> Chunk) {
+ if (Addr >= Data.size())
+ return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "Address out of range.");
+ size_t Size = std::min(Chunk.size(), Data.size() - Addr);
+ std::copy_n(Chunk.begin(), Size, &Data[Addr]);
+ return Size;
+}
+
TEST(NativeProcessProtocolTest, SetBreakpoint) {
NiceMock<MockDelegate> DummyDelegate;
MockProcess Process(DummyDelegate, ArchSpec("x86_64-pc-linux"));
@@ -152,3 +195,27 @@ TEST(NativeProcessProtocolTest, SetBreak
EXPECT_THAT_ERROR(Process.SetBreakpoint(0x47, 0, false).ToError(),
llvm::Failed());
}
+
+TEST(NativeProcessProtocolTest, ReadMemoryWithoutTrap) {
+ NiceMock<MockDelegate> DummyDelegate;
+ MockProcess Process(DummyDelegate, ArchSpec("aarch64-pc-linux"));
+ FakeMemory M{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}};
+ EXPECT_CALL(Process, ReadMemory(_, _))
+ .WillRepeatedly(Invoke(&M, &FakeMemory::Read));
+ EXPECT_CALL(Process, WriteMemory(_, _))
+ .WillRepeatedly(Invoke(&M, &FakeMemory::Write));
+
+ EXPECT_THAT_ERROR(Process.SetBreakpoint(0x4, 0, false).ToError(),
+ llvm::Succeeded());
+ EXPECT_THAT_EXPECTED(
+ Process.ReadMemoryWithoutTrap(0, 10),
+ llvm::HasValue(std::vector<uint8_t>{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}));
+ EXPECT_THAT_EXPECTED(Process.ReadMemoryWithoutTrap(0, 6),
+ llvm::HasValue(std::vector<uint8_t>{0, 1, 2, 3, 4, 5}));
+ EXPECT_THAT_EXPECTED(Process.ReadMemoryWithoutTrap(6, 4),
+ llvm::HasValue(std::vector<uint8_t>{6, 7, 8, 9}));
+ EXPECT_THAT_EXPECTED(Process.ReadMemoryWithoutTrap(6, 2),
+ llvm::HasValue(std::vector<uint8_t>{6, 7}));
+ EXPECT_THAT_EXPECTED(Process.ReadMemoryWithoutTrap(4, 2),
+ llvm::HasValue(std::vector<uint8_t>{4, 5}));
+}
More information about the lldb-commits
mailing list