[Lldb-commits] [lldb] [LLDB][SBSaveCore] Add selectable memory regions to SBSaveCore (PR #105442)
Jacob Lalonde via lldb-commits
lldb-commits at lists.llvm.org
Mon Aug 26 20:12:29 PDT 2024
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/105442
>From e4b2600379ea167482726c9b8c9faeabeba76820 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 1 Aug 2024 12:37:27 -0700
Subject: [PATCH 1/9] Implement a memory region list for sbsavecoreoptions
---
lldb/include/lldb/API/SBMemoryRegionInfo.h | 2 +-
.../include/lldb/API/SBMemoryRegionInfoList.h | 1 -
lldb/include/lldb/API/SBSaveCoreOptions.h | 1 +
lldb/include/lldb/Symbol/SaveCoreOptions.h | 6 +
lldb/include/lldb/Target/Process.h | 55 +---
lldb/source/API/SBSaveCoreOptions.cpp | 9 +
.../ObjectFile/Mach-O/ObjectFileMachO.cpp | 240 +++++++++---------
.../Minidump/MinidumpFileBuilder.cpp | 68 ++---
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 9 +-
.../Minidump/ObjectFileMinidump.cpp | 79 +++---
lldb/source/Symbol/SaveCoreOptions.cpp | 78 +-----
lldb/source/Target/Process.cpp | 84 ++----
12 files changed, 248 insertions(+), 384 deletions(-)
diff --git a/lldb/include/lldb/API/SBMemoryRegionInfo.h b/lldb/include/lldb/API/SBMemoryRegionInfo.h
index be55de4ead1fa8..f9a5dc993d7cb6 100644
--- a/lldb/include/lldb/API/SBMemoryRegionInfo.h
+++ b/lldb/include/lldb/API/SBMemoryRegionInfo.h
@@ -120,7 +120,7 @@ class LLDB_API SBMemoryRegionInfo {
private:
friend class SBProcess;
friend class SBMemoryRegionInfoList;
-
+ friend class SBSaveCoreOptions;
friend class lldb_private::ScriptInterpreter;
lldb_private::MemoryRegionInfo &ref();
diff --git a/lldb/include/lldb/API/SBMemoryRegionInfoList.h b/lldb/include/lldb/API/SBMemoryRegionInfoList.h
index 1d939dff55faa3..7207171bf6d647 100644
--- a/lldb/include/lldb/API/SBMemoryRegionInfoList.h
+++ b/lldb/include/lldb/API/SBMemoryRegionInfoList.h
@@ -45,7 +45,6 @@ class LLDB_API SBMemoryRegionInfoList {
private:
friend class SBProcess;
-
lldb_private::MemoryRegionInfos &ref();
const lldb_private::MemoryRegionInfos &ref() const;
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h
index ba48ba5eaea5a0..5727a9af7395f3 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -79,6 +79,7 @@ class LLDB_API SBSaveCoreOptions {
/// \param thread The thread to remove.
/// \return True if the thread was removed, false if it was not in the list.
bool RemoveThread(lldb::SBThread thread);
+ SBError AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion);
/// Reset all options.
void Clear();
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index f4fed4676fa4ae..ef747920e2dc58 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -9,6 +9,7 @@
#ifndef LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
#define LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
+#include "lldb/Core/AddressRange.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
@@ -16,6 +17,7 @@
#include <optional>
#include <string>
#include <unordered_set>
+#include <set>
namespace lldb_private {
@@ -40,6 +42,9 @@ class SaveCoreOptions {
bool ShouldThreadBeSaved(lldb::tid_t tid) const;
Status EnsureValidConfiguration(lldb::ProcessSP process_sp) const;
+ Status AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion);
+
+ bool ShouldSaveRegion(const lldb_private::MemoryRegionInfo ®ion) const;
void Clear();
@@ -51,6 +56,7 @@ class SaveCoreOptions {
std::optional<lldb::SaveCoreStyle> m_style;
lldb::ProcessSP m_process_sp;
std::unordered_set<lldb::tid_t> m_threads_to_save;
+ std::set<lldb::addr_t> m_regions_to_save;
};
} // namespace lldb_private
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 9cf6f50558570b..4f225d86ad824f 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -738,14 +738,8 @@ class Process : public std::enable_shared_from_this<Process>,
/// Helper function for Process::SaveCore(...) that calculates the address
/// ranges that should be saved. This allows all core file plug-ins to save
/// consistent memory ranges given a \a core_style.
- Status CalculateCoreFileSaveRanges(const SaveCoreOptions &core_options,
- CoreFileMemoryRanges &ranges);
-
- /// Helper function for Process::SaveCore(...) that calculates the thread list
- /// based upon options set within a given \a core_options object.
- /// \note If there is no thread list defined, all threads will be saved.
- std::vector<lldb::ThreadSP>
- CalculateCoreFileThreadList(const SaveCoreOptions &core_options);
+ Status CalculateCoreFileSaveRanges(CoreFileMemoryRanges &ranges,
+ const SaveCoreOptions &options);
protected:
virtual JITLoaderList &GetJITLoaders();
@@ -1320,16 +1314,10 @@ class Process : public std::enable_shared_from_this<Process>,
size_t GetThreadStatus(Stream &ostrm, bool only_threads_with_stop_reason,
uint32_t start_frame, uint32_t num_frames,
- uint32_t num_frames_with_source, bool stop_format);
+ uint32_t num_frames_with_source,
+ bool stop_format);
- /// Send an async interrupt request.
- ///
- /// If \a thread is specified the async interrupt stop will be attributed to
- /// the specified thread.
- ///
- /// \param[in] thread
- /// The thread the async interrupt will be attributed to.
- void SendAsyncInterrupt(Thread *thread = nullptr);
+ void SendAsyncInterrupt();
// Notify this process class that modules got loaded.
//
@@ -2582,19 +2570,6 @@ void PruneThreadPlans();
/// information related to the process.
virtual StructuredData::DictionarySP GetMetadata() { return nullptr; }
- /// Fetch extended crash information held by the process. This will never be
- /// an empty shared pointer, it will always have a dict, though it may be
- /// empty.
- StructuredData::DictionarySP GetExtendedCrashInfoDict() {
- assert(m_crash_info_dict_sp && "We always have a valid dictionary");
- return m_crash_info_dict_sp;
- }
-
- void ResetExtendedCrashInfoDict() {
- // StructuredData::Dictionary is add only, so we have to make a new one:
- m_crash_info_dict_sp.reset(new StructuredData::Dictionary());
- }
-
size_t AddImageToken(lldb::addr_t image_ptr);
lldb::addr_t GetImagePtrFromToken(size_t token) const;
@@ -2879,17 +2854,6 @@ void PruneThreadPlans();
return std::nullopt;
}
- /// Handle thread specific async interrupt and return the original thread
- /// that requested the async interrupt. It can be null if original thread
- /// has exited.
- ///
- /// \param[in] description
- /// Returns the stop reason description of the async interrupt.
- virtual lldb::ThreadSP
- HandleThreadAsyncInterrupt(uint8_t signo, const std::string &description) {
- return lldb::ThreadSP();
- }
-
lldb::StateType GetPrivateState();
/// The "private" side of resuming a process. This doesn't alter the state
@@ -3176,11 +3140,6 @@ void PruneThreadPlans();
// Resume will only request a resume, using this
// flag to check.
- lldb::tid_t m_interrupt_tid; /// The tid of the thread that issued the async
- /// interrupt, used by thread plan timeout. It
- /// can be LLDB_INVALID_THREAD_ID to indicate
- /// user level async interrupt.
-
/// This is set at the beginning of Process::Finalize() to stop functions
/// from looking up or creating things during or after a finalize call.
std::atomic<bool> m_finalizing;
@@ -3227,10 +3186,6 @@ void PruneThreadPlans();
/// Per process source file cache.
SourceManager::SourceFileCache m_source_file_cache;
- /// A repository for extra crash information, consulted in
- /// GetExtendedCrashInformation.
- StructuredData::DictionarySP m_crash_info_dict_sp;
-
size_t RemoveBreakpointOpcodesFromBuffer(lldb::addr_t addr, size_t size,
uint8_t *buf) const;
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp
index 2cd431611ef558..323148807d90ca 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -7,6 +7,9 @@
//===----------------------------------------------------------------------===//
#include "lldb/API/SBSaveCoreOptions.h"
+#include "lldb/API/SBError.h"
+#include "lldb/API/SBFileSpec.h"
+#include "lldb/API/SBMemoryRegionInfo.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Utility/Instrumentation.h"
@@ -90,6 +93,12 @@ bool SBSaveCoreOptions::RemoveThread(lldb::SBThread thread) {
return m_opaque_up->RemoveThread(thread.GetSP());
}
+lldb::SBError SBSaveCoreOptions::AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion) {
+ LLDB_INSTRUMENT_VA(this, region);
+ lldb_private::Status error = m_opaque_up->AddMemoryRegionToSave(region.ref());
+ return SBError(error);
+}
+
void SBSaveCoreOptions::Clear() {
LLDB_INSTRUMENT_VA(this);
m_opaque_up->Clear();
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 22ece4f4dacf79..915e8c5ce6828e 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -137,9 +137,6 @@ using namespace lldb;
using namespace lldb_private;
using namespace llvm::MachO;
-static constexpr llvm::StringLiteral g_loader_path = "@loader_path";
-static constexpr llvm::StringLiteral g_executable_path = "@executable_path";
-
LLDB_PLUGIN_DEFINE(ObjectFileMachO)
static void PrintRegisterValue(RegisterContext *reg_ctx, const char *name,
@@ -5119,119 +5116,121 @@ UUID ObjectFileMachO::GetUUID() {
}
uint32_t ObjectFileMachO::GetDependentModules(FileSpecList &files) {
- ModuleSP module_sp = GetModule();
- if (!module_sp)
- return 0;
-
uint32_t count = 0;
- std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
- llvm::MachO::load_command load_cmd;
- lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
- std::vector<std::string> rpath_paths;
- std::vector<std::string> rpath_relative_paths;
- std::vector<std::string> at_exec_relative_paths;
- uint32_t i;
- for (i = 0; i < m_header.ncmds; ++i) {
- const uint32_t cmd_offset = offset;
- if (m_data.GetU32(&offset, &load_cmd, 2) == nullptr)
- break;
+ ModuleSP module_sp(GetModule());
+ if (module_sp) {
+ std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
+ llvm::MachO::load_command load_cmd;
+ lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
+ std::vector<std::string> rpath_paths;
+ std::vector<std::string> rpath_relative_paths;
+ std::vector<std::string> at_exec_relative_paths;
+ uint32_t i;
+ for (i = 0; i < m_header.ncmds; ++i) {
+ const uint32_t cmd_offset = offset;
+ if (m_data.GetU32(&offset, &load_cmd, 2) == nullptr)
+ break;
- switch (load_cmd.cmd) {
- case LC_RPATH:
- case LC_LOAD_DYLIB:
- case LC_LOAD_WEAK_DYLIB:
- case LC_REEXPORT_DYLIB:
- case LC_LOAD_DYLINKER:
- case LC_LOADFVMLIB:
- case LC_LOAD_UPWARD_DYLIB: {
- uint32_t name_offset = cmd_offset + m_data.GetU32(&offset);
- // For LC_LOAD_DYLIB there is an alternate encoding
- // which adds a uint32_t `flags` field for `DYLD_USE_*`
- // flags. This can be detected by a timestamp field with
- // the `DYLIB_USE_MARKER` constant value.
- bool is_delayed_init = false;
- uint32_t use_command_marker = m_data.GetU32(&offset);
- if (use_command_marker == 0x1a741800 /* DYLIB_USE_MARKER */) {
- offset += 4; /* uint32_t current_version */
- offset += 4; /* uint32_t compat_version */
- uint32_t flags = m_data.GetU32(&offset);
- // If this LC_LOAD_DYLIB is marked delay-init,
- // don't report it as a dependent library -- it
- // may be loaded in the process at some point,
- // but will most likely not be load at launch.
- if (flags & 0x08 /* DYLIB_USE_DELAYED_INIT */)
- is_delayed_init = true;
- }
- const char *path = m_data.PeekCStr(name_offset);
- if (path && !is_delayed_init) {
- if (load_cmd.cmd == LC_RPATH)
- rpath_paths.push_back(path);
- else {
- if (path[0] == '@') {
- if (strncmp(path, "@rpath", strlen("@rpath")) == 0)
- rpath_relative_paths.push_back(path + strlen("@rpath"));
- else if (strncmp(path, "@executable_path",
- strlen("@executable_path")) == 0)
- at_exec_relative_paths.push_back(path +
- strlen("@executable_path"));
- } else {
- FileSpec file_spec(path);
- if (files.AppendIfUnique(file_spec))
- count++;
+ switch (load_cmd.cmd) {
+ case LC_RPATH:
+ case LC_LOAD_DYLIB:
+ case LC_LOAD_WEAK_DYLIB:
+ case LC_REEXPORT_DYLIB:
+ case LC_LOAD_DYLINKER:
+ case LC_LOADFVMLIB:
+ case LC_LOAD_UPWARD_DYLIB: {
+ uint32_t name_offset = cmd_offset + m_data.GetU32(&offset);
+ // For LC_LOAD_DYLIB there is an alternate encoding
+ // which adds a uint32_t `flags` field for `DYLD_USE_*`
+ // flags. This can be detected by a timestamp field with
+ // the `DYLIB_USE_MARKER` constant value.
+ bool is_delayed_init = false;
+ uint32_t use_command_marker = m_data.GetU32(&offset);
+ if (use_command_marker == 0x1a741800 /* DYLIB_USE_MARKER */) {
+ offset += 4; /* uint32_t current_version */
+ offset += 4; /* uint32_t compat_version */
+ uint32_t flags = m_data.GetU32(&offset);
+ // If this LC_LOAD_DYLIB is marked delay-init,
+ // don't report it as a dependent library -- it
+ // may be loaded in the process at some point,
+ // but will most likely not be load at launch.
+ if (flags & 0x08 /* DYLIB_USE_DELAYED_INIT */)
+ is_delayed_init = true;
+ }
+ const char *path = m_data.PeekCStr(name_offset);
+ if (path && !is_delayed_init) {
+ if (load_cmd.cmd == LC_RPATH)
+ rpath_paths.push_back(path);
+ else {
+ if (path[0] == '@') {
+ if (strncmp(path, "@rpath", strlen("@rpath")) == 0)
+ rpath_relative_paths.push_back(path + strlen("@rpath"));
+ else if (strncmp(path, "@executable_path",
+ strlen("@executable_path")) == 0)
+ at_exec_relative_paths.push_back(path +
+ strlen("@executable_path"));
+ } else {
+ FileSpec file_spec(path);
+ if (files.AppendIfUnique(file_spec))
+ count++;
+ }
}
}
- }
- } break;
+ } break;
- default:
- break;
+ default:
+ break;
+ }
+ offset = cmd_offset + load_cmd.cmdsize;
}
- offset = cmd_offset + load_cmd.cmdsize;
- }
- FileSpec this_file_spec(m_file);
- FileSystem::Instance().Resolve(this_file_spec);
-
- if (!rpath_paths.empty()) {
- // Fixup all LC_RPATH values to be absolute paths.
- const std::string this_directory =
- this_file_spec.GetDirectory().GetString();
- for (auto &rpath : rpath_paths) {
- if (llvm::StringRef(rpath).starts_with(g_loader_path))
- rpath = this_directory + rpath.substr(g_loader_path.size());
- else if (llvm::StringRef(rpath).starts_with(g_executable_path))
- rpath = this_directory + rpath.substr(g_executable_path.size());
- }
+ FileSpec this_file_spec(m_file);
+ FileSystem::Instance().Resolve(this_file_spec);
+
+ if (!rpath_paths.empty()) {
+ // Fixup all LC_RPATH values to be absolute paths
+ std::string loader_path("@loader_path");
+ std::string executable_path("@executable_path");
+ for (auto &rpath : rpath_paths) {
+ if (llvm::StringRef(rpath).starts_with(loader_path)) {
+ rpath.erase(0, loader_path.size());
+ rpath.insert(0, this_file_spec.GetDirectory().GetCString());
+ } else if (llvm::StringRef(rpath).starts_with(executable_path)) {
+ rpath.erase(0, executable_path.size());
+ rpath.insert(0, this_file_spec.GetDirectory().GetCString());
+ }
+ }
- for (const auto &rpath_relative_path : rpath_relative_paths) {
- for (const auto &rpath : rpath_paths) {
- std::string path = rpath;
- path += rpath_relative_path;
- // It is OK to resolve this path because we must find a file on disk
- // for us to accept it anyway if it is rpath relative.
- FileSpec file_spec(path);
- FileSystem::Instance().Resolve(file_spec);
- if (FileSystem::Instance().Exists(file_spec) &&
- files.AppendIfUnique(file_spec)) {
- count++;
- break;
+ for (const auto &rpath_relative_path : rpath_relative_paths) {
+ for (const auto &rpath : rpath_paths) {
+ std::string path = rpath;
+ path += rpath_relative_path;
+ // It is OK to resolve this path because we must find a file on disk
+ // for us to accept it anyway if it is rpath relative.
+ FileSpec file_spec(path);
+ FileSystem::Instance().Resolve(file_spec);
+ if (FileSystem::Instance().Exists(file_spec) &&
+ files.AppendIfUnique(file_spec)) {
+ count++;
+ break;
+ }
}
}
}
- }
- // We may have @executable_paths but no RPATHS. Figure those out here.
- // Only do this if this object file is the executable. We have no way to
- // get back to the actual executable otherwise, so we won't get the right
- // path.
- if (!at_exec_relative_paths.empty() && CalculateType() == eTypeExecutable) {
- FileSpec exec_dir = this_file_spec.CopyByRemovingLastPathComponent();
- for (const auto &at_exec_relative_path : at_exec_relative_paths) {
- FileSpec file_spec =
- exec_dir.CopyByAppendingPathComponent(at_exec_relative_path);
- if (FileSystem::Instance().Exists(file_spec) &&
- files.AppendIfUnique(file_spec))
- count++;
+ // We may have @executable_paths but no RPATHS. Figure those out here.
+ // Only do this if this object file is the executable. We have no way to
+ // get back to the actual executable otherwise, so we won't get the right
+ // path.
+ if (!at_exec_relative_paths.empty() && CalculateType() == eTypeExecutable) {
+ FileSpec exec_dir = this_file_spec.CopyByRemovingLastPathComponent();
+ for (const auto &at_exec_relative_path : at_exec_relative_paths) {
+ FileSpec file_spec =
+ exec_dir.CopyByAppendingPathComponent(at_exec_relative_path);
+ if (FileSystem::Instance().Exists(file_spec) &&
+ files.AppendIfUnique(file_spec))
+ count++;
+ }
}
}
return count;
@@ -6347,24 +6346,22 @@ struct segment_vmaddr {
// are some multiple passes over the image list while calculating
// everything.
-static offset_t
-CreateAllImageInfosPayload(const lldb::ProcessSP &process_sp,
- offset_t initial_file_offset,
- StreamString &all_image_infos_payload,
- lldb_private::SaveCoreOptions &options) {
+static offset_t CreateAllImageInfosPayload(
+ const lldb::ProcessSP &process_sp, offset_t initial_file_offset,
+ StreamString &all_image_infos_payload, SaveCoreStyle core_style) {
Target &target = process_sp->GetTarget();
ModuleList modules = target.GetImages();
// stack-only corefiles have no reason to include binaries that
// are not executing; we're trying to make the smallest corefile
// we can, so leave the rest out.
- if (options.GetStyle() == SaveCoreStyle::eSaveCoreStackOnly)
+ if (core_style == SaveCoreStyle::eSaveCoreStackOnly)
modules.Clear();
std::set<std::string> executing_uuids;
- std::vector<ThreadSP> thread_list =
- process_sp->CalculateCoreFileThreadList(options);
- for (const ThreadSP &thread_sp : thread_list) {
+ ThreadList &thread_list(process_sp->GetThreadList());
+ for (uint32_t i = 0; i < thread_list.GetSize(); i++) {
+ ThreadSP thread_sp = thread_list.GetThreadAtIndex(i);
uint32_t stack_frame_count = thread_sp->GetStackFrameCount();
for (uint32_t j = 0; j < stack_frame_count; j++) {
StackFrameSP stack_frame_sp = thread_sp->GetStackFrameAtIndex(j);
@@ -6522,17 +6519,16 @@ struct page_object {
};
bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
- lldb_private::SaveCoreOptions &options,
+ const lldb_private::SaveCoreOptions &options,
Status &error) {
+ auto core_style = options.GetStyle();
+ if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
+ core_style = SaveCoreStyle::eSaveCoreDirtyOnly;
// The FileSpec and Process are already checked in PluginManager::SaveCore.
assert(options.GetOutputFile().has_value());
assert(process_sp);
const FileSpec outfile = options.GetOutputFile().value();
- // MachO defaults to dirty pages
- if (options.GetStyle() == SaveCoreStyle::eSaveCoreUnspecified)
- options.SetStyle(eSaveCoreDirtyOnly);
-
Target &target = process_sp->GetTarget();
const ArchSpec target_arch = target.GetArchitecture();
const llvm::Triple &target_triple = target_arch.GetTriple();
@@ -6562,7 +6558,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
if (make_core) {
Process::CoreFileMemoryRanges core_ranges;
- error = process_sp->CalculateCoreFileSaveRanges(options, core_ranges);
+ error = process_sp->CalculateCoreFileSaveRanges(core_ranges, options);
if (error.Success()) {
const uint32_t addr_byte_size = target_arch.GetAddressByteSize();
const ByteOrder byte_order = target_arch.GetByteOrder();
@@ -6733,8 +6729,8 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
std::make_shared<StructuredData::Dictionary>());
StructuredData::ArraySP threads(
std::make_shared<StructuredData::Array>());
- for (const ThreadSP &thread_sp :
- process_sp->CalculateCoreFileThreadList(options)) {
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
StructuredData::DictionarySP thread(
std::make_shared<StructuredData::Dictionary>());
thread->AddIntegerItem("thread_id", thread_sp->GetID());
@@ -6757,7 +6753,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
all_image_infos_lcnote_up->payload_file_offset = file_offset;
file_offset = CreateAllImageInfosPayload(
process_sp, file_offset, all_image_infos_lcnote_up->payload,
- options);
+ core_style);
lc_notes.push_back(std::move(all_image_infos_lcnote_up));
// Add LC_NOTE load commands
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index c0cc3af638a777..1f38a0c35c86fa 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -69,9 +69,10 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
m_expected_directories += 9;
// Go through all of the threads and check for exceptions.
- std::vector<lldb::ThreadSP> threads =
- m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
- for (const ThreadSP &thread_sp : threads) {
+ lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
+ const uint32_t num_threads = thread_list.GetSize();
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
if (stop_info_sp) {
const StopReason &stop_reason = stop_info_sp->GetStopReason();
@@ -587,13 +588,12 @@ Status MinidumpFileBuilder::FixThreadStacks() {
Status MinidumpFileBuilder::AddThreadList() {
constexpr size_t minidump_thread_size = sizeof(llvm::minidump::Thread);
- std::vector<ThreadSP> thread_list =
- m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
+ lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
// size of the entire thread stream consists of:
// number of threads and threads array
size_t thread_stream_size = sizeof(llvm::support::ulittle32_t) +
- thread_list.size() * minidump_thread_size;
+ thread_list.GetSize() * minidump_thread_size;
// save for the ability to set up RVA
size_t size_before = GetCurrentDataEndOffset();
Status error;
@@ -602,15 +602,17 @@ Status MinidumpFileBuilder::AddThreadList() {
return error;
llvm::support::ulittle32_t thread_count =
- static_cast<llvm::support::ulittle32_t>(thread_list.size());
+ static_cast<llvm::support::ulittle32_t>(thread_list.GetSize());
m_data.AppendData(&thread_count, sizeof(llvm::support::ulittle32_t));
// Take the offset after the thread count.
m_thread_list_start = GetCurrentDataEndOffset();
DataBufferHeap helper_data;
+ const uint32_t num_threads = thread_list.GetSize();
Log *log = GetLog(LLDBLog::Object);
- for (const ThreadSP &thread_sp : thread_list) {
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext());
if (!reg_ctx_sp) {
@@ -648,7 +650,7 @@ Status MinidumpFileBuilder::AddThreadList() {
m_tid_to_reg_ctx[thread_sp->GetID()] = thread_context_memory_locator;
LLDB_LOGF(log, "AddThreadList for thread %d: thread_context %zu bytes",
- thread_sp->GetIndexID(), thread_context.size());
+ thread_idx, thread_context.size());
helper_data.AppendData(thread_context.data(), thread_context.size());
llvm::minidump::Thread t;
@@ -672,10 +674,11 @@ Status MinidumpFileBuilder::AddThreadList() {
}
Status MinidumpFileBuilder::AddExceptions() {
- std::vector<ThreadSP> thread_list =
- m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
+ lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
Status error;
- for (const ThreadSP &thread_sp : thread_list) {
+ const uint32_t num_threads = thread_list.GetSize();
+ for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
+ ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
bool add_exception = false;
if (stop_info_sp) {
@@ -825,26 +828,17 @@ Status MinidumpFileBuilder::AddMemoryList() {
// in accessible with a 32 bit offset.
Process::CoreFileMemoryRanges ranges_32;
Process::CoreFileMemoryRanges ranges_64;
- Process::CoreFileMemoryRanges all_core_memory_ranges;
- error = m_process_sp->CalculateCoreFileSaveRanges(m_save_core_options,
- all_core_memory_ranges);
+ error = m_process_sp->CalculateCoreFileSaveRanges(ranges_32, m_options);
if (error.Fail())
return error;
- // Start by saving all of the stacks and ensuring they fit under the 32b
- // limit.
+ // Calculate totalsize including the current offset.
uint64_t total_size = GetCurrentDataEndOffset();
- auto iterator = all_core_memory_ranges.begin();
- while (iterator != all_core_memory_ranges.end()) {
- if (m_saved_stack_ranges.count(iterator->range.start()) > 0) {
- // We don't save stacks twice.
- ranges_32.push_back(*iterator);
- total_size +=
- iterator->range.size() + sizeof(llvm::minidump::MemoryDescriptor);
- iterator = all_core_memory_ranges.erase(iterator);
- } else {
- iterator++;
- }
+ total_size += ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
+ std::unordered_set<addr_t> stack_start_addresses;
+ for (const auto &core_range : ranges_32) {
+ stack_start_addresses.insert(core_range.range.start());
+ total_size += core_range.range.size();
}
if (total_size >= UINT32_MAX) {
@@ -854,6 +848,13 @@ Status MinidumpFileBuilder::AddMemoryList() {
return error;
}
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ if (m_options.GetStyle() != SaveCoreStyle::eSaveCoreStackOnly) {
+ error = m_process_sp->CalculateCoreFileSaveRanges(all_core_memory_ranges, m_options);
+ if (error.Fail())
+ return error;
+ }
+
// After saving the stacks, we start packing as much as we can into 32b.
// We apply a generous padding here so that the Directory, MemoryList and
// Memory64List sections all begin in 32b addressable space.
@@ -861,13 +862,16 @@ Status MinidumpFileBuilder::AddMemoryList() {
// All core memeroy ranges will either container nothing on stacks only
// or all the memory ranges including stacks
if (!all_core_memory_ranges.empty())
- total_size += 256 + (all_core_memory_ranges.size() *
- sizeof(llvm::minidump::MemoryDescriptor_64));
+ total_size +=
+ 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
+ sizeof(llvm::minidump::MemoryDescriptor_64);
for (const auto &core_range : all_core_memory_ranges) {
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 (stack_start_addresses.count(core_range.range.start()) > 0)
+ // Don't double save stacks.
+ continue;
+
if (total_size + range_size < UINT32_MAX) {
ranges_32.push_back(core_range);
total_size += range_size;
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 762de83db5a39c..690b5daa78a8b3 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -76,9 +76,9 @@ class MinidumpFileBuilder {
public:
MinidumpFileBuilder(lldb::FileUP &&core_file,
const lldb::ProcessSP &process_sp,
- lldb_private::SaveCoreOptions &save_core_options)
- : m_process_sp(process_sp), m_core_file(std::move(core_file)),
- m_save_core_options(save_core_options){}
+ const lldb_private::SaveCoreOptions &options)
+ : m_process_sp(process_sp), m_options(std::move(options)),
+ m_core_file(std::move(core_file)) {};
MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;
@@ -165,9 +165,8 @@ class MinidumpFileBuilder {
// to duplicate it in the exception data.
std::unordered_map<lldb::tid_t, llvm::minidump::LocationDescriptor>
m_tid_to_reg_ctx;
- std::unordered_set<lldb::addr_t> m_saved_stack_ranges;
+ lldb_private::SaveCoreOptions m_options;
lldb::FileUP m_core_file;
- lldb_private::SaveCoreOptions m_save_core_options;
};
#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index 0897895e6bc25d..c0984573a3ddb1 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -56,16 +56,12 @@ size_t ObjectFileMinidump::GetModuleSpecifications(
}
bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
- lldb_private::SaveCoreOptions &options,
+ const lldb_private::SaveCoreOptions &options,
lldb_private::Status &error) {
// Output file and process_sp are both checked in PluginManager::SaveCore.
assert(options.GetOutputFile().has_value());
assert(process_sp);
- // Minidump defaults to stacks only.
- if (options.GetStyle() == SaveCoreStyle::eSaveCoreUnspecified)
- options.SetStyle(SaveCoreStyle::eSaveCoreStackOnly);
-
llvm::Expected<lldb::FileUP> maybe_core_file = FileSystem::Instance().Open(
options.GetOutputFile().value(),
File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate);
@@ -73,8 +69,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
error = maybe_core_file.takeError();
return false;
}
- MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp,
- options);
+ MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp, options);
Log *log = GetLog(LLDBLog::Object);
error = builder.AddHeaderAndCalculateDirectories();
@@ -83,41 +78,41 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
error.AsCString());
return false;
};
- error = builder.AddSystemInfo();
- if (error.Fail()) {
- LLDB_LOGF(log, "AddSystemInfo failed: %s", error.AsCString());
- return false;
- }
-
- error = builder.AddModuleList();
- if (error.Fail()) {
- LLDB_LOGF(log, "AddModuleList failed: %s", error.AsCString());
- return false;
- }
- error = builder.AddMiscInfo();
- if (error.Fail()) {
- LLDB_LOGF(log, "AddMiscInfo failed: %s", error.AsCString());
- return false;
- }
-
- error = builder.AddThreadList();
- if (error.Fail()) {
- LLDB_LOGF(log, "AddThreadList failed: %s", error.AsCString());
- return false;
- }
-
- error = builder.AddLinuxFileStreams();
- if (error.Fail()) {
- LLDB_LOGF(log, "AddLinuxFileStreams failed: %s", error.AsCString());
- return false;
- }
-
- // Add any exceptions but only if there are any in any threads.
- error = builder.AddExceptions();
- if (error.Fail()) {
- LLDB_LOGF(log, "AddExceptions failed: %s", error.AsCString());
- return false;
- }
+ // error = builder.AddSystemInfo();
+ // if (error.Fail()) {
+ // LLDB_LOGF(log, "AddSystemInfo failed: %s", error.AsCString());
+ // return false;
+ // }
+
+ // error = builder.AddModuleList();
+ // if (error.Fail()) {
+ // LLDB_LOGF(log, "AddModuleList failed: %s", error.AsCString());
+ // return false;
+ // }
+ // error = builder.AddMiscInfo();
+ // if (error.Fail()) {
+ // LLDB_LOGF(log, "AddMiscInfo failed: %s", error.AsCString());
+ // return false;
+ // }
+
+ // error = builder.AddThreadList();
+ // if (error.Fail()) {
+ // LLDB_LOGF(log, "AddThreadList failed: %s", error.AsCString());
+ // return false;
+ // }
+
+ // error = builder.AddLinuxFileStreams();
+ // if (error.Fail()) {
+ // LLDB_LOGF(log, "AddLinuxFileStreams failed: %s", error.AsCString());
+ // return false;
+ // }
+
+ // // Add any exceptions but only if there are any in any threads.
+ // error = builder.AddExceptions();
+ // if (error.Fail()) {
+ // LLDB_LOGF(log, "AddExceptions failed: %s", error.AsCString());
+ // return false;
+ // }
// Note: add memory HAS to be the last thing we do. It can overflow into 64b
// land and many RVA's only support 32b
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp
index 3c4ca2d852b76a..6bf6d7c824673f 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -8,8 +8,6 @@
#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Core/PluginManager.h"
-#include "lldb/Target/Process.h"
-#include "lldb/Target/Thread.h"
using namespace lldb;
using namespace lldb_private;
@@ -48,86 +46,18 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
-Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
+Status SaveCoreOptions::AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion) {
Status error;
- if (!process_sp) {
- ClearProcessSpecificData();
- m_process_sp.reset();
- return error;
- }
-
- if (!process_sp->IsValid()) {
- error.SetErrorString("Cannot assign an invalid process.");
- return error;
- }
-
- // Don't clear any process specific data if the process is the same.
- if (m_process_sp == process_sp)
- return error;
-
- ClearProcessSpecificData();
- m_process_sp = process_sp;
- return error;
-}
-
-Status SaveCoreOptions::AddThread(lldb::ThreadSP thread_sp) {
- Status error;
- if (!thread_sp) {
- error.SetErrorString("invalid thread");
- return error;
- }
-
- if (m_process_sp) {
- if (m_process_sp != thread_sp->GetProcess()) {
- error.SetErrorString("Cannot add a thread from a different process.");
- return error;
- }
- } else {
- m_process_sp = thread_sp->GetProcess();
- }
-
- m_threads_to_save.insert(thread_sp->GetID());
- return error;
-}
-
-bool SaveCoreOptions::RemoveThread(lldb::ThreadSP thread_sp) {
- return thread_sp && m_threads_to_save.erase(thread_sp->GetID()) > 0;
-}
-
-bool SaveCoreOptions::ShouldThreadBeSaved(lldb::tid_t tid) const {
- // If the user specified no threads to save, then we save all threads.
- if (m_threads_to_save.empty())
- return true;
- return m_threads_to_save.count(tid) > 0;
-}
-
-Status SaveCoreOptions::EnsureValidConfiguration(
- lldb::ProcessSP process_sp) const {
- Status error;
- std::string error_str;
- if (!m_threads_to_save.empty() && GetStyle() == lldb::eSaveCoreFull)
- error_str += "Cannot save a full core with a subset of threads\n";
-
- if (m_process_sp && m_process_sp != process_sp)
- error_str += "Cannot save core for process using supplied core options. "
- "Options were constructed targeting a different process. \n";
-
- if (!error_str.empty())
- error.SetErrorString(error_str);
-
+ m_regions_to_save.insert(region.GetRange().GetRangeBase());
return error;
}
-void SaveCoreOptions::ClearProcessSpecificData() {
- // Deliberately not following the formatter style here to indicate that
- // this method will be expanded in the future.
- m_threads_to_save.clear();
+bool SaveCoreOptions::ShouldSaveRegion(const lldb_private::MemoryRegionInfo ®ion) const {
+ return m_regions_to_save.empty() || m_regions_to_save.count(region.GetRange().GetRangeBase()) > 0;
}
void SaveCoreOptions::Clear() {
m_file = std::nullopt;
m_plugin_name = std::nullopt;
m_style = std::nullopt;
- m_threads_to_save.clear();
- m_process_sp.reset();
}
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 3c9247fdbbbc96..07b7f4feef8800 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -473,13 +473,11 @@ Process::Process(lldb::TargetSP target_sp, ListenerSP listener_sp,
m_memory_cache(*this), m_allocated_memory_cache(*this),
m_should_detach(false), m_next_event_action_up(), m_public_run_lock(),
m_private_run_lock(), m_currently_handling_do_on_removals(false),
- m_resume_requested(false), m_interrupt_tid(LLDB_INVALID_THREAD_ID),
- m_finalizing(false), m_destructing(false),
+ m_resume_requested(false), m_finalizing(false), m_destructing(false),
m_clear_thread_plans_on_stop(false), m_force_next_event_delivery(false),
m_last_broadcast_state(eStateInvalid), m_destroy_in_process(false),
m_can_interpret_function_calls(false), m_run_thread_plan_lock(),
- m_can_jit(eCanJITDontKnow),
- m_crash_info_dict_sp(new StructuredData::Dictionary()) {
+ m_can_jit(eCanJITDontKnow) {
CheckInWithManager();
Log *log = GetLog(LLDBLog::Object);
@@ -896,7 +894,6 @@ bool Process::HandleProcessStateChangedEvent(
case eStopReasonThreadExiting:
case eStopReasonInstrumentation:
case eStopReasonProcessorTrace:
- case eStopReasonInterrupt:
if (!other_thread)
other_thread = thread;
break;
@@ -3266,10 +3263,6 @@ Status Process::PrivateResume() {
// If signals handing status changed we might want to update our signal
// filters before resuming.
UpdateAutomaticSignalFiltering();
- // Clear any crash info we accumulated for this stop, but don't do so if we
- // are running functions; we don't want to wipe out the real stop's info.
- if (!GetModID().IsLastResumeForUserExpression())
- ResetExtendedCrashInfoDict();
Status error(WillResume());
// Tell the process it is about to resume before the thread list
@@ -3875,11 +3868,7 @@ void Process::ControlPrivateStateThread(uint32_t signal) {
}
}
-void Process::SendAsyncInterrupt(Thread *thread) {
- if (thread != nullptr)
- m_interrupt_tid = thread->GetProtocolID();
- else
- m_interrupt_tid = LLDB_INVALID_THREAD_ID;
+void Process::SendAsyncInterrupt() {
if (PrivateStateThreadIsValid())
m_private_state_broadcaster.BroadcastEvent(Process::eBroadcastBitInterrupt,
nullptr);
@@ -4105,14 +4094,9 @@ thread_result_t Process::RunPrivateStateThread(bool is_secondary_thread) {
if (interrupt_requested) {
if (StateIsStoppedState(internal_state, true)) {
- // Only mark interrupt event if it is not thread specific async
- // interrupt.
- if (m_interrupt_tid == LLDB_INVALID_THREAD_ID) {
- // We requested the interrupt, so mark this as such in the stop
- // event so clients can tell an interrupted process from a natural
- // stop
- ProcessEventData::SetInterruptedInEvent(event_sp.get(), true);
- }
+ // We requested the interrupt, so mark this as such in the stop event
+ // so clients can tell an interrupted process from a natural stop
+ ProcessEventData::SetInterruptedInEvent(event_sp.get(), true);
interrupt_requested = false;
} else if (log) {
LLDB_LOGF(log,
@@ -6535,7 +6519,7 @@ static bool AddDirtyPages(const MemoryRegionInfo ®ion,
// will be added to \a ranges, else the entire range will be added to \a
// ranges.
static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages,
- Process::CoreFileMemoryRanges &ranges) {
+ Process::CoreFileMemoryRanges &ranges, const SaveCoreOptions &options) {
// Don't add empty ranges.
if (region.GetRange().GetByteSize() == 0)
return;
@@ -6544,13 +6528,14 @@ static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages,
return;
if (try_dirty_pages && AddDirtyPages(region, ranges))
return;
+ if (!options.ShouldSaveRegion(region))
+ return;
ranges.push_back(CreateCoreFileMemoryRange(region));
}
static void SaveOffRegionsWithStackPointers(
- Process &process, const SaveCoreOptions &core_options,
- const MemoryRegionInfos ®ions, Process::CoreFileMemoryRanges &ranges,
- std::set<addr_t> &stack_ends) {
+ Process &process, const MemoryRegionInfos ®ions,
+ Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends, const SaveCoreOptions &options) {
const bool try_dirty_pages = true;
// Before we take any dump, we want to save off the used portions of the
@@ -6572,16 +6557,10 @@ static void SaveOffRegionsWithStackPointers(
if (process.GetMemoryRegionInfo(sp, sp_region).Success()) {
const size_t stack_head = (sp - red_zone);
const size_t stack_size = sp_region.GetRange().GetRangeEnd() - stack_head;
- // Even if the SaveCoreOption doesn't want us to save the stack
- // we still need to populate the stack_ends set so it doesn't get saved
- // off in other calls
sp_region.GetRange().SetRangeBase(stack_head);
sp_region.GetRange().SetByteSize(stack_size);
stack_ends.insert(sp_region.GetRange().GetRangeEnd());
- // This will return true if the threadlist the user specified is empty,
- // or contains the thread id from thread_sp.
- if (core_options.ShouldThreadBeSaved(thread_sp->GetID()))
- AddRegion(sp_region, try_dirty_pages, ranges);
+ AddRegion(sp_region, try_dirty_pages, ranges, options);
}
}
}
@@ -6591,13 +6570,14 @@ static void SaveOffRegionsWithStackPointers(
static void GetCoreFileSaveRangesFull(Process &process,
const MemoryRegionInfos ®ions,
Process::CoreFileMemoryRanges &ranges,
- std::set<addr_t> &stack_ends) {
+ std::set<addr_t> &stack_ends,
+ const SaveCoreOptions &options) {
// Don't add only dirty pages, add full regions.
const bool try_dirty_pages = false;
for (const auto ®ion : regions)
if (stack_ends.count(region.GetRange().GetRangeEnd()) == 0)
- AddRegion(region, try_dirty_pages, ranges);
+ AddRegion(region, try_dirty_pages, ranges, options);
}
// Save only the dirty pages to the core file. Make sure the process has at
@@ -6606,7 +6586,8 @@ const bool try_dirty_pages = false;
// page information fall back to saving out all ranges with write permissions.
static void GetCoreFileSaveRangesDirtyOnly(
Process &process, const MemoryRegionInfos ®ions,
- Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
+ Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends,
+ const SaveCoreOptions &options) {
// Iterate over the regions and find all dirty pages.
bool have_dirty_page_info = false;
@@ -6623,7 +6604,7 @@ static void GetCoreFileSaveRangesDirtyOnly(
for (const auto ®ion : regions)
if (stack_ends.count(region.GetRange().GetRangeEnd()) == 0 &&
region.GetWritable() == MemoryRegionInfo::eYes)
- AddRegion(region, try_dirty_pages, ranges);
+ AddRegion(region, try_dirty_pages, ranges, options);
}
}
@@ -6637,7 +6618,8 @@ static void GetCoreFileSaveRangesDirtyOnly(
// stack region.
static void GetCoreFileSaveRangesStackOnly(
Process &process, const MemoryRegionInfos ®ions,
- Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
+ Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends,
+ const SaveCoreOptions &options) {
const bool try_dirty_pages = true;
// Some platforms support annotating the region information that tell us that
// it comes from a thread stack. So look for those regions first.
@@ -6646,12 +6628,12 @@ static void GetCoreFileSaveRangesStackOnly(
// Save all the stack memory ranges not associated with a stack pointer.
if (stack_ends.count(region.GetRange().GetRangeEnd()) == 0 &&
region.IsStackMemory() == MemoryRegionInfo::eYes)
- AddRegion(region, try_dirty_pages, ranges);
+ AddRegion(region, try_dirty_pages, ranges, options);
}
}
-Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
- CoreFileMemoryRanges &ranges) {
+Status Process::CalculateCoreFileSaveRanges(CoreFileMemoryRanges &ranges,
+ const SaveCoreOptions &options) {
lldb_private::MemoryRegionInfos regions;
Status err = GetMemoryRegions(regions);
SaveCoreStyle core_style = options.GetStyle();
@@ -6664,22 +6646,22 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
"eSaveCoreUnspecified");
std::set<addr_t> stack_ends;
- SaveOffRegionsWithStackPointers(*this, options, regions, ranges, stack_ends);
+ SaveOffRegionsWithStackPointers(*this, regions, ranges, stack_ends, options);
switch (core_style) {
case eSaveCoreUnspecified:
break;
case eSaveCoreFull:
- GetCoreFileSaveRangesFull(*this, regions, ranges, stack_ends);
+ GetCoreFileSaveRangesFull(*this, regions, ranges, stack_ends, options);
break;
case eSaveCoreDirtyOnly:
- GetCoreFileSaveRangesDirtyOnly(*this, regions, ranges, stack_ends);
+ GetCoreFileSaveRangesDirtyOnly(*this, regions, ranges, stack_ends, options);
break;
case eSaveCoreStackOnly:
- GetCoreFileSaveRangesStackOnly(*this, regions, ranges, stack_ends);
+ GetCoreFileSaveRangesStackOnly(*this, regions, ranges, stack_ends, options);
break;
}
@@ -6692,18 +6674,6 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
return Status(); // Success!
}
-std::vector<ThreadSP>
-Process::CalculateCoreFileThreadList(const SaveCoreOptions &core_options) {
- std::vector<ThreadSP> thread_list;
- for (const lldb::ThreadSP &thread_sp : m_thread_list.Threads()) {
- if (core_options.ShouldThreadBeSaved(thread_sp->GetID())) {
- thread_list.push_back(thread_sp);
- }
- }
-
- return thread_list;
-}
-
void Process::SetAddressableBitMasks(AddressableBits bit_masks) {
uint32_t low_memory_addr_bits = bit_masks.GetLowmemAddressableBits();
uint32_t high_memory_addr_bits = bit_masks.GetHighmemAddressableBits();
>From 1efc29bf238a085006482a54f323d657b2d8d029 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 16 Aug 2024 13:38:08 -0700
Subject: [PATCH 2/9] Rebase on top of the sbsavecore thread list changes
---
lldb/include/lldb/Target/Process.h | 55 +++-
.../ObjectFile/Mach-O/ObjectFileMachO.cpp | 240 +++++++++---------
.../Minidump/MinidumpFileBuilder.cpp | 68 +++--
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 9 +-
.../Minidump/ObjectFileMinidump.cpp | 79 +++---
lldb/source/Symbol/SaveCoreOptions.cpp | 80 ++++++
lldb/source/Target/Process.cpp | 95 ++++---
7 files changed, 397 insertions(+), 229 deletions(-)
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 4f225d86ad824f..9cf6f50558570b 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -738,8 +738,14 @@ class Process : public std::enable_shared_from_this<Process>,
/// Helper function for Process::SaveCore(...) that calculates the address
/// ranges that should be saved. This allows all core file plug-ins to save
/// consistent memory ranges given a \a core_style.
- Status CalculateCoreFileSaveRanges(CoreFileMemoryRanges &ranges,
- const SaveCoreOptions &options);
+ Status CalculateCoreFileSaveRanges(const SaveCoreOptions &core_options,
+ CoreFileMemoryRanges &ranges);
+
+ /// Helper function for Process::SaveCore(...) that calculates the thread list
+ /// based upon options set within a given \a core_options object.
+ /// \note If there is no thread list defined, all threads will be saved.
+ std::vector<lldb::ThreadSP>
+ CalculateCoreFileThreadList(const SaveCoreOptions &core_options);
protected:
virtual JITLoaderList &GetJITLoaders();
@@ -1314,10 +1320,16 @@ class Process : public std::enable_shared_from_this<Process>,
size_t GetThreadStatus(Stream &ostrm, bool only_threads_with_stop_reason,
uint32_t start_frame, uint32_t num_frames,
- uint32_t num_frames_with_source,
- bool stop_format);
+ uint32_t num_frames_with_source, bool stop_format);
- void SendAsyncInterrupt();
+ /// Send an async interrupt request.
+ ///
+ /// If \a thread is specified the async interrupt stop will be attributed to
+ /// the specified thread.
+ ///
+ /// \param[in] thread
+ /// The thread the async interrupt will be attributed to.
+ void SendAsyncInterrupt(Thread *thread = nullptr);
// Notify this process class that modules got loaded.
//
@@ -2570,6 +2582,19 @@ void PruneThreadPlans();
/// information related to the process.
virtual StructuredData::DictionarySP GetMetadata() { return nullptr; }
+ /// Fetch extended crash information held by the process. This will never be
+ /// an empty shared pointer, it will always have a dict, though it may be
+ /// empty.
+ StructuredData::DictionarySP GetExtendedCrashInfoDict() {
+ assert(m_crash_info_dict_sp && "We always have a valid dictionary");
+ return m_crash_info_dict_sp;
+ }
+
+ void ResetExtendedCrashInfoDict() {
+ // StructuredData::Dictionary is add only, so we have to make a new one:
+ m_crash_info_dict_sp.reset(new StructuredData::Dictionary());
+ }
+
size_t AddImageToken(lldb::addr_t image_ptr);
lldb::addr_t GetImagePtrFromToken(size_t token) const;
@@ -2854,6 +2879,17 @@ void PruneThreadPlans();
return std::nullopt;
}
+ /// Handle thread specific async interrupt and return the original thread
+ /// that requested the async interrupt. It can be null if original thread
+ /// has exited.
+ ///
+ /// \param[in] description
+ /// Returns the stop reason description of the async interrupt.
+ virtual lldb::ThreadSP
+ HandleThreadAsyncInterrupt(uint8_t signo, const std::string &description) {
+ return lldb::ThreadSP();
+ }
+
lldb::StateType GetPrivateState();
/// The "private" side of resuming a process. This doesn't alter the state
@@ -3140,6 +3176,11 @@ void PruneThreadPlans();
// Resume will only request a resume, using this
// flag to check.
+ lldb::tid_t m_interrupt_tid; /// The tid of the thread that issued the async
+ /// interrupt, used by thread plan timeout. It
+ /// can be LLDB_INVALID_THREAD_ID to indicate
+ /// user level async interrupt.
+
/// This is set at the beginning of Process::Finalize() to stop functions
/// from looking up or creating things during or after a finalize call.
std::atomic<bool> m_finalizing;
@@ -3186,6 +3227,10 @@ void PruneThreadPlans();
/// Per process source file cache.
SourceManager::SourceFileCache m_source_file_cache;
+ /// A repository for extra crash information, consulted in
+ /// GetExtendedCrashInformation.
+ StructuredData::DictionarySP m_crash_info_dict_sp;
+
size_t RemoveBreakpointOpcodesFromBuffer(lldb::addr_t addr, size_t size,
uint8_t *buf) const;
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 915e8c5ce6828e..22ece4f4dacf79 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -137,6 +137,9 @@ using namespace lldb;
using namespace lldb_private;
using namespace llvm::MachO;
+static constexpr llvm::StringLiteral g_loader_path = "@loader_path";
+static constexpr llvm::StringLiteral g_executable_path = "@executable_path";
+
LLDB_PLUGIN_DEFINE(ObjectFileMachO)
static void PrintRegisterValue(RegisterContext *reg_ctx, const char *name,
@@ -5116,123 +5119,121 @@ UUID ObjectFileMachO::GetUUID() {
}
uint32_t ObjectFileMachO::GetDependentModules(FileSpecList &files) {
+ ModuleSP module_sp = GetModule();
+ if (!module_sp)
+ return 0;
+
uint32_t count = 0;
- ModuleSP module_sp(GetModule());
- if (module_sp) {
- std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
- llvm::MachO::load_command load_cmd;
- lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
- std::vector<std::string> rpath_paths;
- std::vector<std::string> rpath_relative_paths;
- std::vector<std::string> at_exec_relative_paths;
- uint32_t i;
- for (i = 0; i < m_header.ncmds; ++i) {
- const uint32_t cmd_offset = offset;
- if (m_data.GetU32(&offset, &load_cmd, 2) == nullptr)
- break;
+ std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
+ llvm::MachO::load_command load_cmd;
+ lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
+ std::vector<std::string> rpath_paths;
+ std::vector<std::string> rpath_relative_paths;
+ std::vector<std::string> at_exec_relative_paths;
+ uint32_t i;
+ for (i = 0; i < m_header.ncmds; ++i) {
+ const uint32_t cmd_offset = offset;
+ if (m_data.GetU32(&offset, &load_cmd, 2) == nullptr)
+ break;
- switch (load_cmd.cmd) {
- case LC_RPATH:
- case LC_LOAD_DYLIB:
- case LC_LOAD_WEAK_DYLIB:
- case LC_REEXPORT_DYLIB:
- case LC_LOAD_DYLINKER:
- case LC_LOADFVMLIB:
- case LC_LOAD_UPWARD_DYLIB: {
- uint32_t name_offset = cmd_offset + m_data.GetU32(&offset);
- // For LC_LOAD_DYLIB there is an alternate encoding
- // which adds a uint32_t `flags` field for `DYLD_USE_*`
- // flags. This can be detected by a timestamp field with
- // the `DYLIB_USE_MARKER` constant value.
- bool is_delayed_init = false;
- uint32_t use_command_marker = m_data.GetU32(&offset);
- if (use_command_marker == 0x1a741800 /* DYLIB_USE_MARKER */) {
- offset += 4; /* uint32_t current_version */
- offset += 4; /* uint32_t compat_version */
- uint32_t flags = m_data.GetU32(&offset);
- // If this LC_LOAD_DYLIB is marked delay-init,
- // don't report it as a dependent library -- it
- // may be loaded in the process at some point,
- // but will most likely not be load at launch.
- if (flags & 0x08 /* DYLIB_USE_DELAYED_INIT */)
- is_delayed_init = true;
- }
- const char *path = m_data.PeekCStr(name_offset);
- if (path && !is_delayed_init) {
- if (load_cmd.cmd == LC_RPATH)
- rpath_paths.push_back(path);
- else {
- if (path[0] == '@') {
- if (strncmp(path, "@rpath", strlen("@rpath")) == 0)
- rpath_relative_paths.push_back(path + strlen("@rpath"));
- else if (strncmp(path, "@executable_path",
- strlen("@executable_path")) == 0)
- at_exec_relative_paths.push_back(path +
- strlen("@executable_path"));
- } else {
- FileSpec file_spec(path);
- if (files.AppendIfUnique(file_spec))
- count++;
- }
+ switch (load_cmd.cmd) {
+ case LC_RPATH:
+ case LC_LOAD_DYLIB:
+ case LC_LOAD_WEAK_DYLIB:
+ case LC_REEXPORT_DYLIB:
+ case LC_LOAD_DYLINKER:
+ case LC_LOADFVMLIB:
+ case LC_LOAD_UPWARD_DYLIB: {
+ uint32_t name_offset = cmd_offset + m_data.GetU32(&offset);
+ // For LC_LOAD_DYLIB there is an alternate encoding
+ // which adds a uint32_t `flags` field for `DYLD_USE_*`
+ // flags. This can be detected by a timestamp field with
+ // the `DYLIB_USE_MARKER` constant value.
+ bool is_delayed_init = false;
+ uint32_t use_command_marker = m_data.GetU32(&offset);
+ if (use_command_marker == 0x1a741800 /* DYLIB_USE_MARKER */) {
+ offset += 4; /* uint32_t current_version */
+ offset += 4; /* uint32_t compat_version */
+ uint32_t flags = m_data.GetU32(&offset);
+ // If this LC_LOAD_DYLIB is marked delay-init,
+ // don't report it as a dependent library -- it
+ // may be loaded in the process at some point,
+ // but will most likely not be load at launch.
+ if (flags & 0x08 /* DYLIB_USE_DELAYED_INIT */)
+ is_delayed_init = true;
+ }
+ const char *path = m_data.PeekCStr(name_offset);
+ if (path && !is_delayed_init) {
+ if (load_cmd.cmd == LC_RPATH)
+ rpath_paths.push_back(path);
+ else {
+ if (path[0] == '@') {
+ if (strncmp(path, "@rpath", strlen("@rpath")) == 0)
+ rpath_relative_paths.push_back(path + strlen("@rpath"));
+ else if (strncmp(path, "@executable_path",
+ strlen("@executable_path")) == 0)
+ at_exec_relative_paths.push_back(path +
+ strlen("@executable_path"));
+ } else {
+ FileSpec file_spec(path);
+ if (files.AppendIfUnique(file_spec))
+ count++;
}
}
- } break;
-
- default:
- break;
}
- offset = cmd_offset + load_cmd.cmdsize;
- }
+ } break;
- FileSpec this_file_spec(m_file);
- FileSystem::Instance().Resolve(this_file_spec);
-
- if (!rpath_paths.empty()) {
- // Fixup all LC_RPATH values to be absolute paths
- std::string loader_path("@loader_path");
- std::string executable_path("@executable_path");
- for (auto &rpath : rpath_paths) {
- if (llvm::StringRef(rpath).starts_with(loader_path)) {
- rpath.erase(0, loader_path.size());
- rpath.insert(0, this_file_spec.GetDirectory().GetCString());
- } else if (llvm::StringRef(rpath).starts_with(executable_path)) {
- rpath.erase(0, executable_path.size());
- rpath.insert(0, this_file_spec.GetDirectory().GetCString());
- }
- }
+ default:
+ break;
+ }
+ offset = cmd_offset + load_cmd.cmdsize;
+ }
- for (const auto &rpath_relative_path : rpath_relative_paths) {
- for (const auto &rpath : rpath_paths) {
- std::string path = rpath;
- path += rpath_relative_path;
- // It is OK to resolve this path because we must find a file on disk
- // for us to accept it anyway if it is rpath relative.
- FileSpec file_spec(path);
- FileSystem::Instance().Resolve(file_spec);
- if (FileSystem::Instance().Exists(file_spec) &&
- files.AppendIfUnique(file_spec)) {
- count++;
- break;
- }
- }
- }
+ FileSpec this_file_spec(m_file);
+ FileSystem::Instance().Resolve(this_file_spec);
+
+ if (!rpath_paths.empty()) {
+ // Fixup all LC_RPATH values to be absolute paths.
+ const std::string this_directory =
+ this_file_spec.GetDirectory().GetString();
+ for (auto &rpath : rpath_paths) {
+ if (llvm::StringRef(rpath).starts_with(g_loader_path))
+ rpath = this_directory + rpath.substr(g_loader_path.size());
+ else if (llvm::StringRef(rpath).starts_with(g_executable_path))
+ rpath = this_directory + rpath.substr(g_executable_path.size());
}
- // We may have @executable_paths but no RPATHS. Figure those out here.
- // Only do this if this object file is the executable. We have no way to
- // get back to the actual executable otherwise, so we won't get the right
- // path.
- if (!at_exec_relative_paths.empty() && CalculateType() == eTypeExecutable) {
- FileSpec exec_dir = this_file_spec.CopyByRemovingLastPathComponent();
- for (const auto &at_exec_relative_path : at_exec_relative_paths) {
- FileSpec file_spec =
- exec_dir.CopyByAppendingPathComponent(at_exec_relative_path);
+ for (const auto &rpath_relative_path : rpath_relative_paths) {
+ for (const auto &rpath : rpath_paths) {
+ std::string path = rpath;
+ path += rpath_relative_path;
+ // It is OK to resolve this path because we must find a file on disk
+ // for us to accept it anyway if it is rpath relative.
+ FileSpec file_spec(path);
+ FileSystem::Instance().Resolve(file_spec);
if (FileSystem::Instance().Exists(file_spec) &&
- files.AppendIfUnique(file_spec))
+ files.AppendIfUnique(file_spec)) {
count++;
+ break;
+ }
}
}
}
+
+ // We may have @executable_paths but no RPATHS. Figure those out here.
+ // Only do this if this object file is the executable. We have no way to
+ // get back to the actual executable otherwise, so we won't get the right
+ // path.
+ if (!at_exec_relative_paths.empty() && CalculateType() == eTypeExecutable) {
+ FileSpec exec_dir = this_file_spec.CopyByRemovingLastPathComponent();
+ for (const auto &at_exec_relative_path : at_exec_relative_paths) {
+ FileSpec file_spec =
+ exec_dir.CopyByAppendingPathComponent(at_exec_relative_path);
+ if (FileSystem::Instance().Exists(file_spec) &&
+ files.AppendIfUnique(file_spec))
+ count++;
+ }
+ }
return count;
}
@@ -6346,22 +6347,24 @@ struct segment_vmaddr {
// are some multiple passes over the image list while calculating
// everything.
-static offset_t CreateAllImageInfosPayload(
- const lldb::ProcessSP &process_sp, offset_t initial_file_offset,
- StreamString &all_image_infos_payload, SaveCoreStyle core_style) {
+static offset_t
+CreateAllImageInfosPayload(const lldb::ProcessSP &process_sp,
+ offset_t initial_file_offset,
+ StreamString &all_image_infos_payload,
+ lldb_private::SaveCoreOptions &options) {
Target &target = process_sp->GetTarget();
ModuleList modules = target.GetImages();
// stack-only corefiles have no reason to include binaries that
// are not executing; we're trying to make the smallest corefile
// we can, so leave the rest out.
- if (core_style == SaveCoreStyle::eSaveCoreStackOnly)
+ if (options.GetStyle() == SaveCoreStyle::eSaveCoreStackOnly)
modules.Clear();
std::set<std::string> executing_uuids;
- ThreadList &thread_list(process_sp->GetThreadList());
- for (uint32_t i = 0; i < thread_list.GetSize(); i++) {
- ThreadSP thread_sp = thread_list.GetThreadAtIndex(i);
+ std::vector<ThreadSP> thread_list =
+ process_sp->CalculateCoreFileThreadList(options);
+ for (const ThreadSP &thread_sp : thread_list) {
uint32_t stack_frame_count = thread_sp->GetStackFrameCount();
for (uint32_t j = 0; j < stack_frame_count; j++) {
StackFrameSP stack_frame_sp = thread_sp->GetStackFrameAtIndex(j);
@@ -6519,16 +6522,17 @@ struct page_object {
};
bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
- const lldb_private::SaveCoreOptions &options,
+ lldb_private::SaveCoreOptions &options,
Status &error) {
- auto core_style = options.GetStyle();
- if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
- core_style = SaveCoreStyle::eSaveCoreDirtyOnly;
// The FileSpec and Process are already checked in PluginManager::SaveCore.
assert(options.GetOutputFile().has_value());
assert(process_sp);
const FileSpec outfile = options.GetOutputFile().value();
+ // MachO defaults to dirty pages
+ if (options.GetStyle() == SaveCoreStyle::eSaveCoreUnspecified)
+ options.SetStyle(eSaveCoreDirtyOnly);
+
Target &target = process_sp->GetTarget();
const ArchSpec target_arch = target.GetArchitecture();
const llvm::Triple &target_triple = target_arch.GetTriple();
@@ -6558,7 +6562,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
if (make_core) {
Process::CoreFileMemoryRanges core_ranges;
- error = process_sp->CalculateCoreFileSaveRanges(core_ranges, options);
+ error = process_sp->CalculateCoreFileSaveRanges(options, core_ranges);
if (error.Success()) {
const uint32_t addr_byte_size = target_arch.GetAddressByteSize();
const ByteOrder byte_order = target_arch.GetByteOrder();
@@ -6729,8 +6733,8 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
std::make_shared<StructuredData::Dictionary>());
StructuredData::ArraySP threads(
std::make_shared<StructuredData::Array>());
- for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
- ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ for (const ThreadSP &thread_sp :
+ process_sp->CalculateCoreFileThreadList(options)) {
StructuredData::DictionarySP thread(
std::make_shared<StructuredData::Dictionary>());
thread->AddIntegerItem("thread_id", thread_sp->GetID());
@@ -6753,7 +6757,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
all_image_infos_lcnote_up->payload_file_offset = file_offset;
file_offset = CreateAllImageInfosPayload(
process_sp, file_offset, all_image_infos_lcnote_up->payload,
- core_style);
+ options);
lc_notes.push_back(std::move(all_image_infos_lcnote_up));
// Add LC_NOTE load commands
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 1f38a0c35c86fa..c0cc3af638a777 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -69,10 +69,9 @@ Status MinidumpFileBuilder::AddHeaderAndCalculateDirectories() {
m_expected_directories += 9;
// Go through all of the threads and check for exceptions.
- lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
- const uint32_t num_threads = thread_list.GetSize();
- for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
- ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ std::vector<lldb::ThreadSP> threads =
+ m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
+ for (const ThreadSP &thread_sp : threads) {
StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
if (stop_info_sp) {
const StopReason &stop_reason = stop_info_sp->GetStopReason();
@@ -588,12 +587,13 @@ Status MinidumpFileBuilder::FixThreadStacks() {
Status MinidumpFileBuilder::AddThreadList() {
constexpr size_t minidump_thread_size = sizeof(llvm::minidump::Thread);
- lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
+ std::vector<ThreadSP> thread_list =
+ m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
// size of the entire thread stream consists of:
// number of threads and threads array
size_t thread_stream_size = sizeof(llvm::support::ulittle32_t) +
- thread_list.GetSize() * minidump_thread_size;
+ thread_list.size() * minidump_thread_size;
// save for the ability to set up RVA
size_t size_before = GetCurrentDataEndOffset();
Status error;
@@ -602,17 +602,15 @@ Status MinidumpFileBuilder::AddThreadList() {
return error;
llvm::support::ulittle32_t thread_count =
- static_cast<llvm::support::ulittle32_t>(thread_list.GetSize());
+ static_cast<llvm::support::ulittle32_t>(thread_list.size());
m_data.AppendData(&thread_count, sizeof(llvm::support::ulittle32_t));
// Take the offset after the thread count.
m_thread_list_start = GetCurrentDataEndOffset();
DataBufferHeap helper_data;
- const uint32_t num_threads = thread_list.GetSize();
Log *log = GetLog(LLDBLog::Object);
- for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
- ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ for (const ThreadSP &thread_sp : thread_list) {
RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext());
if (!reg_ctx_sp) {
@@ -650,7 +648,7 @@ Status MinidumpFileBuilder::AddThreadList() {
m_tid_to_reg_ctx[thread_sp->GetID()] = thread_context_memory_locator;
LLDB_LOGF(log, "AddThreadList for thread %d: thread_context %zu bytes",
- thread_idx, thread_context.size());
+ thread_sp->GetIndexID(), thread_context.size());
helper_data.AppendData(thread_context.data(), thread_context.size());
llvm::minidump::Thread t;
@@ -674,11 +672,10 @@ Status MinidumpFileBuilder::AddThreadList() {
}
Status MinidumpFileBuilder::AddExceptions() {
- lldb_private::ThreadList thread_list = m_process_sp->GetThreadList();
+ std::vector<ThreadSP> thread_list =
+ m_process_sp->CalculateCoreFileThreadList(m_save_core_options);
Status error;
- const uint32_t num_threads = thread_list.GetSize();
- for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
- ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx));
+ for (const ThreadSP &thread_sp : thread_list) {
StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
bool add_exception = false;
if (stop_info_sp) {
@@ -828,17 +825,26 @@ Status MinidumpFileBuilder::AddMemoryList() {
// in accessible with a 32 bit offset.
Process::CoreFileMemoryRanges ranges_32;
Process::CoreFileMemoryRanges ranges_64;
- error = m_process_sp->CalculateCoreFileSaveRanges(ranges_32, m_options);
+ Process::CoreFileMemoryRanges all_core_memory_ranges;
+ error = m_process_sp->CalculateCoreFileSaveRanges(m_save_core_options,
+ all_core_memory_ranges);
if (error.Fail())
return error;
- // Calculate totalsize including the current offset.
+ // Start by saving all of the stacks and ensuring they fit under the 32b
+ // limit.
uint64_t total_size = GetCurrentDataEndOffset();
- total_size += ranges_32.size() * sizeof(llvm::minidump::MemoryDescriptor);
- std::unordered_set<addr_t> stack_start_addresses;
- for (const auto &core_range : ranges_32) {
- stack_start_addresses.insert(core_range.range.start());
- total_size += core_range.range.size();
+ auto iterator = all_core_memory_ranges.begin();
+ while (iterator != all_core_memory_ranges.end()) {
+ if (m_saved_stack_ranges.count(iterator->range.start()) > 0) {
+ // We don't save stacks twice.
+ ranges_32.push_back(*iterator);
+ total_size +=
+ iterator->range.size() + sizeof(llvm::minidump::MemoryDescriptor);
+ iterator = all_core_memory_ranges.erase(iterator);
+ } else {
+ iterator++;
+ }
}
if (total_size >= UINT32_MAX) {
@@ -848,13 +854,6 @@ Status MinidumpFileBuilder::AddMemoryList() {
return error;
}
- Process::CoreFileMemoryRanges all_core_memory_ranges;
- if (m_options.GetStyle() != SaveCoreStyle::eSaveCoreStackOnly) {
- error = m_process_sp->CalculateCoreFileSaveRanges(all_core_memory_ranges, m_options);
- if (error.Fail())
- return error;
- }
-
// After saving the stacks, we start packing as much as we can into 32b.
// We apply a generous padding here so that the Directory, MemoryList and
// Memory64List sections all begin in 32b addressable space.
@@ -862,16 +861,13 @@ Status MinidumpFileBuilder::AddMemoryList() {
// All core memeroy ranges will either container nothing on stacks only
// or all the memory ranges including stacks
if (!all_core_memory_ranges.empty())
- total_size +=
- 256 + (all_core_memory_ranges.size() - stack_start_addresses.size()) *
- sizeof(llvm::minidump::MemoryDescriptor_64);
+ total_size += 256 + (all_core_memory_ranges.size() *
+ sizeof(llvm::minidump::MemoryDescriptor_64));
for (const auto &core_range : all_core_memory_ranges) {
const addr_t range_size = core_range.range.size();
- if (stack_start_addresses.count(core_range.range.start()) > 0)
- // Don't double save stacks.
- continue;
-
+ // 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) {
ranges_32.push_back(core_range);
total_size += range_size;
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 690b5daa78a8b3..762de83db5a39c 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -76,9 +76,9 @@ class MinidumpFileBuilder {
public:
MinidumpFileBuilder(lldb::FileUP &&core_file,
const lldb::ProcessSP &process_sp,
- const lldb_private::SaveCoreOptions &options)
- : m_process_sp(process_sp), m_options(std::move(options)),
- m_core_file(std::move(core_file)) {};
+ lldb_private::SaveCoreOptions &save_core_options)
+ : m_process_sp(process_sp), m_core_file(std::move(core_file)),
+ m_save_core_options(save_core_options){}
MinidumpFileBuilder(const MinidumpFileBuilder &) = delete;
MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete;
@@ -165,8 +165,9 @@ class MinidumpFileBuilder {
// to duplicate it in the exception data.
std::unordered_map<lldb::tid_t, llvm::minidump::LocationDescriptor>
m_tid_to_reg_ctx;
- lldb_private::SaveCoreOptions m_options;
+ std::unordered_set<lldb::addr_t> m_saved_stack_ranges;
lldb::FileUP m_core_file;
+ lldb_private::SaveCoreOptions m_save_core_options;
};
#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_MINIDUMPFILEBUILDER_H
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index c0984573a3ddb1..0897895e6bc25d 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -56,12 +56,16 @@ size_t ObjectFileMinidump::GetModuleSpecifications(
}
bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
- const lldb_private::SaveCoreOptions &options,
+ lldb_private::SaveCoreOptions &options,
lldb_private::Status &error) {
// Output file and process_sp are both checked in PluginManager::SaveCore.
assert(options.GetOutputFile().has_value());
assert(process_sp);
+ // Minidump defaults to stacks only.
+ if (options.GetStyle() == SaveCoreStyle::eSaveCoreUnspecified)
+ options.SetStyle(SaveCoreStyle::eSaveCoreStackOnly);
+
llvm::Expected<lldb::FileUP> maybe_core_file = FileSystem::Instance().Open(
options.GetOutputFile().value(),
File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate);
@@ -69,7 +73,8 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
error = maybe_core_file.takeError();
return false;
}
- MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp, options);
+ MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp,
+ options);
Log *log = GetLog(LLDBLog::Object);
error = builder.AddHeaderAndCalculateDirectories();
@@ -78,41 +83,41 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
error.AsCString());
return false;
};
- // error = builder.AddSystemInfo();
- // if (error.Fail()) {
- // LLDB_LOGF(log, "AddSystemInfo failed: %s", error.AsCString());
- // return false;
- // }
-
- // error = builder.AddModuleList();
- // if (error.Fail()) {
- // LLDB_LOGF(log, "AddModuleList failed: %s", error.AsCString());
- // return false;
- // }
- // error = builder.AddMiscInfo();
- // if (error.Fail()) {
- // LLDB_LOGF(log, "AddMiscInfo failed: %s", error.AsCString());
- // return false;
- // }
-
- // error = builder.AddThreadList();
- // if (error.Fail()) {
- // LLDB_LOGF(log, "AddThreadList failed: %s", error.AsCString());
- // return false;
- // }
-
- // error = builder.AddLinuxFileStreams();
- // if (error.Fail()) {
- // LLDB_LOGF(log, "AddLinuxFileStreams failed: %s", error.AsCString());
- // return false;
- // }
-
- // // Add any exceptions but only if there are any in any threads.
- // error = builder.AddExceptions();
- // if (error.Fail()) {
- // LLDB_LOGF(log, "AddExceptions failed: %s", error.AsCString());
- // return false;
- // }
+ error = builder.AddSystemInfo();
+ if (error.Fail()) {
+ LLDB_LOGF(log, "AddSystemInfo failed: %s", error.AsCString());
+ return false;
+ }
+
+ error = builder.AddModuleList();
+ if (error.Fail()) {
+ LLDB_LOGF(log, "AddModuleList failed: %s", error.AsCString());
+ return false;
+ }
+ error = builder.AddMiscInfo();
+ if (error.Fail()) {
+ LLDB_LOGF(log, "AddMiscInfo failed: %s", error.AsCString());
+ return false;
+ }
+
+ error = builder.AddThreadList();
+ if (error.Fail()) {
+ LLDB_LOGF(log, "AddThreadList failed: %s", error.AsCString());
+ return false;
+ }
+
+ error = builder.AddLinuxFileStreams();
+ if (error.Fail()) {
+ LLDB_LOGF(log, "AddLinuxFileStreams failed: %s", error.AsCString());
+ return false;
+ }
+
+ // Add any exceptions but only if there are any in any threads.
+ error = builder.AddExceptions();
+ if (error.Fail()) {
+ LLDB_LOGF(log, "AddExceptions failed: %s", error.AsCString());
+ return false;
+ }
// Note: add memory HAS to be the last thing we do. It can overflow into 64b
// land and many RVA's only support 32b
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp
index 6bf6d7c824673f..f1eda8620f96ae 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -8,6 +8,8 @@
#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Core/PluginManager.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Thread.h"
using namespace lldb;
using namespace lldb_private;
@@ -46,6 +48,59 @@ SaveCoreOptions::GetOutputFile() const {
return m_file;
}
+Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
+ Status error;
+ if (!process_sp) {
+ ClearProcessSpecificData();
+ m_process_sp.reset();
+ return error;
+ }
+
+ if (!process_sp->IsValid()) {
+ error.SetErrorString("Cannot assign an invalid process.");
+ return error;
+ }
+
+ // Don't clear any process specific data if the process is the same.
+ if (m_process_sp == process_sp)
+ return error;
+
+ ClearProcessSpecificData();
+ m_process_sp = process_sp;
+ return error;
+}
+
+Status SaveCoreOptions::AddThread(lldb::ThreadSP thread_sp) {
+ Status error;
+ if (!thread_sp) {
+ error.SetErrorString("invalid thread");
+ return error;
+ }
+
+ if (m_process_sp) {
+ if (m_process_sp != thread_sp->GetProcess()) {
+ error.SetErrorString("Cannot add a thread from a different process.");
+ return error;
+ }
+ } else {
+ m_process_sp = thread_sp->GetProcess();
+ }
+
+ m_threads_to_save.insert(thread_sp->GetID());
+ return error;
+}
+
+bool SaveCoreOptions::RemoveThread(lldb::ThreadSP thread_sp) {
+ return thread_sp && m_threads_to_save.erase(thread_sp->GetID()) > 0;
+}
+
+bool SaveCoreOptions::ShouldThreadBeSaved(lldb::tid_t tid) const {
+ // If the user specified no threads to save, then we save all threads.
+ if (m_threads_to_save.empty())
+ return true;
+ return m_threads_to_save.count(tid) > 0;
+}
+
Status SaveCoreOptions::AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion) {
Status error;
m_regions_to_save.insert(region.GetRange().GetRangeBase());
@@ -56,8 +111,33 @@ bool SaveCoreOptions::ShouldSaveRegion(const lldb_private::MemoryRegionInfo ®
return m_regions_to_save.empty() || m_regions_to_save.count(region.GetRange().GetRangeBase()) > 0;
}
+Status SaveCoreOptions::EnsureValidConfiguration(
+ lldb::ProcessSP process_sp) const {
+ Status error;
+ std::string error_str;
+ if (!m_threads_to_save.empty() && GetStyle() == lldb::eSaveCoreFull)
+ error_str += "Cannot save a full core with a subset of threads\n";
+
+ if (m_process_sp && m_process_sp != process_sp)
+ error_str += "Cannot save core for process using supplied core options. "
+ "Options were constructed targeting a different process. \n";
+
+ if (!error_str.empty())
+ error.SetErrorString(error_str);
+
+ return error;
+}
+
+void SaveCoreOptions::ClearProcessSpecificData() {
+ // Deliberately not following the formatter style here to indicate that
+ // this method will be expanded in the future.
+ m_threads_to_save.clear();
+}
+
void SaveCoreOptions::Clear() {
m_file = std::nullopt;
m_plugin_name = std::nullopt;
m_style = std::nullopt;
+ m_threads_to_save.clear();
+ m_process_sp.reset();
}
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 07b7f4feef8800..713ef42714009b 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -473,11 +473,13 @@ Process::Process(lldb::TargetSP target_sp, ListenerSP listener_sp,
m_memory_cache(*this), m_allocated_memory_cache(*this),
m_should_detach(false), m_next_event_action_up(), m_public_run_lock(),
m_private_run_lock(), m_currently_handling_do_on_removals(false),
- m_resume_requested(false), m_finalizing(false), m_destructing(false),
+ m_resume_requested(false), m_interrupt_tid(LLDB_INVALID_THREAD_ID),
+ m_finalizing(false), m_destructing(false),
m_clear_thread_plans_on_stop(false), m_force_next_event_delivery(false),
m_last_broadcast_state(eStateInvalid), m_destroy_in_process(false),
m_can_interpret_function_calls(false), m_run_thread_plan_lock(),
- m_can_jit(eCanJITDontKnow) {
+ m_can_jit(eCanJITDontKnow),
+ m_crash_info_dict_sp(new StructuredData::Dictionary()) {
CheckInWithManager();
Log *log = GetLog(LLDBLog::Object);
@@ -894,6 +896,7 @@ bool Process::HandleProcessStateChangedEvent(
case eStopReasonThreadExiting:
case eStopReasonInstrumentation:
case eStopReasonProcessorTrace:
+ case eStopReasonInterrupt:
if (!other_thread)
other_thread = thread;
break;
@@ -3263,6 +3266,10 @@ Status Process::PrivateResume() {
// If signals handing status changed we might want to update our signal
// filters before resuming.
UpdateAutomaticSignalFiltering();
+ // Clear any crash info we accumulated for this stop, but don't do so if we
+ // are running functions; we don't want to wipe out the real stop's info.
+ if (!GetModID().IsLastResumeForUserExpression())
+ ResetExtendedCrashInfoDict();
Status error(WillResume());
// Tell the process it is about to resume before the thread list
@@ -3868,7 +3875,11 @@ void Process::ControlPrivateStateThread(uint32_t signal) {
}
}
-void Process::SendAsyncInterrupt() {
+void Process::SendAsyncInterrupt(Thread *thread) {
+ if (thread != nullptr)
+ m_interrupt_tid = thread->GetProtocolID();
+ else
+ m_interrupt_tid = LLDB_INVALID_THREAD_ID;
if (PrivateStateThreadIsValid())
m_private_state_broadcaster.BroadcastEvent(Process::eBroadcastBitInterrupt,
nullptr);
@@ -4094,9 +4105,14 @@ thread_result_t Process::RunPrivateStateThread(bool is_secondary_thread) {
if (interrupt_requested) {
if (StateIsStoppedState(internal_state, true)) {
- // We requested the interrupt, so mark this as such in the stop event
- // so clients can tell an interrupted process from a natural stop
- ProcessEventData::SetInterruptedInEvent(event_sp.get(), true);
+ // Only mark interrupt event if it is not thread specific async
+ // interrupt.
+ if (m_interrupt_tid == LLDB_INVALID_THREAD_ID) {
+ // We requested the interrupt, so mark this as such in the stop
+ // event so clients can tell an interrupted process from a natural
+ // stop
+ ProcessEventData::SetInterruptedInEvent(event_sp.get(), true);
+ }
interrupt_requested = false;
} else if (log) {
LLDB_LOGF(log,
@@ -6518,24 +6534,27 @@ static bool AddDirtyPages(const MemoryRegionInfo ®ion,
// given region. If the region has dirty page information, only dirty pages
// will be added to \a ranges, else the entire range will be added to \a
// ranges.
-static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages,
- Process::CoreFileMemoryRanges &ranges, const SaveCoreOptions &options) {
+static void AddRegion(const MemoryRegionInfo ®ion,
+ const SaveCoreOptions &options,
+ bool try_dirty_pages,
+ Process::CoreFileMemoryRanges &ranges) {
// Don't add empty ranges.
if (region.GetRange().GetByteSize() == 0)
return;
// Don't add ranges with no read permissions.
if ((region.GetLLDBPermissions() & lldb::ePermissionsReadable) == 0)
return;
- if (try_dirty_pages && AddDirtyPages(region, ranges))
- return;
if (!options.ShouldSaveRegion(region))
return;
+ if (try_dirty_pages && AddDirtyPages(region, ranges))
+ return;
ranges.push_back(CreateCoreFileMemoryRange(region));
}
static void SaveOffRegionsWithStackPointers(
- Process &process, const MemoryRegionInfos ®ions,
- Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends, const SaveCoreOptions &options) {
+ Process &process, const SaveCoreOptions &core_options,
+ const MemoryRegionInfos ®ions, Process::CoreFileMemoryRanges &ranges,
+ std::set<addr_t> &stack_ends) {
const bool try_dirty_pages = true;
// Before we take any dump, we want to save off the used portions of the
@@ -6557,10 +6576,16 @@ static void SaveOffRegionsWithStackPointers(
if (process.GetMemoryRegionInfo(sp, sp_region).Success()) {
const size_t stack_head = (sp - red_zone);
const size_t stack_size = sp_region.GetRange().GetRangeEnd() - stack_head;
+ // Even if the SaveCoreOption doesn't want us to save the stack
+ // we still need to populate the stack_ends set so it doesn't get saved
+ // off in other calls
sp_region.GetRange().SetRangeBase(stack_head);
sp_region.GetRange().SetByteSize(stack_size);
stack_ends.insert(sp_region.GetRange().GetRangeEnd());
- AddRegion(sp_region, try_dirty_pages, ranges, options);
+ // This will return true if the threadlist the user specified is empty,
+ // or contains the thread id from thread_sp.
+ if (core_options.ShouldThreadBeSaved(thread_sp->GetID()))
+ AddRegion(sp_region, core_options, try_dirty_pages, ranges);
}
}
}
@@ -6569,15 +6594,15 @@ static void SaveOffRegionsWithStackPointers(
// for a full core file style.
static void GetCoreFileSaveRangesFull(Process &process,
const MemoryRegionInfos ®ions,
+ const SaveCoreOptions &core_options,
Process::CoreFileMemoryRanges &ranges,
- std::set<addr_t> &stack_ends,
- const SaveCoreOptions &options) {
+ std::set<addr_t> &stack_ends) {
- // Don't add only dirty pages, add full regions.
+// Don't add only dirty pages, add full regions.
const bool try_dirty_pages = false;
for (const auto ®ion : regions)
if (stack_ends.count(region.GetRange().GetRangeEnd()) == 0)
- AddRegion(region, try_dirty_pages, ranges, options);
+ AddRegion(region, core_options, try_dirty_pages, ranges);
}
// Save only the dirty pages to the core file. Make sure the process has at
@@ -6586,8 +6611,8 @@ const bool try_dirty_pages = false;
// page information fall back to saving out all ranges with write permissions.
static void GetCoreFileSaveRangesDirtyOnly(
Process &process, const MemoryRegionInfos ®ions,
- Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends,
- const SaveCoreOptions &options) {
+ const SaveCoreOptions &core_options,
+ Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
// Iterate over the regions and find all dirty pages.
bool have_dirty_page_info = false;
@@ -6604,7 +6629,7 @@ static void GetCoreFileSaveRangesDirtyOnly(
for (const auto ®ion : regions)
if (stack_ends.count(region.GetRange().GetRangeEnd()) == 0 &&
region.GetWritable() == MemoryRegionInfo::eYes)
- AddRegion(region, try_dirty_pages, ranges, options);
+ AddRegion(region, core_options, try_dirty_pages, ranges);
}
}
@@ -6618,8 +6643,8 @@ static void GetCoreFileSaveRangesDirtyOnly(
// stack region.
static void GetCoreFileSaveRangesStackOnly(
Process &process, const MemoryRegionInfos ®ions,
- Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends,
- const SaveCoreOptions &options) {
+ const SaveCoreOptions &core_options,
+ Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
const bool try_dirty_pages = true;
// Some platforms support annotating the region information that tell us that
// it comes from a thread stack. So look for those regions first.
@@ -6628,12 +6653,12 @@ static void GetCoreFileSaveRangesStackOnly(
// Save all the stack memory ranges not associated with a stack pointer.
if (stack_ends.count(region.GetRange().GetRangeEnd()) == 0 &&
region.IsStackMemory() == MemoryRegionInfo::eYes)
- AddRegion(region, try_dirty_pages, ranges, options);
+ AddRegion(region, core_options, try_dirty_pages, ranges);
}
}
-Status Process::CalculateCoreFileSaveRanges(CoreFileMemoryRanges &ranges,
- const SaveCoreOptions &options) {
+Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
+ CoreFileMemoryRanges &ranges) {
lldb_private::MemoryRegionInfos regions;
Status err = GetMemoryRegions(regions);
SaveCoreStyle core_style = options.GetStyle();
@@ -6646,22 +6671,22 @@ Status Process::CalculateCoreFileSaveRanges(CoreFileMemoryRanges &ranges,
"eSaveCoreUnspecified");
std::set<addr_t> stack_ends;
- SaveOffRegionsWithStackPointers(*this, regions, ranges, stack_ends, options);
+ SaveOffRegionsWithStackPointers(*this, options, regions, ranges, stack_ends);
switch (core_style) {
case eSaveCoreUnspecified:
break;
case eSaveCoreFull:
- GetCoreFileSaveRangesFull(*this, regions, ranges, stack_ends, options);
+ GetCoreFileSaveRangesFull(*this, regions, options, ranges, stack_ends);
break;
case eSaveCoreDirtyOnly:
- GetCoreFileSaveRangesDirtyOnly(*this, regions, ranges, stack_ends, options);
+ GetCoreFileSaveRangesDirtyOnly(*this, regions, options, ranges, stack_ends);
break;
case eSaveCoreStackOnly:
- GetCoreFileSaveRangesStackOnly(*this, regions, ranges, stack_ends, options);
+ GetCoreFileSaveRangesStackOnly(*this, regions, options, ranges, stack_ends);
break;
}
@@ -6674,6 +6699,18 @@ Status Process::CalculateCoreFileSaveRanges(CoreFileMemoryRanges &ranges,
return Status(); // Success!
}
+std::vector<ThreadSP>
+Process::CalculateCoreFileThreadList(const SaveCoreOptions &core_options) {
+ std::vector<ThreadSP> thread_list;
+ for (const lldb::ThreadSP &thread_sp : m_thread_list.Threads()) {
+ if (core_options.ShouldThreadBeSaved(thread_sp->GetID())) {
+ thread_list.push_back(thread_sp);
+ }
+ }
+
+ return thread_list;
+}
+
void Process::SetAddressableBitMasks(AddressableBits bit_masks) {
uint32_t low_memory_addr_bits = bit_masks.GetLowmemAddressableBits();
uint32_t high_memory_addr_bits = bit_masks.GetHighmemAddressableBits();
>From 5dc8e04740ebef848e31692aa751ac163a672413 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 16 Aug 2024 14:20:42 -0700
Subject: [PATCH 3/9] Add some comments to explain why we're using page bases,
and then change the interal api to void
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 9 ++++
lldb/include/lldb/Symbol/SaveCoreOptions.h | 2 +-
lldb/source/API/SBSaveCoreOptions.cpp | 7 ++-
lldb/source/Symbol/SaveCoreOptions.cpp | 18 ++++++--
.../TestProcessSaveCoreMinidump.py | 46 +++++++++++++++++++
5 files changed, 75 insertions(+), 7 deletions(-)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h
index 5727a9af7395f3..1302863de51b6f 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -79,6 +79,15 @@ class LLDB_API SBSaveCoreOptions {
/// \param thread The thread to remove.
/// \return True if the thread was removed, false if it was not in the list.
bool RemoveThread(lldb::SBThread thread);
+
+ /// Add a memory region to save in the core file.
+ ///
+ /// \param region The memory region to save.
+ /// \returns An empty SBStatus upon success, or an error if the region is
+ /// invalid.
+ /// \note This supercedes SaveCoreStyle, and if this list is set
+ /// only regions within this list will be saved. If the list is empty
+ /// all regions will be saved according to style.
SBError AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion);
/// Reset all options.
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index ef747920e2dc58..ab5e06faabca89 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -42,8 +42,8 @@ class SaveCoreOptions {
bool ShouldThreadBeSaved(lldb::tid_t tid) const;
Status EnsureValidConfiguration(lldb::ProcessSP process_sp) const;
- Status AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion);
+ void AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion);
bool ShouldSaveRegion(const lldb_private::MemoryRegionInfo ®ion) const;
void Clear();
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp
index 323148807d90ca..0dc10ae800ed36 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -95,8 +95,11 @@ bool SBSaveCoreOptions::RemoveThread(lldb::SBThread thread) {
lldb::SBError SBSaveCoreOptions::AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion) {
LLDB_INSTRUMENT_VA(this, region);
- lldb_private::Status error = m_opaque_up->AddMemoryRegionToSave(region.ref());
- return SBError(error);
+ // Currently add memory region can't fail, so we always return a success
+ // SBerror, but because these API's live forever, this is the most future
+ // proof thing to do.
+ m_opaque_up->AddMemoryRegionToSave(region.ref());
+ return SBError();
}
void SBSaveCoreOptions::Clear() {
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp
index f1eda8620f96ae..81717e91baba83 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -31,7 +31,10 @@ Status SaveCoreOptions::SetPluginName(const char *name) {
return error;
}
-void SaveCoreOptions::SetStyle(lldb::SaveCoreStyle style) { m_style = style; }
+void SaveCoreOptions::SetStyle(lldb::SaveCoreStyle style) {
+ if (!m_regions_to_save.empty())
+ m_style = style;
+}
void SaveCoreOptions::SetOutputFile(FileSpec file) { m_file = file; }
@@ -101,10 +104,17 @@ bool SaveCoreOptions::ShouldThreadBeSaved(lldb::tid_t tid) const {
return m_threads_to_save.count(tid) > 0;
}
-Status SaveCoreOptions::AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion) {
- Status error;
+void SaveCoreOptions::AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion) {
+ // Currently we just insert memory regions into the map by their base
+ // This is probably overly simple, but adding ranges of memory introduces
+ // a lot of potential complexity, such as if the range to save is smaller
+ // than the range being evaluated, but both start at the same address
+ // should we truncate or save as is?
+ // To avoid that we simply save off by their base.
m_regions_to_save.insert(region.GetRange().GetRangeBase());
- return error;
+ // Because this list is exclusive, if this map is inserted into, we override
+ // the style with core style full.
+ m_style = lldb::eSaveCoreFull;
}
bool SaveCoreOptions::ShouldSaveRegion(const lldb_private::MemoryRegionInfo ®ion) const {
diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index 5abaa05a90f63e..c54d821b6979bc 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -300,3 +300,49 @@ def test_save_linux_mini_dump_default_options(self):
self.assertTrue(self.dbg.DeleteTarget(target))
if os.path.isfile(default_value_file):
os.unlink(default_value_file)
+
+ @skipUnlessArch("x86_64")
+ @skipUnlessPlatform(["linux"])
+ def test_save_linux_minidump_one_region(self):
+ """Test that we can save a Linux mini dump with one region in sbsavecore regions"""
+
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ default_value_file = self.getBuildArtifact("core.defaults.dmp")
+ try:
+ target = self.dbg.CreateTarget(exe)
+ process = target.LaunchSimple(
+ None, None, self.get_process_working_directory()
+ )
+ self.assertState(process.GetState(), lldb.eStateStopped)
+
+ memory_region = lldb.SBMemoryRegionInfo()
+ memory_list = process.GetMemoryRegions()
+ memory_list.GetMemoryRegionAtIndex(0, memory_region)
+
+
+ # This is almost identical to the single thread test case because
+ # minidump defaults to stacks only, so we want to see if the
+ # default options work as expected.
+ options = lldb.SBSaveCoreOptions()
+ default_value_spec = lldb.SBFileSpec(default_value_file)
+ options.SetOutputFile(default_value_spec)
+ options.SetPluginName("minidump")
+ options.AddMemoryRegionToSave(memory_region)
+ error = process.SaveCore(options)
+ print (f"Error: {error.GetCString()}")
+ self.assertTrue(error.Success(), error.GetCString())
+
+ core_target = self.dbg.CreateTarget(None)
+ core_proc = target.LoadCore(default_value_file)
+ core_memory_list = core_proc.GetMemoryRegions()
+ self.assertEqual(core_memory_list.GetSize(), 1)
+ core_memory_region = lldb.SBMemoryRegionInfo()
+ core_memory_list.GetMemoryRegionAtIndex(0, core_memory_region)
+ self.assertEqual(core_memory_region.GetRegionBase(), memory_region.GetRegionBase())
+ self.assertEqual(core_memory_region.GetRegionEnd(), memory_region.GetRegionEnd())
+
+ finally:
+ self.assertTrue(self.dbg.DeleteTarget(target))
+ if os.path.isfile(default_value_file):
+ os.unlink(default_value_file)
>From 001ed7aa004480e052d5aecf29c7ce8132c60d24 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 20 Aug 2024 14:50:43 -0700
Subject: [PATCH 4/9] Completely change the design for memory regions. Make the
list inclusive to save core by default. Create a custom save core enum to
allow a 'user-only' core dump. Add a check on the save core options for
threads so we can skip thread minification if it's custom and there is only
memory ranges. Needed to extend RangeVector for my use case, and fix where we
have SaveCoreOptions in the lldb private headers
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 7 ++-
lldb/include/lldb/Symbol/SaveCoreOptions.h | 12 ++--
lldb/include/lldb/Target/Process.h | 3 +-
lldb/include/lldb/Utility/RangeMap.h | 4 ++
lldb/include/lldb/lldb-enumerations.h | 1 +
lldb/include/lldb/lldb-forward.h | 1 +
lldb/include/lldb/lldb-private-interfaces.h | 1 -
lldb/source/API/SBSaveCoreOptions.cpp | 1 +
lldb/source/Commands/CommandObjectProcess.cpp | 1 +
.../ObjectFile/Mach-O/ObjectFileMachO.cpp | 4 +-
.../ObjectFile/Mach-O/ObjectFileMachO.h | 1 +
.../Minidump/MinidumpFileBuilder.cpp | 28 ++++++----
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 5 +-
.../ObjectFile/Minidump/ObjectFileMinidump.h | 1 +
.../ObjectFile/PECOFF/ObjectFilePECOFF.cpp | 1 +
.../ObjectFile/PECOFF/ObjectFilePECOFF.h | 1 +
lldb/source/Symbol/SaveCoreOptions.cpp | 23 +++-----
lldb/source/Target/Process.cpp | 56 +++++++++++++------
.../TestProcessSaveCoreMinidump.py | 54 +++++++++++++++---
19 files changed, 142 insertions(+), 63 deletions(-)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h
index 1302863de51b6f..d14ffe023188a2 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -85,9 +85,10 @@ class LLDB_API SBSaveCoreOptions {
/// \param region The memory region to save.
/// \returns An empty SBStatus upon success, or an error if the region is
/// invalid.
- /// \note This supercedes SaveCoreStyle, and if this list is set
- /// only regions within this list will be saved. If the list is empty
- /// all regions will be saved according to style.
+ /// \note Ranges that overlapped with be unioned into a single region this
+ /// also supercedes stack minification. Specifying full regions and a
+ /// non-custom core style will include the specified regions and union them
+ /// with all style specific regions.
SBError AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion);
/// Reset all options.
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index ab5e06faabca89..a95686ada96a79 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -9,16 +9,17 @@
#ifndef LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
#define LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H
-#include "lldb/Core/AddressRange.h"
#include "lldb/Utility/FileSpec.h"
-#include "lldb/lldb-forward.h"
-#include "lldb/lldb-types.h"
+#include "lldb/Utility/RangeMap.h"
+
#include <optional>
#include <string>
#include <unordered_set>
#include <set>
+using MemoryRanges = lldb_private::RangeVector<lldb::addr_t, lldb::addr_t>;
+
namespace lldb_private {
class SaveCoreOptions {
@@ -40,11 +41,12 @@ class SaveCoreOptions {
Status AddThread(lldb::ThreadSP thread_sp);
bool RemoveThread(lldb::ThreadSP thread_sp);
bool ShouldThreadBeSaved(lldb::tid_t tid) const;
+ bool HasSpecifiedThreads() const;
Status EnsureValidConfiguration(lldb::ProcessSP process_sp) const;
+ const MemoryRanges& GetCoreFileMemoryRanges() const;
void AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion);
- bool ShouldSaveRegion(const lldb_private::MemoryRegionInfo ®ion) const;
void Clear();
@@ -56,7 +58,7 @@ class SaveCoreOptions {
std::optional<lldb::SaveCoreStyle> m_style;
lldb::ProcessSP m_process_sp;
std::unordered_set<lldb::tid_t> m_threads_to_save;
- std::set<lldb::addr_t> m_regions_to_save;
+ MemoryRanges m_regions_to_save;
};
} // namespace lldb_private
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 9cf6f50558570b..73228f13dc6715 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -43,6 +43,7 @@
#include "lldb/Target/ThreadList.h"
#include "lldb/Target/ThreadPlanStack.h"
#include "lldb/Target/Trace.h"
+#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Utility/AddressableBits.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/Broadcaster.h"
@@ -733,7 +734,7 @@ class Process : public std::enable_shared_from_this<Process>,
}
};
- using CoreFileMemoryRanges = std::vector<CoreFileMemoryRange>;
+ using CoreFileMemoryRanges = lldb_private::RangeDataVector<lldb::addr_t, lldb::addr_t, CoreFileMemoryRange>;
/// Helper function for Process::SaveCore(...) that calculates the address
/// ranges that should be saved. This allows all core file plug-ins to save
diff --git a/lldb/include/lldb/Utility/RangeMap.h b/lldb/include/lldb/Utility/RangeMap.h
index 8cc382bcc046ce..dacc638c193a10 100644
--- a/lldb/include/lldb/Utility/RangeMap.h
+++ b/lldb/include/lldb/Utility/RangeMap.h
@@ -449,6 +449,10 @@ class RangeDataVector {
~RangeDataVector() = default;
void Append(const Entry &entry) { m_entries.emplace_back(entry); }
+
+ void Append(const B&& b, const S&& s, const T&& t) {
+ m_entries.emplace_back(Entry(b, s, t));
+ }
bool Erase(uint32_t start, uint32_t end) {
if (start >= end || end > m_entries.size())
diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h
index 7bfde8b9de1271..768045c72280a0 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -1222,6 +1222,7 @@ enum SaveCoreStyle {
eSaveCoreFull = 1,
eSaveCoreDirtyOnly = 2,
eSaveCoreStackOnly = 3,
+ eSaveCoreCustom = 4,
};
/// Events that might happen during a trace session.
diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h
index 337eff696fcf3f..5fb288ad43af48 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -207,6 +207,7 @@ class StackFrameRecognizer;
class StackFrameRecognizerManager;
class StackID;
class Status;
+class SaveCoreOptions;
class StopInfo;
class Stoppoint;
class StoppointCallbackContext;
diff --git a/lldb/include/lldb/lldb-private-interfaces.h b/lldb/include/lldb/lldb-private-interfaces.h
index b3c8cda899b95e..5bac5cd3e86b59 100644
--- a/lldb/include/lldb/lldb-private-interfaces.h
+++ b/lldb/include/lldb/lldb-private-interfaces.h
@@ -9,7 +9,6 @@
#ifndef LLDB_LLDB_PRIVATE_INTERFACES_H
#define LLDB_LLDB_PRIVATE_INTERFACES_H
-#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-forward.h"
#include "lldb/lldb-private-enumerations.h"
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp
index 0dc10ae800ed36..03d237d3499727 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -93,6 +93,7 @@ bool SBSaveCoreOptions::RemoveThread(lldb::SBThread thread) {
return m_opaque_up->RemoveThread(thread.GetSP());
}
+
lldb::SBError SBSaveCoreOptions::AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion) {
LLDB_INSTRUMENT_VA(this, region);
// Currently add memory region can't fail, so we always return a success
diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp
index 28413b7a8591f6..398eb574f7d7c9 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -31,6 +31,7 @@
#include "lldb/Target/Target.h"
#include "lldb/Target/Thread.h"
#include "lldb/Target/UnixSignals.h"
+#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Utility/Args.h"
#include "lldb/Utility/ScriptedMetadata.h"
#include "lldb/Utility/State.h"
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 22ece4f4dacf79..f9e1fa1b3b5fda 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6567,7 +6567,9 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
const uint32_t addr_byte_size = target_arch.GetAddressByteSize();
const ByteOrder byte_order = target_arch.GetByteOrder();
std::vector<llvm::MachO::segment_command_64> segment_load_commands;
- for (const auto &core_range : core_ranges) {
+ for (const auto &core_range_info : core_ranges) {
+ // TODO: Refactor RangeDataVector to have a data iterator.
+ const auto &core_range = core_range_info.data;
uint32_t cmd_type = LC_SEGMENT_64;
uint32_t segment_size = sizeof(llvm::MachO::segment_command_64);
if (addr_byte_size == 4) {
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
index 27bc237aaac48d..be87112df7d898 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
@@ -12,6 +12,7 @@
#include "lldb/Core/Address.h"
#include "lldb/Host/SafeMachO.h"
#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/FileSpecList.h"
#include "lldb/Utility/RangeMap.h"
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index c0cc3af638a777..fb323b78735499 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -823,25 +823,31 @@ Status MinidumpFileBuilder::AddMemoryList() {
// 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
// in accessible with a 32 bit offset.
- Process::CoreFileMemoryRanges ranges_32;
- Process::CoreFileMemoryRanges ranges_64;
+ std::vector<Process::CoreFileMemoryRange> ranges_32;
+ std::vector<Process::CoreFileMemoryRange> ranges_64;
Process::CoreFileMemoryRanges all_core_memory_ranges;
error = m_process_sp->CalculateCoreFileSaveRanges(m_save_core_options,
all_core_memory_ranges);
+
+ std::vector<Process::CoreFileMemoryRange> all_core_memory_vec;
+ // Extract all the data into just a vector of data. So we can mutate this in place.
+ for (const auto &core_range : all_core_memory_ranges)
+ all_core_memory_vec.push_back(core_range.data);
+
if (error.Fail())
return error;
// Start by saving all of the stacks and ensuring they fit under the 32b
// limit.
uint64_t total_size = GetCurrentDataEndOffset();
- auto iterator = all_core_memory_ranges.begin();
- while (iterator != all_core_memory_ranges.end()) {
+ auto iterator = all_core_memory_vec.begin();
+ while (iterator != all_core_memory_vec.end()) {
if (m_saved_stack_ranges.count(iterator->range.start()) > 0) {
// We don't save stacks twice.
ranges_32.push_back(*iterator);
total_size +=
iterator->range.size() + sizeof(llvm::minidump::MemoryDescriptor);
- iterator = all_core_memory_ranges.erase(iterator);
+ iterator = all_core_memory_vec.erase(iterator);
} else {
iterator++;
}
@@ -860,11 +866,11 @@ Status MinidumpFileBuilder::AddMemoryList() {
// Then anything overflow extends into 64b addressable space.
// All core memeroy ranges will either container nothing on stacks only
// or all the memory ranges including stacks
- if (!all_core_memory_ranges.empty())
- total_size += 256 + (all_core_memory_ranges.size() *
+ if (!all_core_memory_vec.empty())
+ total_size += 256 + (all_core_memory_vec.size() *
sizeof(llvm::minidump::MemoryDescriptor_64));
- for (const auto &core_range : all_core_memory_ranges) {
+ for (const auto &core_range : all_core_memory_vec) {
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.
@@ -949,7 +955,7 @@ Status MinidumpFileBuilder::DumpDirectories() const {
}
static uint64_t
-GetLargestRangeSize(const Process::CoreFileMemoryRanges &ranges) {
+GetLargestRangeSize(const std::vector<Process::CoreFileMemoryRange> &ranges) {
uint64_t max_size = 0;
for (const auto &core_range : ranges)
max_size = std::max(max_size, core_range.range.size());
@@ -957,7 +963,7 @@ GetLargestRangeSize(const Process::CoreFileMemoryRanges &ranges) {
}
Status
-MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges &ranges) {
+MinidumpFileBuilder::AddMemoryList_32(std::vector<Process::CoreFileMemoryRange> &ranges) {
std::vector<MemoryDescriptor> descriptors;
Status error;
if (ranges.size() == 0)
@@ -1032,7 +1038,7 @@ MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges &ranges) {
}
Status
-MinidumpFileBuilder::AddMemoryList_64(Process::CoreFileMemoryRanges &ranges) {
+MinidumpFileBuilder::AddMemoryList_64(std::vector<Process::CoreFileMemoryRange> &ranges) {
Status error;
if (ranges.empty())
return error;
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 762de83db5a39c..195650de317178 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -25,6 +25,7 @@
#include "lldb/Target/Process.h"
#include "lldb/Target/Target.h"
+#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/Status.h"
#include "lldb/lldb-forward.h"
@@ -120,9 +121,9 @@ class MinidumpFileBuilder {
lldb_private::Status AddData(const void *data, uint64_t size);
// Add MemoryList stream, containing dumps of important memory segments
lldb_private::Status
- AddMemoryList_64(lldb_private::Process::CoreFileMemoryRanges &ranges);
+ AddMemoryList_64(std::vector<lldb_private::Process::CoreFileMemoryRange> &ranges);
lldb_private::Status
- AddMemoryList_32(lldb_private::Process::CoreFileMemoryRanges &ranges);
+ AddMemoryList_32(std::vector<lldb_private::Process::CoreFileMemoryRange> &ranges);
// Update the thread list on disk with the newly emitted stack RVAs.
lldb_private::Status FixThreadStacks();
lldb_private::Status FlushBufferToDisk();
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
index b76fcd0052a8a8..2f45f01558e667 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
@@ -21,6 +21,7 @@
#define LLDB_SOURCE_PLUGINS_OBJECTFILE_MINIDUMP_OBJECTFILEMINIDUMP_H
#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Utility/ArchSpec.h"
class ObjectFileMinidump : public lldb_private::PluginInterface {
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 9d01089745dfc9..8b0e0c5250106b 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -20,6 +20,7 @@
#include "lldb/Target/Process.h"
#include "lldb/Target/SectionLoadList.h"
#include "lldb/Target/Target.h"
+#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/FileSpec.h"
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
index 8bccf3be3e5f63..4f4dedf773c5ba 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -13,6 +13,7 @@
#include <vector>
#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Symbol/SaveCoreOptions.h"
#include "llvm/Object/COFF.h"
class ObjectFilePECOFF : public lldb_private::ObjectFile {
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp
index 81717e91baba83..e6a9c483c8ff69 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -32,8 +32,7 @@ Status SaveCoreOptions::SetPluginName(const char *name) {
}
void SaveCoreOptions::SetStyle(lldb::SaveCoreStyle style) {
- if (!m_regions_to_save.empty())
- m_style = style;
+ m_style = style;
}
void SaveCoreOptions::SetOutputFile(FileSpec file) { m_file = file; }
@@ -104,21 +103,16 @@ bool SaveCoreOptions::ShouldThreadBeSaved(lldb::tid_t tid) const {
return m_threads_to_save.count(tid) > 0;
}
+bool SaveCoreOptions::HasSpecifiedThreads() const {
+ return !m_threads_to_save.empty();
+}
+
void SaveCoreOptions::AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion) {
- // Currently we just insert memory regions into the map by their base
- // This is probably overly simple, but adding ranges of memory introduces
- // a lot of potential complexity, such as if the range to save is smaller
- // than the range being evaluated, but both start at the same address
- // should we truncate or save as is?
- // To avoid that we simply save off by their base.
- m_regions_to_save.insert(region.GetRange().GetRangeBase());
- // Because this list is exclusive, if this map is inserted into, we override
- // the style with core style full.
- m_style = lldb::eSaveCoreFull;
+ m_regions_to_save.Insert(region.GetRange(), /*combine=*/true);
}
-bool SaveCoreOptions::ShouldSaveRegion(const lldb_private::MemoryRegionInfo ®ion) const {
- return m_regions_to_save.empty() || m_regions_to_save.count(region.GetRange().GetRangeBase()) > 0;
+const MemoryRanges &SaveCoreOptions::GetCoreFileMemoryRanges() const {
+ return m_regions_to_save;
}
Status SaveCoreOptions::EnsureValidConfiguration(
@@ -150,4 +144,5 @@ void SaveCoreOptions::Clear() {
m_style = std::nullopt;
m_threads_to_save.clear();
m_process_sp.reset();
+ m_regions_to_save.Clear();
}
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 713ef42714009b..e0b4d0313d4ec1 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6516,14 +6516,14 @@ static bool AddDirtyPages(const MemoryRegionInfo ®ion,
} else {
// Add previous contiguous range and init the new range with the
// current dirty page.
- ranges.push_back({range, lldb_permissions});
+ ranges.Append(range.start(), range.end(), {range, lldb_permissions});
range = llvm::AddressRange(page_addr, page_addr + page_size);
}
}
}
// The last range
if (!range.empty())
- ranges.push_back({range, lldb_permissions});
+ ranges.Append(range.start(), range.end(), {range, lldb_permissions});
return true;
}
@@ -6535,7 +6535,6 @@ static bool AddDirtyPages(const MemoryRegionInfo ®ion,
// will be added to \a ranges, else the entire range will be added to \a
// ranges.
static void AddRegion(const MemoryRegionInfo ®ion,
- const SaveCoreOptions &options,
bool try_dirty_pages,
Process::CoreFileMemoryRanges &ranges) {
// Don't add empty ranges.
@@ -6544,11 +6543,10 @@ static void AddRegion(const MemoryRegionInfo ®ion,
// Don't add ranges with no read permissions.
if ((region.GetLLDBPermissions() & lldb::ePermissionsReadable) == 0)
return;
- if (!options.ShouldSaveRegion(region))
- return;
if (try_dirty_pages && AddDirtyPages(region, ranges))
return;
- ranges.push_back(CreateCoreFileMemoryRange(region));
+
+ ranges.Append(region.GetRange().GetRangeBase(), region.GetRange().GetByteSize(), CreateCoreFileMemoryRange(region));
}
static void SaveOffRegionsWithStackPointers(
@@ -6585,7 +6583,7 @@ static void SaveOffRegionsWithStackPointers(
// This will return true if the threadlist the user specified is empty,
// or contains the thread id from thread_sp.
if (core_options.ShouldThreadBeSaved(thread_sp->GetID()))
- AddRegion(sp_region, core_options, try_dirty_pages, ranges);
+ AddRegion(sp_region, try_dirty_pages, ranges);
}
}
}
@@ -6594,7 +6592,6 @@ static void SaveOffRegionsWithStackPointers(
// for a full core file style.
static void GetCoreFileSaveRangesFull(Process &process,
const MemoryRegionInfos ®ions,
- const SaveCoreOptions &core_options,
Process::CoreFileMemoryRanges &ranges,
std::set<addr_t> &stack_ends) {
@@ -6602,7 +6599,7 @@ static void GetCoreFileSaveRangesFull(Process &process,
const bool try_dirty_pages = false;
for (const auto ®ion : regions)
if (stack_ends.count(region.GetRange().GetRangeEnd()) == 0)
- AddRegion(region, core_options, try_dirty_pages, ranges);
+ AddRegion(region, try_dirty_pages, ranges);
}
// Save only the dirty pages to the core file. Make sure the process has at
@@ -6611,7 +6608,6 @@ const bool try_dirty_pages = false;
// page information fall back to saving out all ranges with write permissions.
static void GetCoreFileSaveRangesDirtyOnly(
Process &process, const MemoryRegionInfos ®ions,
- const SaveCoreOptions &core_options,
Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
// Iterate over the regions and find all dirty pages.
@@ -6629,7 +6625,7 @@ static void GetCoreFileSaveRangesDirtyOnly(
for (const auto ®ion : regions)
if (stack_ends.count(region.GetRange().GetRangeEnd()) == 0 &&
region.GetWritable() == MemoryRegionInfo::eYes)
- AddRegion(region, core_options, try_dirty_pages, ranges);
+ AddRegion(region, try_dirty_pages, ranges);
}
}
@@ -6643,7 +6639,6 @@ static void GetCoreFileSaveRangesDirtyOnly(
// stack region.
static void GetCoreFileSaveRangesStackOnly(
Process &process, const MemoryRegionInfos ®ions,
- const SaveCoreOptions &core_options,
Process::CoreFileMemoryRanges &ranges, std::set<addr_t> &stack_ends) {
const bool try_dirty_pages = true;
// Some platforms support annotating the region information that tell us that
@@ -6653,7 +6648,23 @@ static void GetCoreFileSaveRangesStackOnly(
// Save all the stack memory ranges not associated with a stack pointer.
if (stack_ends.count(region.GetRange().GetRangeEnd()) == 0 &&
region.IsStackMemory() == MemoryRegionInfo::eYes)
- AddRegion(region, core_options, try_dirty_pages, ranges);
+ AddRegion(region, try_dirty_pages, ranges);
+ }
+}
+
+static void GetUserSpecifiedCoreFileSaveRanges(Process &process,
+ const MemoryRegionInfos ®ions,
+ const SaveCoreOptions &options,
+ Process::CoreFileMemoryRanges &ranges) {
+ const auto &option_ranges = options.GetCoreFileMemoryRanges();
+ if (option_ranges.IsEmpty())
+ return;
+
+ for (const auto &range : regions) {
+ auto entry = option_ranges.FindEntryThatContains(range.GetRange());
+ if (entry)
+ ranges.Append(range.GetRange().GetRangeBase(),
+ range.GetRange().GetByteSize(), CreateCoreFileMemoryRange(range));
}
}
@@ -6670,32 +6681,41 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
return Status("callers must set the core_style to something other than "
"eSaveCoreUnspecified");
+ GetUserSpecifiedCoreFileSaveRanges(*this, regions, options, ranges);
+
std::set<addr_t> stack_ends;
- SaveOffRegionsWithStackPointers(*this, options, regions, ranges, stack_ends);
+ // For fully custom set ups, we don't want to even look at threads if there
+ // are no threads specified.
+ if (core_style != lldb::eSaveCoreCustom || options.HasSpecifiedThreads())
+ SaveOffRegionsWithStackPointers(*this, options, regions, ranges, stack_ends);
switch (core_style) {
case eSaveCoreUnspecified:
+ case eSaveCoreCustom:
break;
case eSaveCoreFull:
- GetCoreFileSaveRangesFull(*this, regions, options, ranges, stack_ends);
+ GetCoreFileSaveRangesFull(*this, regions, ranges, stack_ends);
break;
case eSaveCoreDirtyOnly:
- GetCoreFileSaveRangesDirtyOnly(*this, regions, options, ranges, stack_ends);
+ GetCoreFileSaveRangesDirtyOnly(*this, regions, ranges, stack_ends);
break;
case eSaveCoreStackOnly:
- GetCoreFileSaveRangesStackOnly(*this, regions, options, ranges, stack_ends);
+ GetCoreFileSaveRangesStackOnly(*this, regions, ranges, stack_ends);
break;
}
if (err.Fail())
return err;
- if (ranges.empty())
+ if (ranges.IsEmpty())
return Status("no valid address ranges found for core style");
+ // Sort the range data vector to dedupe ranges before returning.
+ ranges.Sort();
+
return Status(); // Success!
}
diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index c54d821b6979bc..61433ea203c665 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -308,7 +308,7 @@ def test_save_linux_minidump_one_region(self):
self.build()
exe = self.getBuildArtifact("a.out")
- default_value_file = self.getBuildArtifact("core.defaults.dmp")
+ one_region_file = self.getBuildArtifact("core.one_region.dmp")
try:
target = self.dbg.CreateTarget(exe)
process = target.LaunchSimple(
@@ -325,24 +325,64 @@ def test_save_linux_minidump_one_region(self):
# minidump defaults to stacks only, so we want to see if the
# default options work as expected.
options = lldb.SBSaveCoreOptions()
- default_value_spec = lldb.SBFileSpec(default_value_file)
- options.SetOutputFile(default_value_spec)
+ file_spec = lldb.SBFileSpec(one_region_file)
+ options.SetOutputFile(file_spec)
options.SetPluginName("minidump")
options.AddMemoryRegionToSave(memory_region)
+ options.SetStyle(lldb.eSaveCoreCustom)
error = process.SaveCore(options)
print (f"Error: {error.GetCString()}")
self.assertTrue(error.Success(), error.GetCString())
core_target = self.dbg.CreateTarget(None)
- core_proc = target.LoadCore(default_value_file)
+ core_proc = target.LoadCore(one_region_file)
core_memory_list = core_proc.GetMemoryRegions()
- self.assertEqual(core_memory_list.GetSize(), 1)
+ # Note because the /proc/pid maps are included on linux, we can't
+ # depend on size for validation, so we'll ensure the first region
+ # is present and then assert we fail on the second.
core_memory_region = lldb.SBMemoryRegionInfo()
core_memory_list.GetMemoryRegionAtIndex(0, core_memory_region)
self.assertEqual(core_memory_region.GetRegionBase(), memory_region.GetRegionBase())
self.assertEqual(core_memory_region.GetRegionEnd(), memory_region.GetRegionEnd())
+
+ region_two = lldb.SBMemoryRegionInfo()
+ core_memory_list.GetMemoryRegionAtIndex(1, region_two)
+ err = lldb.SBError()
+ content = core_proc.ReadMemory(region_two.GetRegionBase(), 1, err)
+ self.assertTrue(err.Fail(), "Should fail to read memory")
+
finally:
self.assertTrue(self.dbg.DeleteTarget(target))
- if os.path.isfile(default_value_file):
- os.unlink(default_value_file)
+ if os.path.isfile(one_region_file):
+ os.unlink(one_region_file)
+
+ @skipUnlessArch("x86_64")
+ @skipUnlessPlatform(["linux"])
+ def test_save_minidump_custom_save_style(self):
+ """Test that verifies a custom and unspecified save style fails for
+ containing no data to save"""
+
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ custom_file = self.getBuildArtifact("core.custom.dmp")
+ try:
+ target = self.dbg.CreateTarget(exe)
+ process = target.LaunchSimple(
+ None, None, self.get_process_working_directory()
+ )
+ self.assertState(process.GetState(), lldb.eStateStopped)
+
+ options = lldb.SBSaveCoreOptions()
+ options.SetOutputFile(lldb.SBFileSpec(custom_file))
+ options.SetPluginName("minidump")
+ options.SetStyle(lldb.eSaveCoreCustom)
+
+ error = process.SaveCore(options)
+ self.assertTrue(error.Fail())
+ self.assertEqual(error.GetCString(), "no valid address ranges found for core style")
+
+ finally:
+ self.assertTrue(self.dbg.DeleteTarget(target))
+ if os.path.isfile(custom_file):
+ os.unlink(custom_file)
>From bbd72ac59bd0dc267f4d42f3797150192bc8cde9 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 21 Aug 2024 09:31:55 -0700
Subject: [PATCH 5/9] Comments, drop const on R-value appends
---
lldb/include/lldb/API/SBSaveCoreOptions.h | 2 +-
lldb/include/lldb/Utility/RangeMap.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h
index d14ffe023188a2..5db06d2a80c53b 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -83,7 +83,7 @@ class LLDB_API SBSaveCoreOptions {
/// Add a memory region to save in the core file.
///
/// \param region The memory region to save.
- /// \returns An empty SBStatus upon success, or an error if the region is
+ /// \returns An empty SBError upon success, or an error if the region is
/// invalid.
/// \note Ranges that overlapped with be unioned into a single region this
/// also supercedes stack minification. Specifying full regions and a
diff --git a/lldb/include/lldb/Utility/RangeMap.h b/lldb/include/lldb/Utility/RangeMap.h
index dacc638c193a10..d7e0a6b200512c 100644
--- a/lldb/include/lldb/Utility/RangeMap.h
+++ b/lldb/include/lldb/Utility/RangeMap.h
@@ -450,7 +450,7 @@ class RangeDataVector {
void Append(const Entry &entry) { m_entries.emplace_back(entry); }
- void Append(const B&& b, const S&& s, const T&& t) {
+ void Append(B&& b, S&& s, T&& t) {
m_entries.emplace_back(Entry(b, s, t));
}
>From 6d0dd3b25181d766308a41374edcaa9a7240da0e Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 26 Aug 2024 09:50:14 -0700
Subject: [PATCH 6/9] Merge overlapping regions but split by permissions. Add
tests to ensure we merge user regions and save style regions
---
lldb/source/Target/Process.cpp | 27 +++++++++--
.../TestProcessSaveCoreMinidump.py | 45 +++++++++++++++++++
2 files changed, 68 insertions(+), 4 deletions(-)
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index e0b4d0313d4ec1..b9387ba80850a0 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6668,6 +6668,28 @@ static void GetUserSpecifiedCoreFileSaveRanges(Process &process,
}
}
+static Status FinalizeCoreFileSaveRanges(Process::CoreFileMemoryRanges &ranges) {
+ Status error;
+ ranges.Sort();
+ for (size_t i = ranges.GetSize() - 1; i > 0; i--) {
+ auto region = ranges.GetMutableEntryAtIndex(i);
+ auto next_region = ranges.GetMutableEntryAtIndex(i - 1);
+ if (next_region->GetRangeEnd() >= region->GetRangeBase()
+ && region->GetRangeBase() <= next_region->GetRangeEnd()
+ && region->data.lldb_permissions == next_region->data.lldb_permissions) {
+ const addr_t base = std::min(region->GetRangeBase(), next_region->GetRangeBase());
+ const addr_t byte_size = std::max(region->GetRangeEnd(), next_region->GetRangeEnd()) - base;
+ next_region->SetRangeBase(base);
+ next_region->SetByteSize(byte_size);
+ if (!ranges.Erase(i, i + 1)) {
+ error.SetErrorString("Core file memory ranges mutated outside of CalculateCoreFileSaveRanges");
+ return error;
+ }
+ }
+ }
+ return error;
+}
+
Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
CoreFileMemoryRanges &ranges) {
lldb_private::MemoryRegionInfos regions;
@@ -6713,10 +6735,7 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
if (ranges.IsEmpty())
return Status("no valid address ranges found for core style");
- // Sort the range data vector to dedupe ranges before returning.
- ranges.Sort();
-
- return Status(); // Success!
+ return FinalizeCoreFileSaveRanges(ranges);
}
std::vector<ThreadSP>
diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index 61433ea203c665..326def05f0ba7c 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -386,3 +386,48 @@ def test_save_minidump_custom_save_style(self):
self.assertTrue(self.dbg.DeleteTarget(target))
if os.path.isfile(custom_file):
os.unlink(custom_file)
+
+ def save_core_with_region(self, process, region_index):
+ try:
+ custom_file = self.getBuildArtifact("core.custom.dmp")
+ memory_region = lldb.SBMemoryRegionInfo()
+ memory_list = process.GetMemoryRegions()
+ memory_list.GetMemoryRegionAtIndex(0, memory_region)
+ options = lldb.SBSaveCoreOptions()
+ options.SetOutputFile(lldb.SBFileSpec(custom_file))
+ options.SetPluginName("minidump")
+ options.SetStyle(lldb.eSaveCoreFull)
+
+ error = process.SaveCore(options)
+ self.assertTrue(error.Success())
+ core_target = self.dbg.CreateTarget(None)
+ core_proc = target.LoadCore(one_region_file)
+ core_memory_list = core_proc.GetMemoryRegions()
+ self.assertEqual(len(core_memory_list), len(memory_list))
+ finally:
+ if os.path.isfile(custom_file):
+ os.unlink(custom_file)
+
+ @skipUnlessArch("x86_64")
+ @skipUnlessPlatform(["linux"])
+ def test_save_minidump_custom_save_style_duplicated_regions(self):
+ """Test that verifies a custom and unspecified save style fails for
+ containing no data to save"""
+
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ try:
+ target = self.dbg.CreateTarget(exe)
+ process = target.LaunchSimple(
+ None, None, self.get_process_working_directory()
+ )
+ self.assertState(process.GetState(), lldb.eStateStopped)
+
+ memory_list = process.GetMemoryRegions()
+ # Test that we don't duplicate regions, by duplicating regions
+ # at various indices.
+ self.save_core_with_region(process, 0)
+ self.save_core_with_region(process, len(memory_list) - 1)
+
+ finally:
+ self.assertTrue(self.dbg.DeleteTarget(target))
>From bcc8433893fc20d5b7c10a0595c1602027881714 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 26 Aug 2024 11:39:28 -0700
Subject: [PATCH 7/9] Feedback on unneeded usings, fix test where we depend on
length of memory regions but can't due to proc pid maps, and fix some
spelling and grammar mistakes
---
.../include/lldb/API/SBMemoryRegionInfoList.h | 1 +
lldb/include/lldb/API/SBSaveCoreOptions.h | 2 +-
lldb/include/lldb/lldb-enumerations.h | 2 +-
lldb/source/API/SBSaveCoreOptions.cpp | 2 --
lldb/source/Target/Process.cpp | 4 ++--
.../TestProcessSaveCoreMinidump.py | 19 ++++++++++++++-----
6 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/lldb/include/lldb/API/SBMemoryRegionInfoList.h b/lldb/include/lldb/API/SBMemoryRegionInfoList.h
index 7207171bf6d647..1d939dff55faa3 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;
+
lldb_private::MemoryRegionInfos &ref();
const lldb_private::MemoryRegionInfos &ref() const;
diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h
index 5db06d2a80c53b..c076d3ce6f7575 100644
--- a/lldb/include/lldb/API/SBSaveCoreOptions.h
+++ b/lldb/include/lldb/API/SBSaveCoreOptions.h
@@ -85,7 +85,7 @@ class LLDB_API SBSaveCoreOptions {
/// \param region The memory region to save.
/// \returns An empty SBError upon success, or an error if the region is
/// invalid.
- /// \note Ranges that overlapped with be unioned into a single region this
+ /// \note Ranges that overlapped will be unioned into a single region, this
/// also supercedes stack minification. Specifying full regions and a
/// non-custom core style will include the specified regions and union them
/// with all style specific regions.
diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h
index 768045c72280a0..938f6e3abe8f2a 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -1222,7 +1222,7 @@ enum SaveCoreStyle {
eSaveCoreFull = 1,
eSaveCoreDirtyOnly = 2,
eSaveCoreStackOnly = 3,
- eSaveCoreCustom = 4,
+ eSaveCoreCustomOnly = 4,
};
/// Events that might happen during a trace session.
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp
index 03d237d3499727..75a36aebcbdc0b 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -7,8 +7,6 @@
//===----------------------------------------------------------------------===//
#include "lldb/API/SBSaveCoreOptions.h"
-#include "lldb/API/SBError.h"
-#include "lldb/API/SBFileSpec.h"
#include "lldb/API/SBMemoryRegionInfo.h"
#include "lldb/Host/FileSystem.h"
#include "lldb/Symbol/SaveCoreOptions.h"
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index b9387ba80850a0..e14b1f6e841195 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6708,12 +6708,12 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
std::set<addr_t> stack_ends;
// For fully custom set ups, we don't want to even look at threads if there
// are no threads specified.
- if (core_style != lldb::eSaveCoreCustom || options.HasSpecifiedThreads())
+ if (core_style != lldb::eSaveCoreCustomOnly || options.HasSpecifiedThreads())
SaveOffRegionsWithStackPointers(*this, options, regions, ranges, stack_ends);
switch (core_style) {
case eSaveCoreUnspecified:
- case eSaveCoreCustom:
+ case eSaveCoreCustomOnly:
break;
case eSaveCoreFull:
diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index 326def05f0ba7c..1c50d2b2046d90 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -329,13 +329,13 @@ def test_save_linux_minidump_one_region(self):
options.SetOutputFile(file_spec)
options.SetPluginName("minidump")
options.AddMemoryRegionToSave(memory_region)
- options.SetStyle(lldb.eSaveCoreCustom)
+ options.SetStyle(lldb.eSaveCoreCustomOnly)
error = process.SaveCore(options)
print (f"Error: {error.GetCString()}")
self.assertTrue(error.Success(), error.GetCString())
core_target = self.dbg.CreateTarget(None)
- core_proc = target.LoadCore(one_region_file)
+ core_proc = core_target.LoadCore(one_region_file)
core_memory_list = core_proc.GetMemoryRegions()
# Note because the /proc/pid maps are included on linux, we can't
# depend on size for validation, so we'll ensure the first region
@@ -376,7 +376,7 @@ def test_save_minidump_custom_save_style(self):
options = lldb.SBSaveCoreOptions()
options.SetOutputFile(lldb.SBFileSpec(custom_file))
options.SetPluginName("minidump")
- options.SetStyle(lldb.eSaveCoreCustom)
+ options.SetStyle(lldb.eSaveCoreCustomOnly)
error = process.SaveCore(options)
self.assertTrue(error.Fail())
@@ -401,9 +401,18 @@ def save_core_with_region(self, process, region_index):
error = process.SaveCore(options)
self.assertTrue(error.Success())
core_target = self.dbg.CreateTarget(None)
- core_proc = target.LoadCore(one_region_file)
+ core_proc = core_target.LoadCore(custom_file)
core_memory_list = core_proc.GetMemoryRegions()
- self.assertEqual(len(core_memory_list), len(memory_list))
+ # proc/pid/ maps are included on linux, so we can't depend on size
+ # for validation, we make a set of all the ranges,
+ # and ensure no duplicates!
+ range_set = set()
+ for x in range(core_memory_list.GetSize()):
+ core_memory_region = lldb.SBMemoryRegionInfo()
+ core_memory_list.GetMemoryRegionAtIndex(x, core_memory_region)
+ mem_tuple = (core_memory_region.GetRegionBase(), core_memory_region.GetRegionEnd())
+ self.assertTrue(mem_tuple not in range_set, "Duplicate memory region found")
+ range_set.add(mem_tuple)
finally:
if os.path.isfile(custom_file):
os.unlink(custom_file)
>From e6e6eede5775461729952b131381f2cac5f73213 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 26 Aug 2024 15:49:37 -0700
Subject: [PATCH 8/9] Run GCF
---
lldb/include/lldb/Symbol/SaveCoreOptions.h | 5 +-
lldb/include/lldb/Target/Process.h | 6 +-
lldb/include/lldb/Utility/RangeMap.h | 6 +-
lldb/source/API/SBSaveCoreOptions.cpp | 6 +-
lldb/source/Commands/CommandObjectProcess.cpp | 2 +-
.../Minidump/MinidumpFileBuilder.cpp | 11 ++--
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 10 +--
.../ObjectFile/PECOFF/ObjectFilePECOFF.cpp | 2 +-
lldb/source/Symbol/SaveCoreOptions.cpp | 7 +-
lldb/source/Target/Process.cpp | 66 ++++++++++---------
10 files changed, 63 insertions(+), 58 deletions(-)
diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h
index a95686ada96a79..d90d08026016dc 100644
--- a/lldb/include/lldb/Symbol/SaveCoreOptions.h
+++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h
@@ -12,11 +12,10 @@
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/RangeMap.h"
-
#include <optional>
+#include <set>
#include <string>
#include <unordered_set>
-#include <set>
using MemoryRanges = lldb_private::RangeVector<lldb::addr_t, lldb::addr_t>;
@@ -44,7 +43,7 @@ class SaveCoreOptions {
bool HasSpecifiedThreads() const;
Status EnsureValidConfiguration(lldb::ProcessSP process_sp) const;
- const MemoryRanges& GetCoreFileMemoryRanges() const;
+ const MemoryRanges &GetCoreFileMemoryRanges() const;
void AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion);
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 73228f13dc6715..a192065c4de4b4 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -35,6 +35,7 @@
#include "lldb/Host/ProcessLaunchInfo.h"
#include "lldb/Host/ProcessRunLock.h"
#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Target/ExecutionContextScope.h"
#include "lldb/Target/InstrumentationRuntime.h"
#include "lldb/Target/Memory.h"
@@ -43,7 +44,6 @@
#include "lldb/Target/ThreadList.h"
#include "lldb/Target/ThreadPlanStack.h"
#include "lldb/Target/Trace.h"
-#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Utility/AddressableBits.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/Broadcaster.h"
@@ -734,7 +734,9 @@ class Process : public std::enable_shared_from_this<Process>,
}
};
- using CoreFileMemoryRanges = lldb_private::RangeDataVector<lldb::addr_t, lldb::addr_t, CoreFileMemoryRange>;
+ using CoreFileMemoryRanges =
+ lldb_private::RangeDataVector<lldb::addr_t, lldb::addr_t,
+ CoreFileMemoryRange>;
/// Helper function for Process::SaveCore(...) that calculates the address
/// ranges that should be saved. This allows all core file plug-ins to save
diff --git a/lldb/include/lldb/Utility/RangeMap.h b/lldb/include/lldb/Utility/RangeMap.h
index d7e0a6b200512c..c636348129b647 100644
--- a/lldb/include/lldb/Utility/RangeMap.h
+++ b/lldb/include/lldb/Utility/RangeMap.h
@@ -449,10 +449,8 @@ class RangeDataVector {
~RangeDataVector() = default;
void Append(const Entry &entry) { m_entries.emplace_back(entry); }
-
- void Append(B&& b, S&& s, T&& t) {
- m_entries.emplace_back(Entry(b, s, t));
- }
+
+ void Append(B &&b, S &&s, T &&t) { m_entries.emplace_back(Entry(b, s, t)); }
bool Erase(uint32_t start, uint32_t end) {
if (start >= end || end > m_entries.size())
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp
index 75a36aebcbdc0b..5e75aa911b650b 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -91,10 +91,10 @@ bool SBSaveCoreOptions::RemoveThread(lldb::SBThread thread) {
return m_opaque_up->RemoveThread(thread.GetSP());
}
-
-lldb::SBError SBSaveCoreOptions::AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion) {
+lldb::SBError
+SBSaveCoreOptions::AddMemoryRegionToSave(const SBMemoryRegionInfo ®ion) {
LLDB_INSTRUMENT_VA(this, region);
- // Currently add memory region can't fail, so we always return a success
+ // Currently add memory region can't fail, so we always return a success
// SBerror, but because these API's live forever, this is the most future
// proof thing to do.
m_opaque_up->AddMemoryRegionToSave(region.ref());
diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp
index 398eb574f7d7c9..1a5ce7de8c67bd 100644
--- a/lldb/source/Commands/CommandObjectProcess.cpp
+++ b/lldb/source/Commands/CommandObjectProcess.cpp
@@ -25,13 +25,13 @@
#include "lldb/Interpreter/OptionArgParser.h"
#include "lldb/Interpreter/OptionGroupPythonClassWithDict.h"
#include "lldb/Interpreter/Options.h"
+#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Target/Platform.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/StopInfo.h"
#include "lldb/Target/Target.h"
#include "lldb/Target/Thread.h"
#include "lldb/Target/UnixSignals.h"
-#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Utility/Args.h"
#include "lldb/Utility/ScriptedMetadata.h"
#include "lldb/Utility/State.h"
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index fb323b78735499..47728d4d1a31e0 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -830,7 +830,8 @@ Status MinidumpFileBuilder::AddMemoryList() {
all_core_memory_ranges);
std::vector<Process::CoreFileMemoryRange> all_core_memory_vec;
- // Extract all the data into just a vector of data. So we can mutate this in place.
+ // Extract all the data into just a vector of data. So we can mutate this in
+ // place.
for (const auto &core_range : all_core_memory_ranges)
all_core_memory_vec.push_back(core_range.data);
@@ -962,8 +963,8 @@ GetLargestRangeSize(const std::vector<Process::CoreFileMemoryRange> &ranges) {
return max_size;
}
-Status
-MinidumpFileBuilder::AddMemoryList_32(std::vector<Process::CoreFileMemoryRange> &ranges) {
+Status MinidumpFileBuilder::AddMemoryList_32(
+ std::vector<Process::CoreFileMemoryRange> &ranges) {
std::vector<MemoryDescriptor> descriptors;
Status error;
if (ranges.size() == 0)
@@ -1037,8 +1038,8 @@ MinidumpFileBuilder::AddMemoryList_32(std::vector<Process::CoreFileMemoryRange>
return error;
}
-Status
-MinidumpFileBuilder::AddMemoryList_64(std::vector<Process::CoreFileMemoryRange> &ranges) {
+Status MinidumpFileBuilder::AddMemoryList_64(
+ std::vector<Process::CoreFileMemoryRange> &ranges) {
Status error;
if (ranges.empty())
return error;
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 195650de317178..8651cddeedb216 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -23,9 +23,9 @@
#include <utility>
#include <variant>
+#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/Target.h"
-#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/Status.h"
#include "lldb/lldb-forward.h"
@@ -120,10 +120,10 @@ class MinidumpFileBuilder {
// trigger a flush.
lldb_private::Status AddData(const void *data, uint64_t size);
// Add MemoryList stream, containing dumps of important memory segments
- lldb_private::Status
- AddMemoryList_64(std::vector<lldb_private::Process::CoreFileMemoryRange> &ranges);
- lldb_private::Status
- AddMemoryList_32(std::vector<lldb_private::Process::CoreFileMemoryRange> &ranges);
+ lldb_private::Status AddMemoryList_64(
+ std::vector<lldb_private::Process::CoreFileMemoryRange> &ranges);
+ lldb_private::Status AddMemoryList_32(
+ std::vector<lldb_private::Process::CoreFileMemoryRange> &ranges);
// Update the thread list on disk with the newly emitted stack RVAs.
lldb_private::Status FixThreadStacks();
lldb_private::Status FlushBufferToDisk();
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 8b0e0c5250106b..8d9c919bc9b101 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -17,10 +17,10 @@
#include "lldb/Interpreter/OptionValueDictionary.h"
#include "lldb/Interpreter/OptionValueProperties.h"
#include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Target/Process.h"
#include "lldb/Target/SectionLoadList.h"
#include "lldb/Target/Target.h"
-#include "lldb/Symbol/SaveCoreOptions.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/FileSpec.h"
diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp
index e6a9c483c8ff69..0d45847a3b3273 100644
--- a/lldb/source/Symbol/SaveCoreOptions.cpp
+++ b/lldb/source/Symbol/SaveCoreOptions.cpp
@@ -31,9 +31,7 @@ Status SaveCoreOptions::SetPluginName(const char *name) {
return error;
}
-void SaveCoreOptions::SetStyle(lldb::SaveCoreStyle style) {
- m_style = style;
-}
+void SaveCoreOptions::SetStyle(lldb::SaveCoreStyle style) { m_style = style; }
void SaveCoreOptions::SetOutputFile(FileSpec file) { m_file = file; }
@@ -107,7 +105,8 @@ bool SaveCoreOptions::HasSpecifiedThreads() const {
return !m_threads_to_save.empty();
}
-void SaveCoreOptions::AddMemoryRegionToSave(const lldb_private::MemoryRegionInfo ®ion) {
+void SaveCoreOptions::AddMemoryRegionToSave(
+ const lldb_private::MemoryRegionInfo ®ion) {
m_regions_to_save.Insert(region.GetRange(), /*combine=*/true);
}
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index e14b1f6e841195..1fbd53c08f9689 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -6534,8 +6534,7 @@ static bool AddDirtyPages(const MemoryRegionInfo ®ion,
// given region. If the region has dirty page information, only dirty pages
// will be added to \a ranges, else the entire range will be added to \a
// ranges.
-static void AddRegion(const MemoryRegionInfo ®ion,
- bool try_dirty_pages,
+static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages,
Process::CoreFileMemoryRanges &ranges) {
// Don't add empty ranges.
if (region.GetRange().GetByteSize() == 0)
@@ -6546,7 +6545,9 @@ static void AddRegion(const MemoryRegionInfo ®ion,
if (try_dirty_pages && AddDirtyPages(region, ranges))
return;
- ranges.Append(region.GetRange().GetRangeBase(), region.GetRange().GetByteSize(), CreateCoreFileMemoryRange(region));
+ ranges.Append(region.GetRange().GetRangeBase(),
+ region.GetRange().GetByteSize(),
+ CreateCoreFileMemoryRange(region));
}
static void SaveOffRegionsWithStackPointers(
@@ -6595,8 +6596,8 @@ static void GetCoreFileSaveRangesFull(Process &process,
Process::CoreFileMemoryRanges &ranges,
std::set<addr_t> &stack_ends) {
-// Don't add only dirty pages, add full regions.
-const bool try_dirty_pages = false;
+ // Don't add only dirty pages, add full regions.
+ const bool try_dirty_pages = false;
for (const auto ®ion : regions)
if (stack_ends.count(region.GetRange().GetRangeEnd()) == 0)
AddRegion(region, try_dirty_pages, ranges);
@@ -6652,10 +6653,9 @@ static void GetCoreFileSaveRangesStackOnly(
}
}
-static void GetUserSpecifiedCoreFileSaveRanges(Process &process,
- const MemoryRegionInfos ®ions,
- const SaveCoreOptions &options,
- Process::CoreFileMemoryRanges &ranges) {
+static void GetUserSpecifiedCoreFileSaveRanges(
+ Process &process, const MemoryRegionInfos ®ions,
+ const SaveCoreOptions &options, Process::CoreFileMemoryRanges &ranges) {
const auto &option_ranges = options.GetCoreFileMemoryRanges();
if (option_ranges.IsEmpty())
return;
@@ -6664,30 +6664,35 @@ static void GetUserSpecifiedCoreFileSaveRanges(Process &process,
auto entry = option_ranges.FindEntryThatContains(range.GetRange());
if (entry)
ranges.Append(range.GetRange().GetRangeBase(),
- range.GetRange().GetByteSize(), CreateCoreFileMemoryRange(range));
+ range.GetRange().GetByteSize(),
+ CreateCoreFileMemoryRange(range));
}
}
-static Status FinalizeCoreFileSaveRanges(Process::CoreFileMemoryRanges &ranges) {
- Status error;
- ranges.Sort();
- for (size_t i = ranges.GetSize() - 1; i > 0; i--) {
- auto region = ranges.GetMutableEntryAtIndex(i);
- auto next_region = ranges.GetMutableEntryAtIndex(i - 1);
- if (next_region->GetRangeEnd() >= region->GetRangeBase()
- && region->GetRangeBase() <= next_region->GetRangeEnd()
- && region->data.lldb_permissions == next_region->data.lldb_permissions) {
- const addr_t base = std::min(region->GetRangeBase(), next_region->GetRangeBase());
- const addr_t byte_size = std::max(region->GetRangeEnd(), next_region->GetRangeEnd()) - base;
- next_region->SetRangeBase(base);
- next_region->SetByteSize(byte_size);
- if (!ranges.Erase(i, i + 1)) {
- error.SetErrorString("Core file memory ranges mutated outside of CalculateCoreFileSaveRanges");
- return error;
- }
- }
+static Status
+FinalizeCoreFileSaveRanges(Process::CoreFileMemoryRanges &ranges) {
+ Status error;
+ ranges.Sort();
+ for (size_t i = ranges.GetSize() - 1; i > 0; i--) {
+ auto region = ranges.GetMutableEntryAtIndex(i);
+ auto next_region = ranges.GetMutableEntryAtIndex(i - 1);
+ if (next_region->GetRangeEnd() >= region->GetRangeBase() &&
+ region->GetRangeBase() <= next_region->GetRangeEnd() &&
+ region->data.lldb_permissions == next_region->data.lldb_permissions) {
+ const addr_t base =
+ std::min(region->GetRangeBase(), next_region->GetRangeBase());
+ const addr_t byte_size =
+ std::max(region->GetRangeEnd(), next_region->GetRangeEnd()) - base;
+ next_region->SetRangeBase(base);
+ next_region->SetByteSize(byte_size);
+ if (!ranges.Erase(i, i + 1)) {
+ error.SetErrorString("Core file memory ranges mutated outside of "
+ "CalculateCoreFileSaveRanges");
+ return error;
+ }
}
- return error;
+ }
+ return error;
}
Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
@@ -6709,7 +6714,8 @@ Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options,
// For fully custom set ups, we don't want to even look at threads if there
// are no threads specified.
if (core_style != lldb::eSaveCoreCustomOnly || options.HasSpecifiedThreads())
- SaveOffRegionsWithStackPointers(*this, options, regions, ranges, stack_ends);
+ SaveOffRegionsWithStackPointers(*this, options, regions, ranges,
+ stack_ends);
switch (core_style) {
case eSaveCoreUnspecified:
>From 99be3f7dd2aeea1b651fc424412128ff05c83f25 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 26 Aug 2024 20:12:15 -0700
Subject: [PATCH 9/9] Format python test case
---
.../TestProcessSaveCoreMinidump.py | 46 ++++++++++++-------
1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index 1c50d2b2046d90..db76228b32bad2 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -283,7 +283,6 @@ def test_save_linux_mini_dump_default_options(self):
expected_threads.append(thread_id)
stacks_to_sp_map[thread_id] = thread.GetFrameAtIndex(0).GetSP()
-
# This is almost identical to the single thread test case because
# minidump defaults to stacks only, so we want to see if the
# default options work as expected.
@@ -294,7 +293,13 @@ def test_save_linux_mini_dump_default_options(self):
error = process.SaveCore(options)
self.assertTrue(error.Success())
- self.verify_core_file(default_value_file, expected_pid, expected_modules, expected_threads, stacks_to_sp_map)
+ self.verify_core_file(
+ default_value_file,
+ expected_pid,
+ expected_modules,
+ expected_threads,
+ stacks_to_sp_map,
+ )
finally:
self.assertTrue(self.dbg.DeleteTarget(target))
@@ -319,7 +324,6 @@ def test_save_linux_minidump_one_region(self):
memory_region = lldb.SBMemoryRegionInfo()
memory_list = process.GetMemoryRegions()
memory_list.GetMemoryRegionAtIndex(0, memory_region)
-
# This is almost identical to the single thread test case because
# minidump defaults to stacks only, so we want to see if the
@@ -331,7 +335,7 @@ def test_save_linux_minidump_one_region(self):
options.AddMemoryRegionToSave(memory_region)
options.SetStyle(lldb.eSaveCoreCustomOnly)
error = process.SaveCore(options)
- print (f"Error: {error.GetCString()}")
+ print(f"Error: {error.GetCString()}")
self.assertTrue(error.Success(), error.GetCString())
core_target = self.dbg.CreateTarget(None)
@@ -342,16 +346,19 @@ def test_save_linux_minidump_one_region(self):
# is present and then assert we fail on the second.
core_memory_region = lldb.SBMemoryRegionInfo()
core_memory_list.GetMemoryRegionAtIndex(0, core_memory_region)
- self.assertEqual(core_memory_region.GetRegionBase(), memory_region.GetRegionBase())
- self.assertEqual(core_memory_region.GetRegionEnd(), memory_region.GetRegionEnd())
-
+ self.assertEqual(
+ core_memory_region.GetRegionBase(), memory_region.GetRegionBase()
+ )
+ self.assertEqual(
+ core_memory_region.GetRegionEnd(), memory_region.GetRegionEnd()
+ )
+
region_two = lldb.SBMemoryRegionInfo()
core_memory_list.GetMemoryRegionAtIndex(1, region_two)
err = lldb.SBError()
content = core_proc.ReadMemory(region_two.GetRegionBase(), 1, err)
self.assertTrue(err.Fail(), "Should fail to read memory")
-
finally:
self.assertTrue(self.dbg.DeleteTarget(target))
if os.path.isfile(one_region_file):
@@ -360,8 +367,8 @@ def test_save_linux_minidump_one_region(self):
@skipUnlessArch("x86_64")
@skipUnlessPlatform(["linux"])
def test_save_minidump_custom_save_style(self):
- """Test that verifies a custom and unspecified save style fails for
- containing no data to save"""
+ """Test that verifies a custom and unspecified save style fails for
+ containing no data to save"""
self.build()
exe = self.getBuildArtifact("a.out")
@@ -380,7 +387,9 @@ def test_save_minidump_custom_save_style(self):
error = process.SaveCore(options)
self.assertTrue(error.Fail())
- self.assertEqual(error.GetCString(), "no valid address ranges found for core style")
+ self.assertEqual(
+ error.GetCString(), "no valid address ranges found for core style"
+ )
finally:
self.assertTrue(self.dbg.DeleteTarget(target))
@@ -410,18 +419,23 @@ def save_core_with_region(self, process, region_index):
for x in range(core_memory_list.GetSize()):
core_memory_region = lldb.SBMemoryRegionInfo()
core_memory_list.GetMemoryRegionAtIndex(x, core_memory_region)
- mem_tuple = (core_memory_region.GetRegionBase(), core_memory_region.GetRegionEnd())
- self.assertTrue(mem_tuple not in range_set, "Duplicate memory region found")
+ mem_tuple = (
+ core_memory_region.GetRegionBase(),
+ core_memory_region.GetRegionEnd(),
+ )
+ self.assertTrue(
+ mem_tuple not in range_set, "Duplicate memory region found"
+ )
range_set.add(mem_tuple)
finally:
if os.path.isfile(custom_file):
os.unlink(custom_file)
-
+
@skipUnlessArch("x86_64")
@skipUnlessPlatform(["linux"])
def test_save_minidump_custom_save_style_duplicated_regions(self):
- """Test that verifies a custom and unspecified save style fails for
- containing no data to save"""
+ """Test that verifies a custom and unspecified save style fails for
+ containing no data to save"""
self.build()
exe = self.getBuildArtifact("a.out")
More information about the lldb-commits
mailing list