diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index edc568a6b47e0..ca22dacb2ba6c 100644 --- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp +++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp @@ -1218,3 +1218,15 @@ Status MinidumpFileBuilder::DumpFile() { return error; } + +void MinidumpFileBuilder::DeleteFile() noexcept { + 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 71001e26c00e9..72e5658718b3c 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() noexcept; + 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 5da69dd4f2ce7..be47991bb09fc 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 DumpFailRemoveHolder { + DumpFailRemoveHolder(MinidumpFileBuilder &builder) : m_builder(builder) {} + + ~DumpFailRemoveHolder() { + 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,6 +90,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, } MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp, options); + DumpFailRemoveHolder request(builder); Log *log = GetLog(LLDBLog::Object); error = builder.AddHeaderAndCalculateDirectories(); @@ -133,5 +149,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp, return false; } + request.SetSuccess(); + return true; } 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 2cbe20ee10b1a..ccdb6653cf16f 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,32 @@ 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.assertIn( + "no valid address ranges found for core style", error.GetCString() + ) + self.assertTrue(not os.path.isfile(custom_file)) + + finally: + self.assertTrue(self.dbg.DeleteTarget(target))