[Lldb-commits] [lldb] 34766d0 - [LLDB][SBSaveCore] Fix bug where default values are not propagated. (#101770)
via lldb-commits
lldb-commits at lists.llvm.org
Fri Aug 2 18:38:08 PDT 2024
Author: Jacob Lalonde
Date: 2024-08-02T18:38:05-07:00
New Revision: 34766d0d488ba2fbefa80dcd0cc8720a0e753448
URL: https://github.com/llvm/llvm-project/commit/34766d0d488ba2fbefa80dcd0cc8720a0e753448
DIFF: https://github.com/llvm/llvm-project/commit/34766d0d488ba2fbefa80dcd0cc8720a0e753448.diff
LOG: [LLDB][SBSaveCore] Fix bug where default values are not propagated. (#101770)
In #100443, Mach-o and Minidump now only call process API's that take a
`SaveCoreOption` as the container for the style and information if a
thread should be included in the core or not. This introduced a bug
where in subsequent method calls we were not honoring the defaults of
both implementations.
~~To solve this I have made a copy of each SaveCoreOptions that is
mutable by the respective plugin. Originally I wanted to leave the
SaveCoreOptions as non const so these default value mutations could be
shown back to the user. Changing that behavior is outside of the scope
of this bugfix, but is context for why we are making a copy.~~
Removed const on the savecoreoptions so defaults can be inspected by the
user
CC: @Michael137
Added:
Modified:
lldb/include/lldb/Core/PluginManager.h
lldb/include/lldb/lldb-private-interfaces.h
lldb/source/Core/PluginManager.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.cpp
lldb/source/Plugins/ObjectFile/PECOFF/WindowsMiniDump.h
lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
Removed:
################################################################################
diff --git a/lldb/include/lldb/Core/PluginManager.h b/lldb/include/lldb/Core/PluginManager.h
index a23f834f471fb..e4e0c3eea67f8 100644
--- a/lldb/include/lldb/Core/PluginManager.h
+++ b/lldb/include/lldb/Core/PluginManager.h
@@ -194,7 +194,7 @@ class PluginManager {
GetObjectFileCreateMemoryCallbackForPluginName(llvm::StringRef name);
static Status SaveCore(const lldb::ProcessSP &process_sp,
- const lldb_private::SaveCoreOptions &core_options);
+ lldb_private::SaveCoreOptions &core_options);
// ObjectContainer
static bool RegisterPlugin(
diff --git a/lldb/include/lldb/lldb-private-interfaces.h b/lldb/include/lldb/lldb-private-interfaces.h
index 87c5ff8d22fb6..b3c8cda899b95 100644
--- a/lldb/include/lldb/lldb-private-interfaces.h
+++ b/lldb/include/lldb/lldb-private-interfaces.h
@@ -57,7 +57,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::SaveCoreOptions &options,
+ lldb_private::SaveCoreOptions &options,
Status &error);
typedef EmulateInstruction *(*EmulateInstructionCreateInstance)(
const ArchSpec &arch, InstructionType inst_type);
diff --git a/lldb/source/Core/PluginManager.cpp b/lldb/source/Core/PluginManager.cpp
index fbd78a7780578..f243807df509e 100644
--- a/lldb/source/Core/PluginManager.cpp
+++ b/lldb/source/Core/PluginManager.cpp
@@ -702,7 +702,7 @@ PluginManager::GetObjectFileCreateMemoryCallbackForPluginName(
}
Status PluginManager::SaveCore(const lldb::ProcessSP &process_sp,
- const lldb_private::SaveCoreOptions &options) {
+ 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 4322bd7e2674f..22ece4f4dacf7 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -6351,7 +6351,7 @@ static offset_t
CreateAllImageInfosPayload(const lldb::ProcessSP &process_sp,
offset_t initial_file_offset,
StreamString &all_image_infos_payload,
- const lldb_private::SaveCoreOptions &options) {
+ lldb_private::SaveCoreOptions &options) {
Target &target = process_sp->GetTarget();
ModuleList modules = target.GetImages();
@@ -6522,16 +6522,17 @@ struct page_object {
};
bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
- const lldb_private::SaveCoreOptions &options,
+ lldb_private::SaveCoreOptions &options,
Status &error) {
- auto core_style = options.GetStyle();
- if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
- core_style = SaveCoreStyle::eSaveCoreDirtyOnly;
// The FileSpec and Process are already checked in PluginManager::SaveCore.
assert(options.GetOutputFile().has_value());
assert(process_sp);
const FileSpec outfile = options.GetOutputFile().value();
+ // MachO defaults to dirty pages
+ if (options.GetStyle() == SaveCoreStyle::eSaveCoreUnspecified)
+ options.SetStyle(eSaveCoreDirtyOnly);
+
Target &target = process_sp->GetTarget();
const ArchSpec target_arch = target.GetArchitecture();
const llvm::Triple &target_triple = target_arch.GetTriple();
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
index e7af90e28bc4b..27bc237aaac48 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::SaveCoreOptions &options,
+ 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/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index c039492aa5c5a..fc1af24819e96 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -76,7 +76,7 @@ class MinidumpFileBuilder {
public:
MinidumpFileBuilder(lldb::FileUP &&core_file,
const lldb::ProcessSP &process_sp,
- const lldb_private::SaveCoreOptions &save_core_options)
+ lldb_private::SaveCoreOptions &save_core_options)
: m_process_sp(process_sp), m_core_file(std::move(core_file)),
m_save_core_options(save_core_options){};
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index 2380ff4c00ca9..0897895e6bc25 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -56,16 +56,15 @@ size_t ObjectFileMinidump::GetModuleSpecifications(
}
bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
- const lldb_private::SaveCoreOptions &options,
+ lldb_private::SaveCoreOptions &options,
lldb_private::Status &error) {
// Output file and process_sp are both checked in PluginManager::SaveCore.
assert(options.GetOutputFile().has_value());
assert(process_sp);
// Minidump defaults to stacks only.
- SaveCoreStyle core_style = options.GetStyle();
- if (core_style == SaveCoreStyle::eSaveCoreUnspecified)
- core_style = SaveCoreStyle::eSaveCoreStackOnly;
+ if (options.GetStyle() == SaveCoreStyle::eSaveCoreUnspecified)
+ options.SetStyle(SaveCoreStyle::eSaveCoreStackOnly);
llvm::Expected<lldb::FileUP> maybe_core_file = FileSystem::Instance().Open(
options.GetOutputFile().value(),
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.h
index 0cd31a0e482d5..b76fcd0052a8a 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::SaveCoreOptions &options,
+ 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 bda691ade8af0..9d01089745dfc 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -355,7 +355,7 @@ size_t ObjectFilePECOFF::GetModuleSpecifications(
}
bool ObjectFilePECOFF::SaveCore(const lldb::ProcessSP &process_sp,
- const lldb_private::SaveCoreOptions &options,
+ lldb_private::SaveCoreOptions &options,
lldb_private::Status &error) {
// Outfile and process_sp are validated by PluginManager::SaveCore
assert(options.GetOutputFile().has_value());
diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
index 2eb2b3b774538..8bccf3be3e5f6 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::SaveCoreOptions &options,
+ 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 61cd74da22223..a6462625fb5b5 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,
- const SaveCoreOptions &core_options,
- lldb_private::Status &error) {
+ SaveCoreOptions &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 03c0ece306143..2f8606d82c974 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,
- const SaveCoreOptions &core_options,
- lldb_private::Status &error);
+ SaveCoreOptions &core_options, lldb_private::Status &error);
} // namespace lldb_private
diff --git a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
index 8573d15733927..c933e01424f56 100644
--- a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
+++ b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
@@ -10,6 +10,11 @@
class ProcessSaveCoreTestCase(TestBase):
+ def validate_core_pid(self, pid, core_path):
+ target = self.dbg.CreateTarget(None)
+ process = target.LoadCore(core_path)
+ return process.GetProcessID() == pid
+
@skipIfRemote
@skipUnlessWindows
def test_cannot_save_core_unless_process_stopped(self):
@@ -88,3 +93,24 @@ def test_save_core_via_process_plugin(self):
os.unlink(core)
except OSError:
pass
+
+ @skipUnlessPlatform(["linux"])
+ def test_save_core_default_values_for_style_minidump(self):
+ """Test we can still save a core for minidump when no
+ core style is specified."""
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ core = self.getBuildArtifact("core.dmp")
+ target = self.dbg.CreateTarget(exe)
+ target.BreakpointCreateByName("bar")
+ process = target.LaunchSimple(None, None, self.get_process_working_directory())
+ self.assertState(process.GetState(), lldb.eStateStopped)
+ pid = process.GetProcessID()
+ options = lldb.SBSaveCoreOptions()
+ minidump_path = core + ".minidump"
+ options.SetOutputFile(lldb.SBFileSpec(minidump_path))
+ options.SetPluginName("minidump")
+ error = process.SaveCore(options)
+ self.assertSuccess(error, error.GetCString())
+ self.assertTrue(os.path.isfile(minidump_path))
+ self.assertTrue(self.validate_core_pid(pid, minidump_path))
More information about the lldb-commits
mailing list