[lldb] Add new commands and tests for getting file perms & exists

[mgorny]

Summary

Add two new commands 'platform get-file-permissions' and 'platform
file-exists' for the respective bits of LLDB protocol. Add tests for
them. Fix error handling in GetFilePermissions().

[mgorny]

Fix colliding test name.

[labath]

I was thinking this would be more useful in the SB form (and indeed we have SBPlatform::GetFilePermissions) already, though I suppose making it available through the command line does not hurt either..

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

664–668

Unless I'm mistaken, the test does not actually run this code (as it tests the client bits).

lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

103–111

Maybe it's time to create a separate test class which would perform this in the setUp/tearDown methods?

[mgorny]

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

664–668

Yes, this is the case. Adding tests for the server is a bit out of scope for what I'm working on.

[labath]

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

664–668

Why is that? You did have them in the other patches I looked at...

Batching these changes in this way is particularly unfortunate because a casual observer might conclude that you actualy _are_ including a test for the bug you've fixed.

[mgorny]

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

664–668

Just to be clear, are we talking of adding a generic server-side test for vFile:mode, or specifically one that triggers an error? Though thinking about it a bit more, using a file that does not exist should be good enough to trigger this.

[mgorny]

Remove now-redundant platform connect calls.

TODO: server tests.

[mgorny]

Add a server test for error condition. Rebase client tests.

[labath]

cool

lldb/source/Commands/CommandObjectPlatform.cpp

if (platform_sp) {
       }
     } else {
       result.AppendError("no platform currently selected\n");
     }
     return result.Succeeded();
   }
 };
 
