diff --git a/lldb/docs/lldb-platform-packets.txt b/lldb/docs/lldb-platform-packets.txt --- a/lldb/docs/lldb-platform-packets.txt +++ b/lldb/docs/lldb-platform-packets.txt @@ -372,12 +372,6 @@ // 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 --- a/lldb/include/lldb/Host/File.h +++ b/lldb/include/lldb/Host/File.h @@ -39,27 +39,29 @@ // 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 --- a/lldb/source/Host/common/File.cpp +++ b/lldb/source/Host/common/File.cpp @@ -90,8 +90,8 @@ .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 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp @@ -501,10 +501,6 @@ 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 --- /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 --- a/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py +++ b/lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py @@ -181,6 +181,8 @@ 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 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