asmith created this revision.
asmith added reviewers: zturner, llvm-commits.
Herald added a subscriber: lldb-commits.

This adds unnamed pipe support in PipeWindows to support communication between 
a debug server and child process.
Modify PipeWindows::CreateNew to support the creation of an unnamed pipe.
Rename the previous method that created a named pipe to 
PipeWindows::CreateNewNamed.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D56234

Files:
  include/lldb/Host/windows/PipeWindows.h
  source/Host/windows/PipeWindows.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
  tools/lldb-server/lldb-gdbserver.cpp

Index: tools/lldb-server/lldb-gdbserver.cpp
===================================================================
--- tools/lldb-server/lldb-gdbserver.cpp
+++ tools/lldb-server/lldb-gdbserver.cpp
@@ -218,21 +218,22 @@
   return writeSocketIdToPipe(port_name_pipe, socket_id);
 }
 
-Status writeSocketIdToPipe(int unnamed_pipe_fd, const std::string &socket_id) {
+Status writeSocketIdToPipe(int64_t unnamed_pipe_fd_or_handle,
+                           const std::string &socket_id) {
 #if defined(_WIN32)
-  return Status("Unnamed pipes are not supported on Windows.");
+  Pipe port_pipe{Pipe::kInvalidHandle, (HANDLE)unnamed_pipe_fd_or_handle};
 #else
-  Pipe port_pipe{Pipe::kInvalidDescriptor, unnamed_pipe_fd};
-  return writeSocketIdToPipe(port_pipe, socket_id);
+  Pipe port_pipe{Pipe::kInvalidDescriptor, (int)unnamed_pipe_fd_or_handle};
 #endif
+  return writeSocketIdToPipe(port_pipe, socket_id);
 }
 
 void ConnectToRemote(MainLoop &mainloop,
                      GDBRemoteCommunicationServerLLGS &gdb_server,
                      bool reverse_connect, const char *const host_and_port,
                      const char *const progname, const char *const subcommand,
-                     const char *const named_pipe_path, int unnamed_pipe_fd,
-                     int connection_fd) {
+                     const char *const named_pipe_path,
+                     int64_t unnamed_pipe_fd_or_handle, int connection_fd) {
   Status error;
 
   std::unique_ptr<Connection> connection_up;
@@ -331,8 +332,8 @@
         }
         // If we have an unnamed pipe to write the socket id back to, do that
         // now.
-        else if (unnamed_pipe_fd >= 0) {
-          error = writeSocketIdToPipe(unnamed_pipe_fd, socket_id);
+        else if (unnamed_pipe_fd_or_handle >= 0) {
+          error = writeSocketIdToPipe(unnamed_pipe_fd_or_handle, socket_id);
           if (error.Fail())
             fprintf(stderr, "failed to write to the unnamed pipe: %s\n",
                     error.AsCString());
@@ -384,7 +385,7 @@
   std::string log_file;
   StringRef
       log_channels; // e.g. "lldb process threads:gdb-remote default:linux all"
-  int unnamed_pipe_fd = -1;
+  int64_t unnamed_pipe_fd_or_handle = -1;
   bool reverse_connect = false;
   int connection_fd = -1;
 
@@ -425,7 +426,7 @@
 
     case 'U': // unnamed pipe
       if (optarg && optarg[0])
-        unnamed_pipe_fd = StringConvert::ToUInt32(optarg, -1);
+        unnamed_pipe_fd_or_handle = StringConvert::ToUInt64(optarg, -1);
       break;
 
     case 'r':
@@ -529,7 +530,7 @@
 
   ConnectToRemote(mainloop, gdb_server, reverse_connect, host_and_port,
                   progname, subcommand, named_pipe_path.c_str(),
-                  unnamed_pipe_fd, connection_fd);
+                  unnamed_pipe_fd_or_handle, connection_fd);
 
   if (!gdb_server.IsConnected()) {
     fprintf(stderr, "no connection information provided, unable to run\n");
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -164,9 +164,6 @@
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerPlatform::Handle_qLaunchGDBServer(
     StringExtractorGDBRemote &packet) {
-#ifdef _WIN32
-  return SendErrorResponse(9);
-#else
   // Spawn a local debugserver as a platform so we can then attach or launch a
   // process...
 
@@ -217,10 +214,9 @@
   PacketResult packet_result = SendPacketNoLock(response.GetString());
   if (packet_result != PacketResult::Success) {
     if (debugserver_pid != LLDB_INVALID_PROCESS_ID)
-      ::kill(debugserver_pid, SIGINT);
+      Host::Kill(debugserver_pid, SIGINT);
   }
   return packet_result;
-#endif
 }
 
 GDBRemoteCommunication::PacketResult
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -1062,6 +1062,18 @@
         }
         debugserver_args.AppendArgument(llvm::StringRef("--named-pipe"));
         debugserver_args.AppendArgument(named_pipe_path);
+#elif defined(_WIN32)
+        error = socket_pipe.CreateNew(true);
+        if (error.Fail()) {
+          if (log)
+            log->Printf("GDBRemoteCommunication::%s() "
+                        "unnamed pipe creation failed: %s",
+                        __FUNCTION__, error.AsCString());
+          return error;
+        }
+        auto write_handle = socket_pipe.GetWriteNativeHandle();
+        debugserver_args.AppendArgument(llvm::StringRef("--pipe"));
+        debugserver_args.AppendArgument(llvm::to_string(write_handle));
 #else
         // Binding to port zero, we need to figure out what port it ends up
         // using using an unnamed pipe...
Index: source/Host/windows/PipeWindows.cpp
===================================================================
--- source/Host/windows/PipeWindows.cpp
+++ source/Host/windows/PipeWindows.cpp
@@ -25,14 +25,36 @@
 
 namespace {
 std::atomic<uint32_t> g_pipe_serial(0);
-}
+static std::string g_pipe_name_prefix = "\\\\.\\Pipe\\";
+} // namespace
+
+HANDLE PipeWindows::kInvalidHandle = INVALID_HANDLE_VALUE;
 
 PipeWindows::PipeWindows() {
   m_read = INVALID_HANDLE_VALUE;
   m_write = INVALID_HANDLE_VALUE;
 
-  m_read_fd = -1;
-  m_write_fd = -1;
+  m_read_fd = PipeWindows::kInvalidDescriptor;
+  m_write_fd = PipeWindows::kInvalidDescriptor;
+  ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
+  ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
+}
+
+PipeWindows::PipeWindows(HANDLE read_handle, HANDLE write_handle) {
+  // Don't risk in passing file descriptors and getting handles from them by
+  // _get_osfhandle since the retrieved handles are highly likely unrecognized
+  // in the current process and usually crashes the program.  Pass handles
+  // instead since the handle can be inheritated.
+  m_read = read_handle;
+  m_write = write_handle;
+
+  m_read_fd = (read_handle == INVALID_HANDLE_VALUE)
+                  ? PipeWindows::kInvalidDescriptor
+                  : _open_osfhandle((intptr_t)read_handle, _O_RDONLY);
+  m_write_fd = (write_handle == INVALID_HANDLE_VALUE)
+                   ? PipeWindows::kInvalidDescriptor
+                   : _open_osfhandle((intptr_t)write_handle, _O_WRONLY);
+
   ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
   ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
 }
@@ -40,6 +62,26 @@
 PipeWindows::~PipeWindows() { Close(); }
 
 Status PipeWindows::CreateNew(bool child_process_inherit) {
+  // Create an anonymous pipe with handle inheritance enabled.
+  SECURITY_ATTRIBUTES sa;
+  sa.nLength = sizeof(sa);
+  sa.lpSecurityDescriptor = 0;
+  sa.bInheritHandle = child_process_inherit ? TRUE : FALSE;
+  BOOL result = ::CreatePipe(&m_read, &m_write, &sa, 1024);
+  if (result == FALSE)
+    return Status(::GetLastError(), eErrorTypeWin32);
+
+  m_read_fd = _open_osfhandle((intptr_t)m_read, _O_RDONLY);
+  ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
+  m_read_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
+
+  m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY);
+  ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
+
+  return Status();
+}
+
+Status PipeWindows::CreateNewNamed(bool child_process_inherit) {
   // Even for anonymous pipes, we open a named pipe.  This is because you
   // cannot get overlapped i/o on Windows without using a named pipe.  So we
   // synthesize a unique name.
@@ -60,7 +102,7 @@
   if (CanRead() || CanWrite())
     return Status(ERROR_ALREADY_EXISTS, eErrorTypeWin32);
 
-  std::string pipe_path = "\\\\.\\Pipe\\";
+  std::string pipe_path = g_pipe_name_prefix;
   pipe_path.append(name);
 
   // Always open for overlapped i/o.  We implement blocking manually in Read
@@ -75,7 +117,10 @@
   ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
   m_read_overlapped.hEvent = ::CreateEvent(nullptr, TRUE, FALSE, nullptr);
 
-  // Open the write end of the pipe.
+  // FIXME: For server end pipe, it's not applicable to use CreateFile to get
+  // read or write end of the pipe for the reason that closing of the file will
+  // directly close the pipe itself.
+
   Status result = OpenNamedPipe(name, child_process_inherit, false);
   if (!result.Success()) {
     CloseReadFileDescriptor();
@@ -111,7 +156,7 @@
 
 Status PipeWindows::OpenAsReader(llvm::StringRef name,
                                  bool child_process_inherit) {
-  if (CanRead() || CanWrite())
+  if (CanRead())
     return Status(ERROR_ALREADY_EXISTS, eErrorTypeWin32);
 
   return OpenNamedPipe(name, child_process_inherit, true);
@@ -121,7 +166,7 @@
 PipeWindows::OpenAsWriterWithTimeout(llvm::StringRef name,
                                      bool child_process_inherit,
                                      const std::chrono::microseconds &timeout) {
-  if (CanRead() || CanWrite())
+  if (CanWrite())
     return Status(ERROR_ALREADY_EXISTS, eErrorTypeWin32);
 
   return OpenNamedPipe(name, child_process_inherit, false);
@@ -137,7 +182,7 @@
   SECURITY_ATTRIBUTES attributes = {};
   attributes.bInheritHandle = child_process_inherit;
 
-  std::string pipe_path = "\\\\.\\Pipe\\";
+  std::string pipe_path = g_pipe_name_prefix;
   pipe_path.append(name);
 
   if (is_read) {
@@ -170,9 +215,9 @@
 
 int PipeWindows::ReleaseReadFileDescriptor() {
   if (!CanRead())
-    return -1;
+    return PipeWindows::kInvalidDescriptor;
   int result = m_read_fd;
-  m_read_fd = -1;
+  m_read_fd = PipeWindows::kInvalidDescriptor;
   if (m_read_overlapped.hEvent)
     ::CloseHandle(m_read_overlapped.hEvent);
   m_read = INVALID_HANDLE_VALUE;
@@ -182,9 +227,9 @@
 
 int PipeWindows::ReleaseWriteFileDescriptor() {
   if (!CanWrite())
-    return -1;
+    return PipeWindows::kInvalidDescriptor;
   int result = m_write_fd;
-  m_write_fd = -1;
+  m_write_fd = PipeWindows::kInvalidDescriptor;
   m_write = INVALID_HANDLE_VALUE;
   ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
   return result;
@@ -198,7 +243,7 @@
     ::CloseHandle(m_read_overlapped.hEvent);
   _close(m_read_fd);
   m_read = INVALID_HANDLE_VALUE;
-  m_read_fd = -1;
+  m_read_fd = PipeWindows::kInvalidDescriptor;
   ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
 }
 
@@ -208,7 +253,7 @@
 
   _close(m_write_fd);
   m_write = INVALID_HANDLE_VALUE;
-  m_write_fd = -1;
+  m_write_fd = PipeWindows::kInvalidDescriptor;
   ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
 }
 
Index: include/lldb/Host/windows/PipeWindows.h
===================================================================
--- include/lldb/Host/windows/PipeWindows.h
+++ include/lldb/Host/windows/PipeWindows.h
@@ -24,10 +24,19 @@
 //----------------------------------------------------------------------
 class PipeWindows : public PipeBase {
 public:
+  static const int kInvalidDescriptor = -1;
+  static HANDLE kInvalidHandle;
+
+public:
   PipeWindows();
+  PipeWindows(HANDLE read_handle, HANDLE write_handle);
   ~PipeWindows() override;
 
+  // Create an unnamed pipe.
   Status CreateNew(bool child_process_inherit) override;
+
+  // Create a named pipe.
+  Status CreateNewNamed(bool child_process_inherit);
   Status CreateNew(llvm::StringRef name, bool child_process_inherit) override;
   Status CreateWithUniqueName(llvm::StringRef prefix,
                               bool child_process_inherit,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to