Author: Jacob Lalonde Date: 2024-09-13T09:17:06-07:00 New Revision: 661382f2c07ba464caa0ad0fb8c64c1c3b20e9a4
URL: https://github.com/llvm/llvm-project/commit/661382f2c07ba464caa0ad0fb8c64c1c3b20e9a4 DIFF: https://github.com/llvm/llvm-project/commit/661382f2c07ba464caa0ad0fb8c64c1c3b20e9a4.diff LOG: [LLDB][Minidump] Minidump erase file on failure (#108259) In #95312 Minidump file creation was moved from being created at the end, to the file being emitted in chunks. This causes some undesirable behavior where the file can still be present after an error has occurred. To resolve this we will now delete the file upon an error. Added: Modified: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py Removed: ################################################################################ diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp index edc568a6b47e00..ca22dacb2ba6c9 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 71001e26c00e91..72e5658718b3c4 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 5da69dd4f2ce79..be47991bb09fcc 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 2cbe20ee10b1af..ccdb6653cf16f8 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)) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