+// "platform get-permissions remote-file-path"
+class CommandObjectPlatformGetPermissions : public CommandObjectParsed {
+public:
+  CommandObjectPlatformGetPermissions(CommandInterpreter &interpreter)
+      : CommandObjectParsed(interpreter, "platform get-permissions",
+                            "Get the file permission bits from the remote end.",
+                            "platform get-permissions <remote-file-spec>", 0) {
+    SetHelpLong(
+        R"(Examples:
+
+(lldb) platform get-permissions /the/remote/file/path
+
+    Get the file permissions from the remote end with path /the/remote/file/path.)");
+
+    CommandArgumentEntry arg1;
+    CommandArgumentData file_arg_remote;
+
+    // Define the first (and only) variant of this arg.
+    file_arg_remote.arg_type = eArgTypeFilename;
+    file_arg_remote.arg_repetition = eArgRepeatPlain;
+    // There is only one variant this argument could be; put it into the
+    // argument entry.
+    arg1.push_back(file_arg_remote);
+
+    // Push the data for the first argument into the m_arguments vector.
+    m_arguments.push_back(arg1);
+  }
+
+  ~CommandObjectPlatformGetPermissions() override = default;
+
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+                           OptionElementVector &opt_element_vector) override {
+    if (request.GetCursorIndex() != 0)
+      return;
+
+    CommandCompletions::InvokeCommonCompletionCallbacks(
+        GetCommandInterpreter(), CommandCompletions::eRemoteDiskFileCompletion,
+        request, nullptr);
+  }
+
+  bool DoExecute(Args &args, CommandReturnObject &result) override {
+    // If the number of arguments is incorrect, issue an error message.
+    if (args.GetArgumentCount() != 1) {
+      result.AppendError("required argument missing; specify the source file "
+                         "path as the only argument");
+      return false;
+    }
+
+    PlatformSP platform_sp(
+        GetDebugger().GetPlatformList().GetSelectedPlatform());
+    if (platform_sp) {
+      std::string remote_file_path(args.GetArgumentAtIndex(0));
+      uint32_t permissions;
+      Status error = platform_sp->GetFilePermissions(FileSpec(remote_file_path),
+                                                     permissions);
+      if (error.Success()) {
+        result.AppendMessageWithFormat(
+            "File permissions of %s (remote): 0o%04" PRIo32 "\n",
+            remote_file_path.c_str(), permissions);
+        result.SetStatus(eReturnStatusSuccessFinishResult);
+      } else
+        result.AppendError(error.AsCString());
+    } else {
+      result.AppendError("no platform currently selected\n");
+    }
+    return result.Succeeded();
+  }
+};
+
+// "platform file-exists remote-file-path"
+class CommandObjectPlatformFileExists : public CommandObjectParsed {
+public:
+  CommandObjectPlatformFileExists(CommandInterpreter &interpreter)
+      : CommandObjectParsed(interpreter, "platform file-exists",
+                            "Check if the file exists on the remote end.",
+                            "platform file-exists <remote-file-spec>", 0) {
+    SetHelpLong(
+        R"(Examples:
+
+(lldb) platform file-exists /the/remote/file/path
+
+    Check if /the/remote/file/path exists on the remote end.)");
+
+    CommandArgumentEntry arg1;
+    CommandArgumentData file_arg_remote;
+
+    // Define the first (and only) variant of this arg.
+    file_arg_remote.arg_type = eArgTypeFilename;
+    file_arg_remote.arg_repetition = eArgRepeatPlain;
+    // There is only one variant this argument could be; put it into the
+    // argument entry.
+    arg1.push_back(file_arg_remote);
+
+    // Push the data for the first argument into the m_arguments vector.
+    m_arguments.push_back(arg1);
+  }
+
+  ~CommandObjectPlatformFileExists() override = default;
+
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+                           OptionElementVector &opt_element_vector) override {
+    if (request.GetCursorIndex() != 0)
+      return;
+
+    CommandCompletions::InvokeCommonCompletionCallbacks(
+        GetCommandInterpreter(), CommandCompletions::eRemoteDiskFileCompletion,
+        request, nullptr);
+  }
+
+  bool DoExecute(Args &args, CommandReturnObject &result) override {
+    // If the number of arguments is incorrect, issue an error message.
+    if (args.GetArgumentCount() != 1) {
+      result.AppendError("required argument missing; specify the source file "
+                         "path as the only argument");
+      return false;
+    }
+
+    PlatformSP platform_sp(
+        GetDebugger().GetPlatformList().GetSelectedPlatform());
+    if (platform_sp) {
+      std::string remote_file_path(args.GetArgumentAtIndex(0));
+      bool exists = platform_sp->GetFileExists(FileSpec(remote_file_path));
+      result.AppendMessageWithFormat(
+          "File %s (remote) %s\n",
+          remote_file_path.c_str(), exists ? "exists" : "does not exist");
+      result.SetStatus(eReturnStatusSuccessFinishResult);
+    } else {
+      result.AppendError("no platform currently selected\n");
+    }
+    return result.Succeeded();
+  }
+};
+
 // "platform put-file"
 class CommandObjectPlatformPutFile : public CommandObjectParsed {
 public:
   CommandObjectPlatformPutFile(CommandInterpreter &interpreter)
       : CommandObjectParsed(
             interpreter, "platform put-file",
             "Transfer a file from this system to the remote end.", nullptr, 0) {
   }
LoadSubCommand(
       "disconnect",
       CommandObjectSP(new CommandObjectPlatformDisconnect(interpreter)));
   LoadSubCommand("settings", CommandObjectSP(new CommandObjectPlatformSettings(
                                  interpreter)));
   LoadSubCommand("mkdir",
                  CommandObjectSP(new CommandObjectPlatformMkDir(interpreter)));
   LoadSubCommand("file",
                  CommandObjectSP(new CommandObjectPlatformFile(interpreter)));
+  LoadSubCommand("file-exists",
+      CommandObjectSP(new CommandObjectPlatformFileExists(interpreter)));
   LoadSubCommand("get-file", CommandObjectSP(new CommandObjectPlatformGetFile(
                                  interpreter)));
+  LoadSubCommand("get-permissions",
+      CommandObjectSP(new CommandObjectPlatformGetPermissions(interpreter)));
   LoadSubCommand("get-size", CommandObjectSP(new CommandObjectPlatformGetSize(
                                  interpreter)));
   LoadSubCommand("put-file", CommandObjectSP(new CommandObjectPlatformPutFile(
                                  interpreter)));
   LoadSubCommand("process", CommandObjectSP(
                                 new CommandObjectPlatformProcess(interpreter)));
   LoadSubCommand("shell",
                  CommandObjectSP(new CommandObjectPlatformShell(interpreter)));
   LoadSubCommand(
       "target-install",
       CommandObjectSP(new CommandObjectPlatformInstall(interpreter)));
 }
 
 CommandObjectPlatform::~CommandObjectPlatform() = default;

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

