[Lldb-commits] [PATCH] D61686: Enable lldb-server on Windows

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu May 9 04:03:51 PDT 2019

labath added inline comments.

Comment at: include/lldb/Host/windows/PosixApi.h:106-109
+inline pid_t waitpid(pid_t pid, int *status, int options) {
+  // To be implemented.
+  return pid_t(-1);
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.

Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:1188-1198
+#if defined(_WIN32)
+  response.Printf("pid:%" PRIx64 ";parent-pid:%" PRIx64 ";",
+                  proc_info.GetProcessID(), proc_info.GetParentProcessID());
   response.Printf("pid:%" PRIx64 ";parent-pid:%" PRIx64
                   proc_info.GetProcessID(), proc_info.GetParentProcessID(),
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()));

Comment at: tools/lldb-server/lldb-platform.cpp:72
 // Watch for signals
 static void signal_handler(int signo) {
+#if !defined(_WIN32)
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.



More information about the lldb-commits mailing list