[lldb] [gdb-server] Implement the vFile:fstat packet

[labath]

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

776

consider:

struct GdbStat {
  llvm::support::ubig32_t st_dev;
  llvm::support::ubig32_t st_ino;
  ...
};

...

translate(gdb_stats.st_dev, file_stats.st_dev, 0); // I'm not sure that this clamping is really necessary.
...

Seems like it could be nicer, particularly as the vFile_Stat function will need to do the same thing...

[mgorny]

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

776

What's this translate() thing? I don't see anything matching in LLVM or LLDB. Or are you saying I should define a helper function?

As for clamping, I think it's better if we send 0 (a "clearly invalid value") than e.g. truncated st_dev that would be simply wrong.

[mgorny]

Use fancy endian-magic types ;-).

[labath]

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

763

static void

764

src <=

776

Yeah, that was a new helper function.

[mgorny]

Thanks!

[mgorny]

Skip st_blocks and st_blksize on Windows.

[labath]

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

791

Just make sure the fields stay initialized (to zero, I guess)

[shafik]

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

764

I am building on a macOS and I seeing the following diagnostic for this line:

warning: comparison of integers of different signs: 'int' and 'std::numeric_limits<unsigned int>::type' (aka 'unsigned int') [-Wsign-compare]
  dest = src <= std::numeric_limits<typename T::value_type>::max() ? src
         ~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

and it is triggered by:

fill_clamp(data.gdb_st_dev, file_stats.st_dev, 0);

In this case it will do the right thing but allowing mixed types is problematic, usually I see clamp done with homogeneous types.

[mgorny]

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

764

Could you suggest how to fix it? Some platforms have different signedness than the GDB struct type.

[shafik]

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

764

Since we don't have C++20 I think you would have to roll your own cmp_less see the possible implementation on cppreference.

For reference see Safe integral comparisons proposal.

I looked and I don't see something similar in llvm anywhere but I could be missing it.

lldb/include/lldb/Utility/StringExtractorGDBRemote.h

 //===-- StringExtractorGDBRemote.h ------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_UTILITY_STRINGEXTRACTORGDBREMOTE_H
 #define LLDB_UTILITY_STRINGEXTRACTORGDBREMOTE_H
 
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StringExtractor.h"
 #include "llvm/ADT/StringRef.h"
 
 #include <string>
 
 #include <cstddef>
 #include <cstdint>
 
enum ServerPacketType {
     eServerPacketType_vFile_open,
     eServerPacketType_vFile_close,
     eServerPacketType_vFile_pread,
     eServerPacketType_vFile_pwrite,
     eServerPacketType_vFile_size,
     eServerPacketType_vFile_mode,
     eServerPacketType_vFile_exists,
     eServerPacketType_vFile_md5,
+    eServerPacketType_vFile_fstat,
     eServerPacketType_vFile_stat,
     eServerPacketType_vFile_symlink,
     eServerPacketType_vFile_unlink,
     // debug server packages
     eServerPacketType_QEnvironmentHexEncoded,
     eServerPacketType_QListThreadsInStopReply,
     eServerPacketType_QPassSignals,
     eServerPacketType_QRestoreRegisterState,

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

 //===-- GDBRemoteCommunicationServerCommon.h --------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_PROCESS_GDB_REMOTE_GDBREMOTECOMMUNICATIONSERVERCOMMON_H
 #define LLDB_SOURCE_PLUGINS_PROCESS_GDB_REMOTE_GDBREMOTECOMMUNICATIONSERVERCOMMON_H
 
 #include <string>
 
 #include "lldb/Host/ProcessLaunchInfo.h"
 #include "lldb/lldb-private-forward.h"
 
 #include "GDBRemoteCommunicationServer.h"
 #include "GDBRemoteCommunicationServerCommon.h"
 
 class StringExtractorGDBRemote;
 
 namespace lldb_private {
protected:
   PacketResult Handle_vFile_Mode(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_vFile_Exists(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_vFile_symlink(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_vFile_unlink(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_vFile_FStat(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_vFile_Stat(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_vFile_MD5(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qEcho(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qModuleInfo(StringExtractorGDBRemote &packet);
 

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

 #include "lldb/Host/FileAction.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/SafeMachO.h"
 #include "lldb/Interpreter/OptionArgParser.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/Platform.h"
 #include "lldb/Utility/Endian.h"
+#include "lldb/Utility/DataEncoder.h"
 #include "lldb/Utility/GDBRemote.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/StructuredData.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/JSON.h"
 
RegisterMemberFunctionHandler(
       &GDBRemoteCommunicationServerCommon::Handle_vFile_pRead);
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_vFile_pwrite,
       &GDBRemoteCommunicationServerCommon::Handle_vFile_pWrite);
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_vFile_size,
       &GDBRemoteCommunicationServerCommon::Handle_vFile_Size);
   RegisterMemberFunctionHandler(
+      StringExtractorGDBRemote::eServerPacketType_vFile_fstat,
+      &GDBRemoteCommunicationServerCommon::Handle_vFile_FStat);
+  RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_vFile_stat,
       &GDBRemoteCommunicationServerCommon::Handle_vFile_Stat);
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_vFile_symlink,
       &GDBRemoteCommunicationServerCommon::Handle_vFile_symlink);
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_vFile_unlink,
       &GDBRemoteCommunicationServerCommon::Handle_vFile_unlink);
if (packet.GetChar() == ',') {
       }
       return SendPacketNoLock(response.GetString());
     }
   }
   return SendErrorResponse(24);
 }
 
 GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerCommon::Handle_vFile_FStat(
+    StringExtractorGDBRemote &packet) {
+  StreamGDBRemote response;
+  packet.SetFilePos(::strlen("vFile:fstat:"));
+  int fd = packet.GetS32(-1, 16);
+
+  struct stat file_stats;
+  if (::fstat(fd, &file_stats) == -1) {
+    const int save_errno = errno;
+    response.Printf("F-1,%x", save_errno);
+    return SendPacketNoLock(response.GetString());
+  }
+
+  std::array<char, 64> data;
+  DataEncoder encoder{data.data(), data.size(), lldb::eByteOrderBig,
+                      sizeof(void *)};
+  uint32_t offset = 0;
+
+  offset = encoder.PutUnsigned(
+      offset, 4, file_stats.st_dev <= UINT32_MAX ? file_stats.st_dev : 0);
+  offset = encoder.PutUnsigned(
+      offset, 4, file_stats.st_ino <= UINT32_MAX ? file_stats.st_ino : 0);
+  offset =
+      encoder.PutUnsigned(offset, 4, static_cast<uint32_t>(file_stats.st_mode));
+  offset = encoder.PutUnsigned(
+      offset, 4,
+      file_stats.st_nlink <= UINT32_MAX ? file_stats.st_nlink : UINT32_MAX);
+  offset = encoder.PutUnsigned(
+      offset, 4, file_stats.st_uid <= UINT32_MAX ? file_stats.st_uid : 0);
+  offset = encoder.PutUnsigned(
+      offset, 4, file_stats.st_gid <= UINT32_MAX ? file_stats.st_gid : 0);
+  offset = encoder.PutUnsigned(
+      offset, 4, file_stats.st_rdev <= UINT32_MAX ? file_stats.st_rdev : 0);
+  offset = encoder.PutUnsigned(offset, 8, file_stats.st_size);
+  offset = encoder.PutUnsigned(offset, 8, file_stats.st_blksize);
+  offset = encoder.PutUnsigned(offset, 8, file_stats.st_blocks);
+  offset = encoder.PutUnsigned(
+      offset, 4, file_stats.st_atime <= UINT32_MAX ? file_stats.st_atime : 0);
+  offset = encoder.PutUnsigned(
+      offset, 4, file_stats.st_mtime <= UINT32_MAX ? file_stats.st_mtime : 0);
+  offset = encoder.PutUnsigned(
+      offset, 4, file_stats.st_ctime <= UINT32_MAX ? file_stats.st_ctime : 0);
+  assert(offset == 64);
+
+  response.Printf("F%zx;", data.size());
+  response.PutEscapedBytes(data.data(), data.size());
+  return SendPacketNoLock(response.GetString());
+}
+
+GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_vFile_Stat(
     StringExtractorGDBRemote &packet) {
   return SendUnimplementedResponse(
       "GDBRemoteCommunicationServerCommon::Handle_vFile_Stat() unimplemented");
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_vFile_MD5(

lldb/source/Utility/StringExtractorGDBRemote.cpp

 //===-- StringExtractorGDBRemote.cpp --------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Utility/StringExtractorGDBRemote.h"
 
 #include <cctype>
 #include <cstring>
 
 constexpr lldb::pid_t StringExtractorGDBRemote::AllProcesses;
 constexpr lldb::tid_t StringExtractorGDBRemote::AllThreads;
 
 StringExtractorGDBRemote::ResponseType
if (PACKET_STARTS_WITH("vFile:")) {
       else if (PACKET_STARTS_WITH("vFile:pread"))
         return eServerPacketType_vFile_pread;
       else if (PACKET_STARTS_WITH("vFile:pwrite"))
         return eServerPacketType_vFile_pwrite;
       else if (PACKET_STARTS_WITH("vFile:size"))
         return eServerPacketType_vFile_size;
       else if (PACKET_STARTS_WITH("vFile:exists"))
         return eServerPacketType_vFile_exists;
+      else if (PACKET_STARTS_WITH("vFile:fstat"))
+        return eServerPacketType_vFile_fstat;
       else if (PACKET_STARTS_WITH("vFile:stat"))
         return eServerPacketType_vFile_stat;
       else if (PACKET_STARTS_WITH("vFile:mode"))
         return eServerPacketType_vFile_mode;
       else if (PACKET_STARTS_WITH("vFile:MD5"))
         return eServerPacketType_vFile_md5;
       else if (PACKET_STARTS_WITH("vFile:symlink"))
         return eServerPacketType_vFile_symlink;

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

 from __future__ import print_function
 
 # lldb test suite imports
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import TestBase
 
 # gdb-remote-specific imports
 import lldbgdbserverutils
 from gdbremote_testcase import GdbRemoteTestCaseBase
 
 import binascii
+import os
 import stat
+import struct
 import tempfile
+import typing
+
+
+class GDBStat(typing.NamedTuple):
+    st_dev: int
+    st_ino: int
+    st_mode: int
+    st_nlink: int
+    st_uid: int
+    st_gid: int
+    st_rdev: int
+    st_size: int
+    st_blksize: int
+    st_blocks: int
+    st_atime: int
+    st_mtime: int
+    st_ctime: int
+
+
+def uint32_or_zero(x):
+    return x if x < 2**32 else 0
+
+
+def uint32_or_max(x):
+    return x if x < 2**32 else 2**32 - 1
+
+
+def uint32_trunc(x):
+    return x & (2**32 - 1)
 
 
 class TestGdbRemotePlatformFile(GdbRemoteTestCaseBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @skipIfWindows
     @add_test_categories(["llgs"])
def test_platform_file_exists_not(self):
             self.do_handshake()
             self.test_sequence.add_log_lines(
                 ["read packet: $vFile:exists:%s#00" % (
                     binascii.b2a_hex(test_path.encode()).decode(),),
                  "send packet: $F,0#00"],
                 True)
             self.expect_gdbremote_sequence()
 
+    @skipIfWindows
+    @add_test_categories(["llgs"])
+    def test_platform_file_fstat(self):
+        server = self.connect_to_debug_monitor()
+        self.assertIsNotNone(server)
+
+        with tempfile.NamedTemporaryFile() as temp_file:
+            temp_file.write(b"some test data for stat")
+            temp_file.flush()
+
+            self.do_handshake()
+            self.test_sequence.add_log_lines(
+                ["read packet: $vFile:open:%s,0,0#00" % (
+                    binascii.b2a_hex(temp_file.name.encode()).decode(),),
+                 {"direction": "send",
+                 "regex": r"^\$F([0-9a-fA-F]+)#[0-9a-fA-F]{2}$",
+                 "capture": {1: "fd"}}],
+                True)
+
+            context = self.expect_gdbremote_sequence()
+            self.assertIsNotNone(context)
+            fd = int(context["fd"], 16)
+
+            self.reset_test_sequence()
+            self.test_sequence.add_log_lines(
+                ["read packet: $vFile:fstat:%x#00" % (fd,),
+                 {"direction": "send",
+                 "regex": r"^\$F([0-9a-fA-F]+);(.*)#[0-9a-fA-F]{2}$",
+                 "capture": {1: "size", 2: "data"}}],
+                True)
+            context = self.expect_gdbremote_sequence()
+            self.assertEqual(int(context["size"], 16), 64)
+            # NB: we're using .encode() as a hack because the test suite
+            # is wrongly using (unicode) str instead of bytes
+            gdb_stat = GDBStat(
+                *struct.unpack(">IIIIIIIQQQIII",
+                               self.decode_gdbremote_binary(context["data"])
+                                .encode("iso-8859-1")))
+            sys_stat = os.fstat(temp_file.fileno())
+
+            self.assertEqual(gdb_stat.st_dev, uint32_or_zero(sys_stat.st_dev))
+            self.assertEqual(gdb_stat.st_ino, uint32_or_zero(sys_stat.st_ino))
+            self.assertEqual(gdb_stat.st_mode, uint32_trunc(sys_stat.st_mode))
+            self.assertEqual(gdb_stat.st_nlink, uint32_or_max(sys_stat.st_nlink))
+            self.assertEqual(gdb_stat.st_uid, uint32_or_zero(sys_stat.st_uid))
+            self.assertEqual(gdb_stat.st_gid, uint32_or_zero(sys_stat.st_gid))
+            self.assertEqual(gdb_stat.st_rdev, uint32_or_zero(sys_stat.st_rdev))
+            self.assertEqual(gdb_stat.st_size, sys_stat.st_size)
+            self.assertEqual(gdb_stat.st_blksize, sys_stat.st_blksize)
+            self.assertEqual(gdb_stat.st_blocks, sys_stat.st_blocks)
+            self.assertEqual(gdb_stat.st_atime,
+                             uint32_or_zero(int(sys_stat.st_atime)))
+            self.assertEqual(gdb_stat.st_mtime,
+                             uint32_or_zero(int(sys_stat.st_mtime)))
+            self.assertEqual(gdb_stat.st_ctime,
+                             uint32_or_zero(int(sys_stat.st_ctime)))
+
+            self.reset_test_sequence()
+            self.test_sequence.add_log_lines(
+                ["read packet: $vFile:close:%x#00" % (fd,),
+                 "send packet: $F0#00"],
+                True)
+            self.expect_gdbremote_sequence()
+
+
     def expect_error(self):
         self.test_sequence.add_log_lines(
             [{"direction": "send",
              "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}],
             True)
         self.expect_gdbremote_sequence()
 
     def vFile_test(self, read=False, write=False, append=False, trunc=False,