https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/146777
>From d17473cc32acb31935759012ca87342d750d68f7 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 2 Jul 2025 09:18:59 -0700
Subject: [PATCH 1/8] Fix logs, prevent accidentally printing a partial read
when that's not true, and fix where we write the base RVA 8 bytes earlier
than it starts
---
.../Minidump/MinidumpFileBuilder.cpp | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 806f256d9da48..34a71f41f3b84 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -977,6 +977,7 @@ Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
const lldb::addr_t addr = range.range.start();
const lldb::addr_t size = range.range.size();
Log *log = GetLog(LLDBLog::Object);
+ uint64_t total_bytes_read = 0;
Status addDataError;
Process::ReadMemoryChunkCallback callback =
[&](Status &error, lldb::addr_t current_addr, const void *buf,
@@ -984,7 +985,7 @@ Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
if (error.Fail() || bytes_read == 0) {
LLDB_LOGF(log,
"Failed to read memory region at: 0x%" PRIx64
- ". Bytes read: %" PRIx64 ", error: %s",
+ ". Bytes read: 0x%" PRIx64 ", error: %s",
current_addr, bytes_read, error.AsCString());
// If we failed in a memory read, we would normally want to skip
@@ -997,6 +998,10 @@ Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
return lldb_private::IterationAction::Stop;
}
+ if (current_addr != addr + total_bytes_read) {
+ LLDB_LOGF(log, "Current addr is at expected address, 0x%" PRIx64 ",
expected at 0x%" PRIx64, current_addr, addr + total_bytes_read);
+ }
+
// Write to the minidump file with the chunk potentially flushing to
// disk.
// This error will be captured by the outer scope and is considered fatal.
@@ -1006,13 +1011,14 @@ Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
if (addDataError.Fail())
return lldb_private::IterationAction::Stop;
+ total_bytes_read += bytes_read;
// If we have a partial read, report it, but only if the partial read
// didn't finish reading the entire region.
if (bytes_read != data_buffer.GetByteSize() &&
- current_addr + bytes_read != size) {
+ total_bytes_read != size) {
LLDB_LOGF(log,
- "Memory region at: %" PRIx64 " partiall read 0x%" PRIx64
- " bytes out of %" PRIx64 " bytes.",
+ "Memory region at: 0x%" PRIx64 " partial read 0x%" PRIx64
+ " bytes out of 0x%" PRIx64 " bytes.",
current_addr, bytes_read,
data_buffer.GetByteSize() - bytes_read);
@@ -1059,7 +1065,7 @@
MinidumpFileBuilder::AddMemoryList_32(std::vector<CoreFileMemoryRange> &ranges,
LLDB_LOGF(log,
"AddMemoryList %zu/%zu reading memory for region "
- "(%" PRIx64 " bytes) [%" PRIx64 ", %" PRIx64 ")",
+ "(0x%" PRIx64 " bytes) [0x%" PRIx64 ", 0x%" PRIx64 ")",
region_index, ranges.size(), size, addr, addr + size);
++region_index;
@@ -1130,7 +1136,7 @@
MinidumpFileBuilder::AddMemoryList_64(std::vector<CoreFileMemoryRange> &ranges,
// Capture the starting offset for all the descriptors so we can clean them
up
// if needed.
offset_t starting_offset =
- GetCurrentDataEndOffset() + sizeof(llvm::support::ulittle64_t);
+ GetCurrentDataEndOffset() + sizeof(llvm::minidump::Memory64ListHeader);
// The base_rva needs to start after the directories, which is right after
// this 8 byte variable.
offset_t base_rva =
>From cbb5ee7914c201323730b73dee9d0394f012673c Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 2 Jul 2025 11:01:03 -0700
Subject: [PATCH 2/8] Add new iterator to SBSaveCoreOptions to enable better
test asserts
---
.../include/lldb/API/SBMemoryRegionInfoList.h | 1 +
lldb/include/lldb/API/SBSaveCoreOptions.h | 8 ++++
lldb/include/lldb/Symbol/SaveCoreOptions.h | 2 +
lldb/source/API/SBSaveCoreOptions.cpp | 19 ++++++++
.../Minidump/MinidumpFileBuilder.cpp | 2 +-
lldb/source/Symbol/SaveCoreOptions.cpp | 23 +++++++++-
.../TestSBSaveCoreOptions.py | 44 +++++++++++++++++++
7 files changed, 97 insertions(+), 2 deletions(-)
diff --git a/lldb/include/lldb/API/SBMemoryRegionInfoList.h
b/lldb/include/lldb/API/SBMemoryRegionInfoList.h
index 1d939dff55faa..8ac9c1aceb6f6 100644
--- a/lldb/include/lldb/API/SBMemoryRegionInfoList.h
+++ b/lldb/include/lldb/API/SBMemoryRegionInfoList.h
@@ -45,6 +45,7 @@ class LLDB_API SBMemoryRegionInfoList {
private:
friend class SBProcess;
+ friend class SBSaveCoreOptions;
lldb_private::MemoryRegionInfos &ref();
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h
b/lldb/include/lldb/API/SBSaveCoreOptions.h
index 37552c13d0f36..a965f4448cbf0 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -15,6 +15,7 @@
#include "lldb/API/SBProcess.h"
#include "lldb/API/SBThread.h"
#include "lldb/API/SBThreadCollection.h"
+#include "lldb/API/SBMemoryRegionInfoList.h"
namespace lldb {
@@ -119,6 +120,13 @@ class LLDB_API SBSaveCoreOptions {
/// an empty collection will be returned.
SBThreadCollection GetThreadsToSave() const;
+ /// Get an unsorted copy of all memory regions to save
+ ///
+ /// \returns
+ /// An unsorted copy of all memory regions to save. If no process or style
+ /// is specified an empty collection will be returned.
+ SBMemoryRegionInfoList GetMemoryRegionsToSave();
+
/// Get the current total number of bytes the core is expected to have
/// excluding the overhead of the core file format. Requires a Process and
/// Style to be specified.
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h
b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index da66b184745db..2a171ba2d8ee2 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -10,6 +10,7 @@
#define LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
#include "lldb/Target/ThreadCollection.h"
+#include "lldb/Target/CoreFileMemoryRanges.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/RangeMap.h"
@@ -47,6 +48,7 @@ class SaveCoreOptions {
void AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion);
+ llvm::Expected<lldb_private::CoreFileMemoryRanges> GetMemoryRegionsToSave();
lldb_private::ThreadCollection::collection GetThreadsToSave() const;
llvm::Expected<uint64_t> GetCurrentSizeInBytes();
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp
b/lldb/source/API/SBSaveCoreOptions.cpp
index 15584abaac013..2b71cb695e584 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -128,6 +128,25 @@ uint64_t SBSaveCoreOptions::GetCurrentSizeInBytes(SBError
&error) {
return *expected_bytes;
}
+lldb::SBMemoryRegionInfoList SBSaveCoreOptions::GetMemoryRegionsToSave() {
+ LLDB_INSTRUMENT_VA(this);
+ llvm::Expected<lldb_private::CoreFileMemoryRanges> memory_ranges =
m_opaque_up->GetMemoryRegionsToSave();
+ if (!memory_ranges) {
+ llvm::consumeError(memory_ranges.takeError());
+ return SBMemoryRegionInfoList();
+ }
+
+
+ SBMemoryRegionInfoList m_memory_region_infos;
+ for (const auto &range : *memory_ranges) {
+ SBMemoryRegionInfo region_info(nullptr,
+ range.GetRangeBase(), range.GetRangeEnd(), range.data.lldb_permissions,
/*mapped=*/true);
+ m_memory_region_infos.Append(region_info);
+ }
+
+ return m_memory_region_infos;
+}
+
lldb_private::SaveCoreOptions &SBSaveCoreOptions::ref() const {
return *m_opaque_up;
}
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 34a71f41f3b84..3adcb2633a29e 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -1138,7 +1138,7 @@
MinidumpFileBuilder::AddMemoryList_64(std::vector<CoreFileMemoryRange> &ranges,
offset_t starting_offset =
GetCurrentDataEndOffset() + sizeof(llvm::minidump::Memory64ListHeader);
// The base_rva needs to start after the directories, which is right after
- // this 8 byte variable.
+ // the descriptors + the size of the header.
offset_t base_rva =
starting_offset +
(ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp
b/lldb/source/Symbol/SaveCoreOptions.cpp
index f93b58f59cf96..dfabe3a62ed1d 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -155,6 +155,23 @@ SaveCoreOptions::GetThreadsToSave() const {
return thread_collection;
}
+llvm::Expected<lldb_private::CoreFileMemoryRanges>
SaveCoreOptions::GetMemoryRegionsToSave() {
+ Status error;
+ if (!m_process_sp)
+ return Status::FromErrorString("Requires a process to be
set.").takeError();
+
+ error = EnsureValidConfiguration(m_process_sp);
+ if (error.Fail())
+ return error.takeError();
+
+ CoreFileMemoryRanges ranges;
+ error = m_process_sp->CalculateCoreFileSaveRanges(*this, ranges);
+ if (error.Fail())
+ return error.takeError();
+
+ return ranges;
+}
+
llvm::Expected<uint64_t> SaveCoreOptions::GetCurrentSizeInBytes() {
Status error;
if (!m_process_sp)
@@ -169,8 +186,12 @@ llvm::Expected<uint64_t>
SaveCoreOptions::GetCurrentSizeInBytes() {
if (error.Fail())
return error.takeError();
+ llvm::Expected<lldb_private::CoreFileMemoryRanges> core_file_ranges_maybe =
GetMemoryRegionsToSave();
+ if (!core_file_ranges_maybe)
+ return core_file_ranges_maybe.takeError();
+ const lldb_private::CoreFileMemoryRanges &core_file_ranges =
*core_file_ranges_maybe;
uint64_t total_in_bytes = 0;
- for (auto &core_range : ranges)
+ for (auto &core_range : core_file_ranges)
total_in_bytes += core_range.data.range.size();
return total_in_bytes;
diff --git
a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
index 31e35e0285f17..ec889db983ea0 100644
--- a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
+++ b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
@@ -164,3 +164,47 @@ def test_get_total_in_bytes_missing_requirements(self):
options.SetStyle(lldb.eSaveCoreCustomOnly)
total = options.GetCurrentSizeInBytes(error)
self.assertTrue(error.Fail(), error.GetCString())
+
+ def test_get_memory_regions_to_save(self):
+ """
+ Tests the matrix of responses for GetMemoryRegionsToSave
+ """
+
+ options = lldb.SBSaveCoreOptions()
+
+ # Not specifying plugin or process should return an empty list.
+ memory_list = options.GetMemoryRegionsToSave()
+ self.assertEqual(0, memory_list.GetSize())
+
+ # No style returns an empty list
+ process = self.get_basic_process()
+ options.SetProcess(process)
+ memory_list = options.GetMemoryRegionsToSave()
+ self.assertEqual(0, memory_list.GetSize())
+ options.Clear()
+
+ # No Process returns an empty list
+ options.SetStyle(lldb.eSaveCoreCustomOnly)
+ memory_list = options.GetMemoryRegionsToSave()
+ self.assertEqual(0, memory_list.GetSize())
+ options.Clear()
+
+
+ # Validate we get back the single region we populate
+ options.SetStyle(lldb.eSaveCoreCustomOnly)
+ process = self.get_basic_process()
+ options.SetProcess(process)
+ memory_range = lldb.SBMemoryRegionInfo()
+
+ # Add the memory range of 0x1000-0x1100
+ process.GetMemoryRegionInfo(0x1000, memory_range)
+ options.AddMemoryRegionToSave(memory_range)
+ memory_list = options.GetMemoryRegionsToSave()
+ self.assertEqual(1, memory_list.GetSize())
+ read_region = lldb.SBMemoryRegionInfo()
+ memory_list.GetMemoryRegionAtIndex(0, read_region)
+
+ # Permissions from Process getLLDBRegion aren't matching up with
+ # the live process permissions, so we're just checking the range for
now.
+ self.assertEqual(memory_range.GetRegionBase(),
read_region.GetRegionBase())
+ self.assertEqual(memory_range.GetRegionEnd(),
read_region.GetRegionEnd())
>From d5d443c49f47d2da93f06e7c672279e3978bd4bd Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 2 Jul 2025 11:16:17 -0700
Subject: [PATCH 3/8] Create flag api, and define it in MinidumpFileBuilder
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 8 ++++++++
lldb/include/lldb/Symbol/SaveCoreOptions.h | 7 ++++++-
lldb/source/API/SBSaveCoreOptions.cpp | 6 ++++++
.../ObjectFile/Minidump/MinidumpFileBuilder.cpp | 5 +++++
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 2 ++
lldb/source/Symbol/SaveCoreOptions.cpp | 14 ++++++++++++++
6 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h
b/lldb/include/lldb/API/SBSaveCoreOptions.h
index a965f4448cbf0..92c5c12a8ae33 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -140,6 +140,14 @@ class LLDB_API SBSaveCoreOptions {
/// The expected size of the data contained in the core in bytes.
uint64_t GetCurrentSizeInBytes(SBError &error);
+ /// Add a flag specific to a plugin provider, null or empty flags
+ /// will be ignored.
+ ///
+ /// \note
+ /// This API is currently only used for testing, with forcing Minidumps to
+ /// to 64b memory list the reason this api was added
+ void AddFlag(const char* flag);
+
/// Reset all options.
void Clear();
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h
b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index 2a171ba2d8ee2..9ff4b494c2bf7 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -24,7 +24,7 @@ namespace lldb_private {
class SaveCoreOptions {
public:
- SaveCoreOptions(){};
+ SaveCoreOptions() = default;
~SaveCoreOptions() = default;
lldb_private::Status SetPluginName(const char *name);
@@ -53,6 +53,10 @@ class SaveCoreOptions {
llvm::Expected<uint64_t> GetCurrentSizeInBytes();
+ void AddFlag(const char *flag);
+
+ bool ContainsFlag(const char *flag) const;
+
void Clear();
private:
@@ -61,6 +65,7 @@ class SaveCoreOptions {
std::optional<std::string> m_plugin_name;
std::optional<lldb_private::FileSpec> m_file;
std::optional<lldb::SaveCoreStyle> m_style;
+ std::optional<std::unordered_set<std::string>> m_flags;
lldb::ProcessSP m_process_sp;
std::unordered_set<lldb::tid_t> m_threads_to_save;
MemoryRanges m_regions_to_save;
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp
b/lldb/source/API/SBSaveCoreOptions.cpp
index 2b71cb695e584..7696912a56bbd 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -147,6 +147,12 @@ lldb::SBMemoryRegionInfoList
SBSaveCoreOptions::GetMemoryRegionsToSave() {
return m_memory_region_infos;
}
+void SBSaveCoreOptions::AddFlag(const char *flag) {
+ LLDB_INSTRUMENT_VA(this, flag);
+
+ m_opaque_up->AddFlag(flag);
+}
+
lldb_private::SaveCoreOptions &SBSaveCoreOptions::ref() const {
return *m_opaque_up;
}
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 3adcb2633a29e..349797c80376b 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -831,6 +831,11 @@ Status MinidumpFileBuilder::AddLinuxFileStreams() {
Status MinidumpFileBuilder::AddMemoryList() {
Status error;
+ // Note this is here for testing. In the past there has been many occasions
that the 64b
+ // code has regressed because it's wasteful and expensive to write a 4.2gb+
on every CI run
+ // to get around this and to exercise this codepath we define a flag in the
options object.
+ bool force_64b_for_non_threads =
m_save_core_options.ContainsFlag(&FORCE_64B_FLAG);
+
// We first save the thread stacks to ensure they fit in the first UINT32_MAX
// bytes of the core file. Thread structures in minidump files can only use
// 32 bit memory descriptiors, so we emit them first to ensure the memory is
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 46b20f90138fe..978fc7b44a384 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -176,6 +176,8 @@ class MinidumpFileBuilder {
static constexpr size_t HEADER_SIZE = sizeof(llvm::minidump::Header);
static constexpr size_t DIRECTORY_SIZE = sizeof(llvm::minidump::Directory);
+ static const char[10] FORCE_64B_FLAG = "force_64b";
+
// More that one place can mention the register thread context locations,
// so when we emit the thread contents, remember where it is so we don't have
// to duplicate it in the exception data.
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp
b/lldb/source/Symbol/SaveCoreOptions.cpp
index dfabe3a62ed1d..58d8b59f7b843 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -211,3 +211,17 @@ void SaveCoreOptions::Clear() {
m_process_sp.reset();
m_regions_to_save.Clear();
}
+
+void SaveCoreOptions::AddFlag(const char *flag) {
+ if (!flag || !flag[0])
+ return;
+
+ if (!m_flags)
+ m_flags = std::unordered_set<std::string>();
+
+ m_flags->emplace(std::string(flag));
+}
+
+bool SaveCoreOptions::ContainsFlag(const char *flag) const {
+ return m_flags && m_flags->find(flag) != m_flags->end();
+}
>From 3c4adde006fd6add000178b79cc4c571adfb220a Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 2 Jul 2025 13:13:20 -0700
Subject: [PATCH 4/8] Add new test class, and do some code fix-up
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 2 +-
.../Minidump/MinidumpFileBuilder.cpp | 4 +-
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 2 +-
.../TestProcessSaveCoreMinidump64b.py | 84 +++++++++++++++++++
4 files changed, 88 insertions(+), 4 deletions(-)
create mode 100644
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump64b.py
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h
b/lldb/include/lldb/API/SBSaveCoreOptions.h
index 92c5c12a8ae33..e42b6e2823775 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -140,7 +140,7 @@ class LLDB_API SBSaveCoreOptions {
/// The expected size of the data contained in the core in bytes.
uint64_t GetCurrentSizeInBytes(SBError &error);
- /// Add a flag specific to a plugin provider, null or empty flags
+ /// Add a flag to be consumed by the specified plugin, null or empty flags
/// will be ignored.
///
/// \note
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 349797c80376b..cb1fb2cb5ca25 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -834,7 +834,7 @@ Status MinidumpFileBuilder::AddMemoryList() {
// Note this is here for testing. In the past there has been many occasions
that the 64b
// code has regressed because it's wasteful and expensive to write a 4.2gb+
on every CI run
// to get around this and to exercise this codepath we define a flag in the
options object.
- bool force_64b_for_non_threads =
m_save_core_options.ContainsFlag(&FORCE_64B_FLAG);
+ bool force_64b_for_non_threads =
m_save_core_options.ContainsFlag(FORCE_64B_FLAG);
// We first save the thread stacks to ensure they fit in the first UINT32_MAX
// bytes of the core file. Thread structures in minidump files can only use
@@ -895,7 +895,7 @@ Status MinidumpFileBuilder::AddMemoryList() {
const addr_t range_size = core_range.range.size();
// We don't need to check for stacks here because we already removed them
// from all_core_memory_ranges.
- if (total_size + range_size < UINT32_MAX) {
+ if (!force_64b_for_non_threads && total_size + range_size < UINT32_MAX) {
ranges_32.push_back(core_range);
total_size += range_size;
} else {
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 978fc7b44a384..cc2520d7f0b16 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -176,7 +176,7 @@ class MinidumpFileBuilder {
static constexpr size_t HEADER_SIZE = sizeof(llvm::minidump::Header);
static constexpr size_t DIRECTORY_SIZE = sizeof(llvm::minidump::Directory);
- static const char[10] FORCE_64B_FLAG = "force_64b";
+ static constexpr const char FORCE_64B_FLAG[] = "force_64b";
// More that one place can mention the register thread context locations,
// so when we emit the thread contents, remember where it is so we don't have
diff --git
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump64b.py
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump64b.py
new file mode 100644
index 0000000000000..5a2300188f140
--- /dev/null
+++
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump64b.py
@@ -0,0 +1,84 @@
+"""
+Test saving a minidumps with the force 64b flag, and evaluate that every
+saved memory region is byte-wise 1:1 with the live process.
+"""
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+# Constant from MinidumpFileBuilder.h, this forces 64b for non threads
+FORCE_64B = "force_64b"
+
+class ProcessSaveCoreMinidump64bTestCase(TestBase):
+
+ def verify_minidump(
+ self,
+ core_proc,
+ live_proc,
+ options,
+ ):
+ """Verify that the minidump is the same byte for byte as the live
process."""
+ # Get the memory regions we saved off in this core, we can't compare
to the core
+ # because we pull from /proc/pid/maps, so even ranges that don't get
mapped in will show up
+ # as ranges in the minidump.
+ #
+ # Instead, we have an API that returns to us the number of regions we
planned to save from the live process
+ # and we compare those
+ memory_regions_to_compare = options.GetMemoryRegionsToSave()
+
+ for region in memory_regions_to_compare:
+ start_addr = region.GetRegionBase()
+ end_addr = region.GetRegionEnd()
+ actual_process_read_error = lldb.SBError()
+ actual = live_proc.ReadMemory(start_addr, end_addr - start_addr,
actual_process_read_error)
+ expected_process_read_error = lldb.SBError()
+ expected = core_proc.ReadMemory(start_addr, end_addr - start_addr,
expected_process_read_error)
+
+ # Both processes could fail to read a given memory region, so if
they both pass
+ # compare, then we'll fail them if the core differs from the live
process.
+ if (actual_process_read_error.Success() and
expected_process_read_error.Success()):
+ self.assertEqual(actual, expected, "Bytes differ between live
process and core")
+
+ # Now we check if the error is the same, error isn't abnormal but
they should fail for the same reason
+ self.assertTrue(
+ (actual_process_read_error.Success() and
expected_process_read_error.Success()) or
+ (actual_process_read_error.Fail() and
expected_process_read_error.Fail())
+ )
+
+ @skipUnlessArch("x86_64")
+ @skipUnlessPlatform(["linux"])
+ def test_minidump_save_style_full(self):
+ """Test that a full minidump is the same byte for byte."""
+
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ minidump_path = self.getBuildArtifact("minidump_full_force64b.dmp")
+
+ try:
+ target = self.dbg.CreateTarget(exe)
+ live_process = target.LaunchSimple(
+ None, None, self.get_process_working_directory()
+ )
+ self.assertState(live_process.GetState(), lldb.eStateStopped)
+ options = lldb.SBSaveCoreOptions()
+
+ options.SetOutputFile(lldb.SBFileSpec(minidump_path))
+ options.SetStyle(lldb.eSaveCoreFull)
+ options.SetPluginName("minidump")
+ options.SetProcess(live_process)
+ options.AddFlag(FORCE_64B)
+
+ error = live_process.SaveCore(options)
+ self.assertTrue(error.Success(), error.GetCString())
+
+ target = self.dbg.CreateTarget(None)
+ core_proc = target.LoadCore(minidump_path)
+
+ self.verify_minidump(core_proc, live_process, options)
+ finally:
+ self.assertTrue(self.dbg.DeleteTarget(target))
+ if os.path.isfile(minidump_path):
+ os.unlink(minidump_path)
>From 6c762afea5a7e1a337cffec37838935743fe46e0 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 2 Jul 2025 13:14:23 -0700
Subject: [PATCH 5/8] GCF
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 6 +++---
lldb/include/lldb/Symbol/SaveCoreOptions.h | 2 +-
lldb/source/API/SBSaveCoreOptions.cpp | 9 +++++----
.../Minidump/MinidumpFileBuilder.cpp | 18 +++++++++++-------
lldb/source/Symbol/SaveCoreOptions.cpp | 9 ++++++---
5 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h
b/lldb/include/lldb/API/SBSaveCoreOptions.h
index e42b6e2823775..e72dd53780ab9 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -12,10 +12,10 @@
#include "lldb/API/SBDefines.h"
#include "lldb/API/SBError.h"
#include "lldb/API/SBFileSpec.h"
+#include "lldb/API/SBMemoryRegionInfoList.h"
#include "lldb/API/SBProcess.h"
#include "lldb/API/SBThread.h"
#include "lldb/API/SBThreadCollection.h"
-#include "lldb/API/SBMemoryRegionInfoList.h"
namespace lldb {
@@ -123,7 +123,7 @@ class LLDB_API SBSaveCoreOptions {
/// Get an unsorted copy of all memory regions to save
///
/// \returns
- /// An unsorted copy of all memory regions to save. If no process or style
+ /// An unsorted copy of all memory regions to save. If no process or style
/// is specified an empty collection will be returned.
SBMemoryRegionInfoList GetMemoryRegionsToSave();
@@ -146,7 +146,7 @@ class LLDB_API SBSaveCoreOptions {
/// \note
/// This API is currently only used for testing, with forcing Minidumps to
/// to 64b memory list the reason this api was added
- void AddFlag(const char* flag);
+ void AddFlag(const char *flag);
/// Reset all options.
void Clear();
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h
b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index 9ff4b494c2bf7..92e474c1131a1 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -9,8 +9,8 @@
#ifndef LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
#define LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
-#include "lldb/Target/ThreadCollection.h"
#include "lldb/Target/CoreFileMemoryRanges.h"
+#include "lldb/Target/ThreadCollection.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/RangeMap.h"
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp
b/lldb/source/API/SBSaveCoreOptions.cpp
index 7696912a56bbd..0d618bc1916b9 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -130,17 +130,18 @@ uint64_t SBSaveCoreOptions::GetCurrentSizeInBytes(SBError
&error) {
lldb::SBMemoryRegionInfoList SBSaveCoreOptions::GetMemoryRegionsToSave() {
LLDB_INSTRUMENT_VA(this);
- llvm::Expected<lldb_private::CoreFileMemoryRanges> memory_ranges =
m_opaque_up->GetMemoryRegionsToSave();
+ llvm::Expected<lldb_private::CoreFileMemoryRanges> memory_ranges =
+ m_opaque_up->GetMemoryRegionsToSave();
if (!memory_ranges) {
llvm::consumeError(memory_ranges.takeError());
return SBMemoryRegionInfoList();
}
-
SBMemoryRegionInfoList m_memory_region_infos;
for (const auto &range : *memory_ranges) {
- SBMemoryRegionInfo region_info(nullptr,
- range.GetRangeBase(), range.GetRangeEnd(), range.data.lldb_permissions,
/*mapped=*/true);
+ SBMemoryRegionInfo region_info(
+ nullptr, range.GetRangeBase(), range.GetRangeEnd(),
+ range.data.lldb_permissions, /*mapped=*/true);
m_memory_region_infos.Append(region_info);
}
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index cb1fb2cb5ca25..8d8d04d1b1aba 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -831,10 +831,12 @@ Status MinidumpFileBuilder::AddLinuxFileStreams() {
Status MinidumpFileBuilder::AddMemoryList() {
Status error;
- // Note this is here for testing. In the past there has been many occasions
that the 64b
- // code has regressed because it's wasteful and expensive to write a 4.2gb+
on every CI run
- // to get around this and to exercise this codepath we define a flag in the
options object.
- bool force_64b_for_non_threads =
m_save_core_options.ContainsFlag(FORCE_64B_FLAG);
+ // Note this is here for testing. In the past there has been many occasions
+ // that the 64b code has regressed because it's wasteful and expensive to
+ // write a 4.2gb+ on every CI run to get around this and to exercise this
+ // codepath we define a flag in the options object.
+ bool force_64b_for_non_threads =
+ m_save_core_options.ContainsFlag(FORCE_64B_FLAG);
// We first save the thread stacks to ensure they fit in the first UINT32_MAX
// bytes of the core file. Thread structures in minidump files can only use
@@ -1004,7 +1006,10 @@ Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
}
if (current_addr != addr + total_bytes_read) {
- LLDB_LOGF(log, "Current addr is at expected address, 0x%" PRIx64 ",
expected at 0x%" PRIx64, current_addr, addr + total_bytes_read);
+ LLDB_LOGF(log,
+ "Current addr is at expected address, 0x%" PRIx64
+ ", expected at 0x%" PRIx64,
+ current_addr, addr + total_bytes_read);
}
// Write to the minidump file with the chunk potentially flushing to
@@ -1019,8 +1024,7 @@ Status MinidumpFileBuilder::ReadWriteMemoryInChunks(
total_bytes_read += bytes_read;
// If we have a partial read, report it, but only if the partial read
// didn't finish reading the entire region.
- if (bytes_read != data_buffer.GetByteSize() &&
- total_bytes_read != size) {
+ if (bytes_read != data_buffer.GetByteSize() && total_bytes_read != size) {
LLDB_LOGF(log,
"Memory region at: 0x%" PRIx64 " partial read 0x%" PRIx64
" bytes out of 0x%" PRIx64 " bytes.",
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp
b/lldb/source/Symbol/SaveCoreOptions.cpp
index 58d8b59f7b843..6b6387f821590 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -155,7 +155,8 @@ SaveCoreOptions::GetThreadsToSave() const {
return thread_collection;
}
-llvm::Expected<lldb_private::CoreFileMemoryRanges>
SaveCoreOptions::GetMemoryRegionsToSave() {
+llvm::Expected<lldb_private::CoreFileMemoryRanges>
+SaveCoreOptions::GetMemoryRegionsToSave() {
Status error;
if (!m_process_sp)
return Status::FromErrorString("Requires a process to be
set.").takeError();
@@ -186,10 +187,12 @@ llvm::Expected<uint64_t>
SaveCoreOptions::GetCurrentSizeInBytes() {
if (error.Fail())
return error.takeError();
- llvm::Expected<lldb_private::CoreFileMemoryRanges> core_file_ranges_maybe =
GetMemoryRegionsToSave();
+ llvm::Expected<lldb_private::CoreFileMemoryRanges> core_file_ranges_maybe =
+ GetMemoryRegionsToSave();
if (!core_file_ranges_maybe)
return core_file_ranges_maybe.takeError();
- const lldb_private::CoreFileMemoryRanges &core_file_ranges =
*core_file_ranges_maybe;
+ const lldb_private::CoreFileMemoryRanges &core_file_ranges =
+ *core_file_ranges_maybe;
uint64_t total_in_bytes = 0;
for (auto &core_range : core_file_ranges)
total_in_bytes += core_range.data.range.size();
>From fa488d0608b961395f0cb41a5b8e2002d0a5fcff Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 2 Jul 2025 13:14:42 -0700
Subject: [PATCH 6/8] Python formatting
---
.../TestProcessSaveCoreMinidump64b.py | 28 +++++++++++++++----
.../TestSBSaveCoreOptions.py | 1 -
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump64b.py
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump64b.py
index 5a2300188f140..b1ce13a047439 100644
---
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump64b.py
+++
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump64b.py
@@ -12,6 +12,7 @@
# Constant from MinidumpFileBuilder.h, this forces 64b for non threads
FORCE_64B = "force_64b"
+
class ProcessSaveCoreMinidump64bTestCase(TestBase):
def verify_minidump(
@@ -33,19 +34,34 @@ def verify_minidump(
start_addr = region.GetRegionBase()
end_addr = region.GetRegionEnd()
actual_process_read_error = lldb.SBError()
- actual = live_proc.ReadMemory(start_addr, end_addr - start_addr,
actual_process_read_error)
+ actual = live_proc.ReadMemory(
+ start_addr, end_addr - start_addr, actual_process_read_error
+ )
expected_process_read_error = lldb.SBError()
- expected = core_proc.ReadMemory(start_addr, end_addr - start_addr,
expected_process_read_error)
+ expected = core_proc.ReadMemory(
+ start_addr, end_addr - start_addr, expected_process_read_error
+ )
# Both processes could fail to read a given memory region, so if
they both pass
# compare, then we'll fail them if the core differs from the live
process.
- if (actual_process_read_error.Success() and
expected_process_read_error.Success()):
- self.assertEqual(actual, expected, "Bytes differ between live
process and core")
+ if (
+ actual_process_read_error.Success()
+ and expected_process_read_error.Success()
+ ):
+ self.assertEqual(
+ actual, expected, "Bytes differ between live process and
core"
+ )
# Now we check if the error is the same, error isn't abnormal but
they should fail for the same reason
self.assertTrue(
- (actual_process_read_error.Success() and
expected_process_read_error.Success()) or
- (actual_process_read_error.Fail() and
expected_process_read_error.Fail())
+ (
+ actual_process_read_error.Success()
+ and expected_process_read_error.Success()
+ )
+ or (
+ actual_process_read_error.Fail()
+ and expected_process_read_error.Fail()
+ )
)
@skipUnlessArch("x86_64")
diff --git
a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
index ec889db983ea0..92ca44ecbbffc 100644
--- a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
+++ b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py
@@ -189,7 +189,6 @@ def test_get_memory_regions_to_save(self):
self.assertEqual(0, memory_list.GetSize())
options.Clear()
-
# Validate we get back the single region we populate
options.SetStyle(lldb.eSaveCoreCustomOnly)
process = self.get_basic_process()
>From 2dc4a46246325ea01cb8583661c412b37547c929 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 2 Jul 2025 13:56:41 -0700
Subject: [PATCH 7/8] Add docstring
---
.../bindings/interface/SBSaveCoreOptionsDocstrings.i | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
b/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
index 6907164a1b95c..a44c0569f40e6 100644
--- a/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
+++ b/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
@@ -63,6 +63,18 @@ Note that currently ELF Core files are not supported."
Get an SBThreadCollection of all threads marked to be saved. This
collection is not sorted according to insertion order."
) lldb::SBSaveCoreOptions::GetThreadsToSave;
+
+%feature("docstring", "
+ Get an SBMemoryRegionInfoList of all the Regions that LLDB will attempt to
write into the Core. Note, reading from these
+ regions can fail, and it's guaraunteed every region will be present. If
called without a valid process or style set an empty
+ collection will be returned."
+) lldb::SBSaveCoreOptions::GetMemoryRegionsToSave;
+
+%feature("docstring", "
+ Add a plugin specific flag to the objects option."
+) lldb::SBSaveCoreOptions::AddFlag;
+
+
%feature("docstring", "
Get the current total number of bytes the core is expected to have,
excluding the overhead of the core file format.
Requires both a Process and a Style to be specified. An error will be
returned if the provided options would result in no data being saved."
>From 082ecc6cb60cc856eb746a3f4a43b84761bee41f Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalo...@fb.com>
Date: Wed, 2 Jul 2025 14:02:35 -0700
Subject: [PATCH 8/8] Manual python formatting
---
.../process_save_core_minidump/TestProcessSaveCoreMinidump64b.py | 1 -
1 file changed, 1 deletion(-)
diff --git
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump64b.py
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump64b.py
index b1ce13a047439..b672629ad7453 100644
---
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump64b.py
+++
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump64b.py
@@ -14,7 +14,6 @@
class ProcessSaveCoreMinidump64bTestCase(TestBase):
-
def verify_minidump(
self,
core_proc,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
