[Lldb-commits] [lldb] 8bbef4f - [lldb] [gdb-remote] Sync vFile:open mode constants with GDB

Michał Górny via lldb-commits lldb-commits at lists.llvm.org
Mon Aug 9 03:35:19 PDT 2021


Author: Michał Górny
Date: 2021-08-09T12:07:18+02:00
New Revision: 8bbef4f9afd8b5f6b4435d5bab48b75f048f353c

URL: https://github.com/llvm/llvm-project/commit/8bbef4f9afd8b5f6b4435d5bab48b75f048f353c
DIFF: https://github.com/llvm/llvm-project/commit/8bbef4f9afd8b5f6b4435d5bab48b75f048f353c.diff

LOG: [lldb] [gdb-remote] Sync vFile:open mode constants with GDB

Sync the mode constants used to drive vFile:open requests with these
used by GDB and defined for the gdb remote protocol.  This makes it
possible to use 'platform file open' after connecting to gdbremote
server (and to some degree to operate on the open file modulo other
incompatibilities).

Differential Revision: https://reviews.llvm.org/D106985

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

Modified: 
    lldb/docs/lldb-platform-packets.txt
    lldb/include/lldb/Host/File.h
    lldb/source/Host/common/File.cpp
    lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
    lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Removed: 
    


################################################################################
diff  --git a/lldb/docs/lldb-platform-packets.txt b/lldb/docs/lldb-platform-packets.txt
index 5d0a399b800f8..0b72c657b86bb 100644
--- a/lldb/docs/lldb-platform-packets.txt
+++ b/lldb/docs/lldb-platform-packets.txt
@@ -372,12 +372,6 @@ incompatible with the flags that gdb specifies.
 //  response is F followed by the opened file descriptor in base 10.
 //  "F-1,errno" with the errno if an error occurs.
 //
-//  COMPATIBILITY
-//    The gdb-remote serial protocol documentatio defines a vFile:open:
-//    packet which uses incompatible flag values, e.g. 1 means O_WRONLY
-//    in gdb's vFile:open:, but it means eOpenOptionReadOnly to lldb's
-//    implementation.
-
 //----------------------------------------------------------------------
 // vFile:close:
 //

