[lldb-server] Improve support on Windows

Summary:
This commit contains the following changes:

  - Rewrite vfile close/read/write packet handlers with portable routines from lldb.
    This removes #if(s) and allows the handlers to work on Windows.

  - Fix a bug in File::Write. This is intended to write data at an offset to a file
    but actually writes at the current position of the file.

  - Add a default boolean argument 'should_close_fd' to FileSystem::Open to
    let the user decide whether to close the fd or not.

Reviewers: zturner, llvm-commits, labath

Reviewed By: zturner

Subscribers: Hui, labath, abidh, lldb-commits

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

llvm-svn: 353446

lldb/include/lldb/Host/File.h

@@ -14,6 +14,7 @@
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-private.h"
 
+#include <mutex>
 #include <stdarg.h>
 #include <stdio.h>
 #include <sys/types.h>
@@ -420,6 +421,7 @@ protected:
   LazyBool m_is_interactive;
   LazyBool m_is_real_terminal;
   LazyBool m_supports_colors;
+  std::mutex offset_access_mutex;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(File);

lldb/include/lldb/Host/FileSystem.h

@@ -64,7 +64,8 @@ public:
   int Open(const char *path, int flags, int mode);
 
   Status Open(File &File, const FileSpec &file_spec, uint32_t options,
-              uint32_t permissions = lldb::eFilePermissionsFileDefault);
+              uint32_t permissions = lldb::eFilePermissionsFileDefault,
+              bool should_close_fd = true);
 
   /// Get a directory iterator.
   /// @{

lldb/source/Host/common/File.cpp

