From 4752adac6b8d39512bbfb46726205ceb2301d1c2 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Tue, 9 Jul 2024 13:30:46 -0700 Subject: [PATCH 01/14] Create CoreDumpOption class, and SB equivalent --- lldb/include/lldb/API/SBCoreDumpOptions.h | 37 +++++++++++++++++++ lldb/source/API/SBCoreDumpOptions.cpp | 16 ++++++++ .../Plugins/ObjectFile/CoreDumpOptions.cpp | 25 +++++++++++++ .../Plugins/ObjectFile/CoreDumpOptions.h | 37 +++++++++++++++++++ 4 files changed, 115 insertions(+) create mode 100644 lldb/include/lldb/API/SBCoreDumpOptions.h create mode 100644 lldb/source/API/SBCoreDumpOptions.cpp create mode 100644 lldb/source/Plugins/ObjectFile/CoreDumpOptions.cpp create mode 100644 lldb/source/Plugins/ObjectFile/CoreDumpOptions.h diff --git a/lldb/include/lldb/API/SBCoreDumpOptions.h b/lldb/include/lldb/API/SBCoreDumpOptions.h new file mode 100644 index 0000000000000..523fa34c17f36 --- /dev/null +++ b/lldb/include/lldb/API/SBCoreDumpOptions.h @@ -0,0 +1,37 @@ +//===-- SBCoreDumpOptions.h -------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_API_SBCOREDUMPOPTIONS_H +#define LLDB_API_SBCOREDUMPOPTIONS_H + +namespace lldb { + +class LLDB_API SBCoreDumpOptions { +public: + SBCoreDumpOptions() {}; + SBStatisticsOptions(const lldb::SBCoreDumpOptions &rhs); + ~SBExpressionOptions() = default; + + const SBStatisticsOptions &operator=(const lldb::SBStatisticsOptions &rhs); + + void SetCoreDumpPluginName(const char* plugin); + const char* GetCoreDumpPluginName(); + + void SetCoreDumpStyle(const char* style); + const char* GetCoreDumpStyle(); + + void SetOutputFilePath(const char* path); + const char* GetOutputFilePath(); + +private: + std::unique_ptr m_opaque_up; +}; // SBCoreDumpOptions + +}; // namespace lldb + +#endif // LLDB_API_SBCOREDUMPOPTIONS_H diff --git a/lldb/source/API/SBCoreDumpOptions.cpp b/lldb/source/API/SBCoreDumpOptions.cpp new file mode 100644 index 0000000000000..d051c5b6bef6b --- /dev/null +++ b/lldb/source/API/SBCoreDumpOptions.cpp @@ -0,0 +1,16 @@ +//===-- SBCoreDumpOptions.cpp -----------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/API/SBCoreDumpOptions.h" + +using namespace lldb; +using namespace lldb_private; + +void SBCoreDumpOptions::SetCoreDumpPluginName(const char *name) { + +}; diff --git a/lldb/source/Plugins/ObjectFile/CoreDumpOptions.cpp b/lldb/source/Plugins/ObjectFile/CoreDumpOptions.cpp new file mode 100644 index 0000000000000..00333d1550790 --- /dev/null +++ b/lldb/source/Plugins/ObjectFile/CoreDumpOptions.cpp @@ -0,0 +1,25 @@ +//===-- CoreDumpOptions.cpp -------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/API/CoreDumpOptions.h" + +CoreDumpOptions::SetCoreDumpPluginName(const char *name) { + m_core_dump_plugin_name = name; +} + +CoreDumpOptions::GetCoreDumpPluginName() { return m_core_dump_plugin_name; } + +CoreDumpOptions::SetCoreDumpStyle(lldb::SaveCoreStyle style) { + m_core_dump_style = style; +} + +CoreDumpOptions::GetCoreDumpStyle() { return m_core_dump_style; } + +CoreDumpOptions::SetCoreDumpFile(const char *file) { m_core_dump_file = file; } + +CoreDumpOptions::GetCoreDumpFile() { return m_core_dump_file; } diff --git a/lldb/source/Plugins/ObjectFile/CoreDumpOptions.h b/lldb/source/Plugins/ObjectFile/CoreDumpOptions.h new file mode 100644 index 0000000000000..fe369b0490674 --- /dev/null +++ b/lldb/source/Plugins/ObjectFile/CoreDumpOptions.h @@ -0,0 +1,37 @@ +//===-- CoreDumpOptions.h ---------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_SOURCE_PLUGINS_OBJECTFILE_COREDUMPOPTIONS_H +#define LLDB_SOURCE_PLUGINS_OBJECTFILE_COREDUMPOPTIONS_H + +#include + +using namespace lldb; + +class CoreDumpOptions { + public: + CoreDumpOptions() {}; + ~CoreDumpOptions() = default; + + + void SetCoreDumpPluginName(const char* name); + const char* GetCoreDumpPluginName() const; + + void SetCoreDumpStyle(lldb::SaveCoreStyle style); + lldb::SaveCoreStyle GetCoreDumpStyle() const; + + void SetOutputFilePath(const char* path); + const char* GetOutputFilePath() const; + +private: + std::string m_core_dump_plugin_name; + std::string m_output_file_path; + lldb::SaveCoreStyle m_core_dump_style = lldb::eSaveCoreStyleNone; +}; + +#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_COREDUMPOPTIONS_H From 3acae92ae678092064164e8e492c4789132b81cc Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 10 Jul 2024 15:07:41 -0700 Subject: [PATCH 02/14] Finish all the plumbing and successfully migrate TestProcessSaveCoreMinidump to the new SBCoreDumpOptions API --- lldb/bindings/headers.swig | 1 + .../interface/SBCoreDumpOptionsDocstrings.i | 0 lldb/bindings/interfaces.swig | 2 + lldb/include/lldb/API/LLDB.h | 1 + lldb/include/lldb/API/SBCoreDumpOptions.h | 29 +++++----- lldb/include/lldb/API/SBDefines.h | 1 + lldb/include/lldb/API/SBProcess.h | 6 +++ lldb/include/lldb/Core/PluginManager.h | 4 +- .../lldb/Symbol}/CoreDumpOptions.h | 24 +++++---- lldb/include/lldb/lldb-private-interfaces.h | 4 +- lldb/source/API/CMakeLists.txt | 1 + lldb/source/API/SBCoreDumpOptions.cpp | 53 ++++++++++++++++++- lldb/source/API/SBProcess.cpp | 19 +++++-- lldb/source/Commands/CommandObjectProcess.cpp | 6 ++- lldb/source/Core/PluginManager.cpp | 22 ++++---- .../Plugins/ObjectFile/CoreDumpOptions.cpp | 25 --------- .../ObjectFile/Mach-O/ObjectFileMachO.cpp | 8 +-- .../ObjectFile/Mach-O/ObjectFileMachO.h | 3 +- .../Minidump/ObjectFileMinidump.cpp | 11 ++-- .../ObjectFile/Minidump/ObjectFileMinidump.h | 3 +- .../ObjectFile/PECOFF/ObjectFilePECOFF.cpp | 7 ++- .../ObjectFile/PECOFF/ObjectFilePECOFF.h | 3 +- .../ObjectFile/PECOFF/WindowsMiniDump.cpp | 3 +- .../ObjectFile/PECOFF/WindowsMiniDump.h | 2 +- lldb/source/Symbol/CMakeLists.txt | 1 + lldb/source/Symbol/CoreDumpOptions.cpp | 35 ++++++++++++ .../postmortem/minidump/TestMiniDump.py | 1 + .../process_save_core/TestProcessSaveCore.py | 1 + .../TestProcessSaveCoreMinidump.py | 15 ++++-- 29 files changed, 198 insertions(+), 93 deletions(-) create mode 100644 lldb/bindings/interface/SBCoreDumpOptionsDocstrings.i rename lldb/{source/Plugins/ObjectFile => include/lldb/Symbol}/CoreDumpOptions.h (54%) delete mode 100644 lldb/source/Plugins/ObjectFile/CoreDumpOptions.cpp create mode 100644 lldb/source/Symbol/CoreDumpOptions.cpp diff --git a/lldb/bindings/headers.swig b/lldb/bindings/headers.swig index c91504604b6ac..d65472648ec59 100644 --- a/lldb/bindings/headers.swig +++ b/lldb/bindings/headers.swig @@ -21,6 +21,7 @@ #include "lldb/API/SBCommandReturnObject.h" #include "lldb/API/SBCommunication.h" #include "lldb/API/SBCompileUnit.h" +#include "lldb/API/SBCoreDumpOptions.h" #include "lldb/API/SBData.h" #include "lldb/API/SBDebugger.h" #include "lldb/API/SBDeclaration.h" diff --git a/lldb/bindings/interface/SBCoreDumpOptionsDocstrings.i b/lldb/bindings/interface/SBCoreDumpOptionsDocstrings.i new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/lldb/bindings/interfaces.swig b/lldb/bindings/interfaces.swig index 0953f4c72a910..d25c25dbfc2af 100644 --- a/lldb/bindings/interfaces.swig +++ b/lldb/bindings/interfaces.swig @@ -25,6 +25,7 @@ %include "./interface/SBCommandReturnObjectDocstrings.i" %include "./interface/SBCommunicationDocstrings.i" %include "./interface/SBCompileUnitDocstrings.i" +%include "./interface/SBCoreDumpOptionsDocstrings.i" %include "./interface/SBDataDocstrings.i" %include "./interface/SBDebuggerDocstrings.i" %include "./interface/SBDeclarationDocstrings.i" @@ -101,6 +102,7 @@ %include "lldb/API/SBCommandReturnObject.h" %include "lldb/API/SBCommunication.h" %include "lldb/API/SBCompileUnit.h" +%include "lldb/API/SBCoreDumpOptions.h" %include "lldb/API/SBData.h" %include "lldb/API/SBDebugger.h" %include "lldb/API/SBDeclaration.h" diff --git a/lldb/include/lldb/API/LLDB.h b/lldb/include/lldb/API/LLDB.h index d8cc9f5067fe9..74817ac99ed2b 100644 --- a/lldb/include/lldb/API/LLDB.h +++ b/lldb/include/lldb/API/LLDB.h @@ -23,6 +23,7 @@ #include "lldb/API/SBCommandReturnObject.h" #include "lldb/API/SBCommunication.h" #include "lldb/API/SBCompileUnit.h" +#include "lldb/API/SBCoreDumpOptions.h" #include "lldb/API/SBData.h" #include "lldb/API/SBDebugger.h" #include "lldb/API/SBDeclaration.h" diff --git a/lldb/include/lldb/API/SBCoreDumpOptions.h b/lldb/include/lldb/API/SBCoreDumpOptions.h index 523fa34c17f36..b3e81ceee852a 100644 --- a/lldb/include/lldb/API/SBCoreDumpOptions.h +++ b/lldb/include/lldb/API/SBCoreDumpOptions.h @@ -9,29 +9,34 @@ #ifndef LLDB_API_SBCOREDUMPOPTIONS_H #define LLDB_API_SBCOREDUMPOPTIONS_H +#include "lldb/API/SBDefines.h" +#include "lldb/Symbol/CoreDumpOptions.h" + namespace lldb { class LLDB_API SBCoreDumpOptions { public: - SBCoreDumpOptions() {}; - SBStatisticsOptions(const lldb::SBCoreDumpOptions &rhs); - ~SBExpressionOptions() = default; + SBCoreDumpOptions(const char* filePath); + SBCoreDumpOptions(const lldb::SBCoreDumpOptions &rhs); + ~SBCoreDumpOptions() = default; - const SBStatisticsOptions &operator=(const lldb::SBStatisticsOptions &rhs); + const SBCoreDumpOptions &operator=(const lldb::SBCoreDumpOptions &rhs); void SetCoreDumpPluginName(const char* plugin); - const char* GetCoreDumpPluginName(); + const std::optional GetCoreDumpPluginName() const; + + void SetCoreDumpStyle(lldb::SaveCoreStyle style); + const std::optional GetCoreDumpStyle() const; - void SetCoreDumpStyle(const char* style); - const char* GetCoreDumpStyle(); + const char * GetOutputFile() const; - void SetOutputFilePath(const char* path); - const char* GetOutputFilePath(); +protected: + friend class SBProcess; + lldb_private::CoreDumpOptions &Ref() const; private: - std::unique_ptr m_opaque_up; + std::unique_ptr m_opaque_up; }; // SBCoreDumpOptions - -}; // namespace lldb +} // namespace lldb #endif // LLDB_API_SBCOREDUMPOPTIONS_H diff --git a/lldb/include/lldb/API/SBDefines.h b/lldb/include/lldb/API/SBDefines.h index 87c0a1c3661ca..eb9c59169eaed 100644 --- a/lldb/include/lldb/API/SBDefines.h +++ b/lldb/include/lldb/API/SBDefines.h @@ -61,6 +61,7 @@ class LLDB_API SBCommandPluginInterface; class LLDB_API SBCommandReturnObject; class LLDB_API SBCommunication; class LLDB_API SBCompileUnit; +class LLDB_API SBCoreDumpOptions; class LLDB_API SBData; class LLDB_API SBDebugger; class LLDB_API SBDeclaration; diff --git a/lldb/include/lldb/API/SBProcess.h b/lldb/include/lldb/API/SBProcess.h index a6ab7ae759918..913aa7992a4fd 100644 --- a/lldb/include/lldb/API/SBProcess.h +++ b/lldb/include/lldb/API/SBProcess.h @@ -378,6 +378,12 @@ class LLDB_API SBProcess { /// \param[in] file_name - The name of the file to save the core file to. lldb::SBError SaveCore(const char *file_name); + /// Save the state of the process with the desired settings + /// as defined in the options object. + /// + /// \param[in] options - The options to use when saving the core file. + lldb::SBError SaveCore(SBCoreDumpOptions &options); + /// Query the address load_addr and store the details of the memory /// region that contains it in the supplied SBMemoryRegionInfo object. /// To iterate over all memory regions use GetMemoryRegionList. diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h index f2296e2920238..dcc3a8a062265 100644 --- a/lldb/include/lldb/Core/PluginManager.h +++ b/lldb/include/lldb/Core/PluginManager.h @@ -191,9 +191,7 @@ class PluginManager { GetObjectFileCreateMemoryCallbackForPluginName(llvm::StringRef name); static Status SaveCore(const lldb::ProcessSP &process_sp, - const FileSpec &outfile, - lldb::SaveCoreStyle &core_style, - llvm::StringRef plugin_name); + lldb_private::CoreDumpOptions &core_options); // ObjectContainer static bool RegisterPlugin( diff --git a/lldb/source/Plugins/ObjectFile/CoreDumpOptions.h b/lldb/include/lldb/Symbol/CoreDumpOptions.h similarity index 54% rename from lldb/source/Plugins/ObjectFile/CoreDumpOptions.h rename to lldb/include/lldb/Symbol/CoreDumpOptions.h index fe369b0490674..435031a510d80 100644 --- a/lldb/source/Plugins/ObjectFile/CoreDumpOptions.h +++ b/lldb/include/lldb/Symbol/CoreDumpOptions.h @@ -9,29 +9,35 @@ #ifndef LLDB_SOURCE_PLUGINS_OBJECTFILE_COREDUMPOPTIONS_H #define LLDB_SOURCE_PLUGINS_OBJECTFILE_COREDUMPOPTIONS_H +#include "lldb/Utility/FileSpec.h" +#include "lldb/lldb-forward.h" +#include "lldb/lldb-types.h" + +#include #include -using namespace lldb; +namespace lldb_private { class CoreDumpOptions { public: - CoreDumpOptions() {}; + CoreDumpOptions(const lldb_private::FileSpec &fspec) : + m_core_dump_file(std::move(fspec)) {}; ~CoreDumpOptions() = default; - void SetCoreDumpPluginName(const char* name); - const char* GetCoreDumpPluginName() const; + void SetCoreDumpPluginName(llvm::StringRef name); + std::optional GetCoreDumpPluginName() const; void SetCoreDumpStyle(lldb::SaveCoreStyle style); lldb::SaveCoreStyle GetCoreDumpStyle() const; - void SetOutputFilePath(const char* path); - const char* GetOutputFilePath() const; + const lldb_private::FileSpec& GetOutputFile() const; private: - std::string m_core_dump_plugin_name; - std::string m_output_file_path; - lldb::SaveCoreStyle m_core_dump_style = lldb::eSaveCoreStyleNone; + std::optional m_core_dump_plugin_name; + const lldb_private::FileSpec m_core_dump_file; + lldb::SaveCoreStyle m_core_dump_style = lldb::eSaveCoreUnspecified; }; +} // namespace lldb_private #endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_COREDUMPOPTIONS_H diff --git a/lldb/include/lldb/lldb-private-interfaces.h b/lldb/include/lldb/lldb-private-interfaces.h index cdd9b51d9329c..0948a0482b201 100644 --- a/lldb/include/lldb/lldb-private-interfaces.h +++ b/lldb/include/lldb/lldb-private-interfaces.h @@ -9,6 +9,7 @@ #ifndef LLDB_LLDB_PRIVATE_INTERFACES_H #define LLDB_LLDB_PRIVATE_INTERFACES_H +#include "lldb/Symbol/CoreDumpOptions.h" #include "lldb/lldb-enumerations.h" #include "lldb/lldb-forward.h" #include "lldb/lldb-private-enumerations.h" @@ -55,8 +56,7 @@ typedef ObjectFile *(*ObjectFileCreateMemoryInstance)( const lldb::ModuleSP &module_sp, lldb::WritableDataBufferSP data_sp, const lldb::ProcessSP &process_sp, lldb::addr_t offset); typedef bool (*ObjectFileSaveCore)(const lldb::ProcessSP &process_sp, - const FileSpec &outfile, - lldb::SaveCoreStyle &core_style, + lldb_private::CoreDumpOptions &core_options, Status &error); typedef EmulateInstruction *(*EmulateInstructionCreateInstance)( const ArchSpec &arch, InstructionType inst_type); diff --git a/lldb/source/API/CMakeLists.txt b/lldb/source/API/CMakeLists.txt index 6397101609315..d8cb532f4015f 100644 --- a/lldb/source/API/CMakeLists.txt +++ b/lldb/source/API/CMakeLists.txt @@ -56,6 +56,7 @@ add_lldb_library(liblldb SHARED ${option_framework} SBCommandReturnObject.cpp SBCommunication.cpp SBCompileUnit.cpp + SBCoreDumpOptions.cpp SBData.cpp SBDebugger.cpp SBDeclaration.cpp diff --git a/lldb/source/API/SBCoreDumpOptions.cpp b/lldb/source/API/SBCoreDumpOptions.cpp index d051c5b6bef6b..f7eab4231d819 100644 --- a/lldb/source/API/SBCoreDumpOptions.cpp +++ b/lldb/source/API/SBCoreDumpOptions.cpp @@ -7,10 +7,59 @@ //===----------------------------------------------------------------------===// #include "lldb/API/SBCoreDumpOptions.h" +#include "lldb/Symbol/CoreDumpOptions.h" +#include "lldb/Utility/Instrumentation.h" +#include "lldb/Host/FileSystem.h" + +#include "Utils.h" using namespace lldb; -using namespace lldb_private; + +SBCoreDumpOptions::SBCoreDumpOptions(const char* filePath) { + LLDB_INSTRUMENT_VA(this, filePath); + lldb_private::FileSpec fspec(filePath); + lldb_private::FileSystem::Instance().Resolve(fspec); + m_opaque_up = std::make_unique(fspec); +} + +SBCoreDumpOptions::SBCoreDumpOptions(const SBCoreDumpOptions &rhs) { + LLDB_INSTRUMENT_VA(this, rhs); + + m_opaque_up = clone(rhs.m_opaque_up); +} + +const SBCoreDumpOptions & +SBCoreDumpOptions::operator=(const SBCoreDumpOptions &rhs) { + LLDB_INSTRUMENT_VA(this, rhs); + + if (this != &rhs) + m_opaque_up = clone(rhs.m_opaque_up); + return *this; +} void SBCoreDumpOptions::SetCoreDumpPluginName(const char *name) { + m_opaque_up->SetCoreDumpPluginName(name); +} + +void SBCoreDumpOptions::SetCoreDumpStyle(lldb::SaveCoreStyle style) { + m_opaque_up->SetCoreDumpStyle(style); +} + +const std::optional SBCoreDumpOptions::GetCoreDumpPluginName() const { + const auto &name = m_opaque_up->GetCoreDumpPluginName(); + if (name->empty()) + return std::nullopt; + return name->data(); +} + +const char * SBCoreDumpOptions::GetOutputFile() const { + return m_opaque_up->GetOutputFile().GetFilename().AsCString(); +} + +const std::optional SBCoreDumpOptions::GetCoreDumpStyle() const { + return m_opaque_up->GetCoreDumpStyle(); +} -}; +lldb_private::CoreDumpOptions& SBCoreDumpOptions::Ref() const { + return *m_opaque_up.get(); +} diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp index efb3c8def2796..ef4641c43464b 100644 --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -34,6 +34,7 @@ #include "lldb/API/SBBroadcaster.h" #include "lldb/API/SBCommandReturnObject.h" +#include "lldb/API/SBCoreDumpOptions.h" #include "lldb/API/SBDebugger.h" #include "lldb/API/SBEvent.h" #include "lldb/API/SBFile.h" @@ -1222,7 +1223,18 @@ lldb::SBError SBProcess::SaveCore(const char *file_name) { lldb::SBError SBProcess::SaveCore(const char *file_name, const char *flavor, SaveCoreStyle core_style) { - LLDB_INSTRUMENT_VA(this, file_name, flavor, core_style); + SBCoreDumpOptions options(file_name); + options.SetCoreDumpPluginName(flavor); + options.SetCoreDumpStyle(core_style); + return SaveCore(options); +} + +lldb::SBError SBProcess::SaveCore(SBCoreDumpOptions &options) { + + LLDB_INSTRUMENT_VA(this, + options.GetOutputFile(), + options.GetCoreDumpPluginName(), + options.GetCoreDumpStyle()); lldb::SBError error; ProcessSP process_sp(GetSP()); @@ -1239,10 +1251,7 @@ lldb::SBError SBProcess::SaveCore(const char *file_name, return error; } - FileSpec core_file(file_name); - FileSystem::Instance().Resolve(core_file); - error.ref() = PluginManager::SaveCore(process_sp, core_file, core_style, - flavor); + error.ref() = PluginManager::SaveCore(process_sp, options.Ref()); return error; } diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index 3587a8f529e4a..382b4058c74b3 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -1304,9 +1304,11 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed { FileSpec output_file(command.GetArgumentAtIndex(0)); FileSystem::Instance().Resolve(output_file); SaveCoreStyle corefile_style = m_options.m_requested_save_core_style; + CoreDumpOptions core_dump_options(output_file); + core_dump_options.SetCoreDumpPluginName(m_options.m_requested_plugin_name); + core_dump_options.SetCoreDumpStyle(corefile_style); Status error = - PluginManager::SaveCore(process_sp, output_file, corefile_style, - m_options.m_requested_plugin_name); + PluginManager::SaveCore(process_sp, core_dump_options); if (error.Success()) { if (corefile_style == SaveCoreStyle::eSaveCoreDirtyOnly || corefile_style == SaveCoreStyle::eSaveCoreStackOnly) { diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index b428370d7f333..346d38248e7c2 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -8,6 +8,7 @@ #include "lldb/Core/PluginManager.h" +#include "lldb/Symbol/CoreDumpOptions.h" #include "lldb/Core/Debugger.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" @@ -689,12 +690,10 @@ PluginManager::GetObjectFileCreateMemoryCallbackForPluginName( } Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp, - const FileSpec &outfile, - lldb::SaveCoreStyle &core_style, - llvm::StringRef plugin_name) { - if (plugin_name.empty()) { + lldb_private::CoreDumpOptions &options) { + if (options.GetCoreDumpPluginName()->empty()) { // Try saving core directly from the process plugin first. - llvm::Expected ret = process_sp->SaveCore(outfile.GetPath()); + llvm::Expected ret = process_sp->SaveCore(options.GetOutputFile().GetPath()); if (!ret) return Status(ret.takeError()); if (ret.get()) @@ -705,17 +704,22 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp, Status error; auto &instances = GetObjectFileInstances().GetInstances(); for (auto &instance : instances) { - if (plugin_name.empty() || instance.name == plugin_name) { + if (options.GetCoreDumpPluginName()->empty() || instance.name == options.GetCoreDumpPluginName()) { if (instance.save_core && - instance.save_core(process_sp, outfile, core_style, error)) + instance.save_core(process_sp, options, error)) return error; } } - error.SetErrorString( - "no ObjectFile plugins were able to save a core for this process"); + + // Check to see if any of the object file plugins tried and failed to save. + // If none ran, set the error message. + if (error.Success()) + error.SetErrorString( + "no ObjectFile plugins were able to save a core for this process"); return error; } + #pragma mark ObjectContainer struct ObjectContainerInstance diff --git a/lldb/source/Plugins/ObjectFile/CoreDumpOptions.cpp b/lldb/source/Plugins/ObjectFile/CoreDumpOptions.cpp deleted file mode 100644 index 00333d1550790..0000000000000 --- a/lldb/source/Plugins/ObjectFile/CoreDumpOptions.cpp +++ /dev/null @@ -1,25 +0,0 @@ -//===-- CoreDumpOptions.cpp -------------------------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#include "lldb/API/CoreDumpOptions.h" - -CoreDumpOptions::SetCoreDumpPluginName(const char *name) { - m_core_dump_plugin_name = name; -} - -CoreDumpOptions::GetCoreDumpPluginName() { return m_core_dump_plugin_name; } - -CoreDumpOptions::SetCoreDumpStyle(lldb::SaveCoreStyle style) { - m_core_dump_style = style; -} - -CoreDumpOptions::GetCoreDumpStyle() { return m_core_dump_style; } - -CoreDumpOptions::SetCoreDumpFile(const char *file) { m_core_dump_file = file; } - -CoreDumpOptions::GetCoreDumpFile() { return m_core_dump_file; } diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index 0dcb1bed23548..f5ade48af07dc 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6518,9 +6518,11 @@ struct page_object { uint32_t prot; }; -bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, - const FileSpec &outfile, - lldb::SaveCoreStyle &core_style, Status &error) { +bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, + lldb_private::CoreDumpOptions &core_options, + Status &error) { + auto core_style = core_options.GetCoreDumpStyle(); + const auto outfile = core_options.GetOutputFile(); if (!process_sp) return false; diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h index 55bc688126eb3..d77f0f68cdf11 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h @@ -62,8 +62,7 @@ class ObjectFileMachO : public lldb_private::ObjectFile { lldb_private::ModuleSpecList &specs); static bool SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::FileSpec &outfile, - lldb::SaveCoreStyle &core_style, + lldb_private::CoreDumpOptions &core_options, lldb_private::Status &error); static bool MagicBytesMatch(lldb::DataBufferSP data_sp, lldb::addr_t offset, diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp index 7c875aa3223ed..f7c833edefb5d 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp @@ -56,18 +56,17 @@ size_t ObjectFileMinidump::GetModuleSpecifications( } bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::FileSpec &outfile, - lldb::SaveCoreStyle &core_style, + lldb_private::CoreDumpOptions &core_options, lldb_private::Status &error) { // Set default core style if it isn't set. - if (core_style == SaveCoreStyle::eSaveCoreUnspecified) - core_style = SaveCoreStyle::eSaveCoreStackOnly; + if (core_options.GetCoreDumpStyle() == SaveCoreStyle::eSaveCoreUnspecified) + core_options.SetCoreDumpStyle(SaveCoreStyle::eSaveCoreStackOnly); if (!process_sp) return false; llvm::Expected maybe_core_file = FileSystem::Instance().Open( - outfile, File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate); + core_options.GetOutputFile(), File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate); if (!maybe_core_file) { error = maybe_core_file.takeError(); return false; @@ -119,7 +118,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(core_style); + error = builder.AddMemoryList(core_options.GetCoreDumpStyle()); if (error.Fail()) { LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString()); return false; diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h index b5c40445fe742..81e7b3339ab39 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h @@ -55,8 +55,7 @@ class ObjectFileMinidump : public lldb_private::PluginInterface { // Saves dump in Minidump file format static bool SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::FileSpec &outfile, - lldb::SaveCoreStyle &core_style, + lldb_private::CoreDumpOptions &core_options, lldb_private::Status &error); private: diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp index be0020cad5bee..4cd01d7f74bd6 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp +++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp @@ -355,11 +355,10 @@ size_t ObjectFilePECOFF::GetModuleSpecifications( } bool ObjectFilePECOFF::SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::FileSpec &outfile, - lldb::SaveCoreStyle &core_style, + lldb_private::CoreDumpOptions &core_options, lldb_private::Status &error) { - core_style = eSaveCoreFull; - return SaveMiniDump(process_sp, outfile, error); + core_options.SetCoreDumpStyle(lldb::eSaveCoreFull); + return SaveMiniDump(process_sp, core_options, error); } bool ObjectFilePECOFF::MagicBytesMatch(DataBufferSP data_sp) { diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h index c59701fa65ec3..1074604864978 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h +++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h @@ -82,8 +82,7 @@ class ObjectFilePECOFF : public lldb_private::ObjectFile { lldb_private::ModuleSpecList &specs); static bool SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::FileSpec &outfile, - lldb::SaveCoreStyle &core_style, + lldb_private::CoreDumpOptions &core_options, lldb_private::Status &error); static bool MagicBytesMatch(lldb::DataBufferSP data_sp); diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp index e5cb36910dd25..7fafc9a4370e8 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp +++ b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp @@ -21,11 +21,12 @@ namespace lldb_private { bool SaveMiniDump(const lldb::ProcessSP &process_sp, - const lldb_private::FileSpec &outfile, + CoreDumpOptions &core_options, lldb_private::Status &error) { if (!process_sp) return false; #ifdef _WIN32 + const auto outfile = core_options.GetOutputFile(); HANDLE process_handle = ::OpenProcess( PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, process_sp->GetID()); const std::string file_name = outfile.GetPath(); diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h index 04b93e221362a..f94d873989a18 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h +++ b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h @@ -14,7 +14,7 @@ namespace lldb_private { bool SaveMiniDump(const lldb::ProcessSP &process_sp, - const lldb_private::FileSpec &outfile, + CoreDumpOptions &core_options, lldb_private::Status &error); } // namespace lldb_private diff --git a/lldb/source/Symbol/CMakeLists.txt b/lldb/source/Symbol/CMakeLists.txt index ad3488dcdfcec..f1a2e25cdef48 100644 --- a/lldb/source/Symbol/CMakeLists.txt +++ b/lldb/source/Symbol/CMakeLists.txt @@ -6,6 +6,7 @@ add_lldb_library(lldbSymbol NO_PLUGIN_DEPENDENCIES CompilerDecl.cpp CompilerDeclContext.cpp CompilerType.cpp + CoreDumpOptions.cpp DWARFCallFrameInfo.cpp DebugMacros.cpp DeclVendor.cpp diff --git a/lldb/source/Symbol/CoreDumpOptions.cpp b/lldb/source/Symbol/CoreDumpOptions.cpp new file mode 100644 index 0000000000000..688105a6aea2f --- /dev/null +++ b/lldb/source/Symbol/CoreDumpOptions.cpp @@ -0,0 +1,35 @@ +//===-- CoreDumpOptions.cpp -------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Symbol/CoreDumpOptions.h" + +using namespace lldb; +using namespace lldb_private; + +void CoreDumpOptions::SetCoreDumpPluginName(const llvm::StringRef name) { + m_core_dump_plugin_name = name.data(); +} + +std::optional CoreDumpOptions::GetCoreDumpPluginName() const { + if (!m_core_dump_plugin_name) + return std::nullopt; + return m_core_dump_plugin_name->data(); +} + +void CoreDumpOptions::SetCoreDumpStyle(lldb::SaveCoreStyle style) { + m_core_dump_style = style; +} + +lldb::SaveCoreStyle CoreDumpOptions::GetCoreDumpStyle() const { + // If unspecified, default to stack only + if (m_core_dump_style == lldb::eSaveCoreUnspecified) + return lldb::eSaveCoreStackOnly; + return m_core_dump_style; +} + +const lldb_private::FileSpec& CoreDumpOptions::GetOutputFile() const { return m_core_dump_file; } diff --git a/lldb/test/API/functionalities/postmortem/minidump/TestMiniDump.py b/lldb/test/API/functionalities/postmortem/minidump/TestMiniDump.py index 8fe5d2a1d8562..101950da16d36 100644 --- a/lldb/test/API/functionalities/postmortem/minidump/TestMiniDump.py +++ b/lldb/test/API/functionalities/postmortem/minidump/TestMiniDump.py @@ -130,6 +130,7 @@ def test_deeper_stack_in_mini_dump(self): process = target.LaunchSimple( None, None, self.get_process_working_directory() ) + self.assertState(process.GetState(), lldb.eStateStopped) self.assertTrue(process.SaveCore(core)) self.assertTrue(os.path.isfile(core)) diff --git a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py index c3f0e7597758a..b6406579f3c55 100644 --- a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py +++ b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py @@ -21,6 +21,7 @@ def test_cannot_save_core_unless_process_stopped(self): target = self.dbg.CreateTarget(exe) process = target.LaunchSimple(None, None, self.get_process_working_directory()) self.assertNotEqual(process.GetState(), lldb.eStateStopped) + options = SBCoreDumpOptions(core) error = process.SaveCore(core) self.assertTrue(error.Fail()) 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 1e171e726fb6b..27492756f6b4a 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -132,8 +132,11 @@ def test_save_linux_mini_dump(self): stacks_to_sp_map, ) + options = lldb.SBCoreDumpOptions(core_sb_stack) + options.SetCoreDumpPluginName("minidump") + options.SetCoreDumpStyle(lldb.eSaveCoreStackOnly) # validate saving via SBProcess - error = process.SaveCore(core_sb_stack, "minidump", lldb.eSaveCoreStackOnly) + error = process.SaveCore(options) self.assertTrue(error.Success()) self.assertTrue(os.path.isfile(core_sb_stack)) self.verify_core_file( @@ -144,7 +147,10 @@ def test_save_linux_mini_dump(self): stacks_to_sp_map, ) - error = process.SaveCore(core_sb_dirty, "minidump", lldb.eSaveCoreDirtyOnly) + options = lldb.SBCoreDumpOptions(core_sb_dirty) + options.SetCoreDumpPluginName("minidump") + options.SetCoreDumpStyle(lldb.eSaveCoreDirtyOnly) + error = process.SaveCore(options) self.assertTrue(error.Success()) self.assertTrue(os.path.isfile(core_sb_dirty)) self.verify_core_file( @@ -157,7 +163,10 @@ def test_save_linux_mini_dump(self): # Minidump can now save full core files, but they will be huge and # they might cause this test to timeout. - error = process.SaveCore(core_sb_full, "minidump", lldb.eSaveCoreFull) + options = lldb.SBCoreDumpOptions(core_sb_full) + options.SetCoreDumpPluginName("minidump") + options.SetCoreDumpStyle(lldb.eSaveCoreFull) + error = process.SaveCore(options) self.assertTrue(error.Success()) self.assertTrue(os.path.isfile(core_sb_full)) self.verify_core_file( From e8425792f02c60e74c0070c3674e54d41f06205b Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 10 Jul 2024 15:20:51 -0700 Subject: [PATCH 03/14] Add descriptions to sbcoredumpoptions methods. Run git-clang-format --- lldb/include/lldb/API/SBCoreDumpOptions.h | 23 ++++++++++++++++--- lldb/include/lldb/Symbol/CoreDumpOptions.h | 11 ++++----- lldb/source/API/SBCoreDumpOptions.cpp | 14 ++++++----- lldb/source/API/SBProcess.cpp | 7 +++--- lldb/source/Commands/CommandObjectProcess.cpp | 6 ++--- lldb/source/Core/PluginManager.cpp | 12 +++++----- .../ObjectFile/Mach-O/ObjectFileMachO.cpp | 2 +- .../Minidump/ObjectFileMinidump.cpp | 3 ++- .../ObjectFile/PECOFF/WindowsMiniDump.cpp | 3 +-- .../ObjectFile/PECOFF/WindowsMiniDump.h | 3 +-- lldb/source/Symbol/CoreDumpOptions.cpp | 8 ++++--- 11 files changed, 55 insertions(+), 37 deletions(-) diff --git a/lldb/include/lldb/API/SBCoreDumpOptions.h b/lldb/include/lldb/API/SBCoreDumpOptions.h index b3e81ceee852a..48baf448492b4 100644 --- a/lldb/include/lldb/API/SBCoreDumpOptions.h +++ b/lldb/include/lldb/API/SBCoreDumpOptions.h @@ -16,19 +16,36 @@ namespace lldb { class LLDB_API SBCoreDumpOptions { public: - SBCoreDumpOptions(const char* filePath); + SBCoreDumpOptions(const char *filePath); SBCoreDumpOptions(const lldb::SBCoreDumpOptions &rhs); ~SBCoreDumpOptions() = default; const SBCoreDumpOptions &operator=(const lldb::SBCoreDumpOptions &rhs); - void SetCoreDumpPluginName(const char* plugin); + /// Set the Core dump plugin name. + /// + /// \param plugin Name of the object file plugin. + void SetCoreDumpPluginName(const char *plugin); + + /// Get the Core dump plugin name, if set. + /// + /// \return The name of the plugin, or nullopt if not set. const std::optional GetCoreDumpPluginName() const; + /// Set the Core dump style. + /// + /// \param style The style of the core dump. void SetCoreDumpStyle(lldb::SaveCoreStyle style); + + /// Get the Core dump style, if set. + /// + /// \return The core dump style, or nullopt if not set. const std::optional GetCoreDumpStyle() const; - const char * GetOutputFile() const; + /// Get the output file path + /// + /// \return The output file path. + const char *GetOutputFile() const; protected: friend class SBProcess; diff --git a/lldb/include/lldb/Symbol/CoreDumpOptions.h b/lldb/include/lldb/Symbol/CoreDumpOptions.h index 435031a510d80..b5482bcf24359 100644 --- a/lldb/include/lldb/Symbol/CoreDumpOptions.h +++ b/lldb/include/lldb/Symbol/CoreDumpOptions.h @@ -19,11 +19,10 @@ namespace lldb_private { class CoreDumpOptions { - public: - CoreDumpOptions(const lldb_private::FileSpec &fspec) : - m_core_dump_file(std::move(fspec)) {}; - ~CoreDumpOptions() = default; - +public: + CoreDumpOptions(const lldb_private::FileSpec &fspec) + : m_core_dump_file(std::move(fspec)){}; + ~CoreDumpOptions() = default; void SetCoreDumpPluginName(llvm::StringRef name); std::optional GetCoreDumpPluginName() const; @@ -31,7 +30,7 @@ class CoreDumpOptions { void SetCoreDumpStyle(lldb::SaveCoreStyle style); lldb::SaveCoreStyle GetCoreDumpStyle() const; - const lldb_private::FileSpec& GetOutputFile() const; + const lldb_private::FileSpec &GetOutputFile() const; private: std::optional m_core_dump_plugin_name; diff --git a/lldb/source/API/SBCoreDumpOptions.cpp b/lldb/source/API/SBCoreDumpOptions.cpp index f7eab4231d819..944f44e75039b 100644 --- a/lldb/source/API/SBCoreDumpOptions.cpp +++ b/lldb/source/API/SBCoreDumpOptions.cpp @@ -7,15 +7,15 @@ //===----------------------------------------------------------------------===// #include "lldb/API/SBCoreDumpOptions.h" +#include "lldb/Host/FileSystem.h" #include "lldb/Symbol/CoreDumpOptions.h" #include "lldb/Utility/Instrumentation.h" -#include "lldb/Host/FileSystem.h" #include "Utils.h" using namespace lldb; -SBCoreDumpOptions::SBCoreDumpOptions(const char* filePath) { +SBCoreDumpOptions::SBCoreDumpOptions(const char *filePath) { LLDB_INSTRUMENT_VA(this, filePath); lldb_private::FileSpec fspec(filePath); lldb_private::FileSystem::Instance().Resolve(fspec); @@ -45,21 +45,23 @@ void SBCoreDumpOptions::SetCoreDumpStyle(lldb::SaveCoreStyle style) { m_opaque_up->SetCoreDumpStyle(style); } -const std::optional SBCoreDumpOptions::GetCoreDumpPluginName() const { +const std::optional +SBCoreDumpOptions::GetCoreDumpPluginName() const { const auto &name = m_opaque_up->GetCoreDumpPluginName(); if (name->empty()) return std::nullopt; return name->data(); } -const char * SBCoreDumpOptions::GetOutputFile() const { +const char *SBCoreDumpOptions::GetOutputFile() const { return m_opaque_up->GetOutputFile().GetFilename().AsCString(); } -const std::optional SBCoreDumpOptions::GetCoreDumpStyle() const { +const std::optional +SBCoreDumpOptions::GetCoreDumpStyle() const { return m_opaque_up->GetCoreDumpStyle(); } -lldb_private::CoreDumpOptions& SBCoreDumpOptions::Ref() const { +lldb_private::CoreDumpOptions &SBCoreDumpOptions::Ref() const { return *m_opaque_up.get(); } diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp index ef4641c43464b..436b615bab920 100644 --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -1231,10 +1231,9 @@ lldb::SBError SBProcess::SaveCore(const char *file_name, lldb::SBError SBProcess::SaveCore(SBCoreDumpOptions &options) { - LLDB_INSTRUMENT_VA(this, - options.GetOutputFile(), - options.GetCoreDumpPluginName(), - options.GetCoreDumpStyle()); + LLDB_INSTRUMENT_VA(this, options.GetOutputFile(), + options.GetCoreDumpPluginName(), + options.GetCoreDumpStyle()); lldb::SBError error; ProcessSP process_sp(GetSP()); diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index 382b4058c74b3..f17c50ee22ac3 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -1305,10 +1305,10 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed { FileSystem::Instance().Resolve(output_file); SaveCoreStyle corefile_style = m_options.m_requested_save_core_style; CoreDumpOptions core_dump_options(output_file); - core_dump_options.SetCoreDumpPluginName(m_options.m_requested_plugin_name); + core_dump_options.SetCoreDumpPluginName( + m_options.m_requested_plugin_name); core_dump_options.SetCoreDumpStyle(corefile_style); - Status error = - PluginManager::SaveCore(process_sp, core_dump_options); + Status error = PluginManager::SaveCore(process_sp, core_dump_options); if (error.Success()) { if (corefile_style == SaveCoreStyle::eSaveCoreDirtyOnly || corefile_style == SaveCoreStyle::eSaveCoreStackOnly) { diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index 346d38248e7c2..90231af030d90 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -8,11 +8,11 @@ #include "lldb/Core/PluginManager.h" -#include "lldb/Symbol/CoreDumpOptions.h" #include "lldb/Core/Debugger.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" #include "lldb/Interpreter/OptionValueProperties.h" +#include "lldb/Symbol/CoreDumpOptions.h" #include "lldb/Target/Process.h" #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/Status.h" @@ -693,7 +693,8 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp, lldb_private::CoreDumpOptions &options) { if (options.GetCoreDumpPluginName()->empty()) { // Try saving core directly from the process plugin first. - llvm::Expected ret = process_sp->SaveCore(options.GetOutputFile().GetPath()); + llvm::Expected ret = + process_sp->SaveCore(options.GetOutputFile().GetPath()); if (!ret) return Status(ret.takeError()); if (ret.get()) @@ -704,9 +705,9 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp, Status error; auto &instances = GetObjectFileInstances().GetInstances(); for (auto &instance : instances) { - if (options.GetCoreDumpPluginName()->empty() || instance.name == options.GetCoreDumpPluginName()) { - if (instance.save_core && - instance.save_core(process_sp, options, error)) + if (options.GetCoreDumpPluginName()->empty() || + instance.name == options.GetCoreDumpPluginName()) { + if (instance.save_core && instance.save_core(process_sp, options, error)) return error; } } @@ -719,7 +720,6 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp, return error; } - #pragma mark ObjectContainer struct ObjectContainerInstance diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index f5ade48af07dc..bfb1d6d69f002 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6518,7 +6518,7 @@ struct page_object { uint32_t prot; }; -bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, +bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, lldb_private::CoreDumpOptions &core_options, Status &error) { auto core_style = core_options.GetCoreDumpStyle(); diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp index f7c833edefb5d..a4c7f9d8cf610 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp @@ -66,7 +66,8 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, return false; llvm::Expected maybe_core_file = FileSystem::Instance().Open( - core_options.GetOutputFile(), File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate); + core_options.GetOutputFile(), + File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate); if (!maybe_core_file) { error = maybe_core_file.takeError(); return false; diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp index 7fafc9a4370e8..79eaf36752a85 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp +++ b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp @@ -21,8 +21,7 @@ namespace lldb_private { bool SaveMiniDump(const lldb::ProcessSP &process_sp, - CoreDumpOptions &core_options, - lldb_private::Status &error) { + CoreDumpOptions &core_options, lldb_private::Status &error) { if (!process_sp) return false; #ifdef _WIN32 diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h index f94d873989a18..eaddfa17b7455 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h +++ b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h @@ -14,8 +14,7 @@ namespace lldb_private { bool SaveMiniDump(const lldb::ProcessSP &process_sp, - CoreDumpOptions &core_options, - lldb_private::Status &error); + CoreDumpOptions &core_options, lldb_private::Status &error); } // namespace lldb_private diff --git a/lldb/source/Symbol/CoreDumpOptions.cpp b/lldb/source/Symbol/CoreDumpOptions.cpp index 688105a6aea2f..78e7e1a3e8617 100644 --- a/lldb/source/Symbol/CoreDumpOptions.cpp +++ b/lldb/source/Symbol/CoreDumpOptions.cpp @@ -15,7 +15,7 @@ void CoreDumpOptions::SetCoreDumpPluginName(const llvm::StringRef name) { m_core_dump_plugin_name = name.data(); } -std::optional CoreDumpOptions::GetCoreDumpPluginName() const { +std::optional CoreDumpOptions::GetCoreDumpPluginName() const { if (!m_core_dump_plugin_name) return std::nullopt; return m_core_dump_plugin_name->data(); @@ -25,11 +25,13 @@ void CoreDumpOptions::SetCoreDumpStyle(lldb::SaveCoreStyle style) { m_core_dump_style = style; } -lldb::SaveCoreStyle CoreDumpOptions::GetCoreDumpStyle() const { +lldb::SaveCoreStyle CoreDumpOptions::GetCoreDumpStyle() const { // If unspecified, default to stack only if (m_core_dump_style == lldb::eSaveCoreUnspecified) return lldb::eSaveCoreStackOnly; return m_core_dump_style; } -const lldb_private::FileSpec& CoreDumpOptions::GetOutputFile() const { return m_core_dump_file; } +const lldb_private::FileSpec &CoreDumpOptions::GetOutputFile() const { + return m_core_dump_file; +} From 2673674ffaff7e1e62480928f88f7525c378dbc7 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Tue, 16 Jul 2024 17:38:16 -0700 Subject: [PATCH 04/14] Code review feedback on all API's, fix tests, and remove default behaviors in each flavor of save core --- lldb/include/lldb/API/SBCoreDumpOptions.h | 25 ++++++++----- lldb/include/lldb/API/SBFileSpec.h | 1 + lldb/include/lldb/Core/PluginManager.h | 2 +- lldb/include/lldb/Symbol/CoreDumpOptions.h | 18 +++++----- lldb/include/lldb/lldb-private-interfaces.h | 2 +- lldb/source/API/SBCoreDumpOptions.cpp | 35 ++++++++++++------- lldb/source/API/SBProcess.cpp | 4 ++- lldb/source/Commands/CommandObjectProcess.cpp | 25 ++++++------- lldb/source/Core/PluginManager.cpp | 11 ++++-- .../ObjectFile/Mach-O/ObjectFileMachO.cpp | 12 +++---- .../ObjectFile/Mach-O/ObjectFileMachO.h | 2 +- .../Minidump/ObjectFileMinidump.cpp | 10 ++---- .../ObjectFile/Minidump/ObjectFileMinidump.h | 2 +- .../ObjectFile/PECOFF/ObjectFilePECOFF.cpp | 3 +- .../ObjectFile/PECOFF/ObjectFilePECOFF.h | 2 +- .../ObjectFile/PECOFF/WindowsMiniDump.cpp | 2 +- .../ObjectFile/PECOFF/WindowsMiniDump.h | 2 +- lldb/source/Symbol/CoreDumpOptions.cpp | 32 ++++++++++------- .../TestProcessSaveCoreMinidump.py | 12 +++++-- 19 files changed, 115 insertions(+), 87 deletions(-) diff --git a/lldb/include/lldb/API/SBCoreDumpOptions.h b/lldb/include/lldb/API/SBCoreDumpOptions.h index 48baf448492b4..c9cec5b841ba3 100644 --- a/lldb/include/lldb/API/SBCoreDumpOptions.h +++ b/lldb/include/lldb/API/SBCoreDumpOptions.h @@ -16,7 +16,7 @@ namespace lldb { class LLDB_API SBCoreDumpOptions { public: - SBCoreDumpOptions(const char *filePath); + SBCoreDumpOptions(); SBCoreDumpOptions(const lldb::SBCoreDumpOptions &rhs); ~SBCoreDumpOptions() = default; @@ -29,8 +29,8 @@ class LLDB_API SBCoreDumpOptions { /// Get the Core dump plugin name, if set. /// - /// \return The name of the plugin, or nullopt if not set. - const std::optional GetCoreDumpPluginName() const; + /// \return The name of the plugin, or null if not set. + const char * GetCoreDumpPluginName() const; /// Set the Core dump style. /// @@ -39,13 +39,22 @@ class LLDB_API SBCoreDumpOptions { /// Get the Core dump style, if set. /// - /// \return The core dump style, or nullopt if not set. - const std::optional GetCoreDumpStyle() const; + /// \return The core dump style, or undefined if not set. + lldb::SaveCoreStyle GetCoreDumpStyle() const; - /// Get the output file path + /// Set the output file path /// - /// \return The output file path. - const char *GetOutputFile() const; + /// \param output_file a + /// \class SBFileSpec object that describes the output file. + void SetOutputFile(SBFileSpec &output_file); + + /// Get the output file spec + /// + /// \return The output file spec. + SBFileSpec GetOutputFile() const; + + /// Reset all options. + void Clear(); protected: friend class SBProcess; diff --git a/lldb/include/lldb/API/SBFileSpec.h b/lldb/include/lldb/API/SBFileSpec.h index beefa19ad7f36..bcf090d4c0ae2 100644 --- a/lldb/include/lldb/API/SBFileSpec.h +++ b/lldb/include/lldb/API/SBFileSpec.h @@ -78,6 +78,7 @@ class LLDB_API SBFileSpec { friend class SBTarget; friend class SBThread; friend class SBTrace; + friend class SBCoreDumpOptions; SBFileSpec(const lldb_private::FileSpec &fspec); diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h index dcc3a8a062265..297381fa911e6 100644 --- a/lldb/include/lldb/Core/PluginManager.h +++ b/lldb/include/lldb/Core/PluginManager.h @@ -191,7 +191,7 @@ class PluginManager { GetObjectFileCreateMemoryCallbackForPluginName(llvm::StringRef name); static Status SaveCore(const lldb::ProcessSP &process_sp, - lldb_private::CoreDumpOptions &core_options); + const lldb_private::CoreDumpOptions &core_options); // ObjectContainer static bool RegisterPlugin( diff --git a/lldb/include/lldb/Symbol/CoreDumpOptions.h b/lldb/include/lldb/Symbol/CoreDumpOptions.h index b5482bcf24359..75d70a5967c9f 100644 --- a/lldb/include/lldb/Symbol/CoreDumpOptions.h +++ b/lldb/include/lldb/Symbol/CoreDumpOptions.h @@ -20,22 +20,24 @@ namespace lldb_private { class CoreDumpOptions { public: - CoreDumpOptions(const lldb_private::FileSpec &fspec) - : m_core_dump_file(std::move(fspec)){}; + CoreDumpOptions() {}; ~CoreDumpOptions() = default; - void SetCoreDumpPluginName(llvm::StringRef name); - std::optional GetCoreDumpPluginName() const; + void SetCoreDumpPluginName(const char * name); + std::optional GetCoreDumpPluginName() const; void SetCoreDumpStyle(lldb::SaveCoreStyle style); lldb::SaveCoreStyle GetCoreDumpStyle() const; - const lldb_private::FileSpec &GetOutputFile() const; + void SetOutputFile(lldb_private::FileSpec file); + const std::optional GetOutputFile() const; + + void Clear(); private: - std::optional m_core_dump_plugin_name; - const lldb_private::FileSpec m_core_dump_file; - lldb::SaveCoreStyle m_core_dump_style = lldb::eSaveCoreUnspecified; + std::optional m_plugin_name; + std::optional m_file; + std::optional m_style; }; } // namespace lldb_private diff --git a/lldb/include/lldb/lldb-private-interfaces.h b/lldb/include/lldb/lldb-private-interfaces.h index 0948a0482b201..044a57f87d492 100644 --- a/lldb/include/lldb/lldb-private-interfaces.h +++ b/lldb/include/lldb/lldb-private-interfaces.h @@ -56,7 +56,7 @@ typedef ObjectFile *(*ObjectFileCreateMemoryInstance)( const lldb::ModuleSP &module_sp, lldb::WritableDataBufferSP data_sp, const lldb::ProcessSP &process_sp, lldb::addr_t offset); typedef bool (*ObjectFileSaveCore)(const lldb::ProcessSP &process_sp, - lldb_private::CoreDumpOptions &core_options, + const lldb_private::CoreDumpOptions &core_options, Status &error); typedef EmulateInstruction *(*EmulateInstructionCreateInstance)( const ArchSpec &arch, InstructionType inst_type); diff --git a/lldb/source/API/SBCoreDumpOptions.cpp b/lldb/source/API/SBCoreDumpOptions.cpp index 944f44e75039b..fc00430a2809f 100644 --- a/lldb/source/API/SBCoreDumpOptions.cpp +++ b/lldb/source/API/SBCoreDumpOptions.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "lldb/API/SBFileSpec.h" #include "lldb/API/SBCoreDumpOptions.h" #include "lldb/Host/FileSystem.h" #include "lldb/Symbol/CoreDumpOptions.h" @@ -15,11 +16,8 @@ using namespace lldb; -SBCoreDumpOptions::SBCoreDumpOptions(const char *filePath) { - LLDB_INSTRUMENT_VA(this, filePath); - lldb_private::FileSpec fspec(filePath); - lldb_private::FileSystem::Instance().Resolve(fspec); - m_opaque_up = std::make_unique(fspec); +SBCoreDumpOptions::SBCoreDumpOptions() { + m_opaque_up = std::make_unique(); } SBCoreDumpOptions::SBCoreDumpOptions(const SBCoreDumpOptions &rhs) { @@ -45,19 +43,26 @@ void SBCoreDumpOptions::SetCoreDumpStyle(lldb::SaveCoreStyle style) { m_opaque_up->SetCoreDumpStyle(style); } -const std::optional +void SBCoreDumpOptions::SetOutputFile(lldb::SBFileSpec &file_spec) { + m_opaque_up->SetOutputFile(file_spec.ref()); +} + +const char * SBCoreDumpOptions::GetCoreDumpPluginName() const { - const auto &name = m_opaque_up->GetCoreDumpPluginName(); - if (name->empty()) - return std::nullopt; - return name->data(); + const auto name = m_opaque_up->GetCoreDumpPluginName(); + if (!name) + return nullptr; + return lldb_private::ConstString(name.value()).GetCString(); } -const char *SBCoreDumpOptions::GetOutputFile() const { - return m_opaque_up->GetOutputFile().GetFilename().AsCString(); +SBFileSpec SBCoreDumpOptions::GetOutputFile() const { + const auto file_spec = m_opaque_up->GetOutputFile(); + if (file_spec) + return SBFileSpec(file_spec.value()); + return SBFileSpec(); } -const std::optional +lldb::SaveCoreStyle SBCoreDumpOptions::GetCoreDumpStyle() const { return m_opaque_up->GetCoreDumpStyle(); } @@ -65,3 +70,7 @@ SBCoreDumpOptions::GetCoreDumpStyle() const { lldb_private::CoreDumpOptions &SBCoreDumpOptions::Ref() const { return *m_opaque_up.get(); } + +void SBCoreDumpOptions::Clear() { + m_opaque_up->Clear(); +} diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp index 436b615bab920..a5a8190d97180 100644 --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -1223,7 +1223,9 @@ lldb::SBError SBProcess::SaveCore(const char *file_name) { lldb::SBError SBProcess::SaveCore(const char *file_name, const char *flavor, SaveCoreStyle core_style) { - SBCoreDumpOptions options(file_name); + SBFileSpec fspec(file_name); + SBCoreDumpOptions options; + options.SetOutputFile(fspec); options.SetCoreDumpPluginName(flavor); options.SetCoreDumpStyle(core_style); return SaveCore(options); diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index f17c50ee22ac3..fedc12b4232fe 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -1256,7 +1256,7 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed { class CommandOptions : public Options { public: - CommandOptions() = default; + CommandOptions() : m_core_dump_options() {}; ~CommandOptions() override = default; @@ -1271,13 +1271,13 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed { switch (short_option) { case 'p': - m_requested_plugin_name = option_arg.str(); + m_core_dump_options.SetCoreDumpPluginName(option_arg.data()); break; case 's': - m_requested_save_core_style = + m_core_dump_options.SetCoreDumpStyle( (lldb::SaveCoreStyle)OptionArgParser::ToOptionEnum( option_arg, GetDefinitions()[option_idx].enum_values, - eSaveCoreUnspecified, error); + eSaveCoreUnspecified, error)); break; default: llvm_unreachable("Unimplemented option"); @@ -1287,13 +1287,11 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed { } void OptionParsingStarting(ExecutionContext *execution_context) override { - m_requested_save_core_style = eSaveCoreUnspecified; - m_requested_plugin_name.clear(); + m_core_dump_options.Clear(); } // Instance variables to hold the values for command options. - SaveCoreStyle m_requested_save_core_style = eSaveCoreUnspecified; - std::string m_requested_plugin_name; + CoreDumpOptions m_core_dump_options; }; protected: @@ -1303,15 +1301,12 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed { if (command.GetArgumentCount() == 1) { FileSpec output_file(command.GetArgumentAtIndex(0)); FileSystem::Instance().Resolve(output_file); - SaveCoreStyle corefile_style = m_options.m_requested_save_core_style; - CoreDumpOptions core_dump_options(output_file); - core_dump_options.SetCoreDumpPluginName( - m_options.m_requested_plugin_name); - core_dump_options.SetCoreDumpStyle(corefile_style); + auto core_dump_options = m_options.m_core_dump_options; + core_dump_options.SetOutputFile(output_file); Status error = PluginManager::SaveCore(process_sp, core_dump_options); if (error.Success()) { - if (corefile_style == SaveCoreStyle::eSaveCoreDirtyOnly || - corefile_style == SaveCoreStyle::eSaveCoreStackOnly) { + if (core_dump_options.GetCoreDumpStyle() == SaveCoreStyle::eSaveCoreDirtyOnly || + core_dump_options.GetCoreDumpStyle() == SaveCoreStyle::eSaveCoreStackOnly) { result.AppendMessageWithFormat( "\nModified-memory or stack-memory only corefile " "created. This corefile may \n" diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index 90231af030d90..c0a0630cccb9e 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -690,11 +690,17 @@ PluginManager::GetObjectFileCreateMemoryCallbackForPluginName( } Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp, - lldb_private::CoreDumpOptions &options) { + const lldb_private::CoreDumpOptions &options) { + Status error; + if (!options.GetOutputFile()) { + error.SetErrorString("No output file specified"); + return error; + } + if (options.GetCoreDumpPluginName()->empty()) { // Try saving core directly from the process plugin first. llvm::Expected ret = - process_sp->SaveCore(options.GetOutputFile().GetPath()); + process_sp->SaveCore(options.GetOutputFile()->GetPath()); if (!ret) return Status(ret.takeError()); if (ret.get()) @@ -702,7 +708,6 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp, } // Fall back to object plugins. - Status error; auto &instances = GetObjectFileInstances().GetInstances(); for (auto &instance : instances) { if (options.GetCoreDumpPluginName()->empty() || diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index bfb1d6d69f002..23183ada04b28 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6519,17 +6519,13 @@ struct page_object { }; bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, - lldb_private::CoreDumpOptions &core_options, + const lldb_private::CoreDumpOptions &core_options, Status &error) { auto core_style = core_options.GetCoreDumpStyle(); const auto outfile = core_options.GetOutputFile(); - if (!process_sp) + if (!process_sp || !outfile) return false; - // Default on macOS is to create a dirty-memory-only corefile. - if (core_style == SaveCoreStyle::eSaveCoreUnspecified) - core_style = SaveCoreStyle::eSaveCoreDirtyOnly; - Target &target = process_sp->GetTarget(); const ArchSpec target_arch = target.GetArchitecture(); const llvm::Triple &target_triple = target_arch.GetTriple(); @@ -6813,9 +6809,9 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, buffer.PutHex32(segment.flags); } - std::string core_file_path(outfile.GetPath()); + std::string core_file_path(core_options.GetOutputFile()->GetPath()); auto core_file = FileSystem::Instance().Open( - outfile, File::eOpenOptionWriteOnly | File::eOpenOptionTruncate | + outfile.value(), File::eOpenOptionWriteOnly | File::eOpenOptionTruncate | File::eOpenOptionCanCreate); if (!core_file) { error = core_file.takeError(); diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h index d77f0f68cdf11..108b4ad506a3b 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h @@ -62,7 +62,7 @@ class ObjectFileMachO : public lldb_private::ObjectFile { lldb_private::ModuleSpecList &specs); static bool SaveCore(const lldb::ProcessSP &process_sp, - lldb_private::CoreDumpOptions &core_options, + const lldb_private::CoreDumpOptions &core_options, lldb_private::Status &error); static bool MagicBytesMatch(lldb::DataBufferSP data_sp, lldb::addr_t offset, diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp index a4c7f9d8cf610..cbcc616fda190 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp @@ -56,17 +56,13 @@ size_t ObjectFileMinidump::GetModuleSpecifications( } bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, - lldb_private::CoreDumpOptions &core_options, + const lldb_private::CoreDumpOptions &core_options, lldb_private::Status &error) { - // Set default core style if it isn't set. - if (core_options.GetCoreDumpStyle() == SaveCoreStyle::eSaveCoreUnspecified) - core_options.SetCoreDumpStyle(SaveCoreStyle::eSaveCoreStackOnly); - - if (!process_sp) + if (!process_sp || !core_options.GetOutputFile()) return false; llvm::Expected maybe_core_file = FileSystem::Instance().Open( - core_options.GetOutputFile(), + core_options.GetOutputFile().value(), File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate); if (!maybe_core_file) { error = maybe_core_file.takeError(); diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h index 81e7b3339ab39..775906b11b3a1 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h @@ -55,7 +55,7 @@ class ObjectFileMinidump : public lldb_private::PluginInterface { // Saves dump in Minidump file format static bool SaveCore(const lldb::ProcessSP &process_sp, - lldb_private::CoreDumpOptions &core_options, + const lldb_private::CoreDumpOptions &core_options, lldb_private::Status &error); private: diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp index 4cd01d7f74bd6..7fad1d9434ad5 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp +++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp @@ -355,9 +355,8 @@ size_t ObjectFilePECOFF::GetModuleSpecifications( } bool ObjectFilePECOFF::SaveCore(const lldb::ProcessSP &process_sp, - lldb_private::CoreDumpOptions &core_options, + const lldb_private::CoreDumpOptions &core_options, lldb_private::Status &error) { - core_options.SetCoreDumpStyle(lldb::eSaveCoreFull); return SaveMiniDump(process_sp, core_options, error); } diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h index 1074604864978..0718d5ff72cfa 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h +++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h @@ -82,7 +82,7 @@ class ObjectFilePECOFF : public lldb_private::ObjectFile { lldb_private::ModuleSpecList &specs); static bool SaveCore(const lldb::ProcessSP &process_sp, - lldb_private::CoreDumpOptions &core_options, + const lldb_private::CoreDumpOptions &core_options, lldb_private::Status &error); static bool MagicBytesMatch(lldb::DataBufferSP data_sp); diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp index 79eaf36752a85..5932fcd70d56f 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp +++ b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp @@ -21,7 +21,7 @@ namespace lldb_private { bool SaveMiniDump(const lldb::ProcessSP &process_sp, - CoreDumpOptions &core_options, lldb_private::Status &error) { + const CoreDumpOptions &core_options, lldb_private::Status &error) { if (!process_sp) return false; #ifdef _WIN32 diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h index eaddfa17b7455..64e5011384f31 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h +++ b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h @@ -14,7 +14,7 @@ namespace lldb_private { bool SaveMiniDump(const lldb::ProcessSP &process_sp, - CoreDumpOptions &core_options, lldb_private::Status &error); + const CoreDumpOptions &core_options, lldb_private::Status &error); } // namespace lldb_private diff --git a/lldb/source/Symbol/CoreDumpOptions.cpp b/lldb/source/Symbol/CoreDumpOptions.cpp index 78e7e1a3e8617..0a8fdbb667487 100644 --- a/lldb/source/Symbol/CoreDumpOptions.cpp +++ b/lldb/source/Symbol/CoreDumpOptions.cpp @@ -11,27 +11,35 @@ using namespace lldb; using namespace lldb_private; -void CoreDumpOptions::SetCoreDumpPluginName(const llvm::StringRef name) { - m_core_dump_plugin_name = name.data(); +void CoreDumpOptions::SetCoreDumpPluginName(const char * name) { + m_plugin_name = name; } -std::optional CoreDumpOptions::GetCoreDumpPluginName() const { - if (!m_core_dump_plugin_name) - return std::nullopt; - return m_core_dump_plugin_name->data(); +void CoreDumpOptions::SetCoreDumpStyle(lldb::SaveCoreStyle style) { + m_style = style; } -void CoreDumpOptions::SetCoreDumpStyle(lldb::SaveCoreStyle style) { - m_core_dump_style = style; +void CoreDumpOptions::SetOutputFile(FileSpec file) { + m_file = file; +} + +std::optional CoreDumpOptions::GetCoreDumpPluginName() const { + return m_plugin_name; } lldb::SaveCoreStyle CoreDumpOptions::GetCoreDumpStyle() const { // If unspecified, default to stack only - if (m_core_dump_style == lldb::eSaveCoreUnspecified) + if (m_style == lldb::eSaveCoreUnspecified) return lldb::eSaveCoreStackOnly; - return m_core_dump_style; + return m_style.value(); +} + +const std::optional CoreDumpOptions::GetOutputFile() const { + return m_file; } -const lldb_private::FileSpec &CoreDumpOptions::GetOutputFile() const { - return m_core_dump_file; +void CoreDumpOptions::Clear() { + m_file = std::nullopt; + m_plugin_name = std::nullopt; + m_style = std::nullopt; } 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 27492756f6b4a..d4df539d28f60 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -132,7 +132,9 @@ def test_save_linux_mini_dump(self): stacks_to_sp_map, ) - options = lldb.SBCoreDumpOptions(core_sb_stack) + options = lldb.SBCoreDumpOptions() + core_sb_stack_spec = lldb.SBFileSpec(core_sb_stack) + options.SetOutputFile(core_sb_stack_spec) options.SetCoreDumpPluginName("minidump") options.SetCoreDumpStyle(lldb.eSaveCoreStackOnly) # validate saving via SBProcess @@ -147,7 +149,9 @@ def test_save_linux_mini_dump(self): stacks_to_sp_map, ) - options = lldb.SBCoreDumpOptions(core_sb_dirty) + options = lldb.SBCoreDumpOptions() + core_sb_dirty_spec = lldb.SBFileSpec(core_sb_dirty) + options.SetOutputFile(core_sb_dirty_spec) options.SetCoreDumpPluginName("minidump") options.SetCoreDumpStyle(lldb.eSaveCoreDirtyOnly) error = process.SaveCore(options) @@ -163,7 +167,9 @@ def test_save_linux_mini_dump(self): # Minidump can now save full core files, but they will be huge and # they might cause this test to timeout. - options = lldb.SBCoreDumpOptions(core_sb_full) + options = lldb.SBCoreDumpOptions() + core_sb_full_spec = lldb.SBFileSpec(core_sb_full) + options.SetOutputFile(core_sb_full_spec) options.SetCoreDumpPluginName("minidump") options.SetCoreDumpStyle(lldb.eSaveCoreFull) error = process.SaveCore(options) From 92bc0b39b5dd9edbb06c360b0946dc1a2b574012 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 17 Jul 2024 13:36:07 -0700 Subject: [PATCH 05/14] Code review feedback. Run git-clang-format. Run Python Format --- lldb/include/lldb/API/SBCoreDumpOptions.h | 16 ++++----- lldb/include/lldb/API/SBError.h | 1 + lldb/include/lldb/Core/PluginManager.h | 2 ++ lldb/include/lldb/Symbol/CoreDumpOptions.h | 10 +++--- lldb/include/lldb/lldb-private-interfaces.h | 2 +- lldb/source/API/SBCoreDumpOptions.cpp | 32 ++++++++--------- lldb/source/API/SBProcess.cpp | 13 +++---- lldb/source/Commands/CommandObjectProcess.cpp | 14 ++++---- lldb/source/Core/PluginManager.cpp | 18 ++++++++-- .../ObjectFile/Mach-O/ObjectFileMachO.cpp | 16 +++++---- .../ObjectFile/Mach-O/ObjectFileMachO.h | 2 +- .../Minidump/ObjectFileMinidump.cpp | 14 +++++--- .../ObjectFile/Minidump/ObjectFileMinidump.h | 2 +- .../ObjectFile/PECOFF/ObjectFilePECOFF.cpp | 5 +-- .../ObjectFile/PECOFF/ObjectFilePECOFF.h | 2 +- .../ObjectFile/PECOFF/WindowsMiniDump.cpp | 5 +-- .../ObjectFile/PECOFF/WindowsMiniDump.h | 3 +- lldb/source/Symbol/CoreDumpOptions.cpp | 36 ++++++++++++------- .../process_save_core/TestProcessSaveCore.py | 4 +-- .../TestProcessSaveCoreMinidump.py | 12 +++---- .../TestSBCoreDumpOptions.py | 25 +++++++++++++ 21 files changed, 148 insertions(+), 86 deletions(-) create mode 100644 lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py diff --git a/lldb/include/lldb/API/SBCoreDumpOptions.h b/lldb/include/lldb/API/SBCoreDumpOptions.h index c9cec5b841ba3..2bbe74dcc6829 100644 --- a/lldb/include/lldb/API/SBCoreDumpOptions.h +++ b/lldb/include/lldb/API/SBCoreDumpOptions.h @@ -22,31 +22,31 @@ class LLDB_API SBCoreDumpOptions { const SBCoreDumpOptions &operator=(const lldb::SBCoreDumpOptions &rhs); - /// Set the Core dump plugin name. + /// Set the plugin name. /// /// \param plugin Name of the object file plugin. - void SetCoreDumpPluginName(const char *plugin); + SBError SetPluginName(const char *plugin); /// Get the Core dump plugin name, if set. /// /// \return The name of the plugin, or null if not set. - const char * GetCoreDumpPluginName() const; + const char *GetPluginName() const; /// Set the Core dump style. /// /// \param style The style of the core dump. - void SetCoreDumpStyle(lldb::SaveCoreStyle style); + void SetStyle(lldb::SaveCoreStyle style); /// Get the Core dump style, if set. /// /// \return The core dump style, or undefined if not set. - lldb::SaveCoreStyle GetCoreDumpStyle() const; + lldb::SaveCoreStyle GetStyle() const; /// Set the output file path /// - /// \param output_file a + /// \param output_file a /// \class SBFileSpec object that describes the output file. - void SetOutputFile(SBFileSpec &output_file); + void SetOutputFile(SBFileSpec output_file); /// Get the output file spec /// @@ -58,7 +58,7 @@ class LLDB_API SBCoreDumpOptions { protected: friend class SBProcess; - lldb_private::CoreDumpOptions &Ref() const; + lldb_private::CoreDumpOptions &ref() const; private: std::unique_ptr m_opaque_up; diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h index 1a720a479d9a6..ef5383fc033f1 100644 --- a/lldb/include/lldb/API/SBError.h +++ b/lldb/include/lldb/API/SBError.h @@ -77,6 +77,7 @@ class LLDB_API SBError { friend class SBBreakpointName; friend class SBCommandReturnObject; friend class SBCommunication; + friend class SBCoreDumpOptions; friend class SBData; friend class SBDebugger; friend class SBFile; diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h index 297381fa911e6..bb5a360059d21 100644 --- a/lldb/include/lldb/Core/PluginManager.h +++ b/lldb/include/lldb/Core/PluginManager.h @@ -178,6 +178,8 @@ class PluginManager { static bool UnregisterPlugin(ObjectFileCreateInstance create_callback); + static bool IsRegisteredPluginName(const char *name); + static ObjectFileCreateInstance GetObjectFileCreateCallbackAtIndex(uint32_t idx); diff --git a/lldb/include/lldb/Symbol/CoreDumpOptions.h b/lldb/include/lldb/Symbol/CoreDumpOptions.h index 75d70a5967c9f..544d7704eb2b5 100644 --- a/lldb/include/lldb/Symbol/CoreDumpOptions.h +++ b/lldb/include/lldb/Symbol/CoreDumpOptions.h @@ -20,14 +20,14 @@ namespace lldb_private { class CoreDumpOptions { public: - CoreDumpOptions() {}; + CoreDumpOptions(){}; ~CoreDumpOptions() = default; - void SetCoreDumpPluginName(const char * name); - std::optional GetCoreDumpPluginName() const; + lldb_private::Status SetPluginName(const char *name); + std::optional GetPluginName() const; - void SetCoreDumpStyle(lldb::SaveCoreStyle style); - lldb::SaveCoreStyle GetCoreDumpStyle() const; + void SetStyle(lldb::SaveCoreStyle style); + lldb::SaveCoreStyle GetStyle() const; void SetOutputFile(lldb_private::FileSpec file); const std::optional GetOutputFile() const; diff --git a/lldb/include/lldb/lldb-private-interfaces.h b/lldb/include/lldb/lldb-private-interfaces.h index 044a57f87d492..985040b98d848 100644 --- a/lldb/include/lldb/lldb-private-interfaces.h +++ b/lldb/include/lldb/lldb-private-interfaces.h @@ -56,7 +56,7 @@ typedef ObjectFile *(*ObjectFileCreateMemoryInstance)( const lldb::ModuleSP &module_sp, lldb::WritableDataBufferSP data_sp, const lldb::ProcessSP &process_sp, lldb::addr_t offset); typedef bool (*ObjectFileSaveCore)(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &core_options, + const lldb_private::CoreDumpOptions &options, Status &error); typedef EmulateInstruction *(*EmulateInstructionCreateInstance)( const ArchSpec &arch, InstructionType inst_type); diff --git a/lldb/source/API/SBCoreDumpOptions.cpp b/lldb/source/API/SBCoreDumpOptions.cpp index fc00430a2809f..2f572d4a8a3ad 100644 --- a/lldb/source/API/SBCoreDumpOptions.cpp +++ b/lldb/source/API/SBCoreDumpOptions.cpp @@ -6,8 +6,9 @@ // //===----------------------------------------------------------------------===// -#include "lldb/API/SBFileSpec.h" #include "lldb/API/SBCoreDumpOptions.h" +#include "lldb/API/SBError.h" +#include "lldb/API/SBFileSpec.h" #include "lldb/Host/FileSystem.h" #include "lldb/Symbol/CoreDumpOptions.h" #include "lldb/Utility/Instrumentation.h" @@ -17,6 +18,8 @@ using namespace lldb; SBCoreDumpOptions::SBCoreDumpOptions() { + LLDB_INSTRUMENT_VA(this) + m_opaque_up = std::make_unique(); } @@ -35,21 +38,21 @@ SBCoreDumpOptions::operator=(const SBCoreDumpOptions &rhs) { return *this; } -void SBCoreDumpOptions::SetCoreDumpPluginName(const char *name) { - m_opaque_up->SetCoreDumpPluginName(name); +SBError SBCoreDumpOptions::SetPluginName(const char *name) { + lldb_private::Status error = m_opaque_up->SetPluginName(name); + return SBError(error); } -void SBCoreDumpOptions::SetCoreDumpStyle(lldb::SaveCoreStyle style) { - m_opaque_up->SetCoreDumpStyle(style); +void SBCoreDumpOptions::SetStyle(lldb::SaveCoreStyle style) { + m_opaque_up->SetStyle(style); } -void SBCoreDumpOptions::SetOutputFile(lldb::SBFileSpec &file_spec) { +void SBCoreDumpOptions::SetOutputFile(lldb::SBFileSpec file_spec) { m_opaque_up->SetOutputFile(file_spec.ref()); } -const char * -SBCoreDumpOptions::GetCoreDumpPluginName() const { - const auto name = m_opaque_up->GetCoreDumpPluginName(); +const char *SBCoreDumpOptions::GetPluginName() const { + const auto name = m_opaque_up->GetPluginName(); if (!name) return nullptr; return lldb_private::ConstString(name.value()).GetCString(); @@ -62,15 +65,12 @@ SBFileSpec SBCoreDumpOptions::GetOutputFile() const { return SBFileSpec(); } -lldb::SaveCoreStyle -SBCoreDumpOptions::GetCoreDumpStyle() const { - return m_opaque_up->GetCoreDumpStyle(); +lldb::SaveCoreStyle SBCoreDumpOptions::GetStyle() const { + return m_opaque_up->GetStyle(); } -lldb_private::CoreDumpOptions &SBCoreDumpOptions::Ref() const { +lldb_private::CoreDumpOptions &SBCoreDumpOptions::ref() const { return *m_opaque_up.get(); } -void SBCoreDumpOptions::Clear() { - m_opaque_up->Clear(); -} +void SBCoreDumpOptions::Clear() { m_opaque_up->Clear(); } diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp index a5a8190d97180..63fc1b28f42b8 100644 --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -1223,19 +1223,16 @@ lldb::SBError SBProcess::SaveCore(const char *file_name) { lldb::SBError SBProcess::SaveCore(const char *file_name, const char *flavor, SaveCoreStyle core_style) { - SBFileSpec fspec(file_name); SBCoreDumpOptions options; - options.SetOutputFile(fspec); - options.SetCoreDumpPluginName(flavor); - options.SetCoreDumpStyle(core_style); + options.SetOutputFile(SBFileSpec(file_name)); + options.SetPluginName(flavor); + options.SetStyle(core_style); return SaveCore(options); } lldb::SBError SBProcess::SaveCore(SBCoreDumpOptions &options) { - LLDB_INSTRUMENT_VA(this, options.GetOutputFile(), - options.GetCoreDumpPluginName(), - options.GetCoreDumpStyle()); + LLDB_INSTRUMENT_VA(this, options); lldb::SBError error; ProcessSP process_sp(GetSP()); @@ -1252,7 +1249,7 @@ lldb::SBError SBProcess::SaveCore(SBCoreDumpOptions &options) { return error; } - error.ref() = PluginManager::SaveCore(process_sp, options.Ref()); + error.ref() = PluginManager::SaveCore(process_sp, options.ref()); return error; } diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index fedc12b4232fe..232e8e22dfcf8 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -1256,7 +1256,7 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed { class CommandOptions : public Options { public: - CommandOptions() : m_core_dump_options() {}; + CommandOptions(){}; ~CommandOptions() override = default; @@ -1271,10 +1271,10 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed { switch (short_option) { case 'p': - m_core_dump_options.SetCoreDumpPluginName(option_arg.data()); + m_core_dump_options.SetPluginName(option_arg.data()); break; case 's': - m_core_dump_options.SetCoreDumpStyle( + m_core_dump_options.SetStyle( (lldb::SaveCoreStyle)OptionArgParser::ToOptionEnum( option_arg, GetDefinitions()[option_idx].enum_values, eSaveCoreUnspecified, error)); @@ -1301,12 +1301,14 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed { if (command.GetArgumentCount() == 1) { FileSpec output_file(command.GetArgumentAtIndex(0)); FileSystem::Instance().Resolve(output_file); - auto core_dump_options = m_options.m_core_dump_options; + auto &core_dump_options = m_options.m_core_dump_options; core_dump_options.SetOutputFile(output_file); Status error = PluginManager::SaveCore(process_sp, core_dump_options); if (error.Success()) { - if (core_dump_options.GetCoreDumpStyle() == SaveCoreStyle::eSaveCoreDirtyOnly || - core_dump_options.GetCoreDumpStyle() == SaveCoreStyle::eSaveCoreStackOnly) { + if (core_dump_options.GetStyle() == + SaveCoreStyle::eSaveCoreDirtyOnly || + core_dump_options.GetStyle() == + SaveCoreStyle::eSaveCoreStackOnly) { result.AppendMessageWithFormat( "\nModified-memory or stack-memory only corefile " "created. This corefile may \n" diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index c0a0630cccb9e..7df8a048253c9 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -640,6 +640,18 @@ static ObjectFileInstances &GetObjectFileInstances() { return g_instances; } +bool PluginManager::IsRegisteredPluginName(const char *name) { + if (!name || !name[0]) + return false; + + const auto &instances = GetObjectFileInstances().GetInstances(); + for (auto &instance : instances) { + if (instance.name == name) + return true; + } + return false; +} + bool PluginManager::RegisterPlugin( llvm::StringRef name, llvm::StringRef description, ObjectFileCreateInstance create_callback, @@ -697,7 +709,7 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp, return error; } - if (options.GetCoreDumpPluginName()->empty()) { + if (!options.GetPluginName().has_value()) { // Try saving core directly from the process plugin first. llvm::Expected ret = process_sp->SaveCore(options.GetOutputFile()->GetPath()); @@ -708,10 +720,10 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp, } // Fall back to object plugins. + const auto &plugin_name = options.GetPluginName().value_or(""); auto &instances = GetObjectFileInstances().GetInstances(); for (auto &instance : instances) { - if (options.GetCoreDumpPluginName()->empty() || - instance.name == options.GetCoreDumpPluginName()) { + if (plugin_name.empty() || instance.name == plugin_name) { if (instance.save_core && instance.save_core(process_sp, options, error)) return error; } diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index 23183ada04b28..4b8cc7546bf6e 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6519,11 +6519,15 @@ struct page_object { }; bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &core_options, + const lldb_private::CoreDumpOptions &options, Status &error) { - auto core_style = core_options.GetCoreDumpStyle(); - const auto outfile = core_options.GetOutputFile(); - if (!process_sp || !outfile) + auto core_style = options.GetStyle(); + if (core_style == SaveCoreStyle::eSaveCoreUnspecified) + core_style = SaveCoreStyle::eSaveCoreDirtyOnly; + // The FileSpec is already checked in PluginManager::SaveCore. + assert(options.GetOutputFile().has_value()); + const FileSpec outfile = options.GetOutputFile().value(); + if (!process_sp) return false; Target &target = process_sp->GetTarget(); @@ -6809,9 +6813,9 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, buffer.PutHex32(segment.flags); } - std::string core_file_path(core_options.GetOutputFile()->GetPath()); + std::string core_file_path(outfile.GetPath()); auto core_file = FileSystem::Instance().Open( - outfile.value(), File::eOpenOptionWriteOnly | File::eOpenOptionTruncate | + outfile, File::eOpenOptionWriteOnly | File::eOpenOptionTruncate | File::eOpenOptionCanCreate); if (!core_file) { error = core_file.takeError(); diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h index 108b4ad506a3b..540597b58f9e7 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h @@ -62,7 +62,7 @@ class ObjectFileMachO : public lldb_private::ObjectFile { lldb_private::ModuleSpecList &specs); static bool SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &core_options, + const lldb_private::CoreDumpOptions &options, lldb_private::Status &error); static bool MagicBytesMatch(lldb::DataBufferSP data_sp, lldb::addr_t offset, diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp index cbcc616fda190..9c9fbc4d54e0d 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp @@ -56,13 +56,19 @@ size_t ObjectFileMinidump::GetModuleSpecifications( } bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &core_options, + const lldb_private::CoreDumpOptions &options, lldb_private::Status &error) { - if (!process_sp || !core_options.GetOutputFile()) + assert(options.GetOutputFile().has_value()); + if (!process_sp) return false; + // Minidump defaults to stacks only. + SaveCoreStyle core_style = options.GetStyle(); + if (core_style == SaveCoreStyle::eSaveCoreUnspecified) + core_style = SaveCoreStyle::eSaveCoreStackOnly; + llvm::Expected maybe_core_file = FileSystem::Instance().Open( - core_options.GetOutputFile().value(), + options.GetOutputFile().value(), File::eOpenOptionWriteOnly | File::eOpenOptionCanCreate); if (!maybe_core_file) { error = maybe_core_file.takeError(); @@ -115,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(core_options.GetCoreDumpStyle()); + error = builder.AddMemoryList(core_style); if (error.Fail()) { LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString()); return false; diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h index 775906b11b3a1..df5727785dc4b 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h @@ -55,7 +55,7 @@ class ObjectFileMinidump : public lldb_private::PluginInterface { // Saves dump in Minidump file format static bool SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &core_options, + const lldb_private::CoreDumpOptions &options, lldb_private::Status &error); private: diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp index 7fad1d9434ad5..05b2c675dea4c 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp +++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp @@ -355,9 +355,10 @@ size_t ObjectFilePECOFF::GetModuleSpecifications( } bool ObjectFilePECOFF::SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &core_options, + const lldb_private::CoreDumpOptions &options, lldb_private::Status &error) { - return SaveMiniDump(process_sp, core_options, error); + assert(options.GetOutputFile().has_value()); + return SaveMiniDump(process_sp, options, error); } bool ObjectFilePECOFF::MagicBytesMatch(DataBufferSP data_sp) { diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h index 0718d5ff72cfa..da71323c89e47 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h +++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h @@ -82,7 +82,7 @@ class ObjectFilePECOFF : public lldb_private::ObjectFile { lldb_private::ModuleSpecList &specs); static bool SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &core_options, + const lldb_private::CoreDumpOptions &options, lldb_private::Status &error); static bool MagicBytesMatch(lldb::DataBufferSP data_sp); diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp index 5932fcd70d56f..ff99ea3778511 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp +++ b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp @@ -21,11 +21,12 @@ namespace lldb_private { bool SaveMiniDump(const lldb::ProcessSP &process_sp, - const CoreDumpOptions &core_options, lldb_private::Status &error) { + const CoreDumpOptions &core_options, + lldb_private::Status &error) { if (!process_sp) return false; #ifdef _WIN32 - const auto outfile = core_options.GetOutputFile(); + const auto &outfile = core_options.GetOutputFile().value(); HANDLE process_handle = ::OpenProcess( PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, process_sp->GetID()); const std::string file_name = outfile.GetPath(); diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h index 64e5011384f31..7501bd8683d57 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h +++ b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h @@ -14,7 +14,8 @@ namespace lldb_private { bool SaveMiniDump(const lldb::ProcessSP &process_sp, - const CoreDumpOptions &core_options, lldb_private::Status &error); + const CoreDumpOptions &core_options, + lldb_private::Status &error); } // namespace lldb_private diff --git a/lldb/source/Symbol/CoreDumpOptions.cpp b/lldb/source/Symbol/CoreDumpOptions.cpp index 0a8fdbb667487..728eaffeabfd9 100644 --- a/lldb/source/Symbol/CoreDumpOptions.cpp +++ b/lldb/source/Symbol/CoreDumpOptions.cpp @@ -7,34 +7,44 @@ //===----------------------------------------------------------------------===// #include "lldb/Symbol/CoreDumpOptions.h" +#include "lldb/Core/PluginManager.h" using namespace lldb; using namespace lldb_private; -void CoreDumpOptions::SetCoreDumpPluginName(const char * name) { +Status CoreDumpOptions::SetPluginName(const char *name) { + Status error; + if (!name || !name[0]) { + m_plugin_name = std::nullopt; + error.SetErrorString("no plugin name specified"); + } + + if (!PluginManager::IsRegisteredPluginName(name)) { + error.SetErrorStringWithFormat( + "plugin name '%s' is not a registered plugin", name); + return error; + } + m_plugin_name = name; + return error; } -void CoreDumpOptions::SetCoreDumpStyle(lldb::SaveCoreStyle style) { - m_style = style; -} +void CoreDumpOptions::SetStyle(lldb::SaveCoreStyle style) { m_style = style; } -void CoreDumpOptions::SetOutputFile(FileSpec file) { - m_file = file; -} +void CoreDumpOptions::SetOutputFile(FileSpec file) { m_file = file; } -std::optional CoreDumpOptions::GetCoreDumpPluginName() const { +std::optional CoreDumpOptions::GetPluginName() const { return m_plugin_name; } -lldb::SaveCoreStyle CoreDumpOptions::GetCoreDumpStyle() const { - // If unspecified, default to stack only - if (m_style == lldb::eSaveCoreUnspecified) - return lldb::eSaveCoreStackOnly; +lldb::SaveCoreStyle CoreDumpOptions::GetStyle() const { + if (!m_style.has_value()) + return lldb::eSaveCoreUnspecified; return m_style.value(); } -const std::optional CoreDumpOptions::GetOutputFile() const { +const std::optional +CoreDumpOptions::GetOutputFile() const { return m_file; } diff --git a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py index b6406579f3c55..c136739a2fb74 100644 --- a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py +++ b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py @@ -2,7 +2,6 @@ Test saving a core file (or mini dump). """ - import os import lldb from lldbsuite.test.decorators import * @@ -21,7 +20,8 @@ def test_cannot_save_core_unless_process_stopped(self): target = self.dbg.CreateTarget(exe) process = target.LaunchSimple(None, None, self.get_process_working_directory()) self.assertNotEqual(process.GetState(), lldb.eStateStopped) - options = SBCoreDumpOptions(core) + options = SBCoreDumpOptions() + options.SetOutputFile(SBFileSpec(core)) error = process.SaveCore(core) self.assertTrue(error.Fail()) 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 d4df539d28f60..20096d7b4d8d6 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -135,8 +135,8 @@ def test_save_linux_mini_dump(self): options = lldb.SBCoreDumpOptions() core_sb_stack_spec = lldb.SBFileSpec(core_sb_stack) options.SetOutputFile(core_sb_stack_spec) - options.SetCoreDumpPluginName("minidump") - options.SetCoreDumpStyle(lldb.eSaveCoreStackOnly) + options.SetPluginName("minidump") + options.SetStyle(lldb.eSaveCoreStackOnly) # validate saving via SBProcess error = process.SaveCore(options) self.assertTrue(error.Success()) @@ -152,8 +152,8 @@ def test_save_linux_mini_dump(self): options = lldb.SBCoreDumpOptions() core_sb_dirty_spec = lldb.SBFileSpec(core_sb_dirty) options.SetOutputFile(core_sb_dirty_spec) - options.SetCoreDumpPluginName("minidump") - options.SetCoreDumpStyle(lldb.eSaveCoreDirtyOnly) + options.SetPluginName("minidump") + options.SetStyle(lldb.eSaveCoreDirtyOnly) error = process.SaveCore(options) self.assertTrue(error.Success()) self.assertTrue(os.path.isfile(core_sb_dirty)) @@ -170,8 +170,8 @@ def test_save_linux_mini_dump(self): options = lldb.SBCoreDumpOptions() core_sb_full_spec = lldb.SBFileSpec(core_sb_full) options.SetOutputFile(core_sb_full_spec) - options.SetCoreDumpPluginName("minidump") - options.SetCoreDumpStyle(lldb.eSaveCoreFull) + options.SetPluginName("minidump") + options.SetStyle(lldb.eSaveCoreFull) error = process.SaveCore(options) self.assertTrue(error.Success()) self.assertTrue(os.path.isfile(core_sb_full)) diff --git a/lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py b/lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py new file mode 100644 index 0000000000000..c9232a56bb768 --- /dev/null +++ b/lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py @@ -0,0 +1,25 @@ +"""Test the SBCoreDumpOptions APIs.""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * + + +class SBCoreDumpOptionsAPICase(TestBase): + def test_plugin_name_assignment(self): + """Test""" + options = lldb.SBCoreDumpOptions() + error = options.SetPluginName(None) + self.assertTrue(error.Fail()) + self.assertEqual(options.GetPluginName(), None) + error = options.SetPluginName("Not a real plugin") + self.assertTrue(error.Fail()) + self.assertEqual(options.GetPluginName(), None) + error = options.SetPluginName("minidump") + self.assertTrue(error.Success()) + self.assertEqual(options.GetPluginName(), "minidump") + + def test_default_corestyle_behavior(self): + """Test that the default core style is unspecified.""" + options = lldb.SBCoreDumpOptions() + self.assertEqual(options.GetStyle(), lldb.eSaveCoreUnspecified) From d4d702dec960cf55af046653d1153876f05ff643 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 17 Jul 2024 13:46:15 -0700 Subject: [PATCH 06/14] Revert TestMinidump.py as it only had a line added, and assign the error output of SetPluginName in CommandOptions for savecore --- lldb/source/Commands/CommandObjectProcess.cpp | 2 +- .../API/functionalities/postmortem/minidump/TestMiniDump.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index 232e8e22dfcf8..dbe8035af2a8c 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -1271,7 +1271,7 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed { switch (short_option) { case 'p': - m_core_dump_options.SetPluginName(option_arg.data()); + error = m_core_dump_options.SetPluginName(option_arg.data()); break; case 's': m_core_dump_options.SetStyle( diff --git a/lldb/test/API/functionalities/postmortem/minidump/TestMiniDump.py b/lldb/test/API/functionalities/postmortem/minidump/TestMiniDump.py index 101950da16d36..8fe5d2a1d8562 100644 --- a/lldb/test/API/functionalities/postmortem/minidump/TestMiniDump.py +++ b/lldb/test/API/functionalities/postmortem/minidump/TestMiniDump.py @@ -130,7 +130,6 @@ def test_deeper_stack_in_mini_dump(self): process = target.LaunchSimple( None, None, self.get_process_working_directory() ) - self.assertState(process.GetState(), lldb.eStateStopped) self.assertTrue(process.SaveCore(core)) self.assertTrue(os.path.isfile(core)) From 117d71a84204b6189a18c202666879d3bc87661c Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 17 Jul 2024 13:48:40 -0700 Subject: [PATCH 07/14] Add omitted test description --- .../API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py b/lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py index c9232a56bb768..c7a6f1519efb7 100644 --- a/lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py +++ b/lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py @@ -7,7 +7,7 @@ class SBCoreDumpOptionsAPICase(TestBase): def test_plugin_name_assignment(self): - """Test""" + """Test assignment ensuring valid plugin names only.""" options = lldb.SBCoreDumpOptions() error = options.SetPluginName(None) self.assertTrue(error.Fail()) From 06c05389be533e56b95ee0d93658301724b25934 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 17 Jul 2024 13:57:27 -0700 Subject: [PATCH 08/14] Have SBProcess savecore always call the method with core dump options, instrument all methods in SBCoreDumpOptions --- lldb/source/API/SBCoreDumpOptions.cpp | 12 +++++++++++- lldb/source/API/SBProcess.cpp | 6 +++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lldb/source/API/SBCoreDumpOptions.cpp b/lldb/source/API/SBCoreDumpOptions.cpp index 2f572d4a8a3ad..c42875715105c 100644 --- a/lldb/source/API/SBCoreDumpOptions.cpp +++ b/lldb/source/API/SBCoreDumpOptions.cpp @@ -39,19 +39,23 @@ SBCoreDumpOptions::operator=(const SBCoreDumpOptions &rhs) { } SBError SBCoreDumpOptions::SetPluginName(const char *name) { + LLDB_INSTRUMENT_VA(this, name); lldb_private::Status error = m_opaque_up->SetPluginName(name); return SBError(error); } void SBCoreDumpOptions::SetStyle(lldb::SaveCoreStyle style) { + LLDB_INSTRUMENT_VA(this, style); m_opaque_up->SetStyle(style); } void SBCoreDumpOptions::SetOutputFile(lldb::SBFileSpec file_spec) { + LLDB_INSTRUMENT_VA(this, file_spec); m_opaque_up->SetOutputFile(file_spec.ref()); } const char *SBCoreDumpOptions::GetPluginName() const { + LLDB_INSTRUMENT_VA(this); const auto name = m_opaque_up->GetPluginName(); if (!name) return nullptr; @@ -59,6 +63,7 @@ const char *SBCoreDumpOptions::GetPluginName() const { } SBFileSpec SBCoreDumpOptions::GetOutputFile() const { + LLDB_INSTRUMENT_VA(this); const auto file_spec = m_opaque_up->GetOutputFile(); if (file_spec) return SBFileSpec(file_spec.value()); @@ -66,11 +71,16 @@ SBFileSpec SBCoreDumpOptions::GetOutputFile() const { } lldb::SaveCoreStyle SBCoreDumpOptions::GetStyle() const { + LLDB_INSTRUMENT_VA(this); return m_opaque_up->GetStyle(); } lldb_private::CoreDumpOptions &SBCoreDumpOptions::ref() const { + LLDB_INSTRUMENT_VA(this); return *m_opaque_up.get(); } -void SBCoreDumpOptions::Clear() { m_opaque_up->Clear(); } +void SBCoreDumpOptions::Clear() { + LLDB_INSTRUMENT_VA(this); + m_opaque_up->Clear(); +} diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp index 63fc1b28f42b8..3abc4ee8861bb 100644 --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -1217,12 +1217,16 @@ bool SBProcess::IsInstrumentationRuntimePresent( lldb::SBError SBProcess::SaveCore(const char *file_name) { LLDB_INSTRUMENT_VA(this, file_name); - return SaveCore(file_name, "", SaveCoreStyle::eSaveCoreFull); + SBCoreDumpOptions options; + options.SetOutputFile(SBFileSpec(file_name)); + options.SetStyle(SaveCoreStyle::eSaveCoreFull); + return SaveCore(options); } lldb::SBError SBProcess::SaveCore(const char *file_name, const char *flavor, SaveCoreStyle core_style) { + LLDB_INSTRUMENT_VA(this, file_name, flavor, core_style); SBCoreDumpOptions options; options.SetOutputFile(SBFileSpec(file_name)); options.SetPluginName(flavor); From 21df3370329ccfba402daa9b25c3c76404edc7b2 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 17 Jul 2024 14:39:13 -0700 Subject: [PATCH 09/14] Further code review feedback and formatting. --- lldb/include/lldb/API/SBCoreDumpOptions.h | 3 ++- lldb/include/lldb/Core/PluginManager.h | 2 +- lldb/source/API/SBCoreDumpOptions.cpp | 6 +++--- lldb/source/Commands/CommandObjectProcess.cpp | 2 +- lldb/source/Core/PluginManager.cpp | 4 ++-- .../source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp | 5 ++--- .../Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp | 4 ++-- lldb/source/Symbol/CoreDumpOptions.cpp | 9 +++------ .../sbcoredumpoptions/TestSBCoreDumpOptions.py | 7 +++++-- 9 files changed, 21 insertions(+), 21 deletions(-) diff --git a/lldb/include/lldb/API/SBCoreDumpOptions.h b/lldb/include/lldb/API/SBCoreDumpOptions.h index 2bbe74dcc6829..b603d9be98ed7 100644 --- a/lldb/include/lldb/API/SBCoreDumpOptions.h +++ b/lldb/include/lldb/API/SBCoreDumpOptions.h @@ -22,7 +22,8 @@ class LLDB_API SBCoreDumpOptions { const SBCoreDumpOptions &operator=(const lldb::SBCoreDumpOptions &rhs); - /// Set the plugin name. + /// Set the plugin name. Supplying null or empty string will reset + /// the option. /// /// \param plugin Name of the object file plugin. SBError SetPluginName(const char *plugin); diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h index bb5a360059d21..15f5dd0cb101d 100644 --- a/lldb/include/lldb/Core/PluginManager.h +++ b/lldb/include/lldb/Core/PluginManager.h @@ -178,7 +178,7 @@ class PluginManager { static bool UnregisterPlugin(ObjectFileCreateInstance create_callback); - static bool IsRegisteredPluginName(const char *name); + static bool IsRegisteredObjectFilePluginName(llvm::StringRef name); static ObjectFileCreateInstance GetObjectFileCreateCallbackAtIndex(uint32_t idx); diff --git a/lldb/source/API/SBCoreDumpOptions.cpp b/lldb/source/API/SBCoreDumpOptions.cpp index c42875715105c..cdf51b3d63fd0 100644 --- a/lldb/source/API/SBCoreDumpOptions.cpp +++ b/lldb/source/API/SBCoreDumpOptions.cpp @@ -80,7 +80,7 @@ lldb_private::CoreDumpOptions &SBCoreDumpOptions::ref() const { return *m_opaque_up.get(); } -void SBCoreDumpOptions::Clear() { - LLDB_INSTRUMENT_VA(this); - m_opaque_up->Clear(); +void SBCoreDumpOptions::Clear() { + LLDB_INSTRUMENT_VA(this); + m_opaque_up->Clear(); } diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index dbe8035af2a8c..f32e5489e200a 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -1256,7 +1256,7 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed { class CommandOptions : public Options { public: - CommandOptions(){}; + CommandOptions() = default; ~CommandOptions() override = default; diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index 7df8a048253c9..8075883a53eac 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -640,8 +640,8 @@ static ObjectFileInstances &GetObjectFileInstances() { return g_instances; } -bool PluginManager::IsRegisteredPluginName(const char *name) { - if (!name || !name[0]) +bool PluginManager::IsRegisteredObjectFilePluginName(llvm::StringRef name) { + if (name.empty()) return false; const auto &instances = GetObjectFileInstances().GetInstances(); diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index 4b8cc7546bf6e..d5d05503fb84d 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6524,11 +6524,10 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, auto core_style = options.GetStyle(); if (core_style == SaveCoreStyle::eSaveCoreUnspecified) core_style = SaveCoreStyle::eSaveCoreDirtyOnly; - // The FileSpec is already checked in PluginManager::SaveCore. + // The FileSpec and Process are already checked in PluginManager::SaveCore. assert(options.GetOutputFile().has_value()); const FileSpec outfile = options.GetOutputFile().value(); - if (!process_sp) - return false; + assert(process_sp); Target &target = process_sp->GetTarget(); const ArchSpec target_arch = target.GetArchitecture(); diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp index 9c9fbc4d54e0d..679870007d75d 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp @@ -58,9 +58,9 @@ size_t ObjectFileMinidump::GetModuleSpecifications( bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, const lldb_private::CoreDumpOptions &options, lldb_private::Status &error) { + // Output file and process_sp are both checked in PluginManager::SaveCore. assert(options.GetOutputFile().has_value()); - if (!process_sp) - return false; + assert(process_sp); // Minidump defaults to stacks only. SaveCoreStyle core_style = options.GetStyle(); diff --git a/lldb/source/Symbol/CoreDumpOptions.cpp b/lldb/source/Symbol/CoreDumpOptions.cpp index 728eaffeabfd9..37f1d6123e76d 100644 --- a/lldb/source/Symbol/CoreDumpOptions.cpp +++ b/lldb/source/Symbol/CoreDumpOptions.cpp @@ -16,12 +16,11 @@ Status CoreDumpOptions::SetPluginName(const char *name) { Status error; if (!name || !name[0]) { m_plugin_name = std::nullopt; - error.SetErrorString("no plugin name specified"); } - if (!PluginManager::IsRegisteredPluginName(name)) { + if (!PluginManager::IsRegisteredObjectFilePluginName(name)) { error.SetErrorStringWithFormat( - "plugin name '%s' is not a registered plugin", name); + "plugin name '%s' is not a valid ObjectFile plugin name", name); return error; } @@ -38,9 +37,7 @@ std::optional CoreDumpOptions::GetPluginName() const { } lldb::SaveCoreStyle CoreDumpOptions::GetStyle() const { - if (!m_style.has_value()) - return lldb::eSaveCoreUnspecified; - return m_style.value(); + return m_style.value_or(lldb::eSaveCoreUnspecified); } const std::optional diff --git a/lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py b/lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py index c7a6f1519efb7..129134c0ce33e 100644 --- a/lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py +++ b/lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py @@ -10,14 +10,17 @@ def test_plugin_name_assignment(self): """Test assignment ensuring valid plugin names only.""" options = lldb.SBCoreDumpOptions() error = options.SetPluginName(None) - self.assertTrue(error.Fail()) + self.assertTrue(error.Success()) self.assertEqual(options.GetPluginName(), None) error = options.SetPluginName("Not a real plugin") - self.assertTrue(error.Fail()) + self.assertTrue(error.Success()) self.assertEqual(options.GetPluginName(), None) error = options.SetPluginName("minidump") self.assertTrue(error.Success()) self.assertEqual(options.GetPluginName(), "minidump") + error = options.SetPluginName("") + self.assertTrue(error.Success()) + self.assertEqual(options.GetPluginName(), None) def test_default_corestyle_behavior(self): """Test that the default core style is unspecified.""" From b2f6d60f6a8b613e0d6e94e4ae59daea4161d9c2 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 17 Jul 2024 14:53:26 -0700 Subject: [PATCH 10/14] Another round of feedback, remove instrumentation and reorder private vs public methods, move assert closer to comment and check process_sp in plu pluginmanager.cpp --- lldb/source/API/SBCoreDumpOptions.cpp | 9 ++++----- lldb/source/Core/PluginManager.cpp | 3 +++ .../source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lldb/source/API/SBCoreDumpOptions.cpp b/lldb/source/API/SBCoreDumpOptions.cpp index cdf51b3d63fd0..6878da0b72804 100644 --- a/lldb/source/API/SBCoreDumpOptions.cpp +++ b/lldb/source/API/SBCoreDumpOptions.cpp @@ -75,12 +75,11 @@ lldb::SaveCoreStyle SBCoreDumpOptions::GetStyle() const { return m_opaque_up->GetStyle(); } -lldb_private::CoreDumpOptions &SBCoreDumpOptions::ref() const { - LLDB_INSTRUMENT_VA(this); - return *m_opaque_up.get(); -} - void SBCoreDumpOptions::Clear() { LLDB_INSTRUMENT_VA(this); m_opaque_up->Clear(); } + +lldb_private::CoreDumpOptions &SBCoreDumpOptions::ref() const { + return *m_opaque_up.get(); +} diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index 8075883a53eac..23d03f348fd1e 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -709,6 +709,9 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp, return error; } + if (!process_sp) + 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 d5d05503fb84d..86f4042911357 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6526,8 +6526,8 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, core_style = SaveCoreStyle::eSaveCoreDirtyOnly; // The FileSpec and Process are already checked in PluginManager::SaveCore. assert(options.GetOutputFile().has_value()); - const FileSpec outfile = options.GetOutputFile().value(); assert(process_sp); + const FileSpec outfile = options.GetOutputFile().value(); Target &target = process_sp->GetTarget(); const ArchSpec target_arch = target.GetArchitecture(); From 1785ad3eebf4a9ab6d7530d76333717742d555aa Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 17 Jul 2024 14:59:09 -0700 Subject: [PATCH 11/14] Add the error string that Greg recommended after I had saved changes locally --- lldb/source/Core/PluginManager.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index 23d03f348fd1e..67ee16e056d2f 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -709,8 +709,10 @@ Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp, return error; } - if (!process_sp) + if (!process_sp) { + error.SetErrorString("Invalid process"); return error; + } if (!options.GetPluginName().has_value()) { // Try saving core directly from the process plugin first. From 6edb284b905fec9da758b4b0ea2bb636f84a9ee1 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 18 Jul 2024 10:28:56 -0700 Subject: [PATCH 12/14] Rename every to SaveCoreOptions, fix null string setting the optional to nullopt --- lldb/bindings/headers.swig | 2 +- ...trings.i => SBSaveCoreOptionsDocstrings.i} | 0 lldb/bindings/interfaces.swig | 4 +-- lldb/include/lldb/API/LLDB.h | 2 +- lldb/include/lldb/API/SBDefines.h | 2 +- lldb/include/lldb/API/SBError.h | 2 +- lldb/include/lldb/API/SBFileSpec.h | 2 +- lldb/include/lldb/API/SBProcess.h | 2 +- ...BCoreDumpOptions.h => SBSaveCoreOptions.h} | 26 +++++++-------- lldb/include/lldb/Core/PluginManager.h | 2 +- .../{CoreDumpOptions.h => SaveCoreOptions.h} | 14 ++++---- lldb/include/lldb/lldb-private-interfaces.h | 4 +-- lldb/source/API/CMakeLists.txt | 2 +- lldb/source/API/SBProcess.cpp | 8 ++--- ...eDumpOptions.cpp => SBSaveCoreOptions.cpp} | 32 +++++++++---------- lldb/source/Commands/CommandObjectProcess.cpp | 2 +- lldb/source/Core/PluginManager.cpp | 4 +-- .../ObjectFile/Mach-O/ObjectFileMachO.cpp | 2 +- .../ObjectFile/Mach-O/ObjectFileMachO.h | 2 +- .../Minidump/ObjectFileMinidump.cpp | 2 +- .../ObjectFile/Minidump/ObjectFileMinidump.h | 2 +- .../ObjectFile/PECOFF/ObjectFilePECOFF.cpp | 4 ++- .../ObjectFile/PECOFF/ObjectFilePECOFF.h | 2 +- .../ObjectFile/PECOFF/WindowsMiniDump.cpp | 2 +- .../ObjectFile/PECOFF/WindowsMiniDump.h | 2 +- lldb/source/Symbol/CMakeLists.txt | 2 +- ...oreDumpOptions.cpp => SaveCoreOptions.cpp} | 19 +++++------ .../process_save_core/TestProcessSaveCore.py | 2 +- .../TestProcessSaveCoreMinidump.py | 6 ++-- .../TestSBSaveCoreOptions.py} | 8 ++--- 30 files changed, 84 insertions(+), 81 deletions(-) rename lldb/bindings/interface/{SBCoreDumpOptionsDocstrings.i => SBSaveCoreOptionsDocstrings.i} (100%) rename lldb/include/lldb/API/{SBCoreDumpOptions.h => SBSaveCoreOptions.h} (71%) rename lldb/include/lldb/Symbol/{CoreDumpOptions.h => SaveCoreOptions.h} (75%) rename lldb/source/API/{SBCoreDumpOptions.cpp => SBSaveCoreOptions.cpp} (63%) rename lldb/source/Symbol/{CoreDumpOptions.cpp => SaveCoreOptions.cpp} (66%) rename lldb/test/API/python_api/{sbcoredumpoptions/TestSBCoreDumpOptions.py => sbsavecoreoptions/TestSBSaveCoreOptions.py} (85%) diff --git a/lldb/bindings/headers.swig b/lldb/bindings/headers.swig index d65472648ec59..c0dde905f986b 100644 --- a/lldb/bindings/headers.swig +++ b/lldb/bindings/headers.swig @@ -21,7 +21,7 @@ #include "lldb/API/SBCommandReturnObject.h" #include "lldb/API/SBCommunication.h" #include "lldb/API/SBCompileUnit.h" -#include "lldb/API/SBCoreDumpOptions.h" +#include "lldb/API/SBSaveCoreOptions.h" #include "lldb/API/SBData.h" #include "lldb/API/SBDebugger.h" #include "lldb/API/SBDeclaration.h" diff --git a/lldb/bindings/interface/SBCoreDumpOptionsDocstrings.i b/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i similarity index 100% rename from lldb/bindings/interface/SBCoreDumpOptionsDocstrings.i rename to lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i diff --git a/lldb/bindings/interfaces.swig b/lldb/bindings/interfaces.swig index d25c25dbfc2af..8a6fed95f0b72 100644 --- a/lldb/bindings/interfaces.swig +++ b/lldb/bindings/interfaces.swig @@ -25,7 +25,7 @@ %include "./interface/SBCommandReturnObjectDocstrings.i" %include "./interface/SBCommunicationDocstrings.i" %include "./interface/SBCompileUnitDocstrings.i" -%include "./interface/SBCoreDumpOptionsDocstrings.i" +%include "./interface/SBSaveCoreOptionsDocstrings.i" %include "./interface/SBDataDocstrings.i" %include "./interface/SBDebuggerDocstrings.i" %include "./interface/SBDeclarationDocstrings.i" @@ -102,7 +102,7 @@ %include "lldb/API/SBCommandReturnObject.h" %include "lldb/API/SBCommunication.h" %include "lldb/API/SBCompileUnit.h" -%include "lldb/API/SBCoreDumpOptions.h" +%include "lldb/API/SBSaveCoreOptions.h" %include "lldb/API/SBData.h" %include "lldb/API/SBDebugger.h" %include "lldb/API/SBDeclaration.h" diff --git a/lldb/include/lldb/API/LLDB.h b/lldb/include/lldb/API/LLDB.h index 74817ac99ed2b..40368e036e0e4 100644 --- a/lldb/include/lldb/API/LLDB.h +++ b/lldb/include/lldb/API/LLDB.h @@ -23,7 +23,6 @@ #include "lldb/API/SBCommandReturnObject.h" #include "lldb/API/SBCommunication.h" #include "lldb/API/SBCompileUnit.h" -#include "lldb/API/SBCoreDumpOptions.h" #include "lldb/API/SBData.h" #include "lldb/API/SBDebugger.h" #include "lldb/API/SBDeclaration.h" @@ -58,6 +57,7 @@ #include "lldb/API/SBQueue.h" #include "lldb/API/SBQueueItem.h" #include "lldb/API/SBReproducer.h" +#include "lldb/API/SBSaveCoreOptions.h" #include "lldb/API/SBSection.h" #include "lldb/API/SBSourceManager.h" #include "lldb/API/SBStatisticsOptions.h" diff --git a/lldb/include/lldb/API/SBDefines.h b/lldb/include/lldb/API/SBDefines.h index eb9c59169eaed..f42e2be558d2e 100644 --- a/lldb/include/lldb/API/SBDefines.h +++ b/lldb/include/lldb/API/SBDefines.h @@ -61,7 +61,7 @@ class LLDB_API SBCommandPluginInterface; class LLDB_API SBCommandReturnObject; class LLDB_API SBCommunication; class LLDB_API SBCompileUnit; -class LLDB_API SBCoreDumpOptions; +class LLDB_API SBSaveCoreOptions; class LLDB_API SBData; class LLDB_API SBDebugger; class LLDB_API SBDeclaration; diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h index ef5383fc033f1..17f2c6c3027af 100644 --- a/lldb/include/lldb/API/SBError.h +++ b/lldb/include/lldb/API/SBError.h @@ -77,7 +77,7 @@ class LLDB_API SBError { friend class SBBreakpointName; friend class SBCommandReturnObject; friend class SBCommunication; - friend class SBCoreDumpOptions; + friend class SBSaveCoreOptions; friend class SBData; friend class SBDebugger; friend class SBFile; diff --git a/lldb/include/lldb/API/SBFileSpec.h b/lldb/include/lldb/API/SBFileSpec.h index bcf090d4c0ae2..36641843aabeb 100644 --- a/lldb/include/lldb/API/SBFileSpec.h +++ b/lldb/include/lldb/API/SBFileSpec.h @@ -78,7 +78,7 @@ class LLDB_API SBFileSpec { friend class SBTarget; friend class SBThread; friend class SBTrace; - friend class SBCoreDumpOptions; + friend class SBSaveCoreOptions; SBFileSpec(const lldb_private::FileSpec &fspec); diff --git a/lldb/include/lldb/API/SBProcess.h b/lldb/include/lldb/API/SBProcess.h index 913aa7992a4fd..778be79583990 100644 --- a/lldb/include/lldb/API/SBProcess.h +++ b/lldb/include/lldb/API/SBProcess.h @@ -382,7 +382,7 @@ class LLDB_API SBProcess { /// as defined in the options object. /// /// \param[in] options - The options to use when saving the core file. - lldb::SBError SaveCore(SBCoreDumpOptions &options); + lldb::SBError SaveCore(SBSaveCoreOptions &options); /// Query the address load_addr and store the details of the memory /// region that contains it in the supplied SBMemoryRegionInfo object. diff --git a/lldb/include/lldb/API/SBCoreDumpOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h similarity index 71% rename from lldb/include/lldb/API/SBCoreDumpOptions.h rename to lldb/include/lldb/API/SBSaveCoreOptions.h index b603d9be98ed7..a19e00c2cef8a 100644 --- a/lldb/include/lldb/API/SBCoreDumpOptions.h +++ b/lldb/include/lldb/API/SBSaveCoreOptions.h @@ -1,4 +1,4 @@ -//===-- SBCoreDumpOptions.h -------------------------------------*- C++ -*-===// +//===-- SBSaveCoreOptions.h -------------------------------------*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,21 +6,21 @@ // //===----------------------------------------------------------------------===// -#ifndef LLDB_API_SBCOREDUMPOPTIONS_H -#define LLDB_API_SBCOREDUMPOPTIONS_H +#ifndef LLDB_API_SBSaveCoreOPTIONS_H +#define LLDB_API_SBSaveCoreOPTIONS_H #include "lldb/API/SBDefines.h" -#include "lldb/Symbol/CoreDumpOptions.h" +#include "lldb/Symbol/SaveCoreOptions.h" namespace lldb { -class LLDB_API SBCoreDumpOptions { +class LLDB_API SBSaveCoreOptions { public: - SBCoreDumpOptions(); - SBCoreDumpOptions(const lldb::SBCoreDumpOptions &rhs); - ~SBCoreDumpOptions() = default; + SBSaveCoreOptions(); + SBSaveCoreOptions(const lldb::SBSaveCoreOptions &rhs); + ~SBSaveCoreOptions() = default; - const SBCoreDumpOptions &operator=(const lldb::SBCoreDumpOptions &rhs); + const SBSaveCoreOptions &operator=(const lldb::SBSaveCoreOptions &rhs); /// Set the plugin name. Supplying null or empty string will reset /// the option. @@ -59,11 +59,11 @@ class LLDB_API SBCoreDumpOptions { protected: friend class SBProcess; - lldb_private::CoreDumpOptions &ref() const; + lldb_private::SaveCoreOptions &ref() const; private: - std::unique_ptr m_opaque_up; -}; // SBCoreDumpOptions + std::unique_ptr m_opaque_up; +}; // SBSaveCoreOptions } // namespace lldb -#endif // LLDB_API_SBCOREDUMPOPTIONS_H +#endif // LLDB_API_SBSaveCoreOPTIONS_H diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h index 15f5dd0cb101d..38a291d9f0afd 100644 --- a/lldb/include/lldb/Core/PluginManager.h +++ b/lldb/include/lldb/Core/PluginManager.h @@ -193,7 +193,7 @@ class PluginManager { GetObjectFileCreateMemoryCallbackForPluginName(llvm::StringRef name); static Status SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &core_options); + const lldb_private::SaveCoreOptions &core_options); // ObjectContainer static bool RegisterPlugin( diff --git a/lldb/include/lldb/Symbol/CoreDumpOptions.h b/lldb/include/lldb/Symbol/SaveCoreOptions.h similarity index 75% rename from lldb/include/lldb/Symbol/CoreDumpOptions.h rename to lldb/include/lldb/Symbol/SaveCoreOptions.h index 544d7704eb2b5..583bc1f483d04 100644 --- a/lldb/include/lldb/Symbol/CoreDumpOptions.h +++ b/lldb/include/lldb/Symbol/SaveCoreOptions.h @@ -1,4 +1,4 @@ -//===-- CoreDumpOptions.h ---------------------------------------*- C++ -*-===// +//===-- SaveCoreOptions.h ---------------------------------------*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,8 +6,8 @@ // //===----------------------------------------------------------------------===// -#ifndef LLDB_SOURCE_PLUGINS_OBJECTFILE_COREDUMPOPTIONS_H -#define LLDB_SOURCE_PLUGINS_OBJECTFILE_COREDUMPOPTIONS_H +#ifndef LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H +#define LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H #include "lldb/Utility/FileSpec.h" #include "lldb/lldb-forward.h" @@ -18,10 +18,10 @@ namespace lldb_private { -class CoreDumpOptions { +class SaveCoreOptions { public: - CoreDumpOptions(){}; - ~CoreDumpOptions() = default; + SaveCoreOptions(){}; + ~SaveCoreOptions() = default; lldb_private::Status SetPluginName(const char *name); std::optional GetPluginName() const; @@ -41,4 +41,4 @@ class CoreDumpOptions { }; } // namespace lldb_private -#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_COREDUMPOPTIONS_H +#endif // LLDB_SOURCE_PLUGINS_OBJECTFILE_SaveCoreOPTIONS_H diff --git a/lldb/include/lldb/lldb-private-interfaces.h b/lldb/include/lldb/lldb-private-interfaces.h index 985040b98d848..10eaf1e6a5add 100644 --- a/lldb/include/lldb/lldb-private-interfaces.h +++ b/lldb/include/lldb/lldb-private-interfaces.h @@ -9,7 +9,7 @@ #ifndef LLDB_LLDB_PRIVATE_INTERFACES_H #define LLDB_LLDB_PRIVATE_INTERFACES_H -#include "lldb/Symbol/CoreDumpOptions.h" +#include "lldb/Symbol/SaveCoreOptions.h" #include "lldb/lldb-enumerations.h" #include "lldb/lldb-forward.h" #include "lldb/lldb-private-enumerations.h" @@ -56,7 +56,7 @@ typedef ObjectFile *(*ObjectFileCreateMemoryInstance)( const lldb::ModuleSP &module_sp, lldb::WritableDataBufferSP data_sp, const lldb::ProcessSP &process_sp, lldb::addr_t offset); typedef bool (*ObjectFileSaveCore)(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &options, + const lldb_private::SaveCoreOptions &options, Status &error); typedef EmulateInstruction *(*EmulateInstructionCreateInstance)( const ArchSpec &arch, InstructionType inst_type); diff --git a/lldb/source/API/CMakeLists.txt b/lldb/source/API/CMakeLists.txt index d8cb532f4015f..a32bc58507d8e 100644 --- a/lldb/source/API/CMakeLists.txt +++ b/lldb/source/API/CMakeLists.txt @@ -56,7 +56,7 @@ add_lldb_library(liblldb SHARED ${option_framework} SBCommandReturnObject.cpp SBCommunication.cpp SBCompileUnit.cpp - SBCoreDumpOptions.cpp + SBSaveCoreOptions.cpp SBData.cpp SBDebugger.cpp SBDeclaration.cpp diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp index 3abc4ee8861bb..8fe42c9416247 100644 --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -34,13 +34,13 @@ #include "lldb/API/SBBroadcaster.h" #include "lldb/API/SBCommandReturnObject.h" -#include "lldb/API/SBCoreDumpOptions.h" #include "lldb/API/SBDebugger.h" #include "lldb/API/SBEvent.h" #include "lldb/API/SBFile.h" #include "lldb/API/SBFileSpec.h" #include "lldb/API/SBMemoryRegionInfo.h" #include "lldb/API/SBMemoryRegionInfoList.h" +#include "lldb/API/SBSaveCoreOptions.h" #include "lldb/API/SBScriptObject.h" #include "lldb/API/SBStream.h" #include "lldb/API/SBStringList.h" @@ -1217,7 +1217,7 @@ bool SBProcess::IsInstrumentationRuntimePresent( lldb::SBError SBProcess::SaveCore(const char *file_name) { LLDB_INSTRUMENT_VA(this, file_name); - SBCoreDumpOptions options; + SBSaveCoreOptions options; options.SetOutputFile(SBFileSpec(file_name)); options.SetStyle(SaveCoreStyle::eSaveCoreFull); return SaveCore(options); @@ -1227,14 +1227,14 @@ lldb::SBError SBProcess::SaveCore(const char *file_name, const char *flavor, SaveCoreStyle core_style) { LLDB_INSTRUMENT_VA(this, file_name, flavor, core_style); - SBCoreDumpOptions options; + SBSaveCoreOptions options; options.SetOutputFile(SBFileSpec(file_name)); options.SetPluginName(flavor); options.SetStyle(core_style); return SaveCore(options); } -lldb::SBError SBProcess::SaveCore(SBCoreDumpOptions &options) { +lldb::SBError SBProcess::SaveCore(SBSaveCoreOptions &options) { LLDB_INSTRUMENT_VA(this, options); diff --git a/lldb/source/API/SBCoreDumpOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp similarity index 63% rename from lldb/source/API/SBCoreDumpOptions.cpp rename to lldb/source/API/SBSaveCoreOptions.cpp index 6878da0b72804..6c3f74596203d 100644 --- a/lldb/source/API/SBCoreDumpOptions.cpp +++ b/lldb/source/API/SBSaveCoreOptions.cpp @@ -1,4 +1,4 @@ -//===-- SBCoreDumpOptions.cpp -----------------------------------*- C++ -*-===// +//===-- SBSaveCoreOptions.cpp -----------------------------------*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,31 +6,31 @@ // //===----------------------------------------------------------------------===// -#include "lldb/API/SBCoreDumpOptions.h" +#include "lldb/API/SBSaveCoreOptions.h" #include "lldb/API/SBError.h" #include "lldb/API/SBFileSpec.h" #include "lldb/Host/FileSystem.h" -#include "lldb/Symbol/CoreDumpOptions.h" +#include "lldb/Symbol/SaveCoreOptions.h" #include "lldb/Utility/Instrumentation.h" #include "Utils.h" using namespace lldb; -SBCoreDumpOptions::SBCoreDumpOptions() { +SBSaveCoreOptions::SBSaveCoreOptions() { LLDB_INSTRUMENT_VA(this) - m_opaque_up = std::make_unique(); + m_opaque_up = std::make_unique(); } -SBCoreDumpOptions::SBCoreDumpOptions(const SBCoreDumpOptions &rhs) { +SBSaveCoreOptions::SBSaveCoreOptions(const SBSaveCoreOptions &rhs) { LLDB_INSTRUMENT_VA(this, rhs); m_opaque_up = clone(rhs.m_opaque_up); } -const SBCoreDumpOptions & -SBCoreDumpOptions::operator=(const SBCoreDumpOptions &rhs) { +const SBSaveCoreOptions & +SBSaveCoreOptions::operator=(const SBSaveCoreOptions &rhs) { LLDB_INSTRUMENT_VA(this, rhs); if (this != &rhs) @@ -38,23 +38,23 @@ SBCoreDumpOptions::operator=(const SBCoreDumpOptions &rhs) { return *this; } -SBError SBCoreDumpOptions::SetPluginName(const char *name) { +SBError SBSaveCoreOptions::SetPluginName(const char *name) { LLDB_INSTRUMENT_VA(this, name); lldb_private::Status error = m_opaque_up->SetPluginName(name); return SBError(error); } -void SBCoreDumpOptions::SetStyle(lldb::SaveCoreStyle style) { +void SBSaveCoreOptions::SetStyle(lldb::SaveCoreStyle style) { LLDB_INSTRUMENT_VA(this, style); m_opaque_up->SetStyle(style); } -void SBCoreDumpOptions::SetOutputFile(lldb::SBFileSpec file_spec) { +void SBSaveCoreOptions::SetOutputFile(lldb::SBFileSpec file_spec) { LLDB_INSTRUMENT_VA(this, file_spec); m_opaque_up->SetOutputFile(file_spec.ref()); } -const char *SBCoreDumpOptions::GetPluginName() const { +const char *SBSaveCoreOptions::GetPluginName() const { LLDB_INSTRUMENT_VA(this); const auto name = m_opaque_up->GetPluginName(); if (!name) @@ -62,7 +62,7 @@ const char *SBCoreDumpOptions::GetPluginName() const { return lldb_private::ConstString(name.value()).GetCString(); } -SBFileSpec SBCoreDumpOptions::GetOutputFile() const { +SBFileSpec SBSaveCoreOptions::GetOutputFile() const { LLDB_INSTRUMENT_VA(this); const auto file_spec = m_opaque_up->GetOutputFile(); if (file_spec) @@ -70,16 +70,16 @@ SBFileSpec SBCoreDumpOptions::GetOutputFile() const { return SBFileSpec(); } -lldb::SaveCoreStyle SBCoreDumpOptions::GetStyle() const { +lldb::SaveCoreStyle SBSaveCoreOptions::GetStyle() const { LLDB_INSTRUMENT_VA(this); return m_opaque_up->GetStyle(); } -void SBCoreDumpOptions::Clear() { +void SBSaveCoreOptions::Clear() { LLDB_INSTRUMENT_VA(this); m_opaque_up->Clear(); } -lldb_private::CoreDumpOptions &SBCoreDumpOptions::ref() const { +lldb_private::SaveCoreOptions &SBSaveCoreOptions::ref() const { return *m_opaque_up.get(); } diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index f32e5489e200a..17e552f2b76ef 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -1291,7 +1291,7 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed { } // Instance variables to hold the values for command options. - CoreDumpOptions m_core_dump_options; + SaveCoreOptions m_core_dump_options; }; protected: diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp index 67ee16e056d2f..759ef3a8afe02 100644 --- a/lldb/source/Core/PluginManager.cpp +++ b/lldb/source/Core/PluginManager.cpp @@ -12,7 +12,7 @@ #include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" #include "lldb/Interpreter/OptionValueProperties.h" -#include "lldb/Symbol/CoreDumpOptions.h" +#include "lldb/Symbol/SaveCoreOptions.h" #include "lldb/Target/Process.h" #include "lldb/Utility/FileSpec.h" #include "lldb/Utility/Status.h" @@ -702,7 +702,7 @@ PluginManager::GetObjectFileCreateMemoryCallbackForPluginName( } Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &options) { + const lldb_private::SaveCoreOptions &options) { Status error; if (!options.GetOutputFile()) { error.SetErrorString("No output file specified"); diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index 86f4042911357..2c7005449f9d7 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6519,7 +6519,7 @@ struct page_object { }; bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &options, + const lldb_private::SaveCoreOptions &options, Status &error) { auto core_style = options.GetStyle(); if (core_style == SaveCoreStyle::eSaveCoreUnspecified) diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h index 540597b58f9e7..e7af90e28bc4b 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h @@ -62,7 +62,7 @@ class ObjectFileMachO : public lldb_private::ObjectFile { lldb_private::ModuleSpecList &specs); static bool SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &options, + const lldb_private::SaveCoreOptions &options, lldb_private::Status &error); static bool MagicBytesMatch(lldb::DataBufferSP data_sp, lldb::addr_t offset, diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp index 679870007d75d..faa144bfb5f6a 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp @@ -56,7 +56,7 @@ size_t ObjectFileMinidump::GetModuleSpecifications( } bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &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()); diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h index df5727785dc4b..0cd31a0e482d5 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h +++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h @@ -55,7 +55,7 @@ class ObjectFileMinidump : public lldb_private::PluginInterface { // Saves dump in Minidump file format static bool SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &options, + const lldb_private::SaveCoreOptions &options, lldb_private::Status &error); private: diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp index 05b2c675dea4c..bda691ade8af0 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp +++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp @@ -355,9 +355,11 @@ size_t ObjectFilePECOFF::GetModuleSpecifications( } bool ObjectFilePECOFF::SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &options, + const lldb_private::SaveCoreOptions &options, lldb_private::Status &error) { + // Outfile and process_sp are validated by PluginManager::SaveCore assert(options.GetOutputFile().has_value()); + assert(process_sp); return SaveMiniDump(process_sp, options, error); } diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h index da71323c89e47..2eb2b3b774538 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h +++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h @@ -82,7 +82,7 @@ class ObjectFilePECOFF : public lldb_private::ObjectFile { lldb_private::ModuleSpecList &specs); static bool SaveCore(const lldb::ProcessSP &process_sp, - const lldb_private::CoreDumpOptions &options, + const lldb_private::SaveCoreOptions &options, lldb_private::Status &error); static bool MagicBytesMatch(lldb::DataBufferSP data_sp); diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp index ff99ea3778511..61cd74da22223 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp +++ b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp @@ -21,7 +21,7 @@ namespace lldb_private { bool SaveMiniDump(const lldb::ProcessSP &process_sp, - const CoreDumpOptions &core_options, + const SaveCoreOptions &core_options, lldb_private::Status &error) { if (!process_sp) return false; diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h index 7501bd8683d57..03c0ece306143 100644 --- a/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h +++ b/lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h @@ -14,7 +14,7 @@ namespace lldb_private { bool SaveMiniDump(const lldb::ProcessSP &process_sp, - const CoreDumpOptions &core_options, + const SaveCoreOptions &core_options, lldb_private::Status &error); } // namespace lldb_private diff --git a/lldb/source/Symbol/CMakeLists.txt b/lldb/source/Symbol/CMakeLists.txt index f1a2e25cdef48..e49477df06c94 100644 --- a/lldb/source/Symbol/CMakeLists.txt +++ b/lldb/source/Symbol/CMakeLists.txt @@ -6,7 +6,7 @@ add_lldb_library(lldbSymbol NO_PLUGIN_DEPENDENCIES CompilerDecl.cpp CompilerDeclContext.cpp CompilerType.cpp - CoreDumpOptions.cpp + SaveCoreOptions.cpp DWARFCallFrameInfo.cpp DebugMacros.cpp DeclVendor.cpp diff --git a/lldb/source/Symbol/CoreDumpOptions.cpp b/lldb/source/Symbol/SaveCoreOptions.cpp similarity index 66% rename from lldb/source/Symbol/CoreDumpOptions.cpp rename to lldb/source/Symbol/SaveCoreOptions.cpp index 37f1d6123e76d..0f6fdac1ce22e 100644 --- a/lldb/source/Symbol/CoreDumpOptions.cpp +++ b/lldb/source/Symbol/SaveCoreOptions.cpp @@ -1,4 +1,4 @@ -//===-- CoreDumpOptions.cpp -------------------------------------*- C++ -*-===// +//===-- SaveCoreOptions.cpp -------------------------------------*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,16 +6,17 @@ // //===----------------------------------------------------------------------===// -#include "lldb/Symbol/CoreDumpOptions.h" +#include "lldb/Symbol/SaveCoreOptions.h" #include "lldb/Core/PluginManager.h" using namespace lldb; using namespace lldb_private; -Status CoreDumpOptions::SetPluginName(const char *name) { +Status SaveCoreOptions::SetPluginName(const char *name) { Status error; if (!name || !name[0]) { m_plugin_name = std::nullopt; + return error; } if (!PluginManager::IsRegisteredObjectFilePluginName(name)) { @@ -28,24 +29,24 @@ Status CoreDumpOptions::SetPluginName(const char *name) { return error; } -void CoreDumpOptions::SetStyle(lldb::SaveCoreStyle style) { m_style = style; } +void SaveCoreOptions::SetStyle(lldb::SaveCoreStyle style) { m_style = style; } -void CoreDumpOptions::SetOutputFile(FileSpec file) { m_file = file; } +void SaveCoreOptions::SetOutputFile(FileSpec file) { m_file = file; } -std::optional CoreDumpOptions::GetPluginName() const { +std::optional SaveCoreOptions::GetPluginName() const { return m_plugin_name; } -lldb::SaveCoreStyle CoreDumpOptions::GetStyle() const { +lldb::SaveCoreStyle SaveCoreOptions::GetStyle() const { return m_style.value_or(lldb::eSaveCoreUnspecified); } const std::optional -CoreDumpOptions::GetOutputFile() const { +SaveCoreOptions::GetOutputFile() const { return m_file; } -void CoreDumpOptions::Clear() { +void SaveCoreOptions::Clear() { m_file = std::nullopt; m_plugin_name = std::nullopt; m_style = std::nullopt; diff --git a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py index c136739a2fb74..07d06bdc116ec 100644 --- a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py +++ b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py @@ -20,7 +20,7 @@ def test_cannot_save_core_unless_process_stopped(self): target = self.dbg.CreateTarget(exe) process = target.LaunchSimple(None, None, self.get_process_working_directory()) self.assertNotEqual(process.GetState(), lldb.eStateStopped) - options = SBCoreDumpOptions() + options = SBSaveCoreOptions() options.SetOutputFile(SBFileSpec(core)) error = process.SaveCore(core) self.assertTrue(error.Fail()) 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 20096d7b4d8d6..96511d790271f 100644 --- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py +++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py @@ -132,7 +132,7 @@ def test_save_linux_mini_dump(self): stacks_to_sp_map, ) - options = lldb.SBCoreDumpOptions() + options = lldb.SBSaveCoreOptions() core_sb_stack_spec = lldb.SBFileSpec(core_sb_stack) options.SetOutputFile(core_sb_stack_spec) options.SetPluginName("minidump") @@ -149,7 +149,7 @@ def test_save_linux_mini_dump(self): stacks_to_sp_map, ) - options = lldb.SBCoreDumpOptions() + options = lldb.SBSaveCoreOptions() core_sb_dirty_spec = lldb.SBFileSpec(core_sb_dirty) options.SetOutputFile(core_sb_dirty_spec) options.SetPluginName("minidump") @@ -167,7 +167,7 @@ def test_save_linux_mini_dump(self): # Minidump can now save full core files, but they will be huge and # they might cause this test to timeout. - options = lldb.SBCoreDumpOptions() + options = lldb.SBSaveCoreOptions() core_sb_full_spec = lldb.SBFileSpec(core_sb_full) options.SetOutputFile(core_sb_full_spec) options.SetPluginName("minidump") diff --git a/lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py similarity index 85% rename from lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py rename to lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py index 129134c0ce33e..f35b3837326e9 100644 --- a/lldb/test/API/python_api/sbcoredumpoptions/TestSBCoreDumpOptions.py +++ b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py @@ -1,14 +1,14 @@ -"""Test the SBCoreDumpOptions APIs.""" +"""Test the SBSaveCoreOptions APIs.""" import lldb from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * -class SBCoreDumpOptionsAPICase(TestBase): +class SBSaveCoreOptionsAPICase(TestBase): def test_plugin_name_assignment(self): """Test assignment ensuring valid plugin names only.""" - options = lldb.SBCoreDumpOptions() + options = lldb.SBSaveCoreOptions() error = options.SetPluginName(None) self.assertTrue(error.Success()) self.assertEqual(options.GetPluginName(), None) @@ -24,5 +24,5 @@ def test_plugin_name_assignment(self): def test_default_corestyle_behavior(self): """Test that the default core style is unspecified.""" - options = lldb.SBCoreDumpOptions() + options = lldb.SBSaveCoreOptions() self.assertEqual(options.GetStyle(), lldb.eSaveCoreUnspecified) From 1e95cc7c7d27b48976b11f7c21a1db6195352cde Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 18 Jul 2024 13:06:57 -0700 Subject: [PATCH 13/14] Fix upper case and return error if plugin name returns an error --- lldb/include/lldb/API/SBSaveCoreOptions.h | 6 +++--- lldb/source/API/SBProcess.cpp | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h index a19e00c2cef8a..b8659bf128a78 100644 --- a/lldb/include/lldb/API/SBSaveCoreOptions.h +++ b/lldb/include/lldb/API/SBSaveCoreOptions.h @@ -6,8 +6,8 @@ // //===----------------------------------------------------------------------===// -#ifndef LLDB_API_SBSaveCoreOPTIONS_H -#define LLDB_API_SBSaveCoreOPTIONS_H +#ifndef LLDB_API_SBSAVECOREOPTIONS_H +#define LLDB_API_SBSAVECOREOPTIONS_H #include "lldb/API/SBDefines.h" #include "lldb/Symbol/SaveCoreOptions.h" @@ -66,4 +66,4 @@ class LLDB_API SBSaveCoreOptions { }; // SBSaveCoreOptions } // namespace lldb -#endif // LLDB_API_SBSaveCoreOPTIONS_H +#endif // LLDB_API_SBSAVECOREOPTIONS_H diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp index 8fe42c9416247..b88f897ff5280 100644 --- a/lldb/source/API/SBProcess.cpp +++ b/lldb/source/API/SBProcess.cpp @@ -1229,8 +1229,10 @@ lldb::SBError SBProcess::SaveCore(const char *file_name, LLDB_INSTRUMENT_VA(this, file_name, flavor, core_style); SBSaveCoreOptions options; options.SetOutputFile(SBFileSpec(file_name)); - options.SetPluginName(flavor); options.SetStyle(core_style); + SBError error = options.SetPluginName(flavor); + if (error.Fail()) + return error; return SaveCore(options); } From d555d849d6368fffd909154fbbfbbf3d17377b3c Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 18 Jul 2024 13:42:58 -0700 Subject: [PATCH 14/14] Fix test that got accidentally set to error.success() --- .../API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py index f35b3837326e9..c509e81d8951a 100644 --- a/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py +++ b/lldb/test/API/python_api/sbsavecoreoptions/TestSBSaveCoreOptions.py @@ -13,7 +13,7 @@ def test_plugin_name_assignment(self): self.assertTrue(error.Success()) self.assertEqual(options.GetPluginName(), None) error = options.SetPluginName("Not a real plugin") - self.assertTrue(error.Success()) + self.assertTrue(error.Fail()) self.assertEqual(options.GetPluginName(), None) error = options.SetPluginName("minidump") self.assertTrue(error.Success())