[Lldb-commits] [lldb] 0151174 - [lldb/Host] Improve error messages on unowned read files
Med Ismail Bennani via lldb-commits
lldb-commits at lists.llvm.org
Mon May 4 08:34:13 PDT 2020
Author: Med Ismail Bennani
Date: 2020-05-04T17:33:55+02:00
New Revision: 015117411e11458f9816ba4359246132164a4297
URL: https://github.com/llvm/llvm-project/commit/015117411e11458f9816ba4359246132164a4297
DIFF: https://github.com/llvm/llvm-project/commit/015117411e11458f9816ba4359246132164a4297.diff
LOG: [lldb/Host] Improve error messages on unowned read files
When trying to read a core file that is not owned by the user running lldb
and that doesn't have read permission on the file, lldb shows a misleading
error message:
```
Unable to find process plug-in for core file
```
This is due to the fact that currently, lldb doesn't check the file
ownership. And when trying to to open and read a core file, the syscall
fails, which prevents a process to be created.
Since lldb already have a portable `open` syscall interface, lets take
advantage of that and delegate the error handling to the syscall
itself. This way, no matter if the file exists or if the user has proper
ownership, lldb will always try to open the file, and behave accordingly
to the error code returned.
rdar://42630030
https://reviews.llvm.org/D78712
Signed-off-by: Med Ismail Bennani <medismail.bennani at gmail.com>
Added:
Modified:
lldb/source/Commands/CommandObjectTarget.cpp
lldb/test/API/commands/target/basic/TestTargetCommand.py
Removed:
################################################################################
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index decdd9c53de2..8db0eef31163 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -271,15 +271,13 @@ class CommandObjectTargetCreate : public CommandObjectParsed {
FileSpec remote_file(m_remote_file.GetOptionValue().GetCurrentValue());
if (core_file) {
- if (!FileSystem::Instance().Exists(core_file)) {
- result.AppendErrorWithFormat("core file '%s' doesn't exist",
- core_file.GetPath().c_str());
- result.SetStatus(eReturnStatusFailed);
- return false;
- }
- if (!FileSystem::Instance().Readable(core_file)) {
- result.AppendErrorWithFormat("core file '%s' is not readable",
- core_file.GetPath().c_str());
+ auto file = FileSystem::Instance().Open(
+ core_file, lldb_private::File::eOpenOptionRead);
+
+ if (!file) {
+ result.AppendErrorWithFormatv("Cannot open '{0}': {1}.",
+ core_file.GetPath(),
+ llvm::toString(file.takeError()));
result.SetStatus(eReturnStatusFailed);
return false;
}
@@ -288,18 +286,13 @@ class CommandObjectTargetCreate : public CommandObjectParsed {
if (argc == 1 || core_file || remote_file) {
FileSpec symfile(m_symbol_file.GetOptionValue().GetCurrentValue());
if (symfile) {
- if (FileSystem::Instance().Exists(symfile)) {
- if (!FileSystem::Instance().Readable(symfile)) {
- result.AppendErrorWithFormat("symbol file '%s' is not readable",
- symfile.GetPath().c_str());
- result.SetStatus(eReturnStatusFailed);
- return false;
- }
- } else {
- char symfile_path[PATH_MAX];
- symfile.GetPath(symfile_path, sizeof(symfile_path));
- result.AppendErrorWithFormat("invalid symbol file path '%s'",
- symfile_path);
+ auto file = FileSystem::Instance().Open(
+ symfile, lldb_private::File::eOpenOptionRead);
+
+ if (!file) {
+ result.AppendErrorWithFormatv("Cannot open '{0}': {1}.",
+ symfile.GetPath(),
+ llvm::toString(file.takeError()));
result.SetStatus(eReturnStatusFailed);
return false;
}
@@ -401,48 +394,34 @@ class CommandObjectTargetCreate : public CommandObjectParsed {
if (module_sp)
module_sp->SetPlatformFileSpec(remote_file);
}
+
if (core_file) {
- char core_path[PATH_MAX];
- core_file.GetPath(core_path, sizeof(core_path));
- if (FileSystem::Instance().Exists(core_file)) {
- if (!FileSystem::Instance().Readable(core_file)) {
- result.AppendMessageWithFormat(
- "Core file '%s' is not readable.\n", core_path);
- result.SetStatus(eReturnStatusFailed);
- return false;
- }
- FileSpec core_file_dir;
- core_file_dir.GetDirectory() = core_file.GetDirectory();
- target_sp->AppendExecutableSearchPaths(core_file_dir);
+ FileSpec core_file_dir;
+ core_file_dir.GetDirectory() = core_file.GetDirectory();
+ target_sp->AppendExecutableSearchPaths(core_file_dir);
- ProcessSP process_sp(target_sp->CreateProcess(
- GetDebugger().GetListener(), llvm::StringRef(), &core_file));
+ ProcessSP process_sp(target_sp->CreateProcess(
+ GetDebugger().GetListener(), llvm::StringRef(), &core_file));
- if (process_sp) {
- // Seems weird that we Launch a core file, but that is what we
- // do!
- error = process_sp->LoadCore();
+ if (process_sp) {
+ // Seems weird that we Launch a core file, but that is what we
+ // do!
+ error = process_sp->LoadCore();
- if (error.Fail()) {
- result.AppendError(
- error.AsCString("can't find plug-in for core file"));
- result.SetStatus(eReturnStatusFailed);
- return false;
- } else {
- result.AppendMessageWithFormat(
- "Core file '%s' (%s) was loaded.\n", core_path,
- target_sp->GetArchitecture().GetArchitectureName());
- result.SetStatus(eReturnStatusSuccessFinishNoResult);
- }
- } else {
- result.AppendErrorWithFormat(
- "Unable to find process plug-in for core file '%s'\n",
- core_path);
+ if (error.Fail()) {
+ result.AppendError(
+ error.AsCString("can't find plug-in for core file"));
result.SetStatus(eReturnStatusFailed);
+ return false;
+ } else {
+ result.AppendMessageWithFormatv("Core file '{0}' ({1}) was loaded.\n", core_file.GetPath(),
+ target_sp->GetArchitecture().GetArchitectureName());
+ result.SetStatus(eReturnStatusSuccessFinishNoResult);
}
} else {
- result.AppendErrorWithFormat("Core file '%s' does not exist\n",
- core_path);
+ result.AppendErrorWithFormatv(
+ "Unable to find process plug-in for core file '{0}'\n",
+ core_file.GetPath());
result.SetStatus(eReturnStatusFailed);
}
} else {
diff --git a/lldb/test/API/commands/target/basic/TestTargetCommand.py b/lldb/test/API/commands/target/basic/TestTargetCommand.py
index 2a5978c9844b..8335753c78af 100644
--- a/lldb/test/API/commands/target/basic/TestTargetCommand.py
+++ b/lldb/test/API/commands/target/basic/TestTargetCommand.py
@@ -326,7 +326,7 @@ def test_target_create_multiple_args(self):
@no_debug_info_test
def test_target_create_nonexistent_core_file(self):
self.expect("target create -c doesntexist", error=True,
- substrs=["core file 'doesntexist' doesn't exist"])
+ substrs=["Cannot open 'doesntexist': No such file or directory"])
# Write only files don't seem to be supported on Windows.
@skipIfWindows
@@ -335,12 +335,12 @@ def test_target_create_unreadable_core_file(self):
tf = tempfile.NamedTemporaryFile()
os.chmod(tf.name, stat.S_IWRITE)
self.expect("target create -c '" + tf.name + "'", error=True,
- substrs=["core file '", "' is not readable"])
+ substrs=["Cannot open '", "': Permission denied"])
@no_debug_info_test
def test_target_create_nonexistent_sym_file(self):
self.expect("target create -s doesntexist doesntexisteither", error=True,
- substrs=["invalid symbol file path 'doesntexist'"])
+ substrs=["Cannot open '", "': No such file or directory"])
@skipIfWindows
@no_debug_info_test
@@ -357,7 +357,7 @@ def test_target_create_unreadable_sym_file(self):
tf = tempfile.NamedTemporaryFile()
os.chmod(tf.name, stat.S_IWRITE)
self.expect("target create -s '" + tf.name + "' no_exe", error=True,
- substrs=["symbol file '", "' is not readable"])
+ substrs=["Cannot open '", "': Permission denied"])
@no_debug_info_test
def test_target_delete_all(self):
More information about the lldb-commits
mailing list