[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 <[email protected]>

Author: medismailben

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 {

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):