Enable lldb-server on Windows

[asmith]

Summary

This commit contains three small changes to enable lldb-server on Windows.

• Add lldb-server for Windows to the build
• Disable pty redirection on Windows for the initial lldb-server bring up
• Add a support to get the parent pid for a process on Windows
• Ifdef some signals which aren't supported on Windows

Thanks to Hui Huang for the help with this patch!

[amccarth]

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

220

What's the scope of "temporarily"? Is there some specific feature or change that will cause this workaround to be removed?

[Hui]

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

220

Currently the absence of pty support is treated as a fatal error that will block the overall usage of lldb-server.exe for windows. In my opinion, the redirection of std -i/o/e are mainly intended to generate I* and O* packets to notify the lldb about the llgs's inferior (shown on lldb console). Without such support, some of the lldb-server functionalities, like launch/attach process and most of the remote packets etc. still can be tested or say be experimented (see the python tests under tools/lldb-server).

Temporarily relax the mentioned codes and wait for the pty support on windows. No concrete idea how to add that support now.

[amccarth]

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

220

Got it. I misunderstood the original comment and thought the "temporarily" applied to the code change itself.

[compnerd]

include/lldb/Host/windows/PosixApi.h

108 ↗

(On Diff #198675)

This should be out-of-lined. Furthermore, is there any place where the use requires process group handling? Otherwise, can't we just replace this with:

pid_t waitpid(pid_t pid, int *status, int options) {
  assert(pid > 0 && "only support waiting for a particular child");
  assert(options ^ ~WNOHANG == 0 && "only WNOHANG is supported');
  HANDLE hProcess;
  DWORD dwResult;
  hProcess = OpenProcess(SYNCHRONIZE, FALSE, pid);
  if (hProcess == NULL) return pid_t(-1); // TODO(compnerd) better error handling
  dwResult = WaitForSingleObject(hProcess, options & WNOHANG ? 0 : INFINITE);
  CloseHandle(hProcess);
  if (dwResult == WAIT_OBJECT_0 || dwResult == WAIT_TIMEOUT) return pid;
  return pid_t(-1);
}

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

220

PTY support is available as of 2019H1.

[labath]

include/lldb/Host/windows/PosixApi.h

106–109 ↗

(On Diff #198675)

As discussed in the review where this was forked from, we shouldn't be mocking posix apis. Instead we should provide platform-independent abstractions that each platform can implement in it's own way. Waitpid is right now used in only one place. Implementing it here (even just as a stub) just encourages others to add more uses of it.

Given that (I think) you don't care about the "server" mode of lldb-platform at the moment, and the fact that the piece of code calling this is already broken because the use of fork, I think that for now we should just #ifdef-out the block of code calling fork and waitpid (it's already structured in a way that's easy to #ifdef. This would enable you to even provide a friendly error message explaining that the requested feature is not available.

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

1188–1198 ↗

(On Diff #198675)

ProcessInstanceInfo already has the ability to say whether some fields are present/valid or not. So I think you should just replace this #ifdef with something like:

if (proc_info.UserIDIsValid())
  response.Printf(";real-uid:%x", proc_info.GetUserID()));
if(proc_info.GroupIDIsValid())
  ...

tools/lldb-server/lldb-platform.cpp

73

It looks like this function will be unused after you disable the signal line below, so you might as well ifdef-out the entire function instead of just its body.

[compnerd]

include/lldb/Host/windows/PosixApi.h

106–109 ↗

(On Diff #198675)

I definitely am in favour of removal of the mocked POSIX API, to the point where I'm willing to entertain reduced functionality for that. If we can completely preprocess away the usage instead of adding this, I am absolutely okay with that.

[JDevlieghere]

cmake/modules/LLDBConfig.cmake

421

Should we just remove LLDB_CAN_USE_LLDB_SERVER instead?

[labath]

cmake/modules/LLDBConfig.cmake

421

Sounds like a good idea.

cmake/modules/LLDBConfig.cmake

 endif()
 
 list(APPEND system_libs ${CMAKE_DL_LIBS})
 
 SET(SKIP_LLDB_SERVER_BUILD OFF CACHE BOOL "Skip building lldb-server")
 
 # Figure out if lldb could use lldb-server.  If so, then we'll
 # ensure we build lldb-server when an lldb target is being built.
-if (CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|FreeBSD|Linux|NetBSD")
+if (CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|FreeBSD|Linux|NetBSD|Windows")
     set(LLDB_CAN_USE_LLDB_SERVER 1)
 else()
     set(LLDB_CAN_USE_LLDB_SERVER 0)
 endif()
 
 # Figure out if lldb could use debugserver.  If so, then we'll
 # ensure we build debugserver when we build lldb.
 if ( CMAKE_SYSTEM_NAME MATCHES "Darwin" )

source/Host/windows/Host.cpp

AutoHandle handle(
       nullptr);
   if (!handle.IsValid())
     return false;
 
   process_info.SetProcessID(pid);
   GetProcessExecutableAndTriple(handle, process_info);
 
   // Need to read the PEB to get parent process and command line arguments.
+
+  AutoHandle snapshot(CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0));
+  if (!snapshot.IsValid())
+    return false;
+
+  PROCESSENTRY32W pe;
+  pe.dwSize = sizeof(PROCESSENTRY32W);
+  if (Process32FirstW(snapshot.get(), &pe)) {
+    do {
+      if (pe.th32ProcessID == pid) {
+        process_info.SetParentProcessID(pe.th32ParentProcessID);
         return true;
       }
+    } while (Process32NextW(snapshot.get(), &pe));
+  }
+
+  return false;
+}
 
 HostThread Host::StartMonitoringChildProcess(
     const Host::MonitorChildProcessCallback &callback, lldb::pid_t pid,
     bool monitor_signals) {
   return HostThread();
 }
 
 Status Host::ShellExpandArguments(ProcessLaunchInfo &launch_info) {

source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

 #include "lldb/Utility/StreamString.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/ScopedPrinter.h"
 
 #include "ProcessGDBRemoteLog.h"
 
 #if defined(__APPLE__)
 #define DEBUGSERVER_BASENAME "debugserver"
+#elif defined(_WIN32)
+#define DEBUGSERVER_BASENAME "lldb-server.exe"
 #else
 #define DEBUGSERVER_BASENAME "lldb-server"
 #endif
 
 #if defined(HAVE_LIBCOMPRESSION)
 #include <compression.h>
 #endif
 

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Status GDBRemoteCommunicationServerLLGS::LaunchProcess() {
   const bool should_forward_stdio =
       m_process_launch_info.GetFileActionForFD(STDIN_FILENO) == nullptr ||
       m_process_launch_info.GetFileActionForFD(STDOUT_FILENO) == nullptr ||
       m_process_launch_info.GetFileActionForFD(STDERR_FILENO) == nullptr;
   m_process_launch_info.SetLaunchInSeparateProcessGroup(true);
   m_process_launch_info.GetFlags().Set(eLaunchFlagDebug);
 
   if (should_forward_stdio) {
+    // No pty support on Windows so far that means neither O* or I*
+    // notification packets will be generated about the inferior. However
+    // in most cases the missing notifications do not affect the major
+    // use of the lldb-server.  So here temporarily relax the following
+    // for Windows.
+#if !defined(_WIN32)
     if (llvm::Error Err = m_process_launch_info.SetUpPtyRedirection())
       return Status(std::move(Err));
+#endif
   }
 
   {
     std::lock_guard<std::recursive_mutex> guard(m_debugged_process_mutex);
     assert(!m_debugged_process_up && "lldb-server creating debugged "
                                      "process but one already exists");
     auto process_or =
         m_process_factory.Launch(m_process_launch_info, *this, m_mainloop);

tools/lldb-server/lldb-gdbserver.cpp

#endif
   // unless
   // explicitly asked to attach with the --attach={pid|program_name} form.
   if (!attach_target.empty())
     handle_attach(gdb_server, attach_target);
   else if (argc > 0)
     handle_launch(gdb_server, argc, argv);
 
   // Print version info.
-  printf("%s-%s", LLGS_PROGRAM_NAME, LLGS_VERSION_STR);
+  printf("%s-%s\n", LLGS_PROGRAM_NAME, LLGS_VERSION_STR);
 
   ConnectToRemote(mainloop, gdb_server, reverse_connect, host_and_port,
                   progname, subcommand, named_pipe_path.c_str(), 
                   unnamed_pipe, connection_fd);
 
   if (!gdb_server.IsConnected()) {
     fprintf(stderr, "no connection information provided, unable to run\n");
     display_usage(progname, subcommand);

tools/lldb-server/lldb-platform.cpp

 #if defined(__APPLE__)
 #include <netinet/in.h>
 #endif
 #include <signal.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#if !defined(_WIN32)
 #include <sys/wait.h>
-
+#endif
 #include <fstream>
 
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
 
 #include "Acceptor.h"
 #include "LLDBServerUtilities.h"
 #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h"

 #if defined(__APPLE__)
 #define LOW_PORT (IPPORT_RESERVED)
 #define HIGH_PORT (IPPORT_HIFIRSTAUTO)
 #else
 #define LOW_PORT (1024u)
 #define HIGH_PORT (49151u)
 #endif
 
+#if !defined(_WIN32)
 // Watch for signals
 static void signal_handler(int signo) {
   switch (signo) {
   case SIGHUP:
     // Use SIGINT first, if that does not work, use SIGHUP as a last resort.
     // And we should not call exit() here because it results in the global
     // destructors
     // to be invoked and wreaking havoc on the threads still running.
     Host::SystemLog(Host::eSystemLogWarning,
                     "SIGHUP received, exiting lldb-server...\n");
     abort();
     break;
   }
 }
+#endif
 
 static void display_usage(const char *progname, const char *subcommand) {
   fprintf(stderr, "Usage:\n  %s %s [--log-file log-file-name] [--log-channels "
                   "log-channel-list] [--port-file port-file-path] --server "
                   "--listen port\n",
           progname, subcommand);
   exit(0);
 }

 }
 
 // main
 int main_platform(int argc, char *argv[]) {
   const char *progname = argv[0];
   const char *subcommand = argv[1];
   argc--;
   argv++;
+#if !defined(_WIN32)
   signal(SIGPIPE, SIG_IGN);
   signal(SIGHUP, signal_handler);
+#endif
   int long_option_index = 0;
   Status error;
   std::string listen_host_port;
   int ch;
 
   std::string log_file;
   StringRef
       log_channels; // e.g. "lldb process threads:gdb-remote default:linux all"