@@ -178,7 +178,7 @@ Status File::Close() {
 
 void File::Clear() {
   m_stream = nullptr;
-  m_descriptor = -1;
+  m_descriptor = kInvalidDescriptor;
   m_options = 0;
   m_own_stream = false;
   m_is_interactive = m_supports_colors = m_is_real_terminal =
@@ -503,6 +503,7 @@ Status File::Read(void *buf, size_t &num_bytes, off_t &offset) {
     error.SetErrorString("invalid file handle");
   }
 #else
+  std::lock_guard<std::mutex> guard(offset_access_mutex);
   long cur = ::lseek(m_descriptor, 0, SEEK_CUR);
   SeekFromStart(offset);
   error = Read(buf, num_bytes);
@@ -602,7 +603,9 @@ Status File::Write(const void *buf, size_t &num_bytes, off_t &offset) {
       num_bytes = bytes_written;
     }
 #else
+    std::lock_guard<std::mutex> guard(offset_access_mutex);
     long cur = ::lseek(m_descriptor, 0, SEEK_CUR);
+    SeekFromStart(offset);
     error = Write(buf, num_bytes);
     long after = ::lseek(m_descriptor, 0, SEEK_CUR);
 

lldb/source/Host/common/FileSystem.cpp

@@ -408,7 +408,7 @@ static mode_t GetOpenMode(uint32_t permissions) {
 }
 
 Status FileSystem::Open(File &File, const FileSpec &file_spec, uint32_t options,
-                        uint32_t permissions) {
+                        uint32_t permissions, bool should_close_fd) {
   if (m_collector)
     m_collector->AddFile(file_spec);
 
@@ -431,7 +431,7 @@ Status FileSystem::Open(File &File, const FileSpec &file_spec, uint32_t options,
     File.SetDescriptor(descriptor, false);
     error.SetErrorToErrno();
   } else {
-    File.SetDescriptor(descriptor, true);
+    File.SetDescriptor(descriptor, should_close_fd);
     File.SetOptions(options);
   }
   return error;

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

@@ -10,7 +10,6 @@
 
 #include <errno.h>
 
-
 #ifdef __APPLE__
 #include <TargetConditionals.h>
 #endif
@@ -214,8 +213,7 @@ GDBRemoteCommunicationServerCommon::Handle_qHostInfo(
   if (sub != LLDB_INVALID_CPUTYPE)
     response.Printf("cpusubtype:%u;", sub);
 
-  if (cpu == llvm::MachO::CPU_TYPE_ARM
-      || cpu == llvm::MachO::CPU_TYPE_ARM64) {
+  if (cpu == llvm::MachO::CPU_TYPE_ARM || cpu == llvm::MachO::CPU_TYPE_ARM64) {
 // Indicate the OS type.
 #if defined(TARGET_OS_TV) && TARGET_OS_TV == 1
     response.PutCString("ostype:tvos;");
@@ -510,18 +508,19 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_Open(
   packet.GetHexByteStringTerminatedBy(path, ',');
   if (!path.empty()) {
     if (packet.GetChar() == ',') {
-      uint32_t flags =
-          File::ConvertOpenOptionsForPOSIXOpen(packet.GetHexMaxU32(false, 0));
+      uint32_t flags = packet.GetHexMaxU32(false, 0);
       if (packet.GetChar() == ',') {
         mode_t mode = packet.GetHexMaxU32(false, 0600);
-        Status error;
         FileSpec path_spec(path);
         FileSystem::Instance().Resolve(path_spec);
-        int fd = ::open(path_spec.GetCString(), flags, mode);
-        const int save_errno = fd == -1 ? errno : 0;
+        File file;
+        // Do not close fd.
+        Status error =
+            FileSystem::Instance().Open(file, path_spec, flags, mode, false);
+        const int save_errno = error.GetError();
         StreamString response;
         response.PutChar('F');
-        response.Printf("%i", fd);
+        response.Printf("%i", file.GetDescriptor());
         if (save_errno)
           response.Printf(",%i", save_errno);
         return SendPacketNoLock(response.GetString());
@@ -536,12 +535,13 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_Close(
     StringExtractorGDBRemote &packet) {
   packet.SetFilePos(::strlen("vFile:close:"));
   int fd = packet.GetS32(-1);
-  Status error;
   int err = -1;
   int save_errno = 0;
   if (fd >= 0) {
-    err = close(fd);
-    save_errno = err == -1 ? errno : 0;
+    File file(fd, true);
+    Status error = file.Close();
+    err = 0;
+    save_errno = error.GetError();
   } else {
     save_errno = EINVAL;
   }
@@ -556,26 +556,23 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_Close(
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_vFile_pRead(
     StringExtractorGDBRemote &packet) {
-#ifdef _WIN32
-  // Not implemented on Windows
-  return SendUnimplementedResponse(
-      "GDBRemoteCommunicationServerCommon::Handle_vFile_pRead() unimplemented");
-#else
   StreamGDBRemote response;
   packet.SetFilePos(::strlen("vFile:pread:"));
   int fd = packet.GetS32(-1);
   if (packet.GetChar() == ',') {
-    uint64_t count = packet.GetU64(UINT64_MAX);
+    size_t count = packet.GetU64(UINT64_MAX);
     if (packet.GetChar() == ',') {
-      uint64_t offset = packet.GetU64(UINT32_MAX);
+      off_t offset = packet.GetU64(UINT32_MAX);
       if (count == UINT64_MAX) {
         response.Printf("F-1:%i", EINVAL);
         return SendPacketNoLock(response.GetString());
       }
 
       std::string buffer(count, 0);
-      const ssize_t bytes_read = ::pread(fd, &buffer[0], buffer.size(), offset);
-      const int save_errno = bytes_read == -1 ? errno : 0;
+      File file(fd, false);
+      Status error = file.Read(static_cast<void *>(&buffer[0]), count, offset);
+      const ssize_t bytes_read = error.Success() ? count : -1;
+      const int save_errno = error.GetError();
       response.PutChar('F');
       response.Printf("%zi", bytes_read);
       if (save_errno)
@@ -588,17 +585,11 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_pRead(
     }
   }
   return SendErrorResponse(21);
-
-#endif
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_vFile_pWrite(
     StringExtractorGDBRemote &packet) {
-#ifdef _WIN32
-  return SendUnimplementedResponse("GDBRemoteCommunicationServerCommon::Handle_"
-                                   "vFile_pWrite() unimplemented");
-#else
   packet.SetFilePos(::strlen("vFile:pwrite:"));
 
   StreamGDBRemote response;
@@ -610,9 +601,12 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_pWrite(
     if (packet.GetChar() == ',') {
       std::string buffer;
       if (packet.GetEscapedBinaryData(buffer)) {
-        const ssize_t bytes_written =
-            ::pwrite(fd, buffer.data(), buffer.size(), offset);
-        const int save_errno = bytes_written == -1 ? errno : 0;
+        File file(fd, false);
+        size_t count = buffer.size();
+        Status error =
+            file.Write(static_cast<const void *>(&buffer[0]), count, offset);
+        const ssize_t bytes_written = error.Success() ? count : -1;
+        const int save_errno = error.GetError();
         response.Printf("%zi", bytes_written);
         if (save_errno)
           response.Printf(",%i", save_errno);
@@ -623,7 +617,6 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_pWrite(
     }
   }
   return SendErrorResponse(27);
-#endif
 }
 
 GDBRemoteCommunication::PacketResult
@@ -971,7 +964,8 @@ GDBRemoteCommunicationServerCommon::Handle_QLaunchArch(
   const uint32_t bytes_left = packet.GetBytesLeft();
   if (bytes_left > 0) {
     const char *arch_triple = packet.Peek();
-    m_process_launch_info.SetArchitecture(HostInfo::GetAugmentedArchSpec(arch_triple));
+    m_process_launch_info.SetArchitecture(
+        HostInfo::GetAugmentedArchSpec(arch_triple));
     return SendOKResponse();
   }
   return SendErrorResponse(13);
@@ -1085,7 +1079,8 @@ GDBRemoteCommunicationServerCommon::Handle_qModuleInfo(
   StreamGDBRemote response;
 
   if (uuid_str.empty()) {
-    auto Result = llvm::sys::fs::md5_contents(matched_module_spec.GetFileSpec().GetPath());
+    auto Result = llvm::sys::fs::md5_contents(
+        matched_module_spec.GetFileSpec().GetPath());
     if (!Result)
       return SendErrorResponse(5);
     response.PutCString("md5:");