diff  --git a/lldb/include/lldb/Host/File.h b/lldb/include/lldb/Host/File.h
index b928222b3fb6b..b0bc7d9535f6c 100644
--- a/lldb/include/lldb/Host/File.h
+++ b/lldb/include/lldb/Host/File.h
@@ -39,27 +39,29 @@ class File : public IOObject {
   // NB this enum is used in the lldb platform gdb-remote packet
   // vFile:open: and existing values cannot be modified.
   //
-  // FIXME
-  // These values do not match the values used by GDB
+  // The first set of values is defined by gdb headers and can be found
+  // in the documentation at:
   // * https://sourceware.org/gdb/onlinedocs/gdb/Open-Flags.html#Open-Flags
-  // * rdar://problem/46788934
+  //
+  // The second half are LLDB extensions and use the highest uint32_t bits
+  // to avoid risk of collisions with future gdb remote protocol changes.
   enum OpenOptions : uint32_t {
-    eOpenOptionReadOnly = (1u << 0),  // Open file for reading (only)
-    eOpenOptionWriteOnly = (1u << 1), // Open file for writing (only)
-    eOpenOptionReadWrite =
-        eOpenOptionReadOnly |
-        eOpenOptionWriteOnly, // Open file for both reading and writing
+    eOpenOptionReadOnly = 0x0,  // Open file for reading (only)
+    eOpenOptionWriteOnly = 0x1, // Open file for writing (only)
+    eOpenOptionReadWrite = 0x2, // Open file for both reading and writing
     eOpenOptionAppend =
-        (1u << 2), // Don't truncate file when opening, append to end of file
-    eOpenOptionTruncate = (1u << 3),    // Truncate file when opening
-    eOpenOptionNonBlocking = (1u << 4), // File reads
-    eOpenOptionCanCreate = (1u << 5),   // Create file if doesn't already exist
+        0x8, // Don't truncate file when opening, append to end of file
+    eOpenOptionCanCreate = 0x200, // Create file if doesn't already exist
+    eOpenOptionTruncate = 0x400,  // Truncate file when opening
     eOpenOptionCanCreateNewOnly =
-        (1u << 6), // Can create file only if it doesn't already exist
-    eOpenOptionDontFollowSymlinks = (1u << 7),
+        0x800, // Can create file only if it doesn't already exist
+
+    eOpenOptionNonBlocking = (1u << 28), // File reads
+    eOpenOptionDontFollowSymlinks = (1u << 29),
     eOpenOptionCloseOnExec =
-        (1u << 8), // Close the file when executing a new process
-    LLVM_MARK_AS_BITMASK_ENUM(/* largest_value= */ eOpenOptionCloseOnExec)
+        (1u << 30), // Close the file when executing a new process
+    eOpenOptionInvalid = (1u << 31), // Used as invalid value
+    LLVM_MARK_AS_BITMASK_ENUM(/* largest_value= */ eOpenOptionInvalid)
   };
 
   static mode_t ConvertOpenOptionsForPOSIXOpen(OpenOptions open_options);

diff  --git a/lldb/source/Host/common/File.cpp b/lldb/source/Host/common/File.cpp
index b92b4c1cf928d..87ed405c4428a 100644
--- a/lldb/source/Host/common/File.cpp
+++ b/lldb/source/Host/common/File.cpp
@@ -90,8 +90,8 @@ Expected<File::OpenOptions> File::GetOptionsFromMode(llvm::StringRef mode) {
           .Cases("a+", "ab+", "a+b",
                  eOpenOptionReadWrite | eOpenOptionAppend |
                      eOpenOptionCanCreate)
-          .Default(OpenOptions());
-  if (opts)
+          .Default(eOpenOptionInvalid);
+  if (opts != eOpenOptionInvalid)
     return opts;
   return llvm::createStringError(
       llvm::inconvertibleErrorCode(),

diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index b81b14f246338..92ac2c7891acb 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -501,10 +501,6 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_Open(
   packet.GetHexByteStringTerminatedBy(path, ',');
   if (!path.empty()) {
     if (packet.GetChar() == ',') {
-      // FIXME
-      // The flag values for OpenOptions do not match the values used by GDB
-      // * https://sourceware.org/gdb/onlinedocs/gdb/Open-Flags.html#Open-Flags
-      // * rdar://problem/46788934
       auto flags = File::OpenOptions(packet.GetHexMaxU32(false, 0));
       if (packet.GetChar() == ',') {
         mode_t mode = packet.GetHexMaxU32(false, 0600);

diff  --git a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
new file mode 100644
index 0000000000000..175270e42f093
--- /dev/null
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -0,0 +1,25 @@
+from gdbclientutils import *
+
+class TestGDBRemotePlatformFile(GDBRemoteTestBase):
+
+    def test_file_open(self):
+        """Test mock-opening a remote file"""
+
+        class Responder(MockGDBServerResponder):
+            def vFile(self, packet):
+                return "F10"
+
+        self.server.responder = Responder()
+
+        try:
+            self.runCmd("platform select remote-gdb-server")
+            self.runCmd("platform connect connect://" +
+                        self.server.get_connect_address())
+            self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+            self.runCmd("platform file open /some/file.txt -v 0755")
+            self.assertPacketLogContains([
+                "vFile:open:2f736f6d652f66696c652e747874,0000020a,000001ed"
+                ])
+        finally:
+            self.dbg.GetSelectedPlatform().DisconnectRemote()

diff  --git a/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py b/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
index c683023d13baa..29c1f126c2d2c 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -181,6 +181,8 @@ def respond(self, packet):
             return self.qfProcessInfo(packet)
         if packet.startswith("qPathComplete:"):
             return self.qPathComplete()
+        if packet.startswith("vFile:"):
+            return self.vFile(packet)
 
         return self.other(packet)
 
@@ -288,6 +290,9 @@ def qMemoryRegionInfo(self, addr):
     def qPathComplete(self):
         return ""
 
+    def vFile(self, packet):
+        return ""
+
     """
     Raised when we receive a packet for which there is no default action.
     Override the responder class to implement behavior suitable for the test at


        


More information about the lldb-commits mailing list