From 91ad83c794eeaed56297aa43a6d36e8c7572e18e Mon Sep 17 00:00:00 2001 From: Bar Soloveychik Date: Mon, 4 Aug 2025 10:13:37 -0700 Subject: [PATCH 1/6] Update Minidump file builder to continue when the Module's section cannot be found --- .../Minidump/MinidumpFileBuilder.cpp | 55 +++++++++++-------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index 25e98882c20c9..aa42ed30a8e09 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -308,38 +308,49 @@ Status MinidumpFileBuilder::AddModuleList() { // the llvm::minidump::Module's structures into helper data size_t size_before = GetCurrentDataEndOffset(); - // This is the size of the main part of the ModuleList stream. - // It consists of a module number and corresponding number of - // structs describing individual modules - size_t module_stream_size = - sizeof(llvm::support::ulittle32_t) + modules_count * minidump_module_size; - - // Adding directory describing this stream. - error = AddDirectory(StreamType::ModuleList, module_stream_size); - if (error.Fail()) - return error; - - m_data.AppendData(&modules_count, sizeof(llvm::support::ulittle32_t)); - // Temporary storage for the helper data (of variable length) // as these cannot be dumped to m_data before dumping entire // array of module structures. DataBufferHeap helper_data; + // Track the count of successfully processed modules and log errors. + uint32_t successful_modules_count = 0; for (size_t i = 0; i < modules_count; ++i) { ModuleSP mod = modules.GetModuleAtIndex(i); std::string module_name = mod->GetSpecificationDescription(); auto maybe_mod_size = getModuleFileSize(target, mod); if (!maybe_mod_size) { llvm::Error mod_size_err = maybe_mod_size.takeError(); - llvm::handleAllErrors(std::move(mod_size_err), - [&](const llvm::ErrorInfoBase &E) { - error = Status::FromErrorStringWithFormat( - "Unable to get the size of module %s: %s.", - module_name.c_str(), E.message().c_str()); - }); - return error; + Log *log = GetLog(LLDBLog::Object); + llvm::handleAllErrors( + std::move(mod_size_err), [&](const llvm::ErrorInfoBase &E) { + if (log) { + LLDB_LOG_ERROR(log, llvm::ErrorSuccess(), + "Unable to get the size of module {}: {}", + module_name, E.message()); + } + }); + } else { + ++successful_modules_count; } + } + + size_t module_stream_size = sizeof(llvm::support::ulittle32_t) + + successful_modules_count * minidump_module_size; + + error = AddDirectory(StreamType::ModuleList, module_stream_size); + if (error.Fail()) + return error; + + m_data.AppendData(&successful_modules_count, + sizeof(llvm::support::ulittle32_t)); + + for (size_t i = 0; i < modules_count; ++i) { + ModuleSP mod = modules.GetModuleAtIndex(i); + std::string module_name = mod->GetSpecificationDescription(); + auto maybe_mod_size = getModuleFileSize(target, mod); + if (!maybe_mod_size) + continue; uint64_t mod_size = std::move(*maybe_mod_size); @@ -367,8 +378,6 @@ Status MinidumpFileBuilder::AddModuleList() { ld.DataSize = static_cast(0u); ld.RVA = static_cast(0u); - // Setting up LocationDescriptor for uuid string. The global offset into - // minidump file is calculated. LocationDescriptor ld_cv; ld_cv.DataSize = static_cast( sizeof(llvm::support::ulittle32_t) + uuid.size()); @@ -1055,7 +1064,7 @@ MinidumpFileBuilder::AddMemoryList_32(std::vector &ranges, const offset_t offset_for_data = GetCurrentDataEndOffset(); const addr_t addr = core_range.range.start(); const addr_t size = core_range.range.size(); - const addr_t end = core_range.range.end(); + const addr_t end = core_range.range.end(); LLDB_LOGF(log, "AddMemoryList %zu/%zu reading memory for region " From fe6e54cbd4f678bb7701b3461bb4fe98174784aa Mon Sep 17 00:00:00 2001 From: Bar Soloveychik Date: Mon, 4 Aug 2025 10:20:38 -0700 Subject: [PATCH 2/6] format fixes --- .../Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index aa42ed30a8e09..db23278722d4f 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -378,6 +378,8 @@ Status MinidumpFileBuilder::AddModuleList() { ld.DataSize = static_cast(0u); ld.RVA = static_cast(0u); + // Setting up LocationDescriptor for uuid string. The global offset into + // minidump file is calculated. LocationDescriptor ld_cv; ld_cv.DataSize = static_cast( sizeof(llvm::support::ulittle32_t) + uuid.size()); @@ -1064,7 +1066,7 @@ MinidumpFileBuilder::AddMemoryList_32(std::vector &ranges, const offset_t offset_for_data = GetCurrentDataEndOffset(); const addr_t addr = core_range.range.start(); const addr_t size = core_range.range.size(); - const addr_t end = core_range.range.end(); + const addr_t end = core_range.range.end(); LLDB_LOGF(log, "AddMemoryList %zu/%zu reading memory for region " From b619ba953ce10d5bf3dbda8bdce64dda6755ef32 Mon Sep 17 00:00:00 2001 From: Bar Soloveychik Date: Mon, 4 Aug 2025 10:39:51 -0700 Subject: [PATCH 3/6] fixed logging format --- .../Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index db23278722d4f..07548d349f8f0 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -325,9 +325,8 @@ Status MinidumpFileBuilder::AddModuleList() { llvm::handleAllErrors( std::move(mod_size_err), [&](const llvm::ErrorInfoBase &E) { if (log) { - LLDB_LOG_ERROR(log, llvm::ErrorSuccess(), - "Unable to get the size of module {}: {}", - module_name, E.message()); + LLDB_LOGF(log, "Unable to get the size of module %s: %s", + module_name.c_str(), E.message().c_str()); } }); } else { From ff15ae8b6bd02563cfa09c2534f69cc946fabd94 Mon Sep 17 00:00:00 2001 From: Bar Soloveychik Date: Mon, 4 Aug 2025 11:31:29 -0700 Subject: [PATCH 4/6] fixed redundancy --- .../Minidump/MinidumpFileBuilder.cpp | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index 07548d349f8f0..b116d15e9467a 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -313,6 +313,16 @@ Status MinidumpFileBuilder::AddModuleList() { // array of module structures. DataBufferHeap helper_data; + // Struct to hold module information. + struct ValidModuleInfo { + ModuleSP module; + std::string module_name; + uint64_t module_size; + }; + + // Vector to store modules that pass validation. + std::vector valid_modules; + // Track the count of successfully processed modules and log errors. uint32_t successful_modules_count = 0; for (size_t i = 0; i < modules_count; ++i) { @@ -329,9 +339,10 @@ Status MinidumpFileBuilder::AddModuleList() { module_name.c_str(), E.message().c_str()); } }); - } else { - ++successful_modules_count; + continue; } + ++successful_modules_count; + valid_modules.push_back({mod, module_name, *maybe_mod_size}); } size_t module_stream_size = sizeof(llvm::support::ulittle32_t) + @@ -344,14 +355,10 @@ Status MinidumpFileBuilder::AddModuleList() { m_data.AppendData(&successful_modules_count, sizeof(llvm::support::ulittle32_t)); - for (size_t i = 0; i < modules_count; ++i) { - ModuleSP mod = modules.GetModuleAtIndex(i); - std::string module_name = mod->GetSpecificationDescription(); - auto maybe_mod_size = getModuleFileSize(target, mod); - if (!maybe_mod_size) - continue; - - uint64_t mod_size = std::move(*maybe_mod_size); + for (const auto &valid_module : valid_modules) { + ModuleSP mod = valid_module.module; + std::string module_name = valid_module.module_name; + uint64_t mod_size = valid_module.module_size; llvm::support::ulittle32_t signature = static_cast( From 3b7c6718248ed9a0070f92630633b4b5176b656b Mon Sep 17 00:00:00 2001 From: Bar Soloveychik Date: Tue, 5 Aug 2025 09:51:19 -0700 Subject: [PATCH 5/6] created a vector to hold the ModuleSP and size --- .../Minidump/MinidumpFileBuilder.cpp | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index b116d15e9467a..8664079637918 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -313,18 +313,9 @@ Status MinidumpFileBuilder::AddModuleList() { // array of module structures. DataBufferHeap helper_data; - // Struct to hold module information. - struct ValidModuleInfo { - ModuleSP module; - std::string module_name; - uint64_t module_size; - }; - // Vector to store modules that pass validation. - std::vector valid_modules; + std::vector> valid_modules; - // Track the count of successfully processed modules and log errors. - uint32_t successful_modules_count = 0; for (size_t i = 0; i < modules_count; ++i) { ModuleSP mod = modules.GetModuleAtIndex(i); std::string module_name = mod->GetSpecificationDescription(); @@ -341,24 +332,25 @@ Status MinidumpFileBuilder::AddModuleList() { }); continue; } - ++successful_modules_count; - valid_modules.push_back({mod, module_name, *maybe_mod_size}); + valid_modules.emplace_back(mod, *maybe_mod_size); } size_t module_stream_size = sizeof(llvm::support::ulittle32_t) + - successful_modules_count * minidump_module_size; + valid_modules.size() * minidump_module_size; error = AddDirectory(StreamType::ModuleList, module_stream_size); if (error.Fail()) return error; - m_data.AppendData(&successful_modules_count, - sizeof(llvm::support::ulittle32_t)); + // Setting the header with the number of modules. + llvm::support::ulittle32_t count = + static_cast(valid_modules.size()); + m_data.AppendData(&count, sizeof(llvm::support::ulittle32_t)); for (const auto &valid_module : valid_modules) { - ModuleSP mod = valid_module.module; - std::string module_name = valid_module.module_name; - uint64_t mod_size = valid_module.module_size; + ModuleSP mod = valid_module.first; + uint64_t module_size = valid_module.second; + std::string module_name = mod->GetSpecificationDescription(); llvm::support::ulittle32_t signature = static_cast( @@ -398,7 +390,8 @@ Status MinidumpFileBuilder::AddModuleList() { llvm::minidump::Module m{}; m.BaseOfImage = static_cast( mod->GetObjectFile()->GetBaseAddress().GetLoadAddress(&target)); - m.SizeOfImage = static_cast(mod_size); + m.SizeOfImage = + static_cast(module_size); m.Checksum = static_cast(0); m.TimeDateStamp = static_cast(std::time(nullptr)); From f9d9a105f1e6dc70e10521c0a9dd9c2a9a2b8eed Mon Sep 17 00:00:00 2001 From: Bar Soloveychik Date: Tue, 5 Aug 2025 11:19:00 -0700 Subject: [PATCH 6/6] fixed format --- .../source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index 8664079637918..c361087730828 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -390,8 +390,7 @@ Status MinidumpFileBuilder::AddModuleList() { llvm::minidump::Module m{}; m.BaseOfImage = static_cast( mod->GetObjectFile()->GetBaseAddress().GetLoadAddress(&target)); - m.SizeOfImage = - static_cast(module_size); + m.SizeOfImage = static_cast(module_size); m.Checksum = static_cast(0); m.TimeDateStamp = static_cast(std::time(nullptr));