Author: Alex Langford Date: 2026-02-05T11:49:17-08:00 New Revision: 9ebdeb2e8cbe8285c8383d8079663b6c73accac8
URL: https://github.com/llvm/llvm-project/commit/9ebdeb2e8cbe8285c8383d8079663b6c73accac8 DIFF: https://github.com/llvm/llvm-project/commit/9ebdeb2e8cbe8285c8383d8079663b6c73accac8.diff LOG: [lldb] Return Expected<ModuleSP> from Process::ReadModuleFromMemory (#179583) I noticed that Module::GetMemoryObjectFile populates a Status object upon error but it's effectively dropped on the floor. Instead, the clients can report the error as desired. At the moment, all clients are either (1) consuming the error because it's only trying to find a module, or (2) log the error and bail out early. I tried to preserve existing behavior as faithfully as possible. Added: Modified: lldb/include/lldb/Target/Process.h lldb/source/API/SBModule.cpp lldb/source/Core/DynamicLoader.cpp lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp lldb/source/Target/Process.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 8614347d1f34a..73104bcb26a99 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -2016,9 +2016,20 @@ class Process : public std::enable_shared_from_this<Process>, /// the instruction has completed executing. bool GetWatchpointReportedAfter(); - lldb::ModuleSP ReadModuleFromMemory(const FileSpec &file_spec, - lldb::addr_t header_addr, - size_t size_to_read = 512); + /// Creates and populates a module using an in-memory object file. + /// + /// \param[in] file_spec + /// The name or path to the module file. May be empty. + /// + /// \param[in] header_addr + /// The address pointing to the beginning of the object file's header. + /// + /// \param[in] size_to_read + /// The number of bytes to read from memory. This should be large enough to + /// identify the object file format. Defaults to 512. + llvm::Expected<lldb::ModuleSP> + ReadModuleFromMemory(const FileSpec &file_spec, lldb::addr_t header_addr, + size_t size_to_read = 512); /// Attempt to get the attributes for a region of memory in the process. /// diff --git a/lldb/source/API/SBModule.cpp b/lldb/source/API/SBModule.cpp index 32067ac1c650f..c48d1abd88c56 100644 --- a/lldb/source/API/SBModule.cpp +++ b/lldb/source/API/SBModule.cpp @@ -52,7 +52,14 @@ SBModule::SBModule(lldb::SBProcess &process, lldb::addr_t header_addr) { ProcessSP process_sp(process.GetSP()); if (process_sp) { - m_opaque_sp = process_sp->ReadModuleFromMemory(FileSpec(), header_addr); + llvm::Expected<ModuleSP> module_sp_or_err = + process_sp->ReadModuleFromMemory(FileSpec(), header_addr); + if (auto err = module_sp_or_err.takeError()) { + llvm::consumeError(std::move(err)); + return; + } + + m_opaque_sp = *module_sp_or_err; if (m_opaque_sp) { Target &target = process_sp->GetTarget(); bool changed = false; diff --git a/lldb/source/Core/DynamicLoader.cpp b/lldb/source/Core/DynamicLoader.cpp index 563a81fb8239b..8426d35cf4bff 100644 --- a/lldb/source/Core/DynamicLoader.cpp +++ b/lldb/source/Core/DynamicLoader.cpp @@ -180,8 +180,15 @@ ModuleSP DynamicLoader::LoadModuleAtAddress(const FileSpec &file, // We have a core file, try to load the image from memory if we didn't find // the module. if (!module_sp && !m_process->IsLiveDebugSession()) { - module_sp = m_process->ReadModuleFromMemory(file, base_addr); - m_process->GetTarget().GetImages().AppendIfNeeded(module_sp, false); + llvm::Expected<ModuleSP> memory_module_sp_or_err = + m_process->ReadModuleFromMemory(file, base_addr); + if (auto err = memory_module_sp_or_err.takeError()) + LLDB_LOG_ERROR(GetLog(LLDBLog::DynamicLoader), std::move(err), + "Failed to read module from memory: {0}"); + else { + module_sp = *memory_module_sp_or_err; + m_process->GetTarget().GetImages().AppendIfNeeded(module_sp, false); + } } if (module_sp) UpdateLoadedSections(module_sp, link_map_addr, base_addr, @@ -196,7 +203,14 @@ static ModuleSP ReadUnnamedMemoryModule(Process *process, addr_t addr, snprintf(namebuf, sizeof(namebuf), "memory-image-0x%" PRIx64, addr); name = namebuf; } - return process->ReadModuleFromMemory(FileSpec(name), addr); + llvm::Expected<ModuleSP> module_sp_or_err = + process->ReadModuleFromMemory(FileSpec(name), addr); + if (auto err = module_sp_or_err.takeError()) { + LLDB_LOG_ERROR(GetLog(LLDBLog::DynamicLoader), std::move(err), + "Failed to read module from memory: {0}"); + return {}; + } + return *module_sp_or_err; } ModuleSP DynamicLoader::LoadBinaryWithUUIDAndAddress( diff --git a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp index 2d0a4f67499ee..142ebb7d1ca6a 100644 --- a/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp +++ b/lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp @@ -465,8 +465,15 @@ DynamicLoaderDarwinKernel::CheckForKernelImageAtAddress(lldb::addr_t addr, if (header.filetype == llvm::MachO::MH_EXECUTE && (header.flags & llvm::MachO::MH_DYLDLINK) == 0) { // Create a full module to get the UUID - ModuleSP memory_module_sp = + llvm::Expected<ModuleSP> memory_module_sp_or_err = process->ReadModuleFromMemory(FileSpec("temp_mach_kernel"), addr); + if (auto err = memory_module_sp_or_err.takeError()) { + LLDB_LOG_ERROR(log, std::move(err), + "DynamicLoaderDarwinKernel::CheckForKernelImageAtAddress: " + "Failed to read module in memory -- {0}"); + return UUID(); + } + ModuleSP memory_module_sp = *memory_module_sp_or_err; if (!memory_module_sp.get()) return UUID(); @@ -663,9 +670,16 @@ bool DynamicLoaderDarwinKernel::KextImageInfo::ReadMemoryModule( size_to_read = sizeof(llvm::MachO::mach_header_64) + mh.sizeofcmds; } - ModuleSP memory_module_sp = + llvm::Expected<ModuleSP> memory_module_sp_or_err = process->ReadModuleFromMemory(file_spec, m_load_address, size_to_read); + if (auto err = memory_module_sp_or_err.takeError()) { + LLDB_LOG_ERROR(log, std::move(err), + "KextImageInfo::ReadMemoryModule failed to read module from " + "memory: {0}"); + return false; + } + ModuleSP memory_module_sp = *memory_module_sp_or_err; if (memory_module_sp.get() == nullptr) return false; diff --git a/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp index a23ba3ad5c545..a0b3a4d85c4e4 100644 --- a/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp +++ b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp @@ -200,9 +200,17 @@ lldb_private::UUID DynamicLoaderFreeBSDKernel::CheckForKernelImageAtAddress( if (header.e_type != llvm::ELF::ET_EXEC) return UUID(); - ModuleSP memory_module_sp = + llvm::Expected<ModuleSP> memory_module_sp_or_err = process->ReadModuleFromMemory(FileSpec("temp_freebsd_kernel"), addr); + if (auto err = memory_module_sp_or_err.takeError()) { + LLDB_LOG_ERROR(log, std::move(err), + "DynamicLoaderFreeBSDKernel::CheckForKernelImageAtAddress: " + "Failed to read module in memory -- {0}"); + *read_error = true; + return UUID(); + } + ModuleSP memory_module_sp = *memory_module_sp_or_err; if (!memory_module_sp.get()) { *read_error = true; return UUID(); @@ -291,8 +299,15 @@ bool DynamicLoaderFreeBSDKernel::KModImageInfo::ReadMemoryModule( } } - memory_module_sp = + llvm::Expected<ModuleSP> memory_module_sp_or_err = process->ReadModuleFromMemory(file_spec, m_load_address, size_to_read); + if (auto err = memory_module_sp_or_err.takeError()) { + LLDB_LOG_ERROR(log, std::move(err), + "KextImageInfo::ReadMemoryModule: Failed to read module " + "from memory -- {0}"); + return false; + } + memory_module_sp = *memory_module_sp_or_err; if (!memory_module_sp) return false; diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp index e7128ca875b94..a3b0001466bbb 100644 --- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp +++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp @@ -147,9 +147,16 @@ ModuleSP DynamicLoaderDarwin::FindTargetModuleForImageInfo( // added to the target, don't let it be called for every one. if (!module_sp) module_sp = target.GetOrCreateModule(module_spec, false /* notify */); - if (!module_sp || module_sp->GetObjectFile() == nullptr) - module_sp = m_process->ReadModuleFromMemory(image_info.file_spec, - image_info.address); + if (!module_sp || module_sp->GetObjectFile() == nullptr) { + llvm::Expected<ModuleSP> module_sp_or_err = m_process->ReadModuleFromMemory( + image_info.file_spec, image_info.address); + if (auto err = module_sp_or_err.takeError()) { + LLDB_LOG_ERROR(GetLog(LLDBLog::DynamicLoader), std::move(err), + "Failed to load module from memory: {0}"); + return {}; + } + module_sp = *module_sp_or_err; + } if (did_create_ptr) *did_create_ptr = (bool)module_sp; @@ -722,19 +729,26 @@ bool DynamicLoaderDarwin::AddModulesUsingPreloadedModules( true /* notify */); if (!commpage_image_module_sp || commpage_image_module_sp->GetObjectFile() == nullptr) { - commpage_image_module_sp = m_process->ReadModuleFromMemory( - image_info.file_spec, image_info.address); - // Always load a memory image right away in the target in case - // we end up trying to read the symbol table from memory... The - // __LINKEDIT will need to be mapped so we can figure out where - // the symbol table bits are... - bool changed = false; - UpdateImageLoadAddress(commpage_image_module_sp.get(), - image_info); - target.GetImages().Append(commpage_image_module_sp); - if (changed) { - image_info.load_stop_id = m_process->GetStopID(); - loaded_module_list.AppendIfNeeded(commpage_image_module_sp); + llvm::Expected<ModuleSP> module_sp_or_err = + m_process->ReadModuleFromMemory(image_info.file_spec, + image_info.address); + if (auto err = module_sp_or_err.takeError()) { + LLDB_LOG_ERROR(log, std::move(err), + "Failed to read module from memory: {0}"); + } else { + // Always load a memory image right away in the target in case + // we end up trying to read the symbol table from memory... + // The __LINKEDIT will need to be mapped so we can figure out + // where the symbol table bits are... + commpage_image_module_sp = *module_sp_or_err; + bool changed = false; + UpdateImageLoadAddress(commpage_image_module_sp.get(), + image_info); + target.GetImages().Append(commpage_image_module_sp); + if (changed) { + image_info.load_stop_id = m_process->GetStopID(); + loaded_module_list.AppendIfNeeded(commpage_image_module_sp); + } } } } diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index 6705ac139f0fb..1d814f93484d8 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -597,16 +597,20 @@ void DynamicLoaderPOSIXDYLD::LoadVDSO() { FileSpec file("[vdso]"); + Log *log = GetLog(LLDBLog::DynamicLoader); MemoryRegionInfo info; Status status = m_process->GetMemoryRegionInfo(m_vdso_base, info); if (status.Fail()) { - Log *log = GetLog(LLDBLog::DynamicLoader); LLDB_LOG(log, "Failed to get vdso region info: {0}", status); return; } - if (ModuleSP module_sp = m_process->ReadModuleFromMemory( - file, m_vdso_base, info.GetRange().GetByteSize())) { + llvm::Expected<ModuleSP> module_sp_or_err = m_process->ReadModuleFromMemory( + file, m_vdso_base, info.GetRange().GetByteSize()); + if (auto err = module_sp_or_err.takeError()) { + LLDB_LOG_ERROR(log, std::move(err), + "Failed to read module from memory: {0}"); + } else if (ModuleSP module_sp = *module_sp_or_err) { UpdateLoadedSections(module_sp, LLDB_INVALID_ADDRESS, m_vdso_base, false); m_process->GetTarget().GetImages().AppendIfNeeded(module_sp); } diff --git a/lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp b/lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp index d019415cb67a6..2aed7c9e00dc4 100644 --- a/lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.cpp @@ -72,7 +72,15 @@ lldb::ModuleSP DynamicLoaderWasmDYLD::LoadModuleAtAddress( file, link_map_addr, base_addr, base_addr_is_offset)) return module_sp; - if (ModuleSP module_sp = m_process->ReadModuleFromMemory(file, base_addr)) { + llvm::Expected<ModuleSP> module_sp_or_err = + m_process->ReadModuleFromMemory(file, base_addr); + if (auto err = module_sp_or_err.takeError()) { + LLDB_LOG_ERROR(GetLog(LLDBLog::DynamicLoader), std::move(err), + "Failed to read module from memory: {0}"); + return nullptr; + } + + if (ModuleSP module_sp = *module_sp_or_err) { UpdateLoadedSections(module_sp, link_map_addr, base_addr, false); m_process->GetTarget().GetImages().AppendIfNeeded(module_sp); return module_sp; diff --git a/lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp b/lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp index b6487d4e8ed4b..b8e33c6568eab 100644 --- a/lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp +++ b/lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp @@ -323,8 +323,16 @@ bool JITLoaderGDB::ReadJITDescriptorImpl(bool all_entries) { char jit_name[64]; snprintf(jit_name, 64, "JIT(0x%" PRIx64 ")", symbolfile_addr); - module_sp = m_process->ReadModuleFromMemory( - FileSpec(jit_name), symbolfile_addr, symbolfile_size); + llvm::Expected<ModuleSP> module_sp_or_err = + m_process->ReadModuleFromMemory(FileSpec(jit_name), symbolfile_addr, + symbolfile_size); + if (auto err = module_sp_or_err.takeError()) + LLDB_LOG_ERROR( + log, std::move(err), + "JITLoaderGDB::{1} failed to read module from memory: {0}", + __FUNCTION__); + else + module_sp = *module_sp_or_err; if (module_sp && module_sp->GetObjectFile()) { // Object formats (like ELF) have no representation for a JIT type. diff --git a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp index 5e4c67af059db..97e84a675baf6 100644 --- a/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp +++ b/lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp @@ -282,9 +282,14 @@ Status ProcessElfCore::DoLoadCore() { break; } } - if (exe_header_addr.has_value()) - exe_module_sp = ReadModuleFromMemory(exe_module_spec.GetFileSpec(), - *exe_header_addr); + if (exe_header_addr) { + if (llvm::Expected<lldb::ModuleSP> module_sp_or_err = + ReadModuleFromMemory(exe_module_spec.GetFileSpec(), + *exe_header_addr)) + exe_module_sp = *module_sp_or_err; + else + llvm::consumeError(module_sp_or_err.takeError()); + } } if (exe_module_sp) GetTarget().SetExecutableModule(exe_module_sp, eLoadDependentsNo); diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 5056a735c8f57..d249b4f4ee25b 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -2619,32 +2619,30 @@ bool Process::GetWatchpointReportedAfter() { return reported_after; } -ModuleSP Process::ReadModuleFromMemory(const FileSpec &file_spec, - lldb::addr_t header_addr, - size_t size_to_read) { - Log *log = GetLog(LLDBLog::Host); - if (log) { - LLDB_LOGF(log, - "Process::ReadModuleFromMemory reading %s binary from memory", - file_spec.GetPath().c_str()); - } - ModuleSP module_sp(new Module(file_spec, ArchSpec())); - if (module_sp) { - Status error; - std::unique_ptr<Progress> progress_up; - // Reading an ObjectFile from a local corefile is very fast, - // only print a progress update if we're reading from a - // live session which might go over gdb remote serial protocol. - if (IsLiveDebugSession()) - progress_up = std::make_unique<Progress>( - "Reading binary from memory", file_spec.GetFilename().GetString()); - - ObjectFile *objfile = module_sp->GetMemoryObjectFile( - shared_from_this(), header_addr, error, size_to_read); - if (objfile) - return module_sp; - } - return ModuleSP(); +llvm::Expected<ModuleSP> +Process::ReadModuleFromMemory(const FileSpec &file_spec, + lldb::addr_t header_addr, size_t size_to_read) { + LLDB_LOGF(GetLog(LLDBLog::Host), + "Process::ReadModuleFromMemory reading %s binary from memory", + file_spec.GetPath().c_str()); + ModuleSP module_sp = std::make_shared<Module>(file_spec, ArchSpec()); + if (!module_sp) + return llvm::createStringError("Failed to allocate Module"); + + Status error; + std::unique_ptr<Progress> progress_up; + // Reading an ObjectFile from a local corefile is very fast, + // only print a progress update if we're reading from a + // live session which might go over gdb remote serial protocol. + if (IsLiveDebugSession()) + progress_up = std::make_unique<Progress>( + "Reading binary from memory", file_spec.GetFilename().GetString()); + + if (ObjectFile *_ = module_sp->GetMemoryObjectFile( + shared_from_this(), header_addr, error, size_to_read)) + return module_sp; + + return error.takeError(); } bool Process::GetLoadAddressPermissions(lldb::addr_t load_addr, _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
