Skip to content

Conversation

@barsolo2000
Copy link
Contributor

@barsolo2000 barsolo2000 commented Aug 4, 2025

Instead of returning an error when:

  • it can't obtain section information from a module.
  • there are other issues calculating the size.

when we encounter such an error we log the error and continue with the other modules.
tested with lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py

@github-actions
Copy link

github-actions bot commented Aug 5, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@barsolo2000 barsolo2000 marked this pull request as ready for review August 5, 2025 23:20
@llvmbot llvmbot added the lldb label Aug 5, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 5, 2025

@llvm/pr-subscribers-lldb

Author: None (barsolo2000)

Changes

Instead of returning an error when:

  • it can't obtain section information from a module.
  • there are other issues calculating the size.

when we encounter such an error we log the error and continue with the other modules.
tested with lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py


Full diff: https://github.com/llvm/llvm-project/pull/152009.diff

1 Files Affected:

  • (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp (+31-22)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 25e98882c20c9..c361087730828 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -308,40 +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;
 
+  // Vector to store modules that pass validation.
+  std::vector<std::pair<ModuleSP, uint64_t>> valid_modules;
+
   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_LOGF(log, "Unable to get the size of module %s: %s",
+                        module_name.c_str(), E.message().c_str());
+            }
+          });
+      continue;
     }
+    valid_modules.emplace_back(mod, *maybe_mod_size);
+  }
+
+  size_t module_stream_size = sizeof(llvm::support::ulittle32_t) +
+                              valid_modules.size() * minidump_module_size;
+
+  error = AddDirectory(StreamType::ModuleList, module_stream_size);
+  if (error.Fail())
+    return error;
 
-    uint64_t mod_size = std::move(*maybe_mod_size);
+  // Setting the header with the number of modules.
+  llvm::support::ulittle32_t count =
+      static_cast<llvm::support::ulittle32_t>(valid_modules.size());
+  m_data.AppendData(&count, sizeof(llvm::support::ulittle32_t));
+
+  for (const auto &valid_module : valid_modules) {
+    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<llvm::support::ulittle32_t>(
@@ -381,7 +390,7 @@ Status MinidumpFileBuilder::AddModuleList() {
     llvm::minidump::Module m{};
     m.BaseOfImage = static_cast<llvm::support::ulittle64_t>(
         mod->GetObjectFile()->GetBaseAddress().GetLoadAddress(&target));
-    m.SizeOfImage = static_cast<llvm::support::ulittle32_t>(mod_size);
+    m.SizeOfImage = static_cast<llvm::support::ulittle32_t>(module_size);
     m.Checksum = static_cast<llvm::support::ulittle32_t>(0);
     m.TimeDateStamp =
         static_cast<llvm::support::ulittle32_t>(std::time(nullptr));

@Jlalond Jlalond merged commit 3cb649a into llvm:main Aug 11, 2025
12 checks passed
imyixinw pushed a commit to imyixinw/llvm-project that referenced this pull request Sep 5, 2025
… section cannot be found (llvm#152009)

Summary:
Instead of returning an error when:
- it can't obtain section information from a module.
- there are other issues calculating the size.

when we encounter such an error we log the error and continue with the
other modules.
NOTE: this is a cherry pick
---------

Co-authored-by: Bar Soloveychik <[email protected]>
(cherry picked from commit 3cb649a)

Test Plan:
test:
```
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
```

Rollback Plan:

Reviewers: jalalonde

Reviewed By: jalalonde

Subscribers: #lldb_team

Differential Revision: https://phabricator.intern.facebook.com/D80036283

(cherry picked from commit 5a4b3e97e8dc06a8ca81f305b87abee714883f3f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants