diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp index cb95da9fc7236..1e2e3a80b18bf 100644 --- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp +++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp @@ -1641,6 +1641,10 @@ NativeProcessLinux::GetSoftwareBreakpointTrapOpcode(size_t size_hint) { Status NativeProcessLinux::ReadMemory(lldb::addr_t addr, void *buf, size_t size, size_t &bytes_read) { + Log *log = GetLog(POSIXLog::Memory); + LLDB_LOG(log, "addr = {0}, buf = {1}, size = {2}", addr, buf, size); + + bytes_read = 0; if (ProcessVmReadvSupported()) { // The process_vm_readv path is about 50 times faster than ptrace api. We // want to use this syscall if it is supported. @@ -1651,32 +1655,29 @@ Status NativeProcessLinux::ReadMemory(lldb::addr_t addr, void *buf, size_t size, remote_iov.iov_base = reinterpret_cast(addr); remote_iov.iov_len = size; - bytes_read = process_vm_readv(GetCurrentThreadID(), &local_iov, 1, - &remote_iov, 1, 0); - const bool success = bytes_read == size; + ssize_t read_result = process_vm_readv(GetCurrentThreadID(), &local_iov, 1, + &remote_iov, 1, 0); + int error = 0; + if (read_result < 0) + error = errno; + else + bytes_read = read_result; - Log *log = GetLog(POSIXLog::Process); LLDB_LOG(log, - "using process_vm_readv to read {0} bytes from inferior " - "address {1:x}: {2}", - size, addr, success ? "Success" : llvm::sys::StrError(errno)); - - if (success) - return Status(); - // else the call failed for some reason, let's retry the read using ptrace - // api. + "process_vm_readv({0}, [iovec({1}, {2})], [iovec({3:x}, {2})], 1, " + "0) => {4} ({5})", + GetCurrentThreadID(), buf, size, addr, read_result, + error > 0 ? llvm::sys::StrError(errno) : "sucesss"); } unsigned char *dst = static_cast(buf); size_t remainder; long data; - Log *log = GetLog(POSIXLog::Memory); - LLDB_LOG(log, "addr = {0}, buf = {1}, size = {2}", addr, buf, size); - - for (bytes_read = 0; bytes_read < size; bytes_read += remainder) { + for (; bytes_read < size; bytes_read += remainder) { Status error = NativeProcessLinux::PtraceWrapper( - PTRACE_PEEKDATA, GetCurrentThreadID(), (void *)addr, nullptr, 0, &data); + PTRACE_PEEKDATA, GetCurrentThreadID(), + reinterpret_cast(addr + bytes_read), nullptr, 0, &data); if (error.Fail()) return error; @@ -1684,11 +1685,7 @@ Status NativeProcessLinux::ReadMemory(lldb::addr_t addr, void *buf, size_t size, remainder = remainder > k_ptrace_word_size ? k_ptrace_word_size : remainder; // Copy the data into our buffer - memcpy(dst, &data, remainder); - - LLDB_LOG(log, "[{0:x}]:{1:x}", addr, data); - addr += k_ptrace_word_size; - dst += k_ptrace_word_size; + memcpy(dst + bytes_read, &data, remainder); } return Status(); } diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp index f6f13a8858727..504c994f980cf 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -2526,28 +2526,18 @@ GDBRemoteCommunicationServerLLGS::Handle_memory_read( size_t bytes_read = 0; Status error = m_current_process->ReadMemoryWithoutTrap( read_addr, &buf[0], byte_count, bytes_read); - if (error.Fail()) { - LLDB_LOGF(log, - "GDBRemoteCommunicationServerLLGS::%s pid %" PRIu64 - " mem 0x%" PRIx64 ": failed to read. Error: %s", - __FUNCTION__, m_current_process->GetID(), read_addr, - error.AsCString()); - return SendErrorResponse(0x08); - } - - if (bytes_read == 0) { - LLDB_LOGF(log, - "GDBRemoteCommunicationServerLLGS::%s pid %" PRIu64 - " mem 0x%" PRIx64 ": read 0 of %" PRIu64 " requested bytes", - __FUNCTION__, m_current_process->GetID(), read_addr, byte_count); + LLDB_LOG( + log, + "ReadMemoryWithoutTrap({0}) read {1} of {2} requested bytes (error: {3})", + read_addr, byte_count, bytes_read, error); + if (bytes_read == 0) return SendErrorResponse(0x08); - } StreamGDBRemote response; packet.SetFilePos(0); char kind = packet.GetChar('?'); if (kind == 'x') - response.PutEscapedBytes(buf.data(), byte_count); + response.PutEscapedBytes(buf.data(), bytes_read); else { assert(kind == 'm'); for (size_t i = 0; i < bytes_read; ++i) diff --git a/lldb/test/API/functionalities/memory/holes/Makefile b/lldb/test/API/functionalities/memory/holes/Makefile new file mode 100644 index 0000000000000..99998b20bcb05 --- /dev/null +++ b/lldb/test/API/functionalities/memory/holes/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py b/lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py new file mode 100644 index 0000000000000..6ecb25b2e4db2 --- /dev/null +++ b/lldb/test/API/functionalities/memory/holes/TestMemoryHoles.py @@ -0,0 +1,61 @@ +""" +Test the memory commands operating on memory regions with holes (inaccessible +pages) +""" + +import lldb +from lldbsuite.test.lldbtest import * +import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.decorators import * + + +class MemoryHolesTestCase(TestBase): + NO_DEBUG_INFO_TESTCASE = True + + def setUp(self): + super().setUp() + # Find the line number to break inside main(). + self.line = line_number("main.cpp", "// break here") + + def _prepare_inferior(self): + self.build() + exe = self.getBuildArtifact("a.out") + self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) + + # Break in main() after the variables are assigned values. + lldbutil.run_break_set_by_file_and_line( + self, "main.cpp", self.line, num_expected_locations=1, loc_exact=True + ) + + self.runCmd("run", RUN_SUCCEEDED) + + # The stop reason of the thread should be breakpoint. + self.expect( + "thread list", + STOPPED_DUE_TO_BREAKPOINT, + substrs=["stopped", "stop reason = breakpoint"], + ) + + # The breakpoint should have a hit count of 1. + lldbutil.check_breakpoint(self, bpno=1, expected_hit_count=1) + + # Avoid the expression evaluator, as it can allocate allocate memory + # inside the holes we've deliberately left empty. + self.memory = self.frame().FindVariable("mem_with_holes").GetValueAsUnsigned() + self.pagesize = self.frame().FindVariable("pagesize").GetValueAsUnsigned() + positions = self.frame().FindVariable("positions") + self.positions = [ + positions.GetChildAtIndex(i).GetValueAsUnsigned() + for i in range(positions.GetNumChildren()) + ] + self.assertEqual(len(self.positions), 5) + + @expectedFailureWindows + def test_memory_read(self): + self._prepare_inferior() + + error = lldb.SBError() + content = self.process().ReadMemory(self.memory, 2 * self.pagesize, error) + self.assertEqual(len(content), self.pagesize) + self.assertEqual(content[0:7], b"needle\0") + self.assertTrue(error.Fail()) diff --git a/lldb/test/API/functionalities/memory/holes/main.cpp b/lldb/test/API/functionalities/memory/holes/main.cpp new file mode 100644 index 0000000000000..3228a1bff7671 --- /dev/null +++ b/lldb/test/API/functionalities/memory/holes/main.cpp @@ -0,0 +1,88 @@ +#include +#include +#include +#include +#include +#include + +constexpr size_t num_pages = 7; +constexpr size_t accessible_pages[] = {0, 2, 4, 6}; + +bool is_accessible(size_t page) { + return std::find(std::begin(accessible_pages), std::end(accessible_pages), + page) != std::end(accessible_pages); +} + +// allocate_memory_with_holes returns a pointer to `num_pages` pages of memory, +// where some of the pages are inaccessible (even to debugging APIs). We use +// this to test lldb's ability to skip over inaccessible blocks. +#ifdef _WIN32 +#include "Windows.h" + +int getpagesize() { + SYSTEM_INFO system_info; + GetSystemInfo(&system_info); + return system_info.dwPageSize; +} + +char *allocate_memory_with_holes() { + int pagesize = getpagesize(); + void *mem = + VirtualAlloc(nullptr, num_pages * pagesize, MEM_RESERVE, PAGE_NOACCESS); + if (!mem) { + std::cerr << std::system_category().message(GetLastError()) << std::endl; + exit(1); + } + char *bytes = static_cast(mem); + for (size_t page = 0; page < num_pages; ++page) { + if (!is_accessible(page)) + continue; + if (!VirtualAlloc(bytes + page * pagesize, pagesize, MEM_COMMIT, + PAGE_READWRITE)) { + std::cerr << std::system_category().message(GetLastError()) << std::endl; + exit(1); + } + } + return bytes; +} +#else +#include "sys/mman.h" +#include "unistd.h" + +char *allocate_memory_with_holes() { + int pagesize = getpagesize(); + void *mem = mmap(nullptr, num_pages * pagesize, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + if (mem == MAP_FAILED) { + perror("mmap"); + exit(1); + } + char *bytes = static_cast(mem); + for (size_t page = 0; page < num_pages; ++page) { + if (is_accessible(page)) + continue; + if (munmap(bytes + page * pagesize, pagesize) != 0) { + perror("munmap"); + exit(1); + } + } + return bytes; +} +#endif + +int main(int argc, char const *argv[]) { + char *mem_with_holes = allocate_memory_with_holes(); + int pagesize = getpagesize(); + char *positions[] = { + mem_with_holes, // Beginning of memory + mem_with_holes + 2 * pagesize, // After a hole + mem_with_holes + 2 * pagesize + + pagesize / 2, // Middle of a block, after an existing match. + mem_with_holes + 5 * pagesize - 7, // End of a block + mem_with_holes + 7 * pagesize - 7, // End of memory + }; + for (char *p : positions) + strcpy(p, "needle"); + + return 0; // break here +}