Author: Jacob Lalonde Date: 2024-08-05T10:17:25-07:00 New Revision: accf5c9bb3b5fe88628a44062a5c0c2f903fca2c
URL: https://github.com/llvm/llvm-project/commit/accf5c9bb3b5fe88628a44062a5c0c2f903fca2c DIFF: https://github.com/llvm/llvm-project/commit/accf5c9bb3b5fe88628a44062a5c0c2f903fca2c.diff LOG: Revert "[LLDB][SBSaveCore] Implement a selectable threadlist for Core… (#102018) … Options. (#100443)" This reverts commit 3e4af616334eae532f308605b89ff158dd195180. @adrian-prantl FYI Reverts #100443 Added: Modified: lldb/include/lldb/API/SBProcess.h lldb/include/lldb/API/SBSaveCoreOptions.h lldb/include/lldb/API/SBThread.h lldb/include/lldb/Symbol/SaveCoreOptions.h lldb/include/lldb/Target/Process.h lldb/source/API/SBSaveCoreOptions.cpp lldb/source/API/SBThread.cpp lldb/source/Core/PluginManager.cpp lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp lldb/source/Symbol/SaveCoreOptions.cpp lldb/source/Target/Process.cpp lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py Removed: lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml lldb/test/API/python_api/sbsavecoreoptions/basic_minidump_different_pid.yaml ################################################################################ diff --git a/lldb/include/lldb/API/SBProcess.h b/lldb/include/lldb/API/SBProcess.h index 1624e02070b1b2..778be795839901 100644 --- a/lldb/include/lldb/API/SBProcess.h +++ b/lldb/include/lldb/API/SBProcess.h @@ -586,7 +586,6 @@ class LLDB_API SBProcess { friend class SBBreakpointCallbackBaton; friend class SBBreakpointLocation; friend class SBCommandInterpreter; - friend class SBSaveCoreOptions; friend class SBDebugger; friend class SBExecutionContext; friend class SBFunction; diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h index df0aa561de4080..e77496bd3a4a0d 100644 --- a/lldb/include/lldb/API/SBSaveCoreOptions.h +++ b/lldb/include/lldb/API/SBSaveCoreOptions.h @@ -10,10 +10,6 @@ #define LLDB_API_SBSAVECOREOPTIONS_H #include "lldb/API/SBDefines.h" -#include "lldb/API/SBError.h" -#include "lldb/API/SBFileSpec.h" -#include "lldb/API/SBProcess.h" -#include "lldb/API/SBThread.h" namespace lldb { @@ -57,29 +53,6 @@ class LLDB_API SBSaveCoreOptions { /// \return The output file spec. SBFileSpec GetOutputFile() const; - /// Set the process to save, or unset if supplied with a default constructed - /// process. - /// - /// \param process The process to save. - /// \return Success if process was set, otherwise an error - /// \note This will clear all process specific options if a diff erent process - /// is specified than the current set process, either explicitly from this - /// api, or implicitly from any function that requires a process. - SBError SetProcess(lldb::SBProcess process); - - /// Add a thread to save in the core file. - /// - /// \param thread The thread to save. - /// \note This will set the process if it is not already set, or return - /// and error if the SBThread is not from the set process. - SBError AddThread(lldb::SBThread thread); - - /// Remove a thread from the list of threads to save. - /// - /// \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); - /// Reset all options. void Clear(); diff --git a/lldb/include/lldb/API/SBThread.h b/lldb/include/lldb/API/SBThread.h index f8ae627da5acee..dcf6aa9d5424e8 100644 --- a/lldb/include/lldb/API/SBThread.h +++ b/lldb/include/lldb/API/SBThread.h @@ -233,7 +233,6 @@ class LLDB_API SBThread { friend class SBBreakpoint; friend class SBBreakpointLocation; friend class SBBreakpointCallbackBaton; - friend class SBSaveCoreOptions; friend class SBExecutionContext; friend class SBFrame; friend class SBProcess; @@ -254,8 +253,6 @@ class LLDB_API SBThread { SBError ResumeNewPlan(lldb_private::ExecutionContext &exe_ctx, lldb_private::ThreadPlan *new_plan); - lldb::ThreadSP GetSP() const; - lldb::ExecutionContextRefSP m_opaque_sp; lldb_private::Thread *operator->(); diff --git a/lldb/include/lldb/Symbol/SaveCoreOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h index f4fed4676fa4ae..583bc1f483d043 100644 --- a/lldb/include/lldb/Symbol/SaveCoreOptions.h +++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h @@ -15,7 +15,6 @@ #include <optional> #include <string> -#include <unordered_set> namespace lldb_private { @@ -33,25 +32,13 @@ class SaveCoreOptions { void SetOutputFile(lldb_private::FileSpec file); const std::optional<lldb_private::FileSpec> GetOutputFile() const; - Status SetProcess(lldb::ProcessSP process_sp); - - Status AddThread(lldb::ThreadSP thread_sp); - bool RemoveThread(lldb::ThreadSP thread_sp); - bool ShouldThreadBeSaved(lldb::tid_t tid) const; - - Status EnsureValidConfiguration(lldb::ProcessSP process_sp) const; - void Clear(); private: - void ClearProcessSpecificData(); - std::optional<std::string> m_plugin_name; std::optional<lldb_private::FileSpec> m_file; std::optional<lldb::SaveCoreStyle> m_style; - lldb::ProcessSP m_process_sp; - std::unordered_set<lldb::tid_t> m_threads_to_save; }; } // namespace lldb_private -#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_SAVECOREOPTIONS_H +#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 7d2e3bddcd4e62..a63d6622949dfe 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -738,15 +738,9 @@ 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, + Status CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style, 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(); diff --git a/lldb/source/API/SBSaveCoreOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp index 6d7aabb1fefa3d..6c3f74596203d6 100644 --- a/lldb/source/API/SBSaveCoreOptions.cpp +++ b/lldb/source/API/SBSaveCoreOptions.cpp @@ -7,6 +7,8 @@ //===----------------------------------------------------------------------===// #include "lldb/API/SBSaveCoreOptions.h" +#include "lldb/API/SBError.h" +#include "lldb/API/SBFileSpec.h" #include "lldb/Host/FileSystem.h" #include "lldb/Symbol/SaveCoreOptions.h" #include "lldb/Utility/Instrumentation.h" @@ -73,21 +75,6 @@ lldb::SaveCoreStyle SBSaveCoreOptions::GetStyle() const { return m_opaque_up->GetStyle(); } -SBError SBSaveCoreOptions::SetProcess(lldb::SBProcess process) { - LLDB_INSTRUMENT_VA(this, process); - return m_opaque_up->SetProcess(process.GetSP()); -} - -SBError SBSaveCoreOptions::AddThread(lldb::SBThread thread) { - LLDB_INSTRUMENT_VA(this, thread); - return m_opaque_up->AddThread(thread.GetSP()); -} - -bool SBSaveCoreOptions::RemoveThread(lldb::SBThread thread) { - LLDB_INSTRUMENT_VA(this, thread); - return m_opaque_up->RemoveThread(thread.GetSP()); -} - void SBSaveCoreOptions::Clear() { LLDB_INSTRUMENT_VA(this); m_opaque_up->Clear(); diff --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp index 55688c9cfa4f1a..53643362421d4f 100644 --- a/lldb/source/API/SBThread.cpp +++ b/lldb/source/API/SBThread.cpp @@ -1331,8 +1331,6 @@ bool SBThread::SafeToCallFunctions() { return true; } -lldb::ThreadSP SBThread::GetSP() const { return m_opaque_sp->GetThreadSP(); } - lldb_private::Thread *SBThread::operator->() { return get(); } diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index fbd78a77805784..01bee8680b7ba5 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -714,10 +714,6 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp, return error; } - error = options.EnsureValidConfiguration(process_sp); - if (error.Fail()) - return error; - if (!options.GetPluginName().has_value()) { // Try saving core directly from the process plugin first. llvm::Expected<bool> ret = diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index 4322bd7e2674f0..ce095bcc48374b 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6347,24 +6347,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, - const 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); @@ -6561,7 +6559,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_style, core_ranges); if (error.Success()) { const uint32_t addr_byte_size = target_arch.GetAddressByteSize(); const ByteOrder byte_order = target_arch.GetByteOrder(); @@ -6732,8 +6730,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()); @@ -6756,7 +6754,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..de212c6b20da7e 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) { @@ -816,7 +819,7 @@ Status MinidumpFileBuilder::AddLinuxFileStreams() { return error; } -Status MinidumpFileBuilder::AddMemoryList() { +Status MinidumpFileBuilder::AddMemoryList(SaveCoreStyle core_style) { Status error; // We first save the thread stacks to ensure they fit in the first UINT32_MAX @@ -825,26 +828,18 @@ 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( + SaveCoreStyle::eSaveCoreStackOnly, ranges_32); 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 +849,14 @@ Status MinidumpFileBuilder::AddMemoryList() { 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. @@ -861,13 +864,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 c039492aa5c5a6..20564e0661f2a2 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h @@ -75,10 +75,8 @@ lldb_private::Status WriteString(const std::string &to_write, class MinidumpFileBuilder { public: MinidumpFileBuilder(lldb::FileUP &&core_file, - const lldb::ProcessSP &process_sp, - const 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::ProcessSP &process_sp) + : m_process_sp(process_sp), m_core_file(std::move(core_file)){}; MinidumpFileBuilder(const MinidumpFileBuilder &) = delete; MinidumpFileBuilder &operator=(const MinidumpFileBuilder &) = delete; @@ -105,7 +103,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 - lldb_private::Status AddMemoryList(); + lldb_private::Status AddMemoryList(lldb::SaveCoreStyle core_style); // Add MiscInfo stream, mainly providing ProcessId lldb_private::Status AddMiscInfo(); // Add informative files about a Linux process @@ -165,9 +163,7 @@ 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::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 2380ff4c00ca9f..faa144bfb5f6a5 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp @@ -74,8 +74,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); Log *log = GetLog(LLDBLog::Object); error = builder.AddHeaderAndCalculateDirectories(); @@ -122,7 +121,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 - error = builder.AddMemoryList(); + error = builder.AddMemoryList(core_style); if (error.Fail()) { LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString()); return false; diff --git a/lldb/source/Symbol/SaveCoreOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp index 3c4ca2d852b76a..0f6fdac1ce22e6 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,8 @@ 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 diff erent 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 diff erent 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 f841c0cfcf8fc8..728f9aadef779a 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -6537,9 +6537,8 @@ static void AddRegion(const MemoryRegionInfo ®ion, bool try_dirty_pages, } 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 bool try_dirty_pages = true; // Before we take any dump, we want to save off the used portions of the @@ -6561,16 +6560,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); } } } @@ -6639,11 +6632,10 @@ static void GetCoreFileSaveRangesStackOnly( } } -Status Process::CalculateCoreFileSaveRanges(const SaveCoreOptions &options, +Status Process::CalculateCoreFileSaveRanges(lldb::SaveCoreStyle core_style, CoreFileMemoryRanges &ranges) { lldb_private::MemoryRegionInfos regions; Status err = GetMemoryRegions(regions); - SaveCoreStyle core_style = options.GetStyle(); if (err.Fail()) return err; if (regions.empty()) @@ -6653,7 +6645,7 @@ 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); switch (core_style) { case eSaveCoreUnspecified: @@ -6681,18 +6673,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(); 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 2e1521cd79fb98..96511d790271fe 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -199,58 +199,3 @@ def test_save_linux_mini_dump(self): os.unlink(core_sb_dirty) if os.path.isfile(core_sb_full): os.unlink(core_sb_full) - - @skipUnlessArch("x86_64") - @skipUnlessPlatform(["linux"]) - def test_save_linux_mini_dump_thread_options(self): - """Test that we can save a Linux mini dump - with a subset of threads""" - - self.build() - exe = self.getBuildArtifact("a.out") - thread_subset_dmp = self.getBuildArtifact("core.thread.subset.dmp") - try: - target = self.dbg.CreateTarget(exe) - process = target.LaunchSimple( - None, None, self.get_process_working_directory() - ) - self.assertState(process.GetState(), lldb.eStateStopped) - - thread_to_include = process.GetThreadAtIndex(0) - options = lldb.SBSaveCoreOptions() - thread_subset_spec = lldb.SBFileSpec(thread_subset_dmp) - options.AddThread(thread_to_include) - options.SetOutputFile(thread_subset_spec) - options.SetPluginName("minidump") - options.SetStyle(lldb.eSaveCoreStackOnly) - error = process.SaveCore(options) - self.assertTrue(error.Success()) - - core_target = self.dbg.CreateTarget(None) - core_process = core_target.LoadCore(thread_subset_dmp) - - self.assertTrue(core_process, PROCESS_IS_VALID) - self.assertEqual(core_process.GetNumThreads(), 1) - saved_thread = core_process.GetThreadAtIndex(0) - expected_thread = process.GetThreadAtIndex(0) - self.assertEqual(expected_thread.GetThreadID(), saved_thread.GetThreadID()) - expected_sp = expected_thread.GetFrameAtIndex(0).GetSP() - saved_sp = saved_thread.GetFrameAtIndex(0).GetSP() - self.assertEqual(expected_sp, saved_sp) - expected_region = lldb.SBMemoryRegionInfo() - saved_region = lldb.SBMemoryRegionInfo() - error = core_process.GetMemoryRegionInfo(saved_sp, saved_region) - self.assertTrue(error.Success(), error.GetCString()) - error = process.GetMemoryRegionInfo(expected_sp, expected_region) - self.assertTrue(error.Success(), error.GetCString()) - self.assertEqual( - expected_region.GetRegionBase(), saved_region.GetRegionBase() - ) - self.assertEqual( - expected_region.GetRegionEnd(), saved_region.GetRegionEnd() - ) - - finally: - self.assertTrue(self.dbg.DeleteTarget(target)) - if os.path.isfile(thread_subset_dmp): - os.unlink(thread_subset_dmp) diff --git a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py index 40d0cc7e96ff48..c509e81d8951a8 100644 --- a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py +++ b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py @@ -4,26 +4,8 @@ from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * -class SBSaveCoreOptionsAPICase(TestBase): - basic_minidump = "basic_minidump.yaml" - basic_minidump_ diff erent_pid = "basic_minidump_ diff erent_pid.yaml" - - def get_process_from_yaml(self, yaml_file): - minidump_path = self.getBuildArtifact(os.path.basename(yaml_file) + ".dmp") - print ("minidump_path: " + minidump_path) - self.yaml2obj(yaml_file, minidump_path) - self.assertTrue(os.path.exists(minidump_path), "yaml2obj did not emit a minidump file") - target = self.dbg.CreateTarget(None) - process = target.LoadCore(minidump_path) - self.assertTrue(process.IsValid(), "Process is not valid") - return process - - def get_basic_process(self): - return self.get_process_from_yaml(self.basic_minidump) - - def get_basic_process_ diff erent_pid(self): - return self.get_process_from_yaml(self.basic_minidump_ diff erent_pid) +class SBSaveCoreOptionsAPICase(TestBase): def test_plugin_name_assignment(self): """Test assignment ensuring valid plugin names only.""" options = lldb.SBSaveCoreOptions() @@ -44,38 +26,3 @@ def test_default_corestyle_behavior(self): """Test that the default core style is unspecified.""" options = lldb.SBSaveCoreOptions() self.assertEqual(options.GetStyle(), lldb.eSaveCoreUnspecified) - - def test_adding_and_removing_thread(self): - """Test adding and removing a thread from save core options.""" - self.assertTrue(self.dbg) - options = lldb.SBSaveCoreOptions() - process = self.get_basic_process() - self.assertTrue(process.IsValid(), "Process is not valid") - thread = process.GetThreadAtIndex(0) - error = options.AddThread(thread) - self.assertTrue(error.Success(), error.GetCString()) - removed_success = options.RemoveThread(thread) - self.assertTrue(removed_success) - removed_success = options.RemoveThread(thread) - self.assertFalse(removed_success) - - - def test_adding_thread_ diff erent_process(self): - """Test adding and removing a thread from save core options.""" - options = lldb.SBSaveCoreOptions() - process = self.get_basic_process() - process_2 = self.get_basic_process_ diff erent_pid() - thread = process.GetThreadAtIndex(0) - error = options.AddThread(thread) - self.assertTrue(error.Success()) - thread_2 = process_2.GetThreadAtIndex(0) - error = options.AddThread(thread_2) - self.assertTrue(error.Fail()) - options.Clear() - error = options.AddThread(thread_2) - self.assertTrue(error.Success()) - options.SetProcess(process) - error = options.AddThread(thread_2) - self.assertTrue(error.Fail()) - error = options.AddThread(thread) - self.assertTrue(error.Success()) diff --git a/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml b/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml deleted file mode 100644 index 993c7da21225a9..00000000000000 --- a/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump.yaml +++ /dev/null @@ -1,26 +0,0 @@ ---- !minidump -Streams: - - Type: SystemInfo - Processor Arch: AMD64 - Processor Level: 6 - Processor Revision: 15876 - Number of Processors: 40 - Platform ID: Linux - CSD Version: 'Linux 3.13.0-91-generic #138-Ubuntu SMP Fri Jun 24 17:00:34 UTC 2016 x86_64' - CPU: - Vendor ID: GenuineIntel - Version Info: 0x00000000 - Feature Info: 0x00000000 - - Type: LinuxProcStatus - Text: | - Name: test-yaml - Umask: 0002 - State: t (tracing stop) - Pid: 8567 - - Type: ThreadList - Threads: - - Thread Id: 0x000074DD - Context: 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B0010000000000033000000000000000000000002020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040109600000000000100000000000000000000000000000068E7D0C8FF7F000068E7D0C8FF7F000097E6D0C8FF7F000010109600000000000000000000000000020000000000000088E4D0C8FF7F0000603FFF85C77F0000F00340000000000080E7D0C8FF7F000000000000000000000000000000000000E0034000000000007F0300000000000000000000000000000000000000000000801F0000FFFF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF252525252525252525252525252525250000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 - Stack: - Start of Memory Range: 0x00007FFFC8D0E000 - Content: 'DEADBEEF' diff --git a/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump_ diff erent_pid.yaml b/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump_ diff erent_pid.yaml deleted file mode 100644 index 7393f1fe62af3d..00000000000000 --- a/lldb/test/API/python_api/sbsavecoreoptions/basic_minidump_ diff erent_pid.yaml +++ /dev/null @@ -1,26 +0,0 @@ ---- !minidump -Streams: - - Type: SystemInfo - Processor Arch: AMD64 - Processor Level: 6 - Processor Revision: 15876 - Number of Processors: 40 - Platform ID: Linux - CSD Version: 'Linux 3.13.0-91-generic #138-Ubuntu SMP Fri Jun 24 17:00:34 UTC 2016 x86_64' - CPU: - Vendor ID: GenuineIntel - Version Info: 0x00000000 - Feature Info: 0x00000000 - - Type: LinuxProcStatus - Text: | - Name: test-yaml - Umask: 0002 - State: t (tracing stop) - Pid: 1967 - - Type: ThreadList - Threads: - - Thread Id: 0x000074DD - Context: 0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B0010000000000033000000000000000000000002020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000040109600000000000100000000000000000000000000000068E7D0C8FF7F000068E7D0C8FF7F000097E6D0C8FF7F000010109600000000000000000000000000020000000000000088E4D0C8FF7F0000603FFF85C77F0000F00340000000000080E7D0C8FF7F000000000000000000000000000000000000E0034000000000007F0300000000000000000000000000000000000000000000801F0000FFFF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF252525252525252525252525252525250000000000000000000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000FF00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 - Stack: - Start of Memory Range: 0x00007FFFC8D0E000 - Content: 'DEADBEEF' _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
