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})); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits