[Lldb-commits] [lldb] [LLDB][Minidump] Minidump erase file on failure (PR #108259)
Jacob Lalonde via lldb-commits
lldb-commits at lists.llvm.org
Wed Sep 11 14:34:40 PDT 2024
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/108259
>From 022173d669e84c96362024feb6512342fdd02d09 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 11 Sep 2024 10:27:31 -0700
Subject: [PATCH 1/3] Unlink file on failure
---
.../ObjectFile/Minidump/MinidumpFileBuilder.cpp | 13 +++++++++++++
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 3 +++
.../ObjectFile/Minidump/ObjectFileMinidump.cpp | 8 ++++++++
3 files changed, 24 insertions(+)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index edc568a6b47e00..7b7745b2953665 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -1218,3 +1218,16 @@ Status MinidumpFileBuilder::DumpFile() {
return error;
}
+
+
+void MinidumpFileBuilder::DeleteFile() {
+ Log *log = GetLog(LLDBLog::Object);
+
+ if (m_core_file) {
+ Status error = m_core_file->Close();
+ if (error.Fail())
+ LLDB_LOGF(log, "Failed to close minidump file: %s", error.AsCString());
+
+ m_core_file.reset();
+ }
+}
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 71001e26c00e91..2dcabe893b496e 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -115,6 +115,9 @@ class MinidumpFileBuilder {
// Run cleanup and write all remaining bytes to file
lldb_private::Status DumpFile();
+ // Delete the file if it exists
+ void DeleteFile();
+
private:
// Add data to the end of the buffer, if the buffer exceeds the flush level,
// trigger a flush.
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index 5da69dd4f2ce79..282e3fdda2daf6 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -81,28 +81,33 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
if (error.Fail()) {
LLDB_LOGF(log, "AddHeaderAndCalculateDirectories failed: %s",
error.AsCString());
+ builder.DeleteFile();
return false;
};
error = builder.AddSystemInfo();
if (error.Fail()) {
LLDB_LOGF(log, "AddSystemInfo failed: %s", error.AsCString());
+ builder.DeleteFile();
return false;
}
error = builder.AddModuleList();
if (error.Fail()) {
LLDB_LOGF(log, "AddModuleList failed: %s", error.AsCString());
+ builder.DeleteFile();
return false;
}
error = builder.AddMiscInfo();
if (error.Fail()) {
LLDB_LOGF(log, "AddMiscInfo failed: %s", error.AsCString());
+ builder.DeleteFile();
return false;
}
error = builder.AddThreadList();
if (error.Fail()) {
LLDB_LOGF(log, "AddThreadList failed: %s", error.AsCString());
+ builder.DeleteFile();
return false;
}
@@ -116,6 +121,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
error = builder.AddExceptions();
if (error.Fail()) {
LLDB_LOGF(log, "AddExceptions failed: %s", error.AsCString());
+ builder.DeleteFile();
return false;
}
@@ -124,12 +130,14 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
error = builder.AddMemoryList();
if (error.Fail()) {
LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString());
+ builder.DeleteFile();
return false;
}
error = builder.DumpFile();
if (error.Fail()) {
LLDB_LOGF(log, "DumpFile failed: %s", error.AsCString());
+ builder.DeleteFile();
return false;
}
>From 8a51bd0c3663c23644334506aa817d6a28739f42 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 11 Sep 2024 10:41:15 -0700
Subject: [PATCH 2/3] Add test to ensure file is deleted upon failure
---
.../TestProcessSaveCoreMinidump.py | 26 +++++++++++++++++++
1 file changed, 26 insertions(+)
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 2cbe20ee10b1af..31af96102f9d22 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -493,3 +493,29 @@ def test_save_minidump_custom_save_style_duplicated_regions(self):
finally:
self.assertTrue(self.dbg.DeleteTarget(target))
+
+ @skipUnlessPlatform(["linux"])
+ def minidump_deleted_on_save_failure(self):
+ """Test that verifies the minidump file is deleted after an error"""
+
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ try:
+ target = self.dbg.CreateTarget(exe)
+ process = target.LaunchSimple(
+ None, None, self.get_process_working_directory()
+ )
+ self.assertState(process.GetState(), lldb.eStateStopped)
+
+ custom_file = self.getBuildArtifact("core.should.be.deleted.custom.dmp")
+ options = lldb.SBSaveCoreOptions()
+ options.SetOutputFile(lldb.SBFileSpec(custom_file))
+ options.SetPluginName("minidump")
+ options.SetStyle(lldb.eSaveCoreCustomOnly)
+ # We set custom only and have no thread list and have no memory.
+ error = process.SaveCore(options)
+ self.assertTrue(error.Fail())
+ self.assertTrue(not os.path.isfile(custom_file))
+
+ finally:
+ self.assertTrue(self.dbg.DeleteTarget(target))
>From 7150acba4947b381edc8b6752da929513dc568d6 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 11 Sep 2024 13:39:44 -0700
Subject: [PATCH 3/3] use RAII object
---
.../Minidump/MinidumpFileBuilder.cpp | 2 +-
.../ObjectFile/Minidump/MinidumpFileBuilder.h | 2 +-
.../Minidump/ObjectFileMinidump.cpp | 26 +++++++++++++------
3 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 7b7745b2953665..d0eefb540dbd84 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -1220,7 +1220,7 @@ Status MinidumpFileBuilder::DumpFile() {
}
-void MinidumpFileBuilder::DeleteFile() {
+void MinidumpFileBuilder::DeleteFile() noexcept {
Log *log = GetLog(LLDBLog::Object);
if (m_core_file) {
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 2dcabe893b496e..72e5658718b3c4 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -116,7 +116,7 @@ class MinidumpFileBuilder {
lldb_private::Status DumpFile();
// Delete the file if it exists
- void DeleteFile();
+ void DeleteFile() noexcept;
private:
// Add data to the end of the buffer, if the buffer exceeds the flush level,
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index 282e3fdda2daf6..a9b8c494f15f9a 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -55,6 +55,21 @@ size_t ObjectFileMinidump::GetModuleSpecifications(
return 0;
}
+struct SaveCoreRequest {
+ SaveCoreRequest(MinidumpFileBuilder &builder) : m_builder(builder) {}
+
+ ~SaveCoreRequest() {
+ if (!m_success)
+ m_builder.DeleteFile();
+ }
+
+ void SetSuccess() { m_success = true; }
+
+ private:
+ MinidumpFileBuilder &m_builder;
+ bool m_success = false;
+};
+
bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
lldb_private::SaveCoreOptions &options,
lldb_private::Status &error) {
@@ -75,39 +90,35 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
}
MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp,
options);
+ SaveCoreRequest request(builder);
Log *log = GetLog(LLDBLog::Object);
error = builder.AddHeaderAndCalculateDirectories();
if (error.Fail()) {
LLDB_LOGF(log, "AddHeaderAndCalculateDirectories failed: %s",
error.AsCString());
- builder.DeleteFile();
return false;
};
error = builder.AddSystemInfo();
if (error.Fail()) {
LLDB_LOGF(log, "AddSystemInfo failed: %s", error.AsCString());
- builder.DeleteFile();
return false;
}
error = builder.AddModuleList();
if (error.Fail()) {
LLDB_LOGF(log, "AddModuleList failed: %s", error.AsCString());
- builder.DeleteFile();
return false;
}
error = builder.AddMiscInfo();
if (error.Fail()) {
LLDB_LOGF(log, "AddMiscInfo failed: %s", error.AsCString());
- builder.DeleteFile();
return false;
}
error = builder.AddThreadList();
if (error.Fail()) {
LLDB_LOGF(log, "AddThreadList failed: %s", error.AsCString());
- builder.DeleteFile();
return false;
}
@@ -121,7 +132,6 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
error = builder.AddExceptions();
if (error.Fail()) {
LLDB_LOGF(log, "AddExceptions failed: %s", error.AsCString());
- builder.DeleteFile();
return false;
}
@@ -130,16 +140,16 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
error = builder.AddMemoryList();
if (error.Fail()) {
LLDB_LOGF(log, "AddMemoryList failed: %s", error.AsCString());
- builder.DeleteFile();
return false;
}
error = builder.DumpFile();
if (error.Fail()) {
LLDB_LOGF(log, "DumpFile failed: %s", error.AsCString());
- builder.DeleteFile();
return false;
}
+ request.SetSuccess();
+
return true;
}
More information about the lldb-commits
mailing list