[lldb] [gdb-remote] Add fallbacks for vFile:mode and vFile:exists

[mgorny]

Summary

Add a GDB-compatible fallback to vFile:fstat for vFile:mode, and to
vFile:open for vFile:exists.

[labath]

It seems useful to note that these are only approximations, as they will fail for unreadable files, even though normally these operations should succeed. However, I think it is the best we can do given the limited set of supported operations (i.e., lack of stat(2) syscall and/or O_PATH argument to open(2)).

[mgorny]

In D107811#2986430, @labath wrote:

It seems useful to note that these are only approximations, as they will fail for unreadable files, even though normally these operations should succeed. However, I think it is the best we can do given the limited set of supported operations (i.e., lack of stat(2) syscall and/or O_PATH argument to open(2)).

Note as in mention in commit message, add a comment, add to documentation somewhere?

[labath]

In the code and/or in the commit message.

[mgorny]

Add fallback notes, as requested.

[mgorny]

Undo extra comments. Fix wrong length of test GDB packet.

[labath]

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

3202–3203

This is exactly the use case that if-declarations (if (Optional<...> st = ...)were designed for.

[mgorny]

Remove now-redundant platform connect from tests. Use if-declaration as requested.

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

protected:
 
   bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
       m_supports_qUserName : 1, m_supports_qGroupName : 1,
       m_supports_qThreadStopInfo : 1, m_supports_z0 : 1, m_supports_z1 : 1,
       m_supports_z2 : 1, m_supports_z3 : 1, m_supports_z4 : 1,
       m_supports_QEnvironment : 1, m_supports_QEnvironmentHexEncoded : 1,
       m_supports_qSymbol : 1, m_qSymbol_requests_done : 1,
       m_supports_qModuleInfo : 1, m_supports_jThreadsInfo : 1,
-      m_supports_jModulesInfo : 1, m_supports_vFileSize : 1;
+      m_supports_jModulesInfo : 1, m_supports_vFileSize : 1,
+      m_supports_vFileMode : 1, m_supports_vFileExists : 1;
 
   /// Current gdb remote protocol process identifier for all other operations
   lldb::pid_t m_curr_pid = LLDB_INVALID_PROCESS_ID;
   /// Current gdb remote protocol process identifier for continue, step, etc
   lldb::pid_t m_curr_pid_run = LLDB_INVALID_PROCESS_ID;
   /// Current gdb remote protocol thread identifier for all other operations
   lldb::tid_t m_curr_tid = LLDB_INVALID_THREAD_ID;
   /// Current gdb remote protocol thread identifier for continue, step, etc

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

: GDBRemoteClientBase("gdb-remote.client", "gdb-remote.client.rx_packet"),
       m_supports_qProcessInfoPID(true), m_supports_qfProcessInfo(true),
       m_supports_qUserName(true), m_supports_qGroupName(true),
       m_supports_qThreadStopInfo(true), m_supports_z0(true),
       m_supports_z1(true), m_supports_z2(true), m_supports_z3(true),
       m_supports_z4(true), m_supports_QEnvironment(true),
       m_supports_QEnvironmentHexEncoded(true), m_supports_qSymbol(true),
       m_qSymbol_requests_done(false), m_supports_qModuleInfo(true),
       m_supports_jThreadsInfo(true), m_supports_jModulesInfo(true),
-      m_supports_vFileSize(true),
+      m_supports_vFileSize(true), m_supports_vFileMode(true),
+      m_supports_vFileExists(true),
 
       m_host_arch(), m_process_arch(), m_os_build(), m_os_kernel(),
       m_hostname(), m_gdb_server_name(), m_default_packet_timeout(0),
       m_qSupported_response(), m_supported_async_json_packets_sp(),
       m_qXfer_memory_map() {}
 
 // Destructor
 GDBRemoteCommunicationClient::~GDBRemoteCommunicationClient() {
while (response.Peek()) {
         break;
     }
   }
 }
 
 Status
 GDBRemoteCommunicationClient::GetFilePermissions(const FileSpec &file_spec,
                                                  uint32_t &file_permissions) {
+  if (m_supports_vFileMode) {
     std::string path{file_spec.GetPath(false)};
     Status error;
     lldb_private::StreamString stream;
     stream.PutCString("vFile:mode:");
     stream.PutStringAsRawHex8(path);
     StringExtractorGDBRemote response;
-  if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
+    if (SendPacketAndWaitForResponse(stream.GetString(), response) !=
         PacketResult::Success) {
+      error.SetErrorStringWithFormat("failed to send '%s' packet",
+                                     stream.GetData());
+      return error;
+    }
+    if (!response.IsUnsupportedResponse()) {
       if (response.GetChar() != 'F') {
         error.SetErrorStringWithFormat("invalid response to '%s' packet",
                                        stream.GetData());
       } else {
         const uint32_t mode = response.GetS32(-1, 16);
         if (static_cast<int32_t>(mode) == -1) {
           if (response.GetChar() == ',') {
             int response_errno = response.GetS32(-1, 16);
             if (response_errno > 0)
               error.SetError(response_errno, lldb::eErrorTypePOSIX);
             else
               error.SetErrorToGenericError();
           } else
             error.SetErrorToGenericError();
         } else {
           file_permissions = mode & (S_IRWXU | S_IRWXG | S_IRWXO);
         }
       }
-  } else {
-    error.SetErrorStringWithFormat("failed to send '%s' packet",
-                                   stream.GetData());
-  }
       return error;
+    } else { // response.IsUnsupportedResponse()
+      m_supports_vFileMode = false;
+    }
+  }
+
+  // Fallback to fstat.
+  if (llvm::Optional<GDBRemoteFStatData> st = Stat(file_spec)) {
+    file_permissions = st->gdb_st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);
+    return Status();
+  }
+  return Status("fstat failed");
 }
 
 uint64_t GDBRemoteCommunicationClient::ReadFile(lldb::user_id_t fd,
                                                 uint64_t offset, void *dst,
                                                 uint64_t dst_len,
                                                 Status &error) {
   lldb_private::StreamString stream;
   stream.Printf("vFile:pread:%x,%" PRIx64 ",%" PRIx64, (int)fd, dst_len,
if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
     error.SetErrorString("failed to send vFile:unlink packet");
   }
   return error;
 }
 
 // Extension of host I/O packets to get whether a file exists.
 bool GDBRemoteCommunicationClient::GetFileExists(
     const lldb_private::FileSpec &file_spec) {
+  if (m_supports_vFileExists) {
     std::string path(file_spec.GetPath(false));
     lldb_private::StreamString stream;
     stream.PutCString("vFile:exists:");
     stream.PutStringAsRawHex8(path);
     StringExtractorGDBRemote response;
-  if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
-      PacketResult::Success) {
+    if (SendPacketAndWaitForResponse(stream.GetString(), response) !=
+        PacketResult::Success)
+      return false;
+    if (!response.IsUnsupportedResponse()) {
       if (response.GetChar() != 'F')
         return false;
       if (response.GetChar() != ',')
         return false;
       bool retcode = (response.GetChar() != '0');
       return retcode;
+    } else
+      m_supports_vFileExists = false;
   }
+
+  // Fallback to open.
+  Status error;
+  lldb::user_id_t fd = OpenFile(file_spec, File::eOpenOptionReadOnly, 0, error);
+  if (fd == UINT64_MAX)
     return false;
+  CloseFile(fd, error);
+  return true;
 }
 
 bool GDBRemoteCommunicationClient::CalculateMD5(
     const lldb_private::FileSpec &file_spec, uint64_t &high, uint64_t &low) {
   std::string path(file_spec.GetPath(false));
   lldb_private::StreamString stream;
   stream.PutCString("vFile:MD5:");
   stream.PutStringAsRawHex8(path);

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

def test_file_permissions(self):
         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_permissions_fallback(self):
+        """Test 'platform get-permissions' fallback to fstat"""
+
+        class Responder(MockGDBServerResponder):
+            def vFile(self, packet):
+                if packet.startswith("vFile:open:"):
+                    return "F5"
+                elif packet.startswith("vFile:fstat:"):
+                    return "F40;" + 8 * "\0" + "\0\0\1\xA4" + 52 * "\0"
+                if packet.startswith("vFile:close:"):
+                    return "F0"
+                return ""
+
+        self.server.responder = Responder()
+
+        try:
+            self.match("platform get-permissions /some/file.txt",
+                       [r"File permissions of /some/file\.txt \(remote\): 0o0644"])
+            self.assertPacketLogContains([
+                "vFile:mode:2f736f6d652f66696c652e747874",
+                "vFile:open:2f736f6d652f66696c652e747874,00000000,00000000",
+                "vFile:fstat:5",
+                "vFile:close:5",
+                ])
+        finally:
+            self.dbg.GetSelectedPlatform().DisconnectRemote()
+
     def test_file_exists(self):
         """Test 'platform file-exists'"""
 
         class Responder(MockGDBServerResponder):
             def vFile(self, packet):
                 return "F,1"
 
         self.server.responder = Responder()
def test_file_exists_not(self):
 
         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",
             ])
+
+    def test_file_exists_fallback(self):
+        """Test 'platform file-exists' fallback to open"""
+
+        class Responder(MockGDBServerResponder):
+            def vFile(self, packet):
+                if packet.startswith("vFile:open:"):
+                    return "F5"
+                if packet.startswith("vFile:close:"):
+                    return "F0"
+                return ""
+
+        self.server.responder = Responder()
+
+        self.match("platform file-exists /some/file.txt",
+                   [r"File /some/file\.txt \(remote\) exists"])
+        self.assertPacketLogContains([
+            "vFile:exists:2f736f6d652f66696c652e747874",
+            "vFile:open:2f736f6d652f66696c652e747874,00000000,00000000",
+            "vFile:close:5",
+            ])
+
+    def test_file_exists_not_fallback(self):
+        """Test 'platform file-exists' fallback to open with non-existing file"""
+
+        class Responder(MockGDBServerResponder):
+            def vFile(self, packet):
+                if packet.startswith("vFile:open:"):
+                    return "F-1,2"
+                return ""
+
+        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",
+            "vFile:open:2f736f6d652f66696c652e747874,00000000,00000000",
+            ])