llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Miro Bucko (mbucko) <details> <summary>Changes</summary> Summary: The current implementation of 'Process::FindInMemory()' utilizes a slow ReadMemory() API to search for 1 byte at a time in memory. This new overload takes advantage of the fact that the process core is already loaded into memory, allowing for a direct and more efficient search. Test Plan: llvm-lit ./llvm-project/lldb/test/API/python_api/find_in_memory/TestFindInMemoryCore.py ./llvm-project/lldb/test/API/python_api/find_in_memory/TestFindRangesInMemory.py ./llvm-project/lldb/test/API/python_api/find_in_memory/TestFindInMemory.py --- Patch is 601.72 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/102536.diff 21 Files Affected: - (modified) lldb/include/lldb/Symbol/ObjectFile.h (+7) - (modified) lldb/include/lldb/Target/PostMortemProcess.h (+18) - (modified) lldb/include/lldb/Target/Process.h (+30-2) - (modified) lldb/include/lldb/Target/ProcessTrace.h (+3) - (modified) lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp (+6) - (modified) lldb/source/Plugins/Process/elf-core/ProcessElfCore.h (+3-7) - (modified) lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp (+6) - (modified) lldb/source/Plugins/Process/mach-core/ProcessMachCore.h (+3-7) - (modified) lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp (+11) - (modified) lldb/source/Plugins/Process/minidump/ProcessMinidump.h (+3) - (modified) lldb/source/Symbol/ObjectFile.cpp (+12) - (modified) lldb/source/Target/CMakeLists.txt (+1) - (added) lldb/source/Target/PostMortemProcess.cpp (+80) - (modified) lldb/source/Target/Process.cpp (+3-25) - (modified) lldb/source/Target/ProcessTrace.cpp (+6) - (modified) lldb/test/API/python_api/find_in_memory/TestFindInMemory.py (+27-40) - (added) lldb/test/API/python_api/find_in_memory/TestFindInMemoryCore.py (+295) - (modified) lldb/test/API/python_api/find_in_memory/TestFindRangesInMemory.py (+35-64) - (modified) lldb/test/API/python_api/find_in_memory/address_ranges_helper.py (+53-36) - (added) lldb/test/API/python_api/find_in_memory/linux-x86_64.yaml (+101) - (modified) lldb/test/API/python_api/find_in_memory/main.cpp (+2-2) ``````````diff diff --git a/lldb/include/lldb/Symbol/ObjectFile.h b/lldb/include/lldb/Symbol/ObjectFile.h index d89314d44bf671..fac35167a7c764 100644 --- a/lldb/include/lldb/Symbol/ObjectFile.h +++ b/lldb/include/lldb/Symbol/ObjectFile.h @@ -669,6 +669,13 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>, // transparent decompression of section contents. size_t CopyData(lldb::offset_t offset, size_t length, void *dst) const; + // Returns a pointer to the data at the specified offset. If the offset is + // invalid, this function will return nullptr. The 'available_bytes' argument + // will be set to the number of bytes available at the given offset, which + // will be at most 'length' bytes. + const uint8_t *PeekData(lldb::addr_t offset, size_t length, + size_t &available_bytes) const; + // This function will transparently decompress section data if the section if // compressed. virtual size_t ReadSectionData(Section *section, diff --git a/lldb/include/lldb/Target/PostMortemProcess.h b/lldb/include/lldb/Target/PostMortemProcess.h index 9c9cd7fa599bc3..a589adda93fe91 100644 --- a/lldb/include/lldb/Target/PostMortemProcess.h +++ b/lldb/include/lldb/Target/PostMortemProcess.h @@ -10,6 +10,7 @@ #define LLDB_TARGET_POSTMORTEMPROCESS_H #include "lldb/Target/Process.h" +#include "lldb/Utility/RangeMap.h" namespace lldb_private { @@ -33,6 +34,23 @@ class PostMortemProcess : public Process { FileSpec GetCoreFile() const override { return m_core_file; } protected: + typedef lldb_private::Range<lldb::addr_t, lldb::addr_t> FileRange; + typedef lldb_private::RangeDataVector<lldb::addr_t, lldb::addr_t, FileRange> + VMRangeToFileOffset; + typedef lldb_private::RangeDataVector<lldb::addr_t, lldb::addr_t, uint32_t> + VMRangeToPermissions; + + virtual const uint8_t *PeekMemory(lldb::addr_t low, lldb::addr_t high, + size_t &size) = 0; + + lldb::addr_t FindInMemory(lldb::addr_t low, lldb::addr_t high, + const uint8_t *buf, size_t size) override; + + const uint8_t *DoPeekMemory(lldb::ModuleSP &core_module_sp, + VMRangeToFileOffset &core_aranges, + lldb::addr_t low, lldb::addr_t high, + size_t &available_bytes); + FileSpec m_core_file; }; diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index cf16fbc812aa48..51dba5b36a21c0 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -2703,8 +2703,8 @@ void PruneThreadPlans(); /// /// \return The address where the pattern was found or LLDB_INVALID_ADDRESS if /// not found. - lldb::addr_t FindInMemory(lldb::addr_t low, lldb::addr_t high, - const uint8_t *buf, size_t size); + virtual lldb::addr_t FindInMemory(lldb::addr_t low, lldb::addr_t high, + const uint8_t *buf, size_t size); AddressRanges FindRangesInMemory(const uint8_t *buf, uint64_t size, const AddressRanges &ranges, @@ -2835,6 +2835,34 @@ void PruneThreadPlans(); AddressRanges &matches, size_t alignment, size_t max_matches); + template <typename IT> + lldb::addr_t FindInMemoryGeneric(IT &&iterator, lldb::addr_t low, + lldb::addr_t high, const uint8_t *buf, + size_t size) { + const size_t region_size = high - low; + + if (region_size < size) + return LLDB_INVALID_ADDRESS; + + std::vector<size_t> bad_char_heuristic(256, size); + + for (size_t idx = 0; idx < size - 1; idx++) { + decltype(bad_char_heuristic)::size_type bcu_idx = buf[idx]; + bad_char_heuristic[bcu_idx] = size - idx - 1; + } + for (size_t s = 0; s <= (region_size - size);) { + int64_t j = size - 1; + while (j >= 0 && buf[j] == iterator[s + j]) + j--; + if (j < 0) + return low + s; + else + s += bad_char_heuristic[iterator[s + size - 1]]; + } + + return LLDB_INVALID_ADDRESS; + } + /// DoGetMemoryRegionInfo is called by GetMemoryRegionInfo after it has /// removed non address bits from load_addr. Override this method in /// subclasses of Process. diff --git a/lldb/include/lldb/Target/ProcessTrace.h b/lldb/include/lldb/Target/ProcessTrace.h index 7a025100f68032..0fc687fe6b9c62 100644 --- a/lldb/include/lldb/Target/ProcessTrace.h +++ b/lldb/include/lldb/Target/ProcessTrace.h @@ -72,6 +72,9 @@ class ProcessTrace : public PostMortemProcess { bool DoUpdateThreadList(ThreadList &old_thread_list, ThreadList &new_thread_list) override; + const uint8_t *PeekMemory(lldb::addr_t low, lldb::addr_t high, + size_t &available_bytes) override; + private: static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp, diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp index 30af9345999c41..2c04cc8905f051 100644 --- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -1095,3 +1095,9 @@ bool ProcessElfCore::GetProcessInfo(ProcessInstanceInfo &info) { } return true; } + +const uint8_t *ProcessElfCore::PeekMemory(lldb::addr_t low, lldb::addr_t high, + size_t &available_bytes) { + return DoPeekMemory(m_core_module_sp, m_core_aranges, low, high, + available_bytes); +} diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h index 668a7c48467475..56afe08f6f3bbe 100644 --- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h +++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.h @@ -111,6 +111,9 @@ class ProcessElfCore : public lldb_private::PostMortemProcess { bool SupportsMemoryTagging() override { return !m_core_tag_ranges.IsEmpty(); } + const uint8_t *PeekMemory(lldb::addr_t low, lldb::addr_t high, + size_t &available_bytes) override; + private: struct NT_FILE_Entry { lldb::addr_t start; @@ -123,13 +126,6 @@ class ProcessElfCore : public lldb_private::PostMortemProcess { lldb_private::UUID uuid; }; - // For ProcessElfCore only - typedef lldb_private::Range<lldb::addr_t, lldb::addr_t> FileRange; - typedef lldb_private::RangeDataVector<lldb::addr_t, lldb::addr_t, FileRange> - VMRangeToFileOffset; - typedef lldb_private::RangeDataVector<lldb::addr_t, lldb::addr_t, uint32_t> - VMRangeToPermissions; - lldb::ModuleSP m_core_module_sp; std::string m_dyld_plugin_name; diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp index 1da7696c9a352a..696de37f78e496 100644 --- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp +++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp @@ -776,3 +776,9 @@ addr_t ProcessMachCore::GetImageInfoAddress() { lldb_private::ObjectFile *ProcessMachCore::GetCoreObjectFile() { return m_core_module_sp->GetObjectFile(); } + +const uint8_t *ProcessMachCore::PeekMemory(lldb::addr_t low, lldb::addr_t high, + size_t &available_bytes) { + return DoPeekMemory(m_core_module_sp, m_core_aranges, low, high, + available_bytes); +} diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h index 8996ae116614bf..1ec7eaa2bf1558 100644 --- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h +++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.h @@ -84,6 +84,9 @@ class ProcessMachCore : public lldb_private::PostMortemProcess { DoGetMemoryRegionInfo(lldb::addr_t load_addr, lldb_private::MemoryRegionInfo ®ion_info) override; + const uint8_t *PeekMemory(lldb::addr_t low, lldb::addr_t high, + size_t &available_bytes) override; + private: void CreateMemoryRegions(); @@ -120,13 +123,6 @@ class ProcessMachCore : public lldb_private::PostMortemProcess { return eKernelCorefile; } - // For ProcessMachCore only - typedef lldb_private::Range<lldb::addr_t, lldb::addr_t> FileRange; - typedef lldb_private::RangeDataVector<lldb::addr_t, lldb::addr_t, FileRange> - VMRangeToFileOffset; - typedef lldb_private::RangeDataVector<lldb::addr_t, lldb::addr_t, uint32_t> - VMRangeToPermissions; - VMRangeToFileOffset m_core_aranges; VMRangeToPermissions m_core_range_infos; lldb::ModuleSP m_core_module_sp; diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp index 13599f4a1553fd..ca8f1e46b02731 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp @@ -564,6 +564,17 @@ JITLoaderList &ProcessMinidump::GetJITLoaders() { #define APPEND_OPT(VAR) \ m_option_group.Append(&VAR, LLDB_OPT_SET_ALL, LLDB_OPT_SET_1) +const uint8_t *ProcessMinidump::PeekMemory(lldb::addr_t low, lldb::addr_t high, + size_t &available_bytes) { + llvm::ArrayRef<uint8_t> mem = m_minidump_parser->GetMemory(low, high - low); + if (mem.empty()) { + available_bytes = 0; + return nullptr; + } + available_bytes = mem.size(); + return mem.data(); +} + class CommandObjectProcessMinidumpDump : public CommandObjectParsed { private: OptionGroupOptions m_option_group; diff --git a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h index 3f3123a0a8b5d3..0eabe22d1d860d 100644 --- a/lldb/source/Plugins/Process/minidump/ProcessMinidump.h +++ b/lldb/source/Plugins/Process/minidump/ProcessMinidump.h @@ -106,6 +106,9 @@ class ProcessMinidump : public PostMortemProcess { JITLoaderList &GetJITLoaders() override; + const uint8_t *PeekMemory(lldb::addr_t low, lldb::addr_t high, + size_t &available_bytes) override; + private: lldb::DataBufferSP m_core_data; llvm::ArrayRef<minidump::Thread> m_thread_list; diff --git a/lldb/source/Symbol/ObjectFile.cpp b/lldb/source/Symbol/ObjectFile.cpp index 35317d209de1f9..e55795307a91ec 100644 --- a/lldb/source/Symbol/ObjectFile.cpp +++ b/lldb/source/Symbol/ObjectFile.cpp @@ -483,6 +483,18 @@ size_t ObjectFile::CopyData(lldb::offset_t offset, size_t length, return m_data.CopyData(offset, length, dst); } +const uint8_t *ObjectFile::PeekData(lldb::addr_t offset, size_t length, + size_t &available_bytes) const { + const uint8_t *data = m_data.PeekData(offset, length); + if (data == nullptr) { + available_bytes = 0; + return nullptr; + } + + available_bytes = length; + return data; +} + size_t ObjectFile::ReadSectionData(Section *section, lldb::offset_t section_offset, void *dst, size_t dst_len) { diff --git a/lldb/source/Target/CMakeLists.txt b/lldb/source/Target/CMakeLists.txt index a42c44b761dc56..20a200fda57633 100644 --- a/lldb/source/Target/CMakeLists.txt +++ b/lldb/source/Target/CMakeLists.txt @@ -26,6 +26,7 @@ add_lldb_library(lldbTarget OperatingSystem.cpp PathMappingList.cpp Platform.cpp + PostMortemProcess.cpp Process.cpp ProcessTrace.cpp Queue.cpp diff --git a/lldb/source/Target/PostMortemProcess.cpp b/lldb/source/Target/PostMortemProcess.cpp new file mode 100644 index 00000000000000..fb2b8b8c6f9180 --- /dev/null +++ b/lldb/source/Target/PostMortemProcess.cpp @@ -0,0 +1,80 @@ +//===-- PostMortemProcess.cpp -----------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Target/PostMortemProcess.h" + +#include "lldb/Core/Module.h" +#include "lldb/Utility/LLDBLog.h" +#include "lldb/lldb-forward.h" + +using namespace lldb; +using namespace lldb_private; + +lldb::addr_t PostMortemProcess::FindInMemory(lldb::addr_t low, + lldb::addr_t high, + const uint8_t *buf, size_t size) { + const size_t region_size = high - low; + if (region_size < size) + return LLDB_INVALID_ADDRESS; + + size_t mem_size = 0; + const uint8_t *data = PeekMemory(low, high, mem_size); + if (data == nullptr || mem_size != region_size) { + LLDB_LOG(GetLog(LLDBLog::Process), + "Failed to get contiguous memory region for search. low: 0x{}, " + "high: 0x{}. Failling back to Process::FindInMemory", + low, high); + // In an edge case when the search has to happen across non-contiguous + // memory, we will have to fall back on the Process::FindInMemory. + return Process::FindInMemory(low, high, buf, size); + } + + return Process::FindInMemoryGeneric(data, low, high, buf, size); +} + +const uint8_t *PostMortemProcess::DoPeekMemory( + lldb::ModuleSP &core_module_sp, VMRangeToFileOffset &core_aranges, + lldb::addr_t low, lldb::addr_t high, size_t &available_bytes) { + + ObjectFile *core_objfile = core_module_sp->GetObjectFile(); + + if (core_objfile == nullptr) { + available_bytes = 0; + return nullptr; + } + + const VMRangeToFileOffset::Entry *core_memory_entry = + core_aranges.FindEntryThatContains(low); + if (core_memory_entry == nullptr || core_memory_entry->GetRangeEnd() < low) { + available_bytes = 0; + return nullptr; + } + const lldb::addr_t offset = low - core_memory_entry->GetRangeBase(); + const lldb::addr_t file_start = core_memory_entry->data.GetRangeBase(); + const lldb::addr_t file_end = core_memory_entry->data.GetRangeEnd(); + + if (file_start == file_end) { + available_bytes = 0; + return nullptr; + } + size_t bytes_available = 0; + if (file_end > file_start + offset) + bytes_available = file_end - (file_start + offset); + + size_t bytes_to_read = high - low; + bytes_to_read = std::min(bytes_to_read, bytes_available); + if (bytes_to_read == 0) { + available_bytes = 0; + return nullptr; + } + + const uint8_t *ret = + core_objfile->PeekData(core_memory_entry->data.GetRangeBase() + offset, + bytes_to_read, available_bytes); + return ret; +} diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index e3c4f2ee398cc4..1c7e284bf09dc6 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -2126,9 +2126,8 @@ lldb::addr_t Process::FindInMemory(const uint8_t *buf, uint64_t size, error.SetErrorString("range load address is invalid"); return LLDB_INVALID_ADDRESS; } - const lldb::addr_t end_addr = start_addr + range.GetByteSize(); - AddressRanges matches; + const lldb::addr_t end_addr = start_addr + range.GetByteSize(); DoFindInMemory(start_addr, end_addr, buf, size, matches, alignment, 1); if (matches.empty()) return LLDB_INVALID_ADDRESS; @@ -3362,29 +3361,8 @@ Status Process::Halt(bool clear_thread_plans, bool use_run_lock) { lldb::addr_t Process::FindInMemory(lldb::addr_t low, lldb::addr_t high, const uint8_t *buf, size_t size) { - const size_t region_size = high - low; - - if (region_size < size) - return LLDB_INVALID_ADDRESS; - - std::vector<size_t> bad_char_heuristic(256, size); - ProcessMemoryIterator iterator(*this, low); - - for (size_t idx = 0; idx < size - 1; idx++) { - decltype(bad_char_heuristic)::size_type bcu_idx = buf[idx]; - bad_char_heuristic[bcu_idx] = size - idx - 1; - } - for (size_t s = 0; s <= (region_size - size);) { - int64_t j = size - 1; - while (j >= 0 && buf[j] == iterator[s + j]) - j--; - if (j < 0) - return low + s; - else - s += bad_char_heuristic[iterator[s + size - 1]]; - } - - return LLDB_INVALID_ADDRESS; + return FindInMemoryGeneric(ProcessMemoryIterator(*this, low), low, high, buf, + size); } Status Process::StopForDestroyOrDetach(lldb::EventSP &exit_event_sp) { diff --git a/lldb/source/Target/ProcessTrace.cpp b/lldb/source/Target/ProcessTrace.cpp index 4718a7ca50a7cb..96769079e61b32 100644 --- a/lldb/source/Target/ProcessTrace.cpp +++ b/lldb/source/Target/ProcessTrace.cpp @@ -94,6 +94,12 @@ size_t ProcessTrace::ReadMemory(addr_t addr, void *buf, size_t size, void ProcessTrace::Clear() { m_thread_list.Clear(); } +const uint8_t *ProcessTrace::PeekMemory(lldb::addr_t low, lldb::addr_t high, + size_t &available_bytes) { + available_bytes = 0; + return nullptr; +} + void ProcessTrace::Initialize() { static llvm::once_flag g_once_flag; diff --git a/lldb/test/API/python_api/find_in_memory/TestFindInMemory.py b/lldb/test/API/python_api/find_in_memory/TestFindInMemory.py index 9ab4619b1f8f4f..5661e9f1a4c9d1 100644 --- a/lldb/test/API/python_api/find_in_memory/TestFindInMemory.py +++ b/lldb/test/API/python_api/find_in_memory/TestFindInMemory.py @@ -13,49 +13,49 @@ class FindInMemoryTestCase(TestBase): def setUp(self): TestBase.setUp(self) + live_pi = ProcessInfo() self.build() ( - self.target, - self.process, - self.thread, - self.bp, + live_pi.target, + live_pi.process, + live_pi.thread, + live_pi.bp, ) = lldbutil.run_to_source_breakpoint( self, "break here", lldb.SBFileSpec("main.cpp"), ) - self.assertTrue(self.bp.IsValid()) + live_pi.frame = live_pi.thread.GetFrameAtIndex(0) + self.assertTrue(live_pi.bp.IsValid()) + self.assertTrue(live_pi.process, PROCESS_IS_VALID) + self.assertState(live_pi.process.GetState(), lldb.eStateStopped, PROCESS_STOPPED) + + self.live_pi = live_pi def test_check_stack_pointer(self): """Make sure the 'stack_pointer' variable lives on the stack""" - self.assertTrue(self.process, PROCESS_IS_VALID) - self.assertState(self.process.GetState(), lldb.eStateStopped, PROCESS_STOPPED) - - frame = self.thread.GetSelectedFrame() - ex = frame.EvaluateExpression("&stack_pointer") + ex = self.live_pi.frame.EvaluateExpression("&stack_pointer") variable_region = lldb.SBMemoryRegionInfo() self.assertTrue( - self.process.GetMemoryRegionInfo( + self.live_pi.process.GetMemoryRegionInfo( ex.GetValueAsUnsigned(), variable_region ).Success(), ) stack_region = lldb.SBMemoryRegionInfo() self.assertTrue( - self.process.GetMemoryRegionInfo(frame.GetSP(), stack_region).Success(), + self.live_pi.process.GetMemoryRegionInfo(self.live_pi.frame.GetSP(), stack_region).Success(), ) self.assertEqual(variable_region, stack_region) def test_find_in_memory_ok(self): """Make sure a match exists in the heap memory and the right address ranges are provided""" - self.assertTrue(self.process, PROCESS_IS_VALID) - self.assertState(self.process.GetState(), lldb.eStateStopped, PROCESS_STOPPED) error = lldb.SBError() - addr = self.process.FindInMemory( + addr = self.live_pi.process.FindInMemory( SINGLE_INSTANCE_PATTERN_STACK, - GetStackRange(self), + GetStackRange(self, self.live_pi), 1, error, ) @@ -65,12 +65,10 @@ def test_find_in_memory_ok(self): def test_find_in_memory_double_instance_ok(self): """Make sure a match exists in the heap memory and the right address ranges are provided""" - self.assertTrue(self.process, PROCESS_IS_VALID) - self.assertState(self.process.GetState(), lldb.e... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/102536 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
