[LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. by Jlalond · Pull Request #100443 · llvm/llvm-project (original) (raw)

@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)

Changes

In #98403 I enabled the SBSaveCoreOptions object, which allows users via the scripting API to define what they want saved into their core file. As the first option I've added a threadlist, so users can scan and identify which threads and corresponding stacks they want to save.

In order to support this, I had to add a new method to Process.h on how we identify which threads are to be saved, and I had to change the book keeping in minidump to ensure we don't double save the stacks.

Important to @jasonmolenda I also changed the MachO coredump to accept these new APIs.


Patch is 20.56 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/100443.diff

11 Files Affected:

diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h index e77496bd3a4a0..b485371ce8f55 100644 --- a/lldb/include/lldb/API/SBSaveCoreOptions.h +++ b/lldb/include/lldb/API/SBSaveCoreOptions.h @@ -53,6 +53,30 @@ class LLDB_API SBSaveCoreOptions { /// \return The output file spec. SBFileSpec GetOutputFile() const; + /// Add a thread to save in the core file. + /// + /// \param thread_id The thread ID to save. + void AddThread(lldb::tid_t thread_id); + + /// Remove a thread from the list of threads to save. + /// + /// \param thread_id The thread ID to remove. + /// \return True if the thread was removed, false if it was not in the list. + bool RemoveThread(lldb::tid_t thread_id); + + /// Get the number of threads to save. If this list is empty all threads will + /// be saved. + /// + /// \return The number of threads to save. + uint32_t GetNumThreads() const; + + /// Get the thread ID at the given index. + /// + /// \param[in] index The index of the thread ID to get. + /// \return The thread ID at the given index, or an error + /// if there is no thread at the index. + lldb::tid_t GetThreadAtIndex(uint32_t index, SBError &error) const; + /// Reset all options. void Clear(); diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h index 583bc1f483d04..d583b32b29508 100644 --- a/lldb/include/lldb/Symbol/SaveCoreOptions.h +++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h @@ -14,6 +14,7 @@ #include "lldb/lldb-types.h" #include +#include #include namespace lldb_private { @@ -32,12 +33,21 @@ class SaveCoreOptions { void SetOutputFile(lldb_private::FileSpec file); const std::optional<lldb_private::FileSpec> GetOutputFile() const; + void AddThread(lldb::tid_t tid); + bool RemoveThread(lldb::tid_t tid); + size_t GetNumThreads() const; + int64_t GetThreadAtIndex(size_t index) const; + bool ShouldSaveThread(lldb::tid_t tid) const; + + Status EnsureValidConfiguration() const; + void Clear(); private: std::optionalstd::string m_plugin_name; std::optional<lldb_private::FileSpec> m_file; std::optionallldb::SaveCoreStyle m_style; + std::setlldb::tid_t m_threads_to_save; }; } // namespace lldb_private diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index c8475db8ae160..ef3907154c20f 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -738,9 +738,14 @@ class Process : public std::enable_shared_from_this, /// 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(lldb::SaveCoreStyle 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. + std::vectorlldb::ThreadSP + CalculateCoreFileThreadList(const SaveCoreOptions &core_options); + protected: virtual JITLoaderList &GetJITLoaders(); diff --git a/lldb/source/API/SBSaveCoreOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp index 6c3f74596203d..1d45313d2426a 100644 --- a/lldb/source/API/SBSaveCoreOptions.cpp +++ b/lldb/source/API/SBSaveCoreOptions.cpp @@ -75,6 +75,26 @@ lldb::SaveCoreStyle SBSaveCoreOptions::GetStyle() const { return m_opaque_up->GetStyle(); } +void SBSaveCoreOptions::AddThread(lldb::tid_t tid) { + m_opaque_up->AddThread(tid); +} + +bool SBSaveCoreOptions::RemoveThread(lldb::tid_t tid) { + return m_opaque_up->RemoveThread(tid); +} + +uint32_t SBSaveCoreOptions::GetNumThreads() const { + return m_opaque_up->GetNumThreads(); +} + +lldb::tid_t SBSaveCoreOptions::GetThreadAtIndex(uint32_t idx, + SBError &error) const { + int64_t tid = m_opaque_up->GetThreadAtIndex(idx); + if (tid == -1) + error.SetErrorString("Invalid index"); + return 0; +} + void SBSaveCoreOptions::Clear() { LLDB_INSTRUMENT_VA(this); m_opaque_up->Clear(); diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index 759ef3a8afe02..94e3cb85f31b9 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -714,6 +714,10 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp, return error; } + error = options.EnsureValidConfiguration(); + if (error.Fail()) + return error; + if (!options.GetPluginName().has_value()) { // Try saving core directly from the process plugin first. llvm::Expected ret = diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index 2c7005449f9d7..f6a9a5dd50d99 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6558,7 +6558,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, if (make_core) { Process::CoreFileMemoryRanges core_ranges; - error = process_sp->CalculateCoreFileSaveRanges(core_style, core_ranges); + 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(); @@ -6608,8 +6608,9 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, mach_header.ncmds = segment_load_commands.size(); mach_header.flags = 0; mach_header.reserved = 0; - ThreadList &thread_list = process_sp->GetThreadList(); - const uint32_t num_threads = thread_list.GetSize(); + std::vector thread_list = + process_sp->CalculateCoreFileThreadList(options); + const uint32_t num_threads = thread_list.size(); // Make an array of LC_THREAD data items. Each one contains the // contents of the LC_THREAD load command. The data doesn't contain @@ -6622,7 +6623,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, LC_THREAD_data.SetByteOrder(byte_order); } for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) { - ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx)); + ThreadSP thread_sp = thread_list.at(thread_idx); if (thread_sp) { switch (mach_header.cputype) { case llvm::MachO::CPU_TYPE_ARM64: @@ -6730,7 +6731,7 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, StructuredData::ArraySP threads( std::make_sharedStructuredData::Array()); for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) { - ThreadSP thread_sp(thread_list.GetThreadAtIndex(thread_idx)); + ThreadSP thread_sp = thread_list.at(thread_idx); StructuredData::DictionarySP thread( std::make_sharedStructuredData::Dictionary()); thread->AddIntegerItem("thread_id", thread_sp->GetID()); diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index de212c6b20da7..3d65596c93522 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -588,12 +588,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 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 +603,17 @@ Status MinidumpFileBuilder::AddThreadList() { return error; llvm::support::ulittle32_t thread_count = - static_castllvm::support::ulittle32_t(thread_list.GetSize()); + static_castllvm::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(); + const uint32_t num_threads = thread_list.size(); 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)); + ThreadSP thread_sp = thread_list.at(thread_idx); RegisterContextSP reg_ctx_sp(thread_sp->GetRegisterContext()); if (!reg_ctx_sp) { @@ -819,7 +820,7 @@ Status MinidumpFileBuilder::AddLinuxFileStreams() { return error; } -Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) { +Status MinidumpFileBuilder::AddMemoryList() { Status error; // We first save the thread stacks to ensure they fit in the first UINT32_MAX @@ -828,18 +829,26 @@ Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) { // in accessible with a 32 bit offset. Process::CoreFileMemoryRanges ranges_32; Process::CoreFileMemoryRanges ranges_64; - error = m_process_sp->CalculateCoreFileSaveRanges( - SaveCoreStyle::eSaveCoreStackOnly, ranges_32); + 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 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) { @@ -849,14 +858,6 @@ Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) { return error; } - Process::CoreFileMemoryRanges all_core_memory_ranges; - if (core_style != SaveCoreStyle::eSaveCoreStackOnly) { - error = m_process_sp->CalculateCoreFileSaveRanges(core_style, - all_core_memory_ranges); - 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. @@ -864,16 +865,13 @@ Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) { // 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;

@@ -103,7 +105,7 @@ class MinidumpFileBuilder { // Add Exception streams for any threads that stopped with exceptions. lldb_private::Status AddExceptions(); // Add MemoryList stream, containing dumps of important memory segments

#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 faa144bfb5f6a..2380ff4c00ca9 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp @@ -74,7 +74,8 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, error = maybe_core_file.takeError(); return false; }

@@ -121,7 +122,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,

// Note: add memory HAS to be the last thing we do. It can overflow into 64b // land and many RVA's only support 32b

+void SaveCoreOptions::AddThread(lldb::tid_t tid) {

+} + +bool SaveCoreOptions::RemoveThread(lldb::tid_t tid) {

static void SaveOffRegionsWithStackPointers(

const bool try_dirty_pages = true;

// Before we take any dump, we want to save off the used portions of the @@ -6555,10 +6556,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;

-Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style, +Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options, CoreFileMemoryRanges &ranges) { lldb_private::MemoryRegionInfos regions; Status err = GetMemoryRegions(regions);

@@ -6668,6 +6676,18 @@ Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style, return Sta... [truncated]