Author: Jonas Devlieghere Date: 2026-01-22T22:32:03-08:00 New Revision: 859052a34ec5ad6c240366837fda12aa6801a560
URL: https://github.com/llvm/llvm-project/commit/859052a34ec5ad6c240366837fda12aa6801a560 DIFF: https://github.com/llvm/llvm-project/commit/859052a34ec5ad6c240366837fda12aa6801a560.diff LOG: [lldb] Improve error message when we can't save core (#177496) When you specify a filename to `process save core`, we'll write it in the current working directory. In Xcode the CWD is `/` and you can't generally write there. ``` (lldb) process save-core --style full foo error: failed to save core file for process: Read-only file system ``` This PR improves the error message by including the output file when we can't save core. However, just printing the filename isn't that much more helpful, because FileSystem::Resolve only makes a path absolute if it exists. ``` error: failed to save core file for process to 'foo': Read-only file system ``` Therefore I also modified the interface to add a flag to force resolve a potentially non-exist path. ``` error: failed to save core file for process to '/foo': Read-only file system ``` Added: Modified: lldb/include/lldb/Host/FileSystem.h lldb/source/Commands/CommandObjectProcess.cpp lldb/source/Host/common/FileSystem.cpp lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py Removed: ################################################################################ diff --git a/lldb/include/lldb/Host/FileSystem.h b/lldb/include/lldb/Host/FileSystem.h index 640f3846e448c..d0ac6edd2b286 100644 --- a/lldb/include/lldb/Host/FileSystem.h +++ b/lldb/include/lldb/Host/FileSystem.h @@ -137,9 +137,13 @@ class FileSystem { /// \} /// Resolve path to make it canonical. + /// + /// If force_make_absolute is specified, we'll make the path absolute even if + /// it does not exist. /// \{ - void Resolve(llvm::SmallVectorImpl<char> &path); - void Resolve(FileSpec &file_spec); + void Resolve(llvm::SmallVectorImpl<char> &path, + bool force_make_absolute = false); + void Resolve(FileSpec &file_spec, bool force_make_absolute = false); /// \} /// Remove a single file. diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index c17f12fae6e79..ca210e145d07e 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -1351,7 +1351,8 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed { if (process_sp) { if (command.GetArgumentCount() == 1) { FileSpec output_file(command.GetArgumentAtIndex(0)); - FileSystem::Instance().Resolve(output_file); + FileSystem::Instance().Resolve(output_file, + /*force_make_absolute=*/true); auto &core_dump_options = m_options.m_core_dump_options; core_dump_options.SetOutputFile(output_file); core_dump_options.SetProcess(process_sp); @@ -1373,8 +1374,9 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed { } result.SetStatus(eReturnStatusSuccessFinishResult); } else { - result.AppendErrorWithFormat( - "Failed to save core file for process: %s\n", error.AsCString()); + result.AppendErrorWithFormatv( + "failed to save core file for process to '{0}': {1}\n", + output_file.GetPath(), error); } } else { result.AppendErrorWithFormat("'%s' takes one arguments:\nUsage: %s\n", diff --git a/lldb/source/Host/common/FileSystem.cpp b/lldb/source/Host/common/FileSystem.cpp index 00919fe9a574d..5ca3b3640ce5f 100644 --- a/lldb/source/Host/common/FileSystem.cpp +++ b/lldb/source/Host/common/FileSystem.cpp @@ -222,7 +222,8 @@ std::error_code FileSystem::GetRealPath(const Twine &path, return m_fs->getRealPath(path, output); } -void FileSystem::Resolve(SmallVectorImpl<char> &path) { +void FileSystem::Resolve(SmallVectorImpl<char> &path, + bool force_make_absolute) { if (path.empty()) return; @@ -237,14 +238,14 @@ void FileSystem::Resolve(SmallVectorImpl<char> &path) { MakeAbsolute(absolute); path.clear(); - if (Exists(absolute)) { + if (force_make_absolute || Exists(absolute)) { path.append(absolute.begin(), absolute.end()); } else { path.append(resolved.begin(), resolved.end()); } } -void FileSystem::Resolve(FileSpec &file_spec) { +void FileSystem::Resolve(FileSpec &file_spec, bool force_make_absolute) { if (!file_spec) return; @@ -253,7 +254,7 @@ void FileSystem::Resolve(FileSpec &file_spec) { file_spec.GetPath(path); // Resolve the path. - Resolve(path); + Resolve(path, force_make_absolute); // Update the FileSpec with the resolved path. if (file_spec.GetFilename().IsEmpty()) diff --git a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py index cf7bd9787d649..98f143f15471b 100644 --- a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py +++ b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py @@ -4,6 +4,8 @@ import os import lldb +import tempfile +import stat from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbutil @@ -106,3 +108,30 @@ def test_help(self): "stack", ], ) + + @skipIfRemote + def test_save_core_to_nonwritable_dir(self): + """Test that saving a core file to a non-writable directory produces a helpful error message.""" + self.build() + exe = self.getBuildArtifact("a.out") + + # Create a non-writable temporary directory. + temp_dir = tempfile.mkdtemp() + os.chmod(temp_dir, stat.S_IRUSR | stat.S_IXUSR) + self.addTearDownHook(lambda: os.rmdir(temp_dir)) + + target = self.dbg.CreateTarget(exe) + breakpoint = target.BreakpointCreateByName("bar") + process = target.LaunchSimple(None, None, self.get_process_working_directory()) + self.assertState(process.GetState(), lldb.eStateStopped) + + # Try to save a core file to the non-writable directory. + core_file = os.path.join(temp_dir, "core") + self.expect( + f"process save-core {core_file}", + error=True, + substrs=[ + "failed to save core file for process to", + os.path.join(temp_dir, "core"), + ], + ) _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
