diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 77122d7d9a9b5..b7ed48d41bd7e 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -1574,6 +1574,28 @@ class Process : public std::enable_shared_from_this, virtual size_t ReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, Status &error); + /// Read from multiple memory ranges and write the results into buffer. + /// This calls ReadMemoryFromInferior multiple times, once per range, + /// bypassing the read cache. Process implementations that can perform this + /// operation more efficiently should override this. + /// + /// \param[in] ranges + /// A collection of ranges (base address + size) to read from. + /// + /// \param[out] buffer + /// A buffer where the read memory will be written to. It must be at least + /// as long as the sum of the sizes of each range. + /// + /// \return + /// A vector of MutableArrayRef, where each MutableArrayRef is a slice of + /// the input buffer into which the memory contents were copied. The size + /// of the slice indicates how many bytes were read successfully. Partial + /// reads are always performed from the start of the requested range, + /// never from the middle or end. + virtual llvm::SmallVector> + ReadMemoryRanges(llvm::ArrayRef> ranges, + llvm::MutableArrayRef buffer); + /// Read of memory from a process. /// /// This function has the same semantics of ReadMemory except that it @@ -1647,11 +1669,24 @@ class Process : public std::enable_shared_from_this, size_t byte_size, uint64_t fail_value, Status &error); + /// Use Process::ReadMemoryRanges to efficiently read multiple unsigned + /// integers from memory at once. + /// TODO: this should be upstream once there is a use for it there. + llvm::SmallVector> + ReadUnsignedIntegersFromMemory(llvm::ArrayRef addresses, + unsigned byte_size); + int64_t ReadSignedIntegerFromMemory(lldb::addr_t load_addr, size_t byte_size, int64_t fail_value, Status &error); lldb::addr_t ReadPointerFromMemory(lldb::addr_t vm_addr, Status &error); + /// Use Process::ReadMemoryRanges to efficiently read multiple pointers from + /// memory at once. + /// TODO: this should be upstream once there is a use for it there. + llvm::SmallVector> + ReadPointersFromMemory(llvm::ArrayRef ptr_locs); + bool WritePointerToMemory(lldb::addr_t vm_addr, lldb::addr_t ptr_value, Status &error); diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp index 5e2007e013b85..8eedb5878dda4 100644 --- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp +++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp @@ -279,22 +279,23 @@ ClassDescriptorV2::ReadMethods(llvm::ArrayRef addresses, const size_t num_methods = addresses.size(); llvm::SmallVector buffer(num_methods * size, 0); - llvm::DenseSet failed_indices; - for (auto [idx, addr] : llvm::enumerate(addresses)) { - Status error; - process->ReadMemory(addr, buffer.data() + idx * size, size, error); - if (error.Fail()) - failed_indices.insert(idx); - } + llvm::SmallVector> mem_ranges = + llvm::to_vector(llvm::map_range(llvm::seq(num_methods), [&](size_t idx) { + return Range(addresses[idx], size); + })); + + llvm::SmallVector> read_results = + process->ReadMemoryRanges(mem_ranges, buffer); llvm::SmallVector methods; methods.reserve(num_methods); - for (auto [idx, addr] : llvm::enumerate(addresses)) { - if (failed_indices.contains(idx)) + for (auto [addr, memory] : llvm::zip(addresses, read_results)) { + // Ignore partial reads. + if (memory.size() != size) continue; - DataExtractor extractor(buffer.data() + idx * size, size, - process->GetByteOrder(), + + DataExtractor extractor(memory.data(), size, process->GetByteOrder(), process->GetAddressByteSize()); methods.push_back(method_t()); methods.back().Read(extractor, process, addr, relative_selector_base_addr, diff --git a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp index a43c0a6b80b3b..95a752d0541ad 100644 --- a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp +++ b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp @@ -2337,14 +2337,15 @@ class CommandObjectLanguageSwiftTaskInfo final : public CommandObjectParsed { } TaskInspector task_inspector; - auto task_addr_or_err = task_inspector.GetTaskAddrFromThreadLocalStorage( - m_exe_ctx.GetThreadRef()); - if (auto error = task_addr_or_err.takeError()) { - result.AppendError(toString(std::move(error))); + std::optional maybe_task_addr = + task_inspector.GetTaskAddrFromThreadLocalStorage( + m_exe_ctx.GetThreadRef()); + if (!task_addr) { + result.AppendError("could find the task address"); return; } - task_addr = task_addr_or_err.get(); + task_addr = *maybe_task_addr; } auto ts_or_err = m_exe_ctx.GetTargetRef().GetScratchTypeSystemForLanguage( @@ -2935,19 +2936,6 @@ std::optional SwiftLanguageRuntime::TrySkipVirtualParentProlog( return pc_value; } -/// Attempts to read the memory location at `task_addr_location`, producing -/// the Task pointer if possible. -static llvm::Expected -ReadTaskAddr(lldb::addr_t task_addr_location, Process &process) { - Status status; - addr_t task_addr = process.ReadPointerFromMemory(task_addr_location, status); - if (status.Fail()) - return llvm::joinErrors( - llvm::createStringError("could not get current task from thread"), - status.takeError()); - return task_addr; -} - /// Compute the location where the Task pointer for `real_thread` is stored by /// the runtime. static llvm::Expected @@ -2975,48 +2963,146 @@ ComputeTaskAddrLocationFromThreadLocalStorage(Thread &real_thread) { #endif } -llvm::Expected +/// Helper function to read all `pointers` from process memory at once. +/// Consumes any errors from the input by propagating them to the output. +static llvm::SmallVector> +MultiReadPointers(Process &process, + llvm::MutableArrayRef> maybe_pointers) { + llvm::SmallVector> final_results; + llvm::SmallVector to_read; + final_results.reserve(maybe_pointers.size()); + to_read.reserve(maybe_pointers.size()); + + /// Filter the input: propagate input errors directly to the output, forward + /// proper inputs to `to_read`. + for (std::optional &maybe_ptr : maybe_pointers) { + if (!maybe_ptr) + final_results.emplace_back(std::nullopt); + else { + final_results.push_back(LLDB_INVALID_ADDRESS); + to_read.push_back(*maybe_ptr); + } + } + + llvm::SmallVector> read_results = + process.ReadPointersFromMemory(to_read); + llvm::MutableArrayRef> results_ref = read_results; + + // Move the results in the slots not filled by errors from the input. + for (std::optional &maybe_result : final_results) + if (maybe_result) { + maybe_result = results_ref.front(); + results_ref = results_ref.drop_front(); + } + + assert(results_ref.empty()); + return final_results; +} + +/// Helper function to read `addr` from process memory. Errors in the input are +/// propagated to to the output. +static std::optional ReadPointer(Process &process, + std::optional addr) { + return MultiReadPointers(process, addr)[0]; +} + +std::optional TaskInspector::GetTaskAddrFromThreadLocalStorage(Thread &thread) { - // Look through backing threads when inspecting TLS. - Thread &real_thread = - thread.GetBackingThread() ? *thread.GetBackingThread() : thread; + return GetTaskAddrFromThreadLocalStorage(&thread)[0]; +} - if (auto it = m_tid_to_task_addr_location.find(real_thread.GetID()); - it != m_tid_to_task_addr_location.end()) { +llvm::SmallVector> +TaskInspector::GetTaskAddrLocations(llvm::ArrayRef threads) { + llvm::SmallVector> addr_locations; + addr_locations.reserve(threads.size()); + + for (auto [idx, thread] : llvm::enumerate(threads)) { + Thread &real_thread = + thread->GetBackingThread() ? *thread->GetBackingThread() : *thread; + + auto it = m_tid_to_task_addr_location.find(real_thread.GetID()); + if (it != m_tid_to_task_addr_location.end()) { + addr_locations.push_back(it->second); #ifndef NDEBUG - // In assert builds, check that caching did not produce incorrect results. - llvm::Expected task_addr_location = - ComputeTaskAddrLocationFromThreadLocalStorage(real_thread); - assert(task_addr_location); - assert(it->second == *task_addr_location); + // In assert builds, check that caching did not produce incorrect results. + llvm::Expected task_addr_location = + ComputeTaskAddrLocationFromThreadLocalStorage(real_thread); + assert(task_addr_location); + assert(it->second == *task_addr_location); #endif - llvm::Expected task_addr = - ReadTaskAddr(it->second, *thread.GetProcess()); - if (task_addr) - return task_addr; - // If the cached task addr location became invalid, invalidate the cache. - m_tid_to_task_addr_location.erase(it); - LLDB_LOG_ERROR(GetLog(LLDBLog::OS), task_addr.takeError(), - "TaskInspector: evicted task location address due to " - "invalid memory read: {0}"); - } - - llvm::Expected task_addr_location = + continue; + } + llvm::Expected addr_loc = + ComputeTaskAddrLocationFromThreadLocalStorage(real_thread); + if (!addr_loc) { + LLDB_LOG_ERROR(GetLog(LLDBLog::OS), addr_loc.takeError(), + "TaskInspector: failed to compute task address location " + "from TLS: {0}"); + addr_locations.push_back(std::nullopt); + } else + addr_locations.push_back(*addr_loc); + } + return addr_locations; +} + +std::optional TaskInspector::RetryRead(Thread &thread, + addr_t task_addr_location) { + Thread &real_thread = + thread.GetBackingThread() ? *thread.GetBackingThread() : thread; + user_id_t tid = real_thread.GetID(); + + // For unsuccessful reads whose address was not cached, don't try again. + if (!m_tid_to_task_addr_location.erase(tid)) + return std::nullopt; + + LLDB_LOG(GetLog(LLDBLog::OS), "TaskInspector: evicted task location " + "address due to invalid memory read"); + + // The cached address could not be loaded. "This should never happen", but + // recompute the address and try again for completeness. + llvm::Expected task_addr_loc = ComputeTaskAddrLocationFromThreadLocalStorage(real_thread); - if (!task_addr_location) - return task_addr_location; - - llvm::Expected task_addr = - ReadTaskAddr(*task_addr_location, *thread.GetProcess()); - - // If the read from this TLS address is successful, cache the TLS address. - // Caching without a valid read is dangerous: earlier in the thread - // lifetime, the result of GetExtendedInfo can be invalid. - if (task_addr && - real_thread.GetProcess()->GetTarget().GetSwiftCacheTaskPointerLocation()) - m_tid_to_task_addr_location.try_emplace(real_thread.GetID(), - *task_addr_location); - return task_addr; + if (!task_addr_loc) { + LLDB_LOG_ERROR(GetLog(LLDBLog::OS), task_addr_loc.takeError(), + "TaskInspector: failed to compute task address location " + "from TLS: {0}"); + return std::nullopt; + } + + std::optional read_retry_result = + ReadPointer(*thread.GetProcess(), *task_addr_loc); + if (read_retry_result) + m_tid_to_task_addr_location[tid] = *task_addr_loc; + return read_retry_result; +} + +llvm::SmallVector> +TaskInspector::GetTaskAddrFromThreadLocalStorage( + llvm::ArrayRef threads) { + if (threads.empty()) + return {}; + + llvm::SmallVector> addr_locations = + GetTaskAddrLocations(threads); + + Process &process = *threads[0]->GetProcess(); + llvm::SmallVector> mem_read_results = + MultiReadPointers(process, addr_locations); + + for (auto [idx, thread] : llvm::enumerate(threads)) { + if (!addr_locations[idx]) + continue; + // If the read was successful, cache the address. + if (mem_read_results[idx]) { + Thread &real_thread = + thread->GetBackingThread() ? *thread->GetBackingThread() : *thread; + m_tid_to_task_addr_location[real_thread.GetID()] = *addr_locations[idx]; + continue; + } + mem_read_results[idx] = RetryRead(*thread, *addr_locations[idx]); + } + + return mem_read_results; } namespace { diff --git a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.h b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.h index d72bc0522ff34..c02c15cded236 100644 --- a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.h +++ b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.h @@ -934,10 +934,24 @@ class TaskInspector { public: /// Inspects thread local storage to find the address of the currently /// executing task, if any. - llvm::Expected - GetTaskAddrFromThreadLocalStorage(Thread &thread); + std::optional GetTaskAddrFromThreadLocalStorage(Thread &thread); + + /// Inspects thread local storage to find the address of the currently + /// executing task, if any. + llvm::SmallVector> + GetTaskAddrFromThreadLocalStorage(llvm::ArrayRef threads); private: + /// For each thread in `threads`, return the location of the its task + /// pointer, if it exists. + llvm::SmallVector> + GetTaskAddrLocations(llvm::ArrayRef threads); + + /// If reading from a cached task address location failed, invalidate the + /// cache and try again. + std::optional RetryRead(Thread &thread, + lldb::addr_t task_addr_location); + llvm::DenseMap m_tid_to_task_addr_location; }; diff --git a/lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.cpp b/lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.cpp index 08d220e333b01..169fd6abe7319 100644 --- a/lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.cpp +++ b/lldb/source/Plugins/OperatingSystem/SwiftTasks/OperatingSystemSwiftTasks.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "llvm/ADT/Sequence.h" #include "llvm/Support/Error.h" #include #if LLDB_ENABLE_SWIFT @@ -151,36 +152,72 @@ OperatingSystemSwiftTasks::FindOrCreateSwiftThread(ThreadList &old_thread_list, /*register_data_addr*/ 0); } -static std::optional FindTaskAddress(TaskInspector &task_inspector, - Thread &thread) { - llvm::Expected task_addr = - task_inspector.GetTaskAddrFromThreadLocalStorage(thread); - if (!task_addr) { - LLDB_LOG_ERROR(GetLog(LLDBLog::OS), task_addr.takeError(), - "OperatingSystemSwiftTasks: failed to find task address in " - "thread local storage: {0}"); - return {}; +/// For each thread in `threads_it`, computes the task address that is being run +/// by the thread, if any. +static llvm::SmallVector> +FindTaskAddresses(TaskInspector &task_inspector, + ThreadCollection::ThreadIterable &threads_it) { + llvm::SmallVector threads; + for (const ThreadSP &thread : threads_it) + threads.push_back(thread.get()); + + llvm::SmallVector> task_addrs; + task_addrs.reserve(threads.size()); + + for (std::optional &task_addr : + task_inspector.GetTaskAddrFromThreadLocalStorage(threads)) { + if (!task_addr) { + LLDB_LOG(GetLog(LLDBLog::OS), "OperatingSystemSwiftTasks: failed to find " + "task address in thread local storage"); + task_addrs.push_back(std::nullopt); + continue; + } + if (*task_addr == 0 || *task_addr == LLDB_INVALID_ADDRESS) + task_addrs.push_back(std::nullopt); + else + task_addrs.push_back(*task_addr); } - if (*task_addr == 0 || *task_addr == LLDB_INVALID_ADDRESS) - return std::nullopt; - return *task_addr; + return task_addrs; } -static std::optional FindTaskId(addr_t task_addr, Process &process) { - size_t ptr_size = process.GetAddressByteSize(); +static llvm::SmallVector> +FindTaskIds(llvm::ArrayRef> maybe_task_addrs, + Process &process) { + llvm::SmallVector> final_results; + llvm::SmallVector to_read; + final_results.reserve(maybe_task_addrs.size()); + to_read.reserve(maybe_task_addrs.size()); + // Offset of a Task ID inside a Task data structure, guaranteed by the ABI. // See Job in swift/RemoteInspection/RuntimeInternals.h. + size_t ptr_size = process.GetAddressByteSize(); const offset_t job_id_offset = 4 * ptr_size + 4; - Status error; - // The Task ID is at offset job_id_offset from the Task pointer. + /// Propagate input errors directly to the output, forward proper inputs to + /// `to_read`. + for (std::optional maybe_ptr : maybe_task_addrs) { + if (!maybe_ptr) + final_results.emplace_back(std::nullopt); + else { + final_results.push_back(0); + to_read.push_back(*maybe_ptr + job_id_offset); + } + } + constexpr uint32_t num_bytes_task_id = 4; - auto task_id = process.ReadUnsignedIntegerFromMemory( - task_addr + job_id_offset, num_bytes_task_id, LLDB_INVALID_ADDRESS, - error); - if (error.Fail()) - return {}; - return task_id; + llvm::SmallVector> read_results = + process.ReadUnsignedIntegersFromMemory(to_read, num_bytes_task_id); + + // Move the results into the slots not filled by errors from the input. + llvm::ArrayRef> results_ref = read_results; + for (std::optional &maybe_result : final_results) + if (maybe_result) { + maybe_result = results_ref.front(); + results_ref = results_ref.drop_front(); + } + assert(results_ref.empty()); + + return final_results; } bool OperatingSystemSwiftTasks::UpdateThreadList(ThreadList &old_thread_list, @@ -189,37 +226,37 @@ bool OperatingSystemSwiftTasks::UpdateThreadList(ThreadList &old_thread_list, Log *log = GetLog(LLDBLog::OS); LLDB_LOG(log, "OperatingSystemSwiftTasks: Updating thread list"); - for (const ThreadSP &real_thread : core_thread_list.Threads()) { - std::optional task_addr = - FindTaskAddress(m_task_inspector, *real_thread); + ThreadCollection::ThreadIterable locked_core_threads = + core_thread_list.Threads(); + llvm::SmallVector> task_addrs = + FindTaskAddresses(m_task_inspector, locked_core_threads); - // If this is not a thread running a Task, add it to the list as is. - if (!task_addr) { - new_thread_list.AddThread(real_thread); - LLDB_LOGF(log, - "OperatingSystemSwiftTasks: thread %" PRIx64 - " is not executing a Task", - real_thread->GetID()); - continue; - } + assert(m_process != nullptr); + llvm::SmallVector> task_ids = + FindTaskIds(task_addrs, *m_process); - assert(m_process != nullptr); - std::optional task_id = FindTaskId(*task_addr, *m_process); + for (uint64_t idx : llvm::seq(task_addrs.size())) { + auto real_thread = + core_thread_list.GetThreadAtIndex(idx, /*can_update=*/false); + auto task_addr = task_addrs[idx]; + auto task_id = task_ids[idx]; + // If this is not a thread running a Task, add it to the list as is. if (!task_id) { - LLDB_LOG(log, "OperatingSystemSwiftTasks: could not get ID of Task {0:x}", - *task_addr); + LLDB_LOG(log, + "OperatingSystemSwiftTasks: thread {0:x} is not running a task", + real_thread->GetID()); + new_thread_list.AddThread(real_thread); continue; } + LLDB_LOG(log, "OperatingSystemSwiftTasks: TID to task ID: {0:x} -> {1:x}", + real_thread->GetID(), *task_id); ThreadSP swift_thread = FindOrCreateSwiftThread(old_thread_list, *task_id); static_cast(*swift_thread) .UpdateBackingThread(real_thread, *task_addr); new_thread_list.AddThread(swift_thread); - LLDB_LOGF(log, - "OperatingSystemSwiftTasks: mapping thread IDs: %" PRIx64 - " -> %" PRIx64, - real_thread->GetID(), swift_thread->GetID()); } + return true; } diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 9a67015199473..47273f2bcf8c9 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -85,6 +85,7 @@ #include "lldb/Host/Host.h" #include "lldb/Utility/StringExtractorGDBRemote.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringSwitch.h" @@ -2629,6 +2630,108 @@ size_t ProcessGDBRemote::DoReadMemory(addr_t addr, void *buf, size_t size, return 0; } +llvm::SmallVector> +ProcessGDBRemote::ReadMemoryRanges( + llvm::ArrayRef> ranges, + llvm::MutableArrayRef buffer) { + if (!m_gdb_comm.GetMultiMemReadSupported()) + return Process::ReadMemoryRanges(ranges, buffer); + + llvm::Expected response = + SendMultiMemReadPacket(ranges); + if (!response) { + LLDB_LOG_ERROR(GetLog(GDBRLog::Process), response.takeError(), + "MultiMemRead error response: {0}"); + return Process::ReadMemoryRanges(ranges, buffer); + } + + llvm::StringRef response_str = response->GetStringRef(); + const unsigned expected_num_ranges = ranges.size(); + llvm::Expected>> + parsed_response = + ParseMultiMemReadPacket(response_str, buffer, expected_num_ranges); + if (!parsed_response) { + LLDB_LOG_ERROR(GetLog(GDBRLog::Process), parsed_response.takeError(), + "MultiMemRead error parsing response: {0}"); + return Process::ReadMemoryRanges(ranges, buffer); + } + return std::move(*parsed_response); +} + +llvm::Expected +ProcessGDBRemote::SendMultiMemReadPacket( + llvm::ArrayRef> ranges) { + std::string packet_str; + llvm::raw_string_ostream stream(packet_str); + stream << "MultiMemRead:ranges:"; + + auto range_to_stream = [&](auto range) { + // the "-" marker omits the '0x' prefix. + stream << llvm::formatv("{0:x-},{1:x-}", range.base, range.size); + }; + llvm::interleave(ranges, stream, range_to_stream, ","); + stream << ";"; + + StringExtractorGDBRemote response; + GDBRemoteCommunication::PacketResult packet_result = + m_gdb_comm.SendPacketAndWaitForResponse(packet_str.data(), response, + GetInterruptTimeout()); + if (packet_result != GDBRemoteCommunication::PacketResult::Success) + return llvm::createStringError( + llvm::formatv("MultiMemRead failed to send packet: '{0}'", packet_str)); + + if (response.IsErrorResponse()) + return llvm::createStringError( + llvm::formatv("MultiMemRead failed: '{0}'", response.GetStringRef())); + + if (!response.IsNormalResponse()) + return llvm::createStringError(llvm::formatv( + "MultiMemRead unexpected response: '{0}'", response.GetStringRef())); + + return response; +} + +llvm::Expected>> +ProcessGDBRemote::ParseMultiMemReadPacket(llvm::StringRef response_str, + llvm::MutableArrayRef buffer, + unsigned expected_num_ranges) { + // The sizes and the data are separated by a `;`. + auto [sizes_str, memory_data] = response_str.split(';'); + if (sizes_str.size() == response_str.size()) + return llvm::createStringError(llvm::formatv( + "MultiMemRead response missing field separator ';' in: '{0}'", + response_str)); + + llvm::SmallVector> read_results; + + // Sizes are separated by a `,`. + for (llvm::StringRef size_str : llvm::split(sizes_str, ',')) { + uint64_t read_size; + if (size_str.getAsInteger(16, read_size)) + return llvm::createStringError(llvm::formatv( + "MultiMemRead response has invalid size string: {0}", size_str)); + + if (memory_data.size() < read_size) + return llvm::createStringError( + llvm::formatv("MultiMemRead response did not have enough data, " + "requested sizes: {0}", + sizes_str)); + + llvm::StringRef region_to_read = memory_data.take_front(read_size); + memory_data = memory_data.drop_front(read_size); + + assert(buffer.size() >= read_size); + llvm::MutableArrayRef region_to_write = + buffer.take_front(read_size); + buffer = buffer.drop_front(read_size); + + memcpy(region_to_write.data(), region_to_read.data(), read_size); + read_results.push_back(region_to_write); + } + + return read_results; +} + bool ProcessGDBRemote::SupportsMemoryTagging() { return m_gdb_comm.GetMemoryTaggingSupported(); } diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index 881a6b1376b19..b3496b66d8de2 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -138,6 +138,22 @@ class ProcessGDBRemote : public Process, size_t DoReadMemory(lldb::addr_t addr, void *buf, size_t size, Status &error) override; + /// Override of ReadMemoryRanges that uses MultiMemRead to optimize this + /// operation. + llvm::SmallVector> + ReadMemoryRanges(llvm::ArrayRef> ranges, + llvm::MutableArrayRef buf) override; + +private: + llvm::Expected + SendMultiMemReadPacket(llvm::ArrayRef> ranges); + + llvm::Expected>> + ParseMultiMemReadPacket(llvm::StringRef response_str, + llvm::MutableArrayRef buffer, + unsigned expected_num_ranges); + +public: Status WriteObjectFile(std::vector entries) override; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 36a572df840e5..1a1e2a7fbf287 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -2119,6 +2119,50 @@ size_t Process::ReadMemory(addr_t addr, void *buf, size_t size, Status &error) { } } +llvm::SmallVector> +Process::ReadMemoryRanges(llvm::ArrayRef> ranges, + llvm::MutableArrayRef buffer) { + uint64_t total_ranges_len = 0; + for (auto range : ranges) + total_ranges_len += range.size; + // If the buffer is not large enough, this is a programmer error. + // In production builds, gracefully fail by returning a length of 0 for all + // ranges. + assert(buffer.size() >= total_ranges_len && "provided buffer is too short"); + if (buffer.size() < total_ranges_len) { + llvm::MutableArrayRef empty; + return {ranges.size(), empty}; + } + + llvm::SmallVector> results; + + // While `buffer` has space, take the next requested range and read + // memory into a `buffer` piece, then slice it to remove the used memory. + for (auto [addr, range_len] : ranges) { + Status status; + size_t num_bytes_read = + ReadMemoryFromInferior(addr, buffer.data(), range_len, status); + // FIXME: ReadMemoryFromInferior promises to return 0 in case of errors, but + // it doesn't; it never checks for errors. + if (status.Fail()) + num_bytes_read = 0; + + assert(num_bytes_read <= range_len && "read more than requested bytes"); + if (num_bytes_read > range_len) { + // In production builds, gracefully fail by returning length zero for this + // range. + results.emplace_back(); + continue; + } + + results.push_back(buffer.take_front(num_bytes_read)); + // Slice buffer to remove the used memory. + buffer = buffer.drop_front(num_bytes_read); + } + + return results; +} + void Process::DoFindInMemory(lldb::addr_t start_addr, lldb::addr_t end_addr, const uint8_t *buf, size_t size, AddressRanges &matches, size_t alignment, @@ -2350,6 +2394,45 @@ uint64_t Process::ReadUnsignedIntegerFromMemory(lldb::addr_t vm_addr, return fail_value; } +llvm::SmallVector> +Process::ReadUnsignedIntegersFromMemory(llvm::ArrayRef addresses, + unsigned integer_byte_size) { + if (addresses.empty()) + return {}; + // Like ReadUnsignedIntegerFromMemory, this only supports a handful + // of widths. + if (!llvm::is_contained({1u, 2u, 4u, 8u}, integer_byte_size)) + return llvm::SmallVector>(addresses.size(), + std::nullopt); + + llvm::SmallVector> ranges{ + llvm::map_range(addresses, [=](addr_t ptr) { + return Range(ptr, integer_byte_size); + })}; + + std::vector buffer(integer_byte_size * addresses.size(), 0); + llvm::SmallVector> memory = + ReadMemoryRanges(ranges, buffer); + + llvm::SmallVector> result; + result.reserve(addresses.size()); + const uint32_t addr_size = GetAddressByteSize(); + const ByteOrder byte_order = GetByteOrder(); + + for (llvm::MutableArrayRef range : memory) { + if (range.size() != integer_byte_size) { + result.push_back(std::nullopt); + continue; + } + + DataExtractor data(range.data(), integer_byte_size, byte_order, addr_size); + offset_t offset = 0; + result.push_back(data.GetMaxU64(&offset, integer_byte_size)); + assert(offset == integer_byte_size); + } + return result; +} + int64_t Process::ReadSignedIntegerFromMemory(lldb::addr_t vm_addr, size_t integer_byte_size, int64_t fail_value, @@ -2369,6 +2452,12 @@ addr_t Process::ReadPointerFromMemory(lldb::addr_t vm_addr, Status &error) { return LLDB_INVALID_ADDRESS; } +llvm::SmallVector> +Process::ReadPointersFromMemory(llvm::ArrayRef ptr_locs) { + const size_t ptr_size = GetAddressByteSize(); + return ReadUnsignedIntegersFromMemory(ptr_locs, ptr_size); +} + bool Process::WritePointerToMemory(lldb::addr_t vm_addr, lldb::addr_t ptr_value, Status &error) { Scalar scalar; diff --git a/lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py b/lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py index c9e801422b46c..a07591f4cf458 100644 --- a/lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py +++ b/lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py @@ -45,3 +45,30 @@ def test_NSError_p(self): ], ) self.runCmd("process continue") + + @skipIfOutOfTreeDebugserver + def test_runtime_types_efficient_memreads(self): + # Test that we use an efficient reading of memory when reading + # Objective-C method descriptions. + logfile = os.path.join(self.getBuildDir(), "log.txt") + self.runCmd(f"log enable -f {logfile} gdb-remote packets process") + self.addTearDownHook(lambda: self.runCmd("log disable gdb-remote packets")) + + self.build() + self.target, process, thread, bkpt = lldbutil.run_to_source_breakpoint( + self, "// Break here for NSString tests", lldb.SBFileSpec("main.m", False) + ) + + self.runCmd(f"proc plugin packet send StartTesting", check=False) + self.expect('expression str = [NSString stringWithCString: "new"]') + self.runCmd(f"proc plugin packet send EndTesting", check=False) + + self.assertTrue(os.path.exists(logfile)) + log_text = open(logfile).read() + log_text = log_text.split("StartTesting", 1)[-1].split("EndTesting", 1)[0] + + # This test is only checking that the packet it used at all (and that + # no errors are produced). It doesn't check that the packet is being + # used to solve a problem in an optimal way. + self.assertIn("MultiMemRead:", log_text) + self.assertNotIn("MultiMemRead error", log_text) diff --git a/lldb/test/API/lang/swift/async/stepping/step_over/TestSwiftAsyncStepOver.py b/lldb/test/API/lang/swift/async/stepping/step_over/TestSwiftAsyncStepOver.py index 1cd15380be230..5d0ebcbdcc6de 100644 --- a/lldb/test/API/lang/swift/async/stepping/step_over/TestSwiftAsyncStepOver.py +++ b/lldb/test/API/lang/swift/async/stepping/step_over/TestSwiftAsyncStepOver.py @@ -38,3 +38,32 @@ def test(self): self.assertStopReason(stop_reason, lldb.eStopReasonPlanComplete) self.check_is_in_line(thread, expected_line_num) self.check_x_is_available(thread.frames[0]) + + @skipIfOutOfTreeDebugserver + @swiftTest + @skipIf(oslist=["windows", "linux"]) + def test_efficient_step_over_packets(self): + """Test that MultiMemRead is used while stepping""" + logfile = os.path.join(self.getBuildDir(), "log.txt") + self.runCmd(f"log enable -f {logfile} gdb-remote packets process") + self.addTearDownHook(lambda: self.runCmd("log disable gdb-remote packets")) + + self.build() + + source_file = lldb.SBFileSpec("main.swift") + target, process, thread, bkpt = lldbutil.run_to_source_breakpoint( + self, "BREAK HERE", source_file + ) + bkpt.SetEnabled(False) # avoid hitting multiple locations in async breakpoints + + self.runCmd(f"proc plugin packet send StartTesting", check=False) + thread.StepOver() + self.runCmd(f"proc plugin packet send EndTesting", check=False) + stop_reason = thread.GetStopReason() + self.assertStopReason(stop_reason, lldb.eStopReasonPlanComplete) + + self.assertTrue(os.path.exists(logfile)) + log_text = open(logfile).read() + log_text = log_text.split("StartTesting", 1)[-1].split("EndTesting", 1)[0] + self.assertIn("MultiMemRead:", log_text) + self.assertNotIn("MultiMemRead error", log_text) diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp index 4d7c4d5b4c784..e9739b3e47e65 100644 --- a/lldb/unittests/Target/MemoryTest.cpp +++ b/lldb/unittests/Target/MemoryTest.cpp @@ -17,6 +17,7 @@ #include "lldb/Utility/ArchSpec.h" #include "lldb/Utility/DataBufferHeap.h" #include "gtest/gtest.h" +#include using namespace lldb_private; using namespace lldb_private::repro; @@ -226,3 +227,238 @@ TEST_F(MemoryTest, TesetMemoryCacheRead) { // instead of using an // old cache } + +/// A process class that, when asked to read memory from some address X, returns +/// the least significant byte of X. +class DummyReaderProcess : public Process { +public: + // If true, `DoReadMemory` will not return all requested bytes. + // It's not possible to control exactly how many bytes will be read, because + // Process::ReadMemoryFromInferior tries to fulfill the entire request by + // reading smaller chunks until it gets nothing back. + bool read_less_than_requested = false; + bool read_more_than_requested = false; + + size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, + Status &error) override { + if (read_less_than_requested && size > 0) + size--; + if (read_more_than_requested) + size *= 2; + uint8_t *buffer = static_cast(buf); + for (size_t addr = vm_addr; addr < vm_addr + size; addr++) + buffer[addr - vm_addr] = static_cast(addr); // LSB of addr. + return size; + } + // Boilerplate, nothing interesting below. + DummyReaderProcess(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp) + : Process(target_sp, listener_sp) {} + bool CanDebug(lldb::TargetSP, bool) override { return true; } + Status DoDestroy() override { return {}; } + void RefreshStateAfterStop() override {} + bool DoUpdateThreadList(ThreadList &, ThreadList &) override { return false; } + llvm::StringRef GetPluginName() override { return "Dummy"; } +}; + +TEST_F(MemoryTest, TestReadMemoryRanges) { + ArchSpec arch("x86_64-apple-macosx-"); + + Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch)); + + DebuggerSP debugger_sp = Debugger::CreateInstance(); + ASSERT_TRUE(debugger_sp); + + TargetSP target_sp = CreateTarget(debugger_sp, arch); + ASSERT_TRUE(target_sp); + + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process_sp = + std::make_shared(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + + { + llvm::SmallVector buffer(1024, 0); + // Read 8 ranges of 128 bytes with arbitrary base addresses. + llvm::SmallVector> ranges = { + {0x12345, 128}, {0x11112222, 128}, {0x77777777, 128}, + {0xffaabbccdd, 128}, {0x0, 128}, {0x4242424242, 128}, + {0x17171717, 128}, {0x99999, 128}}; + + llvm::SmallVector> read_results = + process_sp->ReadMemoryRanges(ranges, buffer); + + for (auto [range, memory] : llvm::zip(ranges, read_results)) { + ASSERT_EQ(memory.size(), 128u); + addr_t range_base = range.GetRangeBase(); + for (auto [idx, byte] : llvm::enumerate(memory)) + ASSERT_EQ(byte, static_cast(range_base + idx)); + } + } + + auto &dummy_process = static_cast(*process_sp); + dummy_process.read_less_than_requested = true; + { + llvm::SmallVector buffer(1024, 0); + llvm::SmallVector> ranges = { + {0x12345, 128}, {0x11112222, 128}, {0x77777777, 128}}; + llvm::SmallVector> read_results = + dummy_process.ReadMemoryRanges(ranges, buffer); + for (auto [range, memory] : llvm::zip(ranges, read_results)) { + ASSERT_LT(memory.size(), 128u); + addr_t range_base = range.GetRangeBase(); + for (auto [idx, byte] : llvm::enumerate(memory)) + ASSERT_EQ(byte, static_cast(range_base + idx)); + } + } +} + +using MemoryDeathTest = MemoryTest; + +TEST_F(MemoryDeathTest, TestReadMemoryRangesReturnsTooMuch) { + ArchSpec arch("x86_64-apple-macosx-"); + Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch)); + DebuggerSP debugger_sp = Debugger::CreateInstance(); + ASSERT_TRUE(debugger_sp); + TargetSP target_sp = CreateTarget(debugger_sp, arch); + ASSERT_TRUE(target_sp); + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process_sp = + std::make_shared(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + + auto &dummy_process = static_cast(*process_sp); + dummy_process.read_more_than_requested = true; + llvm::SmallVector buffer(1024, 0); + llvm::SmallVector> ranges = {{0x12345, 128}}; + + llvm::SmallVector> read_results; + ASSERT_DEBUG_DEATH( + { read_results = process_sp->ReadMemoryRanges(ranges, buffer); }, + "read more than requested bytes"); +#ifdef NDEBUG + // With asserts off, the read should return empty ranges. + ASSERT_EQ(read_results.size(), 1u); + ASSERT_TRUE(read_results[0].empty()); +#endif +} + +TEST_F(MemoryDeathTest, TestReadMemoryRangesWithShortBuffer) { + ArchSpec arch("x86_64-apple-macosx-"); + Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch)); + DebuggerSP debugger_sp = Debugger::CreateInstance(); + ASSERT_TRUE(debugger_sp); + TargetSP target_sp = CreateTarget(debugger_sp, arch); + ASSERT_TRUE(target_sp); + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process_sp = + std::make_shared(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + + llvm::SmallVector short_buffer(10, 0); + llvm::SmallVector> ranges = {{0x12345, 128}, + {0x11, 128}}; + llvm::SmallVector> read_results; + ASSERT_DEBUG_DEATH( + { read_results = process_sp->ReadMemoryRanges(ranges, short_buffer); }, + "provided buffer is too short"); +#ifdef NDEBUG + // With asserts off, the read should return empty ranges. + ASSERT_EQ(read_results.size(), ranges.size()); + for (llvm::MutableArrayRef result : read_results) + ASSERT_TRUE(result.empty()); +#endif +} +TEST_F(MemoryTest, TestReadPointersFromMemory) { + ArchSpec arch("x86_64-apple-macosx-"); + Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch)); + DebuggerSP debugger_sp = Debugger::CreateInstance(); + ASSERT_TRUE(debugger_sp); + TargetSP target_sp = CreateTarget(debugger_sp, arch); + ASSERT_TRUE(target_sp); + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process = + std::make_shared(target_sp, listener_sp); + ASSERT_TRUE(process); + + // Read pointers at arbitrary addresses. + llvm::SmallVector ptr_locs = {0x0, 0x100, 0x2000, 0x123400}; + // Because of how DummyReaderProcess works, each byte of a memory read result + // is its address modulo 256: + constexpr addr_t expected_result = 0x0706050403020100; + + llvm::SmallVector> read_results = + process->ReadPointersFromMemory(ptr_locs); + + for (std::optional maybe_ptr : read_results) { + ASSERT_TRUE(maybe_ptr.has_value()); + EXPECT_EQ(*maybe_ptr, expected_result); + } +} + +TEST_F(MemoryTest, TestReadUnsignedIntegersFromMemory) { + ArchSpec arch("x86_64-apple-macosx-"); + + Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch)); + DebuggerSP debugger_sp = Debugger::CreateInstance(); + ASSERT_TRUE(debugger_sp); + TargetSP target_sp = CreateTarget(debugger_sp, arch); + ASSERT_TRUE(target_sp); + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process = + std::make_shared(target_sp, listener_sp); + ASSERT_TRUE(process); + + { // Test reads of size 1 + llvm::SmallVector locs = {0x0, 0x101, 0x2002, 0x123403}; + llvm::SmallVector> read_results = + process->ReadUnsignedIntegersFromMemory(locs, /*byte_size=*/1); + + for (auto [maybe_int, loc] : llvm::zip(read_results, locs)) { + ASSERT_TRUE(maybe_int.has_value()); + EXPECT_EQ(*maybe_int, static_cast(loc)); + } + } + + { // Test reads of size 2 + llvm::SmallVector locs = {0x0, 0x101, 0x2002, 0x123403}; + llvm::SmallVector> read_results = + process->ReadUnsignedIntegersFromMemory(locs, /*byte_size=*/2); + + for (auto [maybe_int, loc] : llvm::zip(read_results, locs)) { + ASSERT_TRUE(maybe_int.has_value()); + uint64_t lsb = static_cast(loc); + uint64_t expected_result = ((lsb + 1) << 8) | lsb; + EXPECT_EQ(*maybe_int, expected_result); + } + } + + { // Test reads of size 4 + llvm::SmallVector locs = {0x0, 0x101, 0x2002, 0x123403}; + llvm::SmallVector> read_results = + process->ReadUnsignedIntegersFromMemory(locs, /*byte_size=*/4); + + for (auto [maybe_int, loc] : llvm::zip(read_results, locs)) { + ASSERT_TRUE(maybe_int.has_value()); + uint64_t lsb = static_cast(loc); + uint64_t expected_result = + ((lsb + 3) << 24) | ((lsb + 2) << 16) | ((lsb + 1) << 8) | lsb; + EXPECT_EQ(*maybe_int, expected_result); + } + } + + { // Test reads of size 8 + llvm::SmallVector locs = {0x0, 0x101, 0x2002, 0x123403}; + llvm::SmallVector> read_results = + process->ReadUnsignedIntegersFromMemory(locs, /*byte_size=*/8); + + for (auto [maybe_int, loc] : llvm::zip(read_results, locs)) { + ASSERT_TRUE(maybe_int.has_value()); + uint64_t lsb = static_cast(loc); + uint64_t expected_result = ((lsb + 7) << 56) | ((lsb + 6) << 48) | + ((lsb + 5) << 40) | ((lsb + 4) << 32) | + ((lsb + 3) << 24) | ((lsb + 2) << 16) | + ((lsb + 1) << 8) | lsb; + EXPECT_EQ(*maybe_int, expected_result); + } + } +}