GDBRemoteCommunicationServerCommon::Handle_vFile_Mode(
   std::string path;
   packet.GetHexByteString(path);
   if (!path.empty()) {
     FileSpec file_spec(path);
     FileSystem::Instance().Resolve(file_spec);
     std::error_code ec;
     const uint32_t mode = FileSystem::Instance().GetPermissions(file_spec, ec);
     StreamString response;
+    if (mode != llvm::sys::fs::perms_not_known)
       response.Printf("F%x", mode);
-    if (mode == 0 || ec)
-      response.Printf(",%x", (int)Status(ec).GetError());
+    else
+      response.Printf("F-1,%x", (int)Status(ec).GetError());
     return SendPacketNoLock(response.GetString());
   }
   return SendErrorResponse(23);
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_vFile_Exists(
     StringExtractorGDBRemote &packet) {
   packet.SetFilePos(::strlen("vFile:exists:"));

lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

def test_file_size_fallback(self):
         self.match("platform get-size /some/file.txt",
                    [r"File size of /some/file\.txt \(remote\): 66051"])
         self.assertPacketLogContains([
             "vFile:size:2f736f6d652f66696c652e747874",
             "vFile:open:2f736f6d652f66696c652e747874,00000000,00000000",
             "vFile:fstat:5",
             "vFile:close:5",
             ])
+
+    def test_file_permissions(self):
+        """Test 'platform get-permissions'"""
+
+        class Responder(MockGDBServerResponder):
+            def vFile(self, packet):
+                return "F1a4"
+
+        self.server.responder = Responder()
+
+        self.match("platform get-permissions /some/file.txt",
+                   [r"File permissions of /some/file\.txt \(remote\): 0o0644"])
+        self.assertPacketLogContains([
+            "vFile:mode:2f736f6d652f66696c652e747874",
+            ])
+
+    def test_file_exists(self):
+        """Test 'platform file-exists'"""
+
+        class Responder(MockGDBServerResponder):
+            def vFile(self, packet):
+                return "F,1"
+
+        self.server.responder = Responder()
+
+        self.match("platform file-exists /some/file.txt",
+                   [r"File /some/file\.txt \(remote\) exists"])
+        self.assertPacketLogContains([
+            "vFile:exists:2f736f6d652f66696c652e747874",
+            ])
+
+    def test_file_exists_not(self):
+        """Test 'platform file-exists' with non-existing file"""
+
+        class Responder(MockGDBServerResponder):
+            def vFile(self, packet):
+                return "F,0"
+
+        self.server.responder = Responder()
+
+        self.match("platform file-exists /some/file.txt",
+                   [r"File /some/file\.txt \(remote\) does not exist"])
+        self.assertPacketLogContains([
+            "vFile:exists:2f736f6d652f66696c652e747874",
+            ])

lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

def test_platform_file_mode(self):
              "regex": r"^\$F([0-9a-fA-F]+)+#[0-9a-fA-F]{2}$",
              "capture": {1: "mode"}}],
             True)
         context = self.expect_gdbremote_sequence()
         self.assertEqual(int(context["mode"], 16), test_mode)
 
     @skipIfWindows
     @add_test_categories(["llgs"])
+    def test_platform_file_mode_fail(self):
+        server = self.connect_to_debug_monitor()
+        self.assertIsNotNone(server)
+
+        temp_path = self.getBuildArtifact("nonexist")
+
+        self.do_handshake()
+        self.test_sequence.add_log_lines(
+            ["read packet: $vFile:mode:%s#00" % (
+                binascii.b2a_hex(temp_path.encode()).decode(),),
+             {"direction": "send",
+             "regex": r"^\$F-1,0*2+#[0-9a-fA-F]{2}$"}],
+            True)
+        self.expect_gdbremote_sequence()
+
+    @skipIfWindows
+    @add_test_categories(["llgs"])
     def test_platform_file_exists(self):
         server = self.connect_to_debug_monitor()
         self.assertIsNotNone(server)
 
         temp_path = self.getBuildArtifact("test")
         with open(temp_path, "wb"):
             pass