Add a NativeProcessProtocol Factory class

[labath]

Summary

This replaces the static functions used for creating
NativeProcessProtocol instances with a factory pattern, and modernizes
the interface of the new class in the process -- I use llvm::Expected
instead of the Status+value combo. I also move some of the common code
(like the Delegate registration into the base class). The new
arrangement has multiple benefits:

• it removes the NativeProcess*** dependency from Process/gdb-remote (which for example means that liblldb no longer pulls in this code).
• it enables unit testing of the GDBRemoteCommunicationServerLLGS class (by providing a mock Native Process).
• serves as another example on how to use the llvm::Expected class (I couldn't get rid of the Initialize-type functions completely here because of the use of shared_from_this, but that's the next thing on my list here)

Tests still pass on Linux and I've made sure NetBSD compiles after this.

[labath]

forgot to reformat the code

[zturner]

source/Plugins/Process/Linux/NativeProcessLinux.h

148 ↗

(On Diff #101023)

Before it was only returning 1, now it's returning a vector. Any reason?

152 ↗

(On Diff #101023)

Shouldn't this be tid_t?

[labath]

source/Plugins/Process/Linux/NativeProcessLinux.h

148 ↗

(On Diff #101023)

I've refactored the function a bit. It now returns a list of threads that it has attached to. Previously it stored them in the object itself, but now it can't as I don't construct a process object until I know that the attach has succeeded.

I should probably document the return value though.

152 ↗

(On Diff #101023)

I'm using native thread types in this code. I think that makes more sense as this is the thing that actually interfaces with system APIs. linux does not have a separate type for thread IDs.

[krytarowski]

source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

112 ↗

(On Diff #101023)

I can imagine that in some cases there might be returned errno=0 and eg. WIFSIGNALED()=true. wstatus wouldn't be readable for end-user - I think it's better to check for (wpid != -1) following the same code between {} and add dedicated code for (wpid != pid || !WIFSTOPPED(wstatus)) that the state is unexpected and terminating.

133 ↗

(On Diff #101023)

ReinitializeThreads() was designed to not set stopped reason in threads.

  for (const auto &thread_sp : m_threads) {
    static_pointer_cast<NativeThreadNetBSD>(thread_sp)->SetStoppedBySignal(
        SIGSTOP);
}

I think we need the equivalent of this code there.

Similar for:

/* Set process stopped */
SetState(StateType::eStateStopped);

834 ↗

(On Diff #101023)

Technically it is possible on NetBSD, with uid=root and in kernel in INSECURE mode. This way root can debug init. In all other cases ptrace(2) returns error for PID 1.

I think it should be allowed, or rather not explicitly prohibited behavior.

tools/lldb-server/lldb-gdbserver.cpp

65 ↗

(On Diff #101023)

Included twice?

[eugene]

source/Plugins/Process/Linux/NativeProcessLinux.h

148 ↗

(On Diff #101023)

This method would certainly benefit from a comment that explains the nature of its return value.

[labath]

I am going to come back to this later, I'm going to create one or two more cleanup diffs which this will depend on.

[labath]

Address review comments

source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

112 ↗

(On Diff #101023)

After further consideration, I believe that waitpid will always succeed because the process launcher will make sure for us that the process has actually started (so I changed the wpid == -1 part into an assert). The only part that can fail here is the WIFSTOPPED, which can happen if someone kills the child process before we manage to stop it (in which case we will get a WIFEXITED/WIFSIGNALED instead.

133 ↗

(On Diff #101023)

That sounds correct. I have the equivalent code in the attach case, I must have missed this one somehow.

834 ↗

(On Diff #101023)

Agreed. In fact, I have removed the same check from linux code. If the user has enough privileges, and really wants to debug init, I don't think we should stop him.

[krytarowski]

source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

85 ↗

(On Diff #105065)

We can set BSD specific: -1 -> WAIT_ANY.

I verified that it exists on NetBSD, FreeBSD and OpenBSD.

87 ↗

(On Diff #105065)

Is this line needed?

738 ↗

(On Diff #105065)

We can set BSD specific: -1 -> WAIT_ANY.

[labath]

source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

85 ↗

(On Diff #105065)

I think that's semantically incorrect, WAIT_ANY means "any process" when used in the pid argument of waitpid. The -1 here is the "failure value" returned by waitpid, and that one is documented as -1 even on netbsd.

87 ↗

(On Diff #105065)

Yes, otherwise you get unused variable warnings in no-asserts build. (Or at least you would get, if I had removed the wpid != pid check like I was planning to).

[labath]

remove extra pid check

[krytarowski]

source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

85 ↗

(On Diff #105065)

Right, my mistake.

[krytarowski]

source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

87 ↗

(On Diff #105065)

This is right, I viewed a differential commit and it was misleading.

lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h

 #include "lldb/Host/MainLoop.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/TraceOptions.h"
 #include "lldb/lldb-private-forward.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include <vector>
 
 #include "NativeBreakpointList.h"
 #include "NativeWatchpointList.h"
 
 namespace lldb_private {
 class MemoryRegionInfo;
public:
   bool UnregisterNativeDelegate(NativeDelegate &native_delegate);
 
   virtual Status GetLoadedModuleFileSpec(const char *module_path,
                                          FileSpec &file_spec) = 0;
 
   virtual Status GetFileLoadAddress(const llvm::StringRef &file_name,
                                     lldb::addr_t &load_addr) = 0;
 
+  class Factory {
+  public:
+    virtual ~Factory();
     //------------------------------------------------------------------
-  /// Launch a process for debugging. This method will create an concrete
-  /// instance of NativeProcessProtocol, based on the host platform.
-  /// (e.g. NativeProcessLinux on linux, etc.)
+    /// Launch a process for debugging.
     ///
     /// @param[in] launch_info
     ///     Information required to launch the process.
     ///
     /// @param[in] native_delegate
     ///     The delegate that will receive messages regarding the
     ///     inferior.  Must outlive the NativeProcessProtocol
     ///     instance.
     ///
     /// @param[in] mainloop
     ///     The mainloop instance with which the process can register
     ///     callbacks. Must outlive the NativeProcessProtocol
     ///     instance.
     ///
-  /// @param[out] process_sp
-  ///     On successful return from the method, this parameter
-  ///     contains the shared pointer to the
-  ///     NativeProcessProtocol that can be used to manipulate
-  ///     the native process.
-  ///
     /// @return
-  ///     An error object indicating if the operation succeeded,
-  ///     and if not, what error occurred.
+    ///     A NativeProcessProtocol shared pointer if the operation succeeded or
+    ///     an error object if it failed.
     //------------------------------------------------------------------
-  static Status Launch(ProcessLaunchInfo &launch_info,
-                       NativeDelegate &native_delegate, MainLoop &mainloop,
-                       NativeProcessProtocolSP &process_sp);
+    virtual llvm::Expected<NativeProcessProtocolSP>
+    Launch(ProcessLaunchInfo &launch_info, NativeDelegate &native_delegate,
+           MainLoop &mainloop) const = 0;
 
     //------------------------------------------------------------------
-  /// Attach to an existing process. This method will create an concrete
-  /// instance of NativeProcessProtocol, based on the host platform.
-  /// (e.g. NativeProcessLinux on linux, etc.)
+    /// Attach to an existing process.
     ///
     /// @param[in] pid
     ///     pid of the process locatable
     ///
     /// @param[in] native_delegate
     ///     The delegate that will receive messages regarding the
     ///     inferior.  Must outlive the NativeProcessProtocol
     ///     instance.
     ///
     /// @param[in] mainloop
     ///     The mainloop instance with which the process can register
     ///     callbacks. Must outlive the NativeProcessProtocol
     ///     instance.
     ///
-  /// @param[out] process_sp
-  ///     On successful return from the method, this parameter
-  ///     contains the shared pointer to the
-  ///     NativeProcessProtocol that can be used to manipulate
-  ///     the native process.
-  ///
     /// @return
-  ///     An error object indicating if the operation succeeded,
-  ///     and if not, what error occurred.
+    ///     A NativeProcessProtocol shared pointer if the operation succeeded or
+    ///     an error object if it failed.
     //------------------------------------------------------------------
-  static Status Attach(lldb::pid_t pid, NativeDelegate &native_delegate,
-                       MainLoop &mainloop, NativeProcessProtocolSP &process_sp);
+    virtual llvm::Expected<NativeProcessProtocolSP>
+    Attach(lldb::pid_t pid, NativeDelegate &native_delegate,
+           MainLoop &mainloop) const = 0;
+  };
 
   //------------------------------------------------------------------
   /// StartTracing API for starting a tracing instance with the
   /// TraceOptions on a specific thread or process.
   ///
   /// @param[in] config
   ///     The configuration to use when starting tracing.
   ///
public:
   virtual Status GetTraceConfig(lldb::user_id_t traceid, TraceOptions &config) {
     return Status("Not implemented");
   }
 
 protected:
   lldb::pid_t m_pid;
 
   std::vector<NativeThreadProtocolSP> m_threads;
-  lldb::tid_t m_current_thread_id;
+  lldb::tid_t m_current_thread_id = LLDB_INVALID_THREAD_ID;
   mutable std::recursive_mutex m_threads_mutex;
 
-  lldb::StateType m_state;
+  lldb::StateType m_state = lldb::eStateInvalid;
   mutable std::recursive_mutex m_state_mutex;
 
   llvm::Optional<WaitStatus> m_exit_status;
 
   std::recursive_mutex m_delegates_mutex;
   std::vector<NativeDelegate *> m_delegates;
   NativeBreakpointList m_breakpoint_list;
   NativeWatchpointList m_watchpoint_list;
   HardwareBreakpointMap m_hw_breakpoints_map;
   int m_terminal_fd;
-  uint32_t m_stop_id;
+  uint32_t m_stop_id = 0;
 
   // Set of signal numbers that LLDB directly injects back to inferior
   // without stopping it.
   llvm::DenseSet<int> m_signals_to_ignore;
 
   // lldb_private::Host calls should be used to launch a process for debugging,
   // and
   // then the process should be attached to. When attaching to a process
   // lldb_private::Host calls should be used to locate the process to attach to,
   // and then this function should be called.
-  NativeProcessProtocol(lldb::pid_t pid);
+  NativeProcessProtocol(lldb::pid_t pid, int terminal_fd,
+                        NativeDelegate &delegate);
 
   // -----------------------------------------------------------
   // Internal interface for state handling
   // -----------------------------------------------------------
   void SetState(lldb::StateType state, bool notify_delegates = true);
 
   // Derived classes need not implement this.  It can be used as a
   // hook to clear internal caches that should be invalidated when

lldb/trunk/source/Host/common/NativeProcessProtocol.cpp

 
 using namespace lldb;
 using namespace lldb_private;
 
 // -----------------------------------------------------------------------------
 // NativeProcessProtocol Members
 // -----------------------------------------------------------------------------
 
-NativeProcessProtocol::NativeProcessProtocol(lldb::pid_t pid)
-    : m_pid(pid), m_threads(), m_current_thread_id(LLDB_INVALID_THREAD_ID),
-      m_threads_mutex(), m_state(lldb::eStateInvalid), m_state_mutex(),
-      m_delegates_mutex(), m_delegates(), m_breakpoint_list(),
-      m_watchpoint_list(), m_terminal_fd(-1), m_stop_id(0) {}
+NativeProcessProtocol::NativeProcessProtocol(lldb::pid_t pid, int terminal_fd,
+                                             NativeDelegate &delegate)
+    : m_pid(pid), m_terminal_fd(terminal_fd) {
+  bool registered = RegisterNativeDelegate(delegate);
+  assert(registered);
+  (void)registered;
+}
 
 lldb_private::Status NativeProcessProtocol::Interrupt() {
   Status error;
 #if !defined(SIGSTOP)
   error.SetErrorString("local host does not support signaling");
   return error;
 #else
   return Signal(SIGSTOP);
Status NativeProcessProtocol::ResolveProcessArchitecture(lldb::pid_t pid,
   arch = module_specs.GetModuleSpecRefAtIndex(0).GetArchitecture();
   if (arch.IsValid())
     return Status();
   else
     return Status(
         "failed to retrieve a valid architecture from the exe module");
 }
 
-#if !defined(__linux__) && !defined(__NetBSD__)
-// These need to be implemented to support lldb-gdb-server on a given platform.
-// Stubs are
-// provided to make the rest of the code link on non-supported platforms.
-
-Status NativeProcessProtocol::Launch(ProcessLaunchInfo &launch_info,
-                                     NativeDelegate &native_delegate,
-                                     MainLoop &mainloop,
-                                     NativeProcessProtocolSP &process_sp) {
-  llvm_unreachable("Platform has no NativeProcessProtocol support");
-}
-
-Status NativeProcessProtocol::Attach(lldb::pid_t pid,
-                                     NativeDelegate &native_delegate,
-                                     MainLoop &mainloop,
-                                     NativeProcessProtocolSP &process_sp) {
-  llvm_unreachable("Platform has no NativeProcessProtocol support");
-}
-
-#endif
+NativeProcessProtocol::Factory::~Factory() = default;

lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h

 /// @class NativeProcessLinux
 /// @brief Manages communication with the inferior (debugee) process.
 ///
 /// Upon construction, this class prepares and launches an inferior process for
 /// debugging.
 ///
 /// Changes in the inferior process state are broadcasted.
 class NativeProcessLinux : public NativeProcessProtocol {
-  friend Status NativeProcessProtocol::Launch(
-      ProcessLaunchInfo &launch_info, NativeDelegate &native_delegate,
-      MainLoop &mainloop, NativeProcessProtocolSP &process_sp);
-
-  friend Status NativeProcessProtocol::Attach(
-      lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &native_delegate,
-      MainLoop &mainloop, NativeProcessProtocolSP &process_sp);
-
 public:
+  class Factory : public NativeProcessProtocol::Factory {
+  public:
+    llvm::Expected<NativeProcessProtocolSP>
+    Launch(ProcessLaunchInfo &launch_info, NativeDelegate &native_delegate,
+           MainLoop &mainloop) const override;
+
+    llvm::Expected<NativeProcessProtocolSP>
+    Attach(lldb::pid_t pid, NativeDelegate &native_delegate,
+           MainLoop &mainloop) const override;
+  };
+
   // ---------------------------------------------------------------------
   // NativeProcessProtocol Interface
   // ---------------------------------------------------------------------
   Status Resume(const ResumeActionList &resume_actions) override;
 
   Status Halt() override;
 
   Status Detach() override;
protected:
   GetSoftwareBreakpointTrapOpcode(size_t trap_opcode_size_hint,
                                   size_t &actual_opcode_size,
                                   const uint8_t *&trap_opcode_bytes) override;
 
 private:
   MainLoop::SignalHandleUP m_sigchld_handle;
   ArchSpec m_arch;
 
-  LazyBool m_supports_mem_region;
+  LazyBool m_supports_mem_region = eLazyBoolCalculate;
   std::vector<std::pair<MemoryRegionInfo, FileSpec>> m_mem_region_cache;
 
-  lldb::tid_t m_pending_notification_tid;
+  lldb::tid_t m_pending_notification_tid = LLDB_INVALID_THREAD_ID;
 
   // List of thread ids stepping with a breakpoint with the address of
   // the relevan breakpoint
   std::map<lldb::tid_t, lldb::addr_t> m_threads_stepping_with_breakpoint;
 
   // ---------------------------------------------------------------------
   // Private Instance Methods
   // ---------------------------------------------------------------------
-  NativeProcessLinux();
-
-  Status LaunchInferior(MainLoop &mainloop, ProcessLaunchInfo &launch_info);
-
-  /// Attaches to an existing process.  Forms the
-  /// implementation of Process::DoAttach
-  void AttachToInferior(MainLoop &mainloop, lldb::pid_t pid, Status &error);
+  NativeProcessLinux(::pid_t pid, int terminal_fd, NativeDelegate &delegate,
+                     const ArchSpec &arch, MainLoop &mainloop);
 
-  ::pid_t Attach(lldb::pid_t pid, Status &error);
+  // Returns a list of process threads that we have attached to.
+  static llvm::Expected<std::vector<::pid_t>> Attach(::pid_t pid);
 
   static Status SetDefaultPtraceOpts(const lldb::pid_t);
 
-  static void *MonitorThread(void *baton);
+  void InitializeThreads(llvm::ArrayRef<::pid_t> tids);
 
   void MonitorCallback(lldb::pid_t pid, bool exited, WaitStatus status);
 
   void WaitForNewThread(::pid_t tid);
 
   void MonitorSIGTRAP(const siginfo_t &info, NativeThreadLinux &thread);
 
   void MonitorTrace(NativeThreadLinux &thread);
#endif
 
   llvm::DenseMap<lldb::tid_t, ProcessorTraceMonitorUP>
       m_processor_trace_monitor;
 
   // Set for tracking threads being traced under
   // same process user id.
   llvm::DenseSet<lldb::tid_t> m_pt_traced_thread_group;
 
-  lldb::user_id_t m_pt_proces_trace_id;
+  lldb::user_id_t m_pt_proces_trace_id = LLDB_INVALID_UID;
   TraceOptions m_pt_process_trace_config;
 };
 
 } // namespace process_linux
 } // namespace lldb_private
 
 #endif // #ifndef liblldb_NativeProcessLinux_H_

lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp

static Status EnsureFDFlags(int fd, int flags) {
 
   return error;
 }
 
 // -----------------------------------------------------------------------------
 // Public Static Methods
 // -----------------------------------------------------------------------------
 
-Status NativeProcessProtocol::Launch(
-    ProcessLaunchInfo &launch_info,
-    NativeProcessProtocol::NativeDelegate &native_delegate, MainLoop &mainloop,
-    NativeProcessProtocolSP &native_process_sp) {
+llvm::Expected<NativeProcessProtocolSP>
+NativeProcessLinux::Factory::Launch(ProcessLaunchInfo &launch_info,
+                                    NativeDelegate &native_delegate,
+                                    MainLoop &mainloop) const {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
 
-  Status error;
+  MaybeLogLaunchInfo(launch_info);
 
-  // Verify the working directory is valid if one was specified.
-  FileSpec working_dir{launch_info.GetWorkingDirectory()};
-  if (working_dir && (!working_dir.ResolvePath() ||
-                      !llvm::sys::fs::is_directory(working_dir.GetPath()))) {
-    error.SetErrorStringWithFormat("No such file or directory: %s",
-                                   working_dir.GetCString());
-    return error;
+  Status status;
+  ::pid_t pid = ProcessLauncherPosixFork()
+                    .LaunchProcess(launch_info, status)
+                    .GetProcessId();
+  LLDB_LOG(log, "pid = {0:x}", pid);
+  if (status.Fail()) {
+    LLDB_LOG(log, "failed to launch process: {0}", status);
+    return status.ToError();
   }
 
-  // Create the NativeProcessLinux in launch mode.
-  native_process_sp.reset(new NativeProcessLinux());
-
-  if (!native_process_sp->RegisterNativeDelegate(native_delegate)) {
-    native_process_sp.reset();
-    error.SetErrorStringWithFormat("failed to register the native delegate");
-    return error;
+  // Wait for the child process to trap on its call to execve.
+  int wstatus;
+  ::pid_t wpid = llvm::sys::RetryAfterSignal(-1, ::waitpid, pid, &wstatus, 0);
+  assert(wpid == pid);
+  (void)wpid;
+  if (!WIFSTOPPED(wstatus)) {
+    LLDB_LOG(log, "Could not sync with inferior process: wstatus={1}",
+             WaitStatus::Decode(wstatus));
+    return llvm::make_error<StringError>("Could not sync with inferior process",
+                                         llvm::inconvertibleErrorCode());
   }
+  LLDB_LOG(log, "inferior started, now in stopped state");
 
-  error = std::static_pointer_cast<NativeProcessLinux>(native_process_sp)
-              ->LaunchInferior(mainloop, launch_info);
+  ArchSpec arch;
+  if ((status = ResolveProcessArchitecture(pid, arch)).Fail())
+    return status.ToError();
 
-  if (error.Fail()) {
-    native_process_sp.reset();
-    LLDB_LOG(log, "failed to launch process: {0}", error);
-    return error;
-  }
+  // Set the architecture to the exe architecture.
+  LLDB_LOG(log, "pid = {0:x}, detected architecture {1}", pid,
+           arch.GetArchitectureName());
 
-  launch_info.SetProcessID(native_process_sp->GetID());
+  status = SetDefaultPtraceOpts(pid);
+  if (status.Fail()) {
+    LLDB_LOG(log, "failed to set default ptrace options: {0}", status);
+    return status.ToError();
+  }
 
-  return error;
+  std::shared_ptr<NativeProcessLinux> process_sp(new NativeProcessLinux(
+      pid, launch_info.GetPTY().ReleaseMasterFileDescriptor(), native_delegate,
+      arch, mainloop));
+  process_sp->InitializeThreads({pid});
+  return process_sp;
 }
 
-Status NativeProcessProtocol::Attach(
+llvm::Expected<NativeProcessProtocolSP> NativeProcessLinux::Factory::Attach(
     lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &native_delegate,
-    MainLoop &mainloop, NativeProcessProtocolSP &native_process_sp) {
+    MainLoop &mainloop) const {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
   LLDB_LOG(log, "pid = {0:x}", pid);
 
   // Retrieve the architecture for the running process.
-  ArchSpec process_arch;
-  Status error = ResolveProcessArchitecture(pid, process_arch);
-  if (!error.Success())
-    return error;
-
-  std::shared_ptr<NativeProcessLinux> native_process_linux_sp(
-      new NativeProcessLinux());
-
-  if (!native_process_linux_sp->RegisterNativeDelegate(native_delegate)) {
-    error.SetErrorStringWithFormat("failed to register the native delegate");
-    return error;
-  }
-
-  native_process_linux_sp->AttachToInferior(mainloop, pid, error);
-  if (!error.Success())
-    return error;
-
-  native_process_sp = native_process_linux_sp;
-  return error;
+  ArchSpec arch;
+  Status status = ResolveProcessArchitecture(pid, arch);
+  if (!status.Success())
+    return status.ToError();
+
+  auto tids_or = NativeProcessLinux::Attach(pid);
+  if (!tids_or)
+    return tids_or.takeError();
+
+  std::shared_ptr<NativeProcessLinux> process_sp(
+      new NativeProcessLinux(pid, -1, native_delegate, arch, mainloop));
+  process_sp->InitializeThreads(*tids_or);
+  return process_sp;
 }
 
 // -----------------------------------------------------------------------------
 // Public Instance Methods
 // -----------------------------------------------------------------------------
 
-NativeProcessLinux::NativeProcessLinux()
-    : NativeProcessProtocol(LLDB_INVALID_PROCESS_ID), m_arch(),
-      m_supports_mem_region(eLazyBoolCalculate), m_mem_region_cache(),
-      m_pending_notification_tid(LLDB_INVALID_THREAD_ID),
-      m_pt_proces_trace_id(LLDB_INVALID_UID) {}
-
-void NativeProcessLinux::AttachToInferior(MainLoop &mainloop, lldb::pid_t pid,
-                                          Status &error) {
-  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
-  LLDB_LOG(log, "pid = {0:x}", pid);
-
-  m_sigchld_handle = mainloop.RegisterSignal(
-      SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, error);
-  if (!m_sigchld_handle)
-    return;
-
-  error = ResolveProcessArchitecture(pid, m_arch);
-  if (!error.Success())
-    return;
-
-  // Set the architecture to the exe architecture.
-  LLDB_LOG(log, "pid = {0:x}, detected architecture {1}", pid,
-           m_arch.GetArchitectureName());
-  m_pid = pid;
-  SetState(eStateAttaching);
-
-  Attach(pid, error);
+NativeProcessLinux::NativeProcessLinux(::pid_t pid, int terminal_fd,
+                                       NativeDelegate &delegate,
+                                       const ArchSpec &arch, MainLoop &mainloop)
+    : NativeProcessProtocol(pid, terminal_fd, delegate), m_arch(arch) {
+  if (m_terminal_fd != -1) {
+    Status status = EnsureFDFlags(m_terminal_fd, O_NONBLOCK);
+    assert(status.Success());
   }
 
-Status NativeProcessLinux::LaunchInferior(MainLoop &mainloop,
-                                          ProcessLaunchInfo &launch_info) {
-  Status error;
+  Status status;
   m_sigchld_handle = mainloop.RegisterSignal(
-      SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, error);
-  if (!m_sigchld_handle)
-    return error;
-
-  SetState(eStateLaunching);
-
-  MaybeLogLaunchInfo(launch_info);
-
-  ::pid_t pid =
-      ProcessLauncherPosixFork().LaunchProcess(launch_info, error).GetProcessId();
-  if (error.Fail())
-    return error;
-
-  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
-
-  // Wait for the child process to trap on its call to execve.
-  ::pid_t wpid;
-  int status;
-  if ((wpid = waitpid(pid, &status, 0)) < 0) {
-    error.SetErrorToErrno();
-    LLDB_LOG(log, "waitpid for inferior failed with %s", error);
-
-    // Mark the inferior as invalid.
-    // FIXME this could really use a new state - eStateLaunchFailure.  For now,
-    // using eStateInvalid.
-    SetState(StateType::eStateInvalid);
-
-    return error;
-  }
-  assert(WIFSTOPPED(status) && (wpid == static_cast<::pid_t>(pid)) &&
-         "Could not sync with inferior process.");
-
-  LLDB_LOG(log, "inferior started, now in stopped state");
-  error = SetDefaultPtraceOpts(pid);
-  if (error.Fail()) {
-    LLDB_LOG(log, "failed to set default ptrace options: {0}", error);
-
-    // Mark the inferior as invalid.
-    // FIXME this could really use a new state - eStateLaunchFailure.  For now,
-    // using eStateInvalid.
-    SetState(StateType::eStateInvalid);
+      SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, status);
+  assert(m_sigchld_handle && status.Success());
 
-    return error;
-  }
-
-  // Release the master terminal descriptor and pass it off to the
-  // NativeProcessLinux instance.  Similarly stash the inferior pid.
-  m_terminal_fd = launch_info.GetPTY().ReleaseMasterFileDescriptor();
-  m_pid = pid;
-  launch_info.SetProcessID(pid);
-
-  if (m_terminal_fd != -1) {
-    error = EnsureFDFlags(m_terminal_fd, O_NONBLOCK);
-    if (error.Fail()) {
-      LLDB_LOG(log,
-               "inferior EnsureFDFlags failed for ensuring terminal "
-               "O_NONBLOCK setting: {0}",
-               error);
-
-      // Mark the inferior as invalid.
-      // FIXME this could really use a new state - eStateLaunchFailure.  For
-      // now, using eStateInvalid.
-      SetState(StateType::eStateInvalid);
-
-      return error;
-    }
-  }
-
-  LLDB_LOG(log, "adding pid = {0}", pid);
-  ResolveProcessArchitecture(m_pid, m_arch);
-  NativeThreadLinuxSP thread_sp = AddThread(pid);
+void NativeProcessLinux::InitializeThreads(llvm::ArrayRef<::pid_t> tids) {
+  for (const auto &tid : tids) {
+    NativeThreadLinuxSP thread_sp = AddThread(tid);
     assert(thread_sp && "AddThread() returned a nullptr thread");
     thread_sp->SetStoppedBySignal(SIGSTOP);
     ThreadWasCreated(*thread_sp);
+  }
 
   // Let our process instance know the thread has stopped.
-  SetCurrentThreadID(thread_sp->GetID());
-  SetState(StateType::eStateStopped);
+  SetCurrentThreadID(tids[0]);
+  SetState(StateType::eStateStopped, false);
 
-  if (error.Fail())
-    LLDB_LOG(log, "inferior launching failed {0}", error);
-  return error;
+  // Proccess any signals we received before installing our handler
+  SigchldHandler();
 }
 
-::pid_t NativeProcessLinux::Attach(lldb::pid_t pid, Status &error) {
+llvm::Expected<std::vector<::pid_t>> NativeProcessLinux::Attach(::pid_t pid) {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
 
+  Status status;
   // Use a map to keep track of the threads which we have attached/need to
   // attach.
   Host::TidMap tids_to_attach;
-  if (pid <= 1) {
-    error.SetErrorToGenericError();
-    error.SetErrorString("Attaching to process 1 is not allowed.");
-    return -1;
-  }
-
   while (Host::FindProcessThreads(pid, tids_to_attach)) {
     for (Host::TidMap::iterator it = tids_to_attach.begin();
          it != tids_to_attach.end();) {
       if (it->second == false) {
         lldb::tid_t tid = it->first;
 
         // Attach to the requested process.
         // An attach will cause the thread to stop with a SIGSTOP.
-        error = PtraceWrapper(PTRACE_ATTACH, tid);
-        if (error.Fail()) {
+        if ((status = PtraceWrapper(PTRACE_ATTACH, tid)).Fail()) {
           // No such thread. The thread may have exited.
           // More error handling may be needed.
-          if (error.GetError() == ESRCH) {
+          if (status.GetError() == ESRCH) {
             it = tids_to_attach.erase(it);
             continue;
-          } else
-            return -1;
+          }
+          return status.ToError();
         }
 
-        int status;
+        int wpid =
+            llvm::sys::RetryAfterSignal(-1, ::waitpid, tid, nullptr, __WALL);
         // Need to use __WALL otherwise we receive an error with errno=ECHLD
         // At this point we should have a thread stopped if waitpid succeeds.
-        if ((status = waitpid(tid, NULL, __WALL)) < 0) {
+        if (wpid < 0) {
           // No such thread. The thread may have exited.
           // More error handling may be needed.
           if (errno == ESRCH) {
             it = tids_to_attach.erase(it);
             continue;
-          } else {
-            error.SetErrorToErrno();
-            return -1;
           }
+          return llvm::errorCodeToError(
+              std::error_code(errno, std::generic_category()));
         }
 
-        error = SetDefaultPtraceOpts(tid);
-        if (error.Fail())
-          return -1;
+        if ((status = SetDefaultPtraceOpts(tid)).Fail())
+          return status.ToError();
 
         LLDB_LOG(log, "adding tid = {0}", tid);
         it->second = true;
-
-        // Create the thread, mark it as stopped.
-        NativeThreadLinuxSP thread_sp(AddThread(static_cast<lldb::tid_t>(tid)));
-        assert(thread_sp && "AddThread() returned a nullptr");
-
-        // This will notify this is a new thread and tell the system it is
-        // stopped.
-        thread_sp->SetStoppedBySignal(SIGSTOP);
-        ThreadWasCreated(*thread_sp);
-        SetCurrentThreadID(thread_sp->GetID());
       }
 
       // move the loop forward
       ++it;
     }
   }
 
-  if (tids_to_attach.size() > 0) {
-    m_pid = pid;
-    // Let our process instance know the thread has stopped.
-    SetState(StateType::eStateStopped);
-  } else {
-    error.SetErrorToGenericError();
-    error.SetErrorString("No such process.");
-    return -1;
-  }
-
-  return pid;
+  size_t tid_count = tids_to_attach.size();
+  if (tid_count == 0)
+    return llvm::make_error<StringError>("No such process",
+                                         llvm::inconvertibleErrorCode());
+
+  std::vector<::pid_t> tids;
+  tids.reserve(tid_count);
+  for (const auto &p : tids_to_attach)
+    tids.push_back(p.first);
+  return std::move(tids);
 }
 
 Status NativeProcessLinux::SetDefaultPtraceOpts(lldb::pid_t pid) {
   long ptrace_opts = 0;
 
   // Have the child raise an event on exit.  This is used to keep the child in
   // limbo until it is destroyed.
   ptrace_opts |= PTRACE_O_TRACEEXIT;

lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h

 /// @class NativeProcessNetBSD
 /// @brief Manages communication with the inferior (debugee) process.
 ///
 /// Upon construction, this class prepares and launches an inferior process for
 /// debugging.
 ///
 /// Changes in the inferior process state are broadcasted.
 class NativeProcessNetBSD : public NativeProcessProtocol {
-  friend Status NativeProcessProtocol::Launch(
-      ProcessLaunchInfo &launch_info, NativeDelegate &native_delegate,
-      MainLoop &mainloop, NativeProcessProtocolSP &process_sp);
-
-  friend Status NativeProcessProtocol::Attach(
-      lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &native_delegate,
-      MainLoop &mainloop, NativeProcessProtocolSP &process_sp);
-
 public:
+  class Factory : public NativeProcessProtocol::Factory {
+  public:
+    llvm::Expected<NativeProcessProtocolSP>
+    Launch(ProcessLaunchInfo &launch_info, NativeDelegate &native_delegate,
+           MainLoop &mainloop) const override;
+
+    llvm::Expected<NativeProcessProtocolSP>
+    Attach(lldb::pid_t pid, NativeDelegate &native_delegate,
+           MainLoop &mainloop) const override;
+  };
+
   // ---------------------------------------------------------------------
   // NativeProcessProtocol Interface
   // ---------------------------------------------------------------------
   Status Resume(const ResumeActionList &resume_actions) override;
 
   Status Halt() override;
 
   Status Detach() override;
protected:
   Status
   GetSoftwareBreakpointTrapOpcode(size_t trap_opcode_size_hint,
                                   size_t &actual_opcode_size,
                                   const uint8_t *&trap_opcode_bytes) override;
 
 private:
   MainLoop::SignalHandleUP m_sigchld_handle;
   ArchSpec m_arch;
-  LazyBool m_supports_mem_region;
+  LazyBool m_supports_mem_region = eLazyBoolCalculate;
   std::vector<std::pair<MemoryRegionInfo, FileSpec>> m_mem_region_cache;
 
   // ---------------------------------------------------------------------
   // Private Instance Methods
   // ---------------------------------------------------------------------
-  NativeProcessNetBSD();
+  NativeProcessNetBSD(::pid_t pid, int terminal_fd, NativeDelegate &delegate,
+                      const ArchSpec &arch, MainLoop &mainloop);
 
   bool HasThreadNoLock(lldb::tid_t thread_id);
 
   NativeThreadNetBSDSP AddThread(lldb::tid_t thread_id);
 
-  Status LaunchInferior(MainLoop &mainloop, ProcessLaunchInfo &launch_info);
-  void AttachToInferior(MainLoop &mainloop, lldb::pid_t pid, Status &error);
-
   void MonitorCallback(lldb::pid_t pid, int signal);
   void MonitorExited(lldb::pid_t pid, WaitStatus status);
   void MonitorSIGSTOP(lldb::pid_t pid);
   void MonitorSIGTRAP(lldb::pid_t pid);
   void MonitorSignal(lldb::pid_t pid, int signal);
 
   Status GetSoftwareBreakpointPCOffset(uint32_t &actual_opcode_size);
   Status FixupBreakpointPCAsNeeded(NativeThreadNetBSD &thread);
   Status PopulateMemoryRegionCache();
   void SigchldHandler();
 
-  ::pid_t Attach(lldb::pid_t pid, Status &error);
-
+  Status Attach();
   Status ReinitializeThreads();
 };
 
 } // namespace process_netbsd
 } // namespace lldb_private
 
 #endif // #ifndef liblldb_NativeProcessNetBSD_H_

lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

static Status EnsureFDFlags(int fd, int flags) {
 
   return error;
 }
 
 // -----------------------------------------------------------------------------
 // Public Static Methods
 // -----------------------------------------------------------------------------
 
-Status NativeProcessProtocol::Launch(
-    ProcessLaunchInfo &launch_info,
-    NativeProcessProtocol::NativeDelegate &native_delegate, MainLoop &mainloop,
-    NativeProcessProtocolSP &native_process_sp) {
+llvm::Expected<NativeProcessProtocolSP>
+NativeProcessNetBSD::Factory::Launch(ProcessLaunchInfo &launch_info,
+                                     NativeDelegate &native_delegate,
+                                     MainLoop &mainloop) const {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
 
-  Status error;
+  Status status;
+  ::pid_t pid = ProcessLauncherPosixFork()
+                    .LaunchProcess(launch_info, status)
+                    .GetProcessId();
+  LLDB_LOG(log, "pid = {0:x}", pid);
+  if (status.Fail()) {
+    LLDB_LOG(log, "failed to launch process: {0}", status);
+    return status.ToError();
+  }
 
-  // Verify the working directory is valid if one was specified.
-  FileSpec working_dir{launch_info.GetWorkingDirectory()};
-  if (working_dir && (!working_dir.ResolvePath() ||
-                      !llvm::sys::fs::is_directory(working_dir.GetPath()))) {
-    error.SetErrorStringWithFormat("No such file or directory: %s",
-                                   working_dir.GetCString());
-    return error;
+  // Wait for the child process to trap on its call to execve.
+  int wstatus;
+  ::pid_t wpid = llvm::sys::RetryAfterSignal(-1, ::waitpid, pid, &wstatus, 0);
+  assert(wpid == pid);
+  (void)wpid;
+  if (!WIFSTOPPED(wstatus)) {
+    LLDB_LOG(log, "Could not sync with inferior process: wstatus={1}",
+             WaitStatus::Decode(wstatus));
+    return llvm::make_error<StringError>("Could not sync with inferior process",
+                                         llvm::inconvertibleErrorCode());
   }
+  LLDB_LOG(log, "inferior started, now in stopped state");
 
-  // Create the NativeProcessNetBSD in launch mode.
-  native_process_sp.reset(new NativeProcessNetBSD());
+  ArchSpec arch;
+  if ((status = ResolveProcessArchitecture(pid, arch)).Fail())
+    return status.ToError();
 
-  if (!native_process_sp->RegisterNativeDelegate(native_delegate)) {
-    native_process_sp.reset();
-    error.SetErrorStringWithFormat("failed to register the native delegate");
-    return error;
-  }
+  // Set the architecture to the exe architecture.
+  LLDB_LOG(log, "pid = {0:x}, detected architecture {1}", pid,
+           arch.GetArchitectureName());
 
-  error = std::static_pointer_cast<NativeProcessNetBSD>(native_process_sp)
-              ->LaunchInferior(mainloop, launch_info);
+  std::shared_ptr<NativeProcessNetBSD> process_sp(new NativeProcessNetBSD(
+      pid, launch_info.GetPTY().ReleaseMasterFileDescriptor(), native_delegate,
+      arch, mainloop));
 
-  if (error.Fail()) {
-    native_process_sp.reset();
-    LLDB_LOG(log, "failed to launch process: {0}", error);
-    return error;
-  }
+  status = process_sp->ReinitializeThreads();
+  if (status.Fail())
+    return status.ToError();
 
-  launch_info.SetProcessID(native_process_sp->GetID());
+  for (const auto &thread_sp : process_sp->m_threads) {
+    static_pointer_cast<NativeThreadNetBSD>(thread_sp)->SetStoppedBySignal(
+        SIGSTOP);
+  }
+  process_sp->SetState(StateType::eStateStopped);
 
-  return error;
+  return process_sp;
 }
 
-Status NativeProcessProtocol::Attach(
+llvm::Expected<NativeProcessProtocolSP> NativeProcessNetBSD::Factory::Attach(
     lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &native_delegate,
-    MainLoop &mainloop, NativeProcessProtocolSP &native_process_sp) {
+    MainLoop &mainloop) const {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
   LLDB_LOG(log, "pid = {0:x}", pid);
 
   // Retrieve the architecture for the running process.
-  ArchSpec process_arch;
-  Status error = ResolveProcessArchitecture(pid, process_arch);
-  if (!error.Success())
-    return error;
+  ArchSpec arch;
+  Status status = ResolveProcessArchitecture(pid, arch);
+  if (!status.Success())
+    return status.ToError();
+
+  std::shared_ptr<NativeProcessNetBSD> process_sp(
+      new NativeProcessNetBSD(pid, -1, native_delegate, arch, mainloop));
+
+  status = process_sp->Attach();
+  if (!status.Success())
+    return status.ToError();
 
-  std::shared_ptr<NativeProcessNetBSD> native_process_netbsd_sp(
-      new NativeProcessNetBSD());
-
-  if (!native_process_netbsd_sp->RegisterNativeDelegate(native_delegate)) {
-    error.SetErrorStringWithFormat("failed to register the native delegate");
-    return error;
-  }
-
-  native_process_netbsd_sp->AttachToInferior(mainloop, pid, error);
-  if (!error.Success())
-    return error;
-
-  native_process_sp = native_process_netbsd_sp;
-  return error;
+  return process_sp;
 }
 
 // -----------------------------------------------------------------------------
 // Public Instance Methods
 // -----------------------------------------------------------------------------
 
-NativeProcessNetBSD::NativeProcessNetBSD()
-    : NativeProcessProtocol(LLDB_INVALID_PROCESS_ID), m_arch(),
-      m_supports_mem_region(eLazyBoolCalculate), m_mem_region_cache() {}
+NativeProcessNetBSD::NativeProcessNetBSD(::pid_t pid, int terminal_fd,
+                                         NativeDelegate &delegate,
+                                         const ArchSpec &arch,
+                                         MainLoop &mainloop)
+    : NativeProcessProtocol(pid, terminal_fd, delegate), m_arch(arch) {
+  if (m_terminal_fd != -1) {
+    Status status = EnsureFDFlags(m_terminal_fd, O_NONBLOCK);
+    assert(status.Success());
+  }
+
+  Status status;
+  m_sigchld_handle = mainloop.RegisterSignal(
+      SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, status);
+  assert(m_sigchld_handle && status.Success());
+}
 
 // Handles all waitpid events from the inferior process.
 void NativeProcessNetBSD::MonitorCallback(lldb::pid_t pid, int signal) {
   switch (signal) {
   case SIGTRAP:
     return MonitorSIGTRAP(pid);
   case SIGSTOP:
     return MonitorSIGSTOP(pid);

 }
 
 Status NativeProcessNetBSD::GetFileLoadAddress(const llvm::StringRef &file_name,
                                                lldb::addr_t &load_addr) {
   load_addr = LLDB_INVALID_ADDRESS;
   return Status();
 }
 
-Status NativeProcessNetBSD::LaunchInferior(MainLoop &mainloop,
-                                           ProcessLaunchInfo &launch_info) {
-  Status error;
-  m_sigchld_handle = mainloop.RegisterSignal(
-      SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, error);
-  if (!m_sigchld_handle)
-    return error;
-
-  SetState(eStateLaunching);
-
-  ::pid_t pid = ProcessLauncherPosixFork()
-                    .LaunchProcess(launch_info, error)
-                    .GetProcessId();
-  if (error.Fail())
-    return error;
-
-  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
-
-  // Wait for the child process to trap on its call to execve.
-  ::pid_t wpid;
-  int status;
-  if ((wpid = waitpid(pid, &status, 0)) < 0) {
-    error.SetErrorToErrno();
-    LLDB_LOG(log, "waitpid for inferior failed with %s", error);
-
-    // Mark the inferior as invalid.
-    // FIXME this could really use a new state - eStateLaunchFailure.  For
-    // now, using eStateInvalid.
-    SetState(StateType::eStateInvalid);
-
-    return error;
-  }
-  assert(WIFSTOPPED(status) && (wpid == static_cast<::pid_t>(pid)) &&
-         "Could not sync with inferior process.");
-
-  LLDB_LOG(log, "inferior started, now in stopped state");
-
-  // Release the master terminal descriptor and pass it off to the
-  // NativeProcessNetBSD instance.  Similarly stash the inferior pid.
-  m_terminal_fd = launch_info.GetPTY().ReleaseMasterFileDescriptor();
-  m_pid = pid;
-  launch_info.SetProcessID(pid);
-
-  if (m_terminal_fd != -1) {
-    error = EnsureFDFlags(m_terminal_fd, O_NONBLOCK);
-    if (error.Fail()) {
-      LLDB_LOG(log,
-               "inferior EnsureFDFlags failed for ensuring terminal "
-               "O_NONBLOCK setting: {0}",
-               error);
-
-      // Mark the inferior as invalid.
-      // FIXME this could really use a new state - eStateLaunchFailure.  For
-      // now, using eStateInvalid.
-      SetState(StateType::eStateInvalid);
-
-      return error;
-    }
-  }
-
-  LLDB_LOG(log, "adding pid = {0}", pid);
-
-  ResolveProcessArchitecture(m_pid, m_arch);
-
-  error = ReinitializeThreads();
-  if (error.Fail()) {
-    SetState(StateType::eStateInvalid);
-    return error;
-  }
-
-  for (const auto &thread_sp : m_threads) {
-    static_pointer_cast<NativeThreadNetBSD>(thread_sp)->SetStoppedBySignal(
-        SIGSTOP);
-  }
-
-  /* Set process stopped */
-  SetState(StateType::eStateStopped);
-
-  if (error.Fail())
-    LLDB_LOG(log, "inferior launching failed {0}", error);
-  return error;
-}
-
-void NativeProcessNetBSD::AttachToInferior(MainLoop &mainloop, lldb::pid_t pid,
-                                           Status &error) {
-  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
-  LLDB_LOG(log, "pid = {0:x}", pid);
-
-  m_sigchld_handle = mainloop.RegisterSignal(
-      SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, error);
-  if (!m_sigchld_handle)
-    return;
-
-  error = ResolveProcessArchitecture(pid, m_arch);
-  if (!error.Success())
-    return;
-
-  // Set the architecture to the exe architecture.
-  LLDB_LOG(log, "pid = {0:x}, detected architecture {1}", pid,
-           m_arch.GetArchitectureName());
-
-  m_pid = pid;
-  SetState(eStateAttaching);
-
-  Attach(pid, error);
-}
-
 void NativeProcessNetBSD::SigchldHandler() {
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
   // Process all pending waitpid notifications.
   int status;
   ::pid_t wait_pid =
       llvm::sys::RetryAfterSignal(-1, waitpid, GetID(), &status, WALLSIG | WNOHANG);
 
   if (wait_pid == 0)
NativeThreadNetBSDSP NativeProcessNetBSD::AddThread(lldb::tid_t thread_id) {
   if (m_threads.empty())
     SetCurrentThreadID(thread_id);
 
   auto thread_sp = std::make_shared<NativeThreadNetBSD>(this, thread_id);
   m_threads.push_back(thread_sp);
   return thread_sp;
 }
 
-::pid_t NativeProcessNetBSD::Attach(lldb::pid_t pid, Status &error) {
-  if (pid <= 1) {
-    error.SetErrorToGenericError();
-    error.SetErrorString("Attaching to process 1 is not allowed.");
-    return -1;
-  }
-
+Status NativeProcessNetBSD::Attach() {
   // Attach to the requested process.
   // An attach will cause the thread to stop with a SIGSTOP.
-  error = PtraceWrapper(PT_ATTACH, pid);
-  if (error.Fail())
-    return -1;
+  Status status = PtraceWrapper(PT_ATTACH, m_pid);
+  if (status.Fail())
+    return status;
 
-  int status;
+  int wstatus;
   // Need to use WALLSIG otherwise we receive an error with errno=ECHLD
   // At this point we should have a thread stopped if waitpid succeeds.
-  if ((status = waitpid(pid, NULL, WALLSIG)) < 0)
-    return -1;
-
-  m_pid = pid;
+  if ((wstatus = waitpid(m_pid, NULL, WALLSIG)) < 0)
+    return Status(errno, eErrorTypePOSIX);
 
   /* Initialize threads */
-  error = ReinitializeThreads();
-  if (error.Fail()) {
-    SetState(StateType::eStateInvalid);
-    return -1;
-  }
+  status = ReinitializeThreads();
+  if (status.Fail())
+    return status;
 
   for (const auto &thread_sp : m_threads) {
     static_pointer_cast<NativeThreadNetBSD>(thread_sp)->SetStoppedBySignal(
         SIGSTOP);
   }
 
   // Let our process instance know the thread has stopped.
   SetState(StateType::eStateStopped);
-
-  return pid;
+  return Status();
 }
 
 Status NativeProcessNetBSD::ReadMemory(lldb::addr_t addr, void *buf,
                                        size_t size, size_t &bytes_read) {
   unsigned char *dst = static_cast<unsigned char *>(buf);
   struct ptrace_io_desc io;
 
   Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_MEMORY));

lldb/trunk/source/Plugins/Process/gdb-remote/CMakeLists.txt

 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   include_directories(${LIBXML2_INCLUDE_DIR})
 endif()
 
 set(LLDB_PLUGINS
   lldbPluginProcessUtility
   lldbPluginPlatformMacOSX
 )
 
-if(CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
-  list(APPEND LLDB_PLUGINS lldbPluginProcessLinux)
-endif()
-
-if(CMAKE_SYSTEM_NAME MATCHES "NetBSD")
-  list(APPEND LLDB_PLUGINS lldbPluginProcessNetBSD)
-endif()
-
 add_lldb_library(lldbPluginProcessGDBRemote PLUGIN
   GDBRemoteClientBase.cpp
   GDBRemoteCommunication.cpp
   GDBRemoteCommunicationClient.cpp
   GDBRemoteCommunicationServer.cpp
   GDBRemoteCommunicationServerCommon.cpp
   GDBRemoteCommunicationServerLLGS.cpp
   GDBRemoteCommunicationServerPlatform.cpp

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

else {
           }
         }
       }
     }
   }
 
   if (success) {
     m_process_launch_error = LaunchProcess();
-    if (m_process_launch_info.GetProcessID() != LLDB_INVALID_PROCESS_ID) {
+    if (m_process_launch_error.Success())
       return SendOKResponse();
-    } else {
-      Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
-      if (log)
-        log->Printf("LLGSPacketHandler::%s failed to launch exe: %s",
-                    __FUNCTION__, m_process_launch_error.AsCString());
-    }
+    LLDB_LOG(log, "failed to launch exe: {0}", m_process_launch_error);
   }
   return SendErrorResponse(8);
 }
 
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_qEcho(
     StringExtractorGDBRemote &packet) {
   // Just echo back the exact same packet for qEcho...

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

 
 class GDBRemoteCommunicationServerLLGS
     : public GDBRemoteCommunicationServerCommon,
       public NativeProcessProtocol::NativeDelegate {
 public:
   //------------------------------------------------------------------
   // Constructors and Destructors
   //------------------------------------------------------------------
-  GDBRemoteCommunicationServerLLGS(MainLoop &mainloop);
+  GDBRemoteCommunicationServerLLGS(
+      MainLoop &mainloop,
+      const NativeProcessProtocol::Factory &process_factory);
 
   //------------------------------------------------------------------
   /// Specify the program to launch and its arguments.
   ///
   /// @param[in] args
   ///     The command line to launch.
   ///
   /// @param[in] argc
public:
 
   void DidExec(NativeProcessProtocol *process) override;
 
   Status InitializeConnection(std::unique_ptr<Connection> &&connection);
 
 protected:
   MainLoop &m_mainloop;
   MainLoop::ReadHandleUP m_network_handle_up;
-  lldb::tid_t m_current_tid;
-  lldb::tid_t m_continue_tid;
+  const NativeProcessProtocol::Factory &m_process_factory;
+  lldb::tid_t m_current_tid = LLDB_INVALID_THREAD_ID;
+  lldb::tid_t m_continue_tid = LLDB_INVALID_THREAD_ID;
   std::recursive_mutex m_debugged_process_mutex;
   NativeProcessProtocolSP m_debugged_process_sp;
 
   Communication m_stdio_communication;
   MainLoop::ReadHandleUP m_stdio_handle_up;
 
-  lldb::StateType m_inferior_prev_state;
+  lldb::StateType m_inferior_prev_state = lldb::StateType::eStateInvalid;
   std::unique_ptr<llvm::MemoryBuffer> m_active_auxv_buffer_up;
   std::mutex m_saved_registers_mutex;
   std::unordered_map<uint32_t, lldb::DataBufferSP> m_saved_registers_map;
-  uint32_t m_next_saved_registers_id;
-  bool m_handshake_completed : 1;
+  uint32_t m_next_saved_registers_id = 1;
+  bool m_handshake_completed = false;
 
   PacketResult SendONotification(const char *buffer, uint32_t len);
 
   PacketResult SendWResponse(NativeProcessProtocol *process);
 
   PacketResult SendStopReplyPacketForThread(lldb::tid_t tid);
 
   PacketResult SendStopReasonForState(lldb::StateType process_state);

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

enum GDBRemoteServerError {
   eErrorExitStatus
 };
 }
 
 //----------------------------------------------------------------------
 // GDBRemoteCommunicationServerLLGS constructor
 //----------------------------------------------------------------------
 GDBRemoteCommunicationServerLLGS::GDBRemoteCommunicationServerLLGS(
-    MainLoop &mainloop)
+    MainLoop &mainloop, const NativeProcessProtocol::Factory &process_factory)
     : GDBRemoteCommunicationServerCommon("gdb-remote.server",
                                          "gdb-remote.server.rx_packet"),
-      m_mainloop(mainloop), m_current_tid(LLDB_INVALID_THREAD_ID),
-      m_continue_tid(LLDB_INVALID_THREAD_ID), m_debugged_process_mutex(),
-      m_debugged_process_sp(), m_stdio_communication("process.stdio"),
-      m_inferior_prev_state(StateType::eStateInvalid),
-      m_saved_registers_map(), m_next_saved_registers_id(1),
-      m_handshake_completed(false) {
+      m_mainloop(mainloop), m_process_factory(process_factory),
+      m_stdio_communication("process.stdio") {
   RegisterPacketHandlers();
 }
 
 void GDBRemoteCommunicationServerLLGS::RegisterPacketHandlers() {
   RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_C,
                                 &GDBRemoteCommunicationServerLLGS::Handle_C);
   RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_c,
                                 &GDBRemoteCommunicationServerLLGS::Handle_c);
const bool should_forward_stdio =
       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);
 
   const bool default_to_use_pty = true;
   m_process_launch_info.FinalizeFileActions(nullptr, default_to_use_pty);
 
-  Status error;
   {
     std::lock_guard<std::recursive_mutex> guard(m_debugged_process_mutex);
     assert(!m_debugged_process_sp && "lldb-server creating debugged "
                                      "process but one already exists");
-    error = NativeProcessProtocol::Launch(m_process_launch_info, *this,
-                                          m_mainloop, m_debugged_process_sp);
+    auto process_or =
+        m_process_factory.Launch(m_process_launch_info, *this, m_mainloop);
+    if (!process_or) {
+      Status status(process_or.takeError());
+      llvm::errs() << llvm::formatv(
+          "failed to launch executable `{0}`: {1}",
+          m_process_launch_info.GetArguments().GetArgumentAtIndex(0), status);
+      return status;
     }
-
-  if (!error.Success()) {
-    fprintf(stderr, "%s: failed to launch executable %s", __FUNCTION__,
-            m_process_launch_info.GetArguments().GetArgumentAtIndex(0));
-    return error;
+    m_debugged_process_sp = *process_or;
   }
 
   // Handle mirroring of inferior stdout/stderr over the gdb-remote protocol
   // as needed.
   // llgs local-process debugging may specify PTY paths, which will make these
   // file actions non-null
   // process launch -i/e/o will also make these file actions non-null
   // nullptr means that the traffic is expected to flow over gdb-remote protocol
if (should_forward_stdio) {
 
     // Setup stdout/stderr mapping from inferior to $O
     auto terminal_fd = m_debugged_process_sp->GetTerminalFileDescriptor();
     if (terminal_fd >= 0) {
       if (log)
         log->Printf("ProcessGDBRemoteCommunicationServerLLGS::%s setting "
                     "inferior STDIO fd to %d",
                     __FUNCTION__, terminal_fd);
-      error = SetSTDIOFileDescriptor(terminal_fd);
-      if (error.Fail())
-        return error;
+      Status status = SetSTDIOFileDescriptor(terminal_fd);
+      if (status.Fail())
+        return status;
     } else {
       if (log)
         log->Printf("ProcessGDBRemoteCommunicationServerLLGS::%s ignoring "
                     "inferior STDIO since terminal fd reported as %d",
                     __FUNCTION__, terminal_fd);
     }
   } else {
     if (log)
       log->Printf("GDBRemoteCommunicationServerLLGS::%s pid %" PRIu64
                   " skipping stdout/stderr redirection via $O: inferior will "
                   "communicate over client-provided file descriptors",
                   __FUNCTION__, m_debugged_process_sp->GetID());
   }
 
   printf("Launched '%s' as process %" PRIu64 "...\n",
          m_process_launch_info.GetArguments().GetArgumentAtIndex(0),
-         m_process_launch_info.GetProcessID());
+         m_debugged_process_sp->GetID());
 
-  return error;
+  return Status();
 }
 
 Status GDBRemoteCommunicationServerLLGS::AttachToProcess(lldb::pid_t pid) {
-  Status error;
-
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
   if (log)
     log->Printf("GDBRemoteCommunicationServerLLGS::%s pid %" PRIu64,
                 __FUNCTION__, pid);
 
   // Before we try to attach, make sure we aren't already monitoring something
   // else.
   if (m_debugged_process_sp &&
       m_debugged_process_sp->GetID() != LLDB_INVALID_PROCESS_ID)
     return Status("cannot attach to a process %" PRIu64
                   " when another process with pid %" PRIu64
                   " is being debugged.",
                   pid, m_debugged_process_sp->GetID());
 
   // Try to attach.
-  error = NativeProcessProtocol::Attach(pid, *this, m_mainloop,
-                                        m_debugged_process_sp);
-  if (!error.Success()) {
-    fprintf(stderr, "%s: failed to attach to process %" PRIu64 ": %s",
-            __FUNCTION__, pid, error.AsCString());
-    return error;
+  auto process_or = m_process_factory.Attach(pid, *this, m_mainloop);
+  if (!process_or) {
+    Status status(process_or.takeError());
+    llvm::errs() << llvm::formatv("failed to attach to process {0}: {1}", pid,
+                                  status);
+    return status;
   }
+  m_debugged_process_sp = *process_or;
 
   // Setup stdout/stderr mapping from inferior.
   auto terminal_fd = m_debugged_process_sp->GetTerminalFileDescriptor();
   if (terminal_fd >= 0) {
     if (log)
       log->Printf("ProcessGDBRemoteCommunicationServerLLGS::%s setting "
                   "inferior STDIO fd to %d",
                   __FUNCTION__, terminal_fd);
-    error = SetSTDIOFileDescriptor(terminal_fd);
-    if (error.Fail())
-      return error;
+    Status status = SetSTDIOFileDescriptor(terminal_fd);
+    if (status.Fail())
+      return status;
   } else {
     if (log)
       log->Printf("ProcessGDBRemoteCommunicationServerLLGS::%s ignoring "
                   "inferior STDIO since terminal fd reported as %d",
                   __FUNCTION__, terminal_fd);
   }
 
   printf("Attached to process %" PRIu64 "...\n", pid);
-
-  return error;
+  return Status();
 }
 
 void GDBRemoteCommunicationServerLLGS::InitializeDelegate(
     NativeProcessProtocol *process) {
   assert(process && "process cannot be NULL");
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
   if (log) {
     log->Printf("GDBRemoteCommunicationServerLLGS::%s called with "

lldb/trunk/tools/lldb-server/CMakeLists.txt

if (LLVM_BUILD_STATIC)
   if (NOT LLDB_DISABLE_PYTHON)
     list(APPEND LLDB_SYSTEM_LIBS python2.7 util)
   endif()
   if (NOT LLDB_DISABLE_CURSES)
     list(APPEND LLDB_SYSTEM_LIBS gpm)
   endif()
 endif()
 
+set(LLDB_PLUGINS)
+
+if(CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
+  list(APPEND LLDB_PLUGINS lldbPluginProcessLinux)
+endif()
+
+if(CMAKE_SYSTEM_NAME MATCHES "NetBSD")
+  list(APPEND LLDB_PLUGINS lldbPluginProcessNetBSD)
+endif()
+
 add_lldb_tool(lldb-server INCLUDE_IN_FRAMEWORK
     Acceptor.cpp
     lldb-gdbserver.cpp
     lldb-platform.cpp
     lldb-server.cpp
     LLDBServerUtilities.cpp
 
     LINK_LIBS
       lldbBase
       lldbCore
       lldbHost
       lldbInitialization
       lldbInterpreter
-      ${EXTRA_LLDB_LIBS}
+      ${LLDB_PLUGINS}
       ${LLDB_SYSTEM_LIBS}
 
     LINK_COMPONENTS
       Support
 )
 
 target_link_libraries(lldb-server ${LLDB_SYSTEM_LIBS})

lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp

 #include "Plugins/Process/gdb-remote/ProcessGDBRemoteLog.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/HostGetOpt.h"
 #include "lldb/Host/OptionParser.h"
 #include "lldb/Host/Pipe.h"
 #include "lldb/Host/Socket.h"
 #include "lldb/Host/StringConvert.h"
+#include "lldb/Host/common/NativeProcessProtocol.h"
 #include "lldb/Utility/Status.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Errno.h"
 
+#if defined(__linux__)
+#include "Plugins/Process/Linux/NativeProcessLinux.h"
+#elif defined(__NetBSD__)
+#include "Plugins/Process/NetBSD/NativeProcessNetBSD.h"
+#endif
+
 #ifndef LLGS_PROGRAM_NAME
 #define LLGS_PROGRAM_NAME "lldb-server"
 #endif
 
 #ifndef LLGS_VERSION_STR
 #define LLGS_VERSION_STR "local_build"
 #endif
 
 using namespace llvm;
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::lldb_server;
 using namespace lldb_private::process_gdb_remote;
 
+namespace {
+#if defined(__linux__)
+typedef process_linux::NativeProcessLinux::Factory NativeProcessFactory;
+#elif defined(__NetBSD__)
+typedef process_netbsd::NativeProcessNetBSD::Factory NativeProcessFactory;
+#else
+// Dummy implementation to make sure the code compiles
+class NativeProcessFactory : public NativeProcessProtocol::Factory {
+public:
+  llvm::Expected<NativeProcessProtocolSP>
+  Launch(ProcessLaunchInfo &launch_info,
+         NativeProcessProtocol::NativeDelegate &delegate,
+         MainLoop &mainloop) const override {
+    llvm_unreachable("Not implemented");
+  }
+  llvm::Expected<NativeProcessProtocolSP>
+  Attach(lldb::pid_t pid, NativeProcessProtocol::NativeDelegate &delegate,
+         MainLoop &mainloop) const override {
+    llvm_unreachable("Not implemented");
+  }
+};
+#endif
+}
+
 //----------------------------------------------------------------------
 // option descriptors for getopt_long_only()
 //----------------------------------------------------------------------
 
 static int g_debug = 0;
 static int g_verbose = 0;
 
 static struct option g_long_options[] = {
#endif
   argc -= optind;
   argv += optind;
 
   if (argc == 0) {
     display_usage(progname, subcommand);
     exit(255);
   }
 
-  GDBRemoteCommunicationServerLLGS gdb_server(mainloop);
+  NativeProcessFactory factory;
+  GDBRemoteCommunicationServerLLGS gdb_server(mainloop, factory);
 
   const char *const host_and_port = argv[0];
   argc -= 1;
   argv += 1;
 
   // Any arguments left over are for the program that we need to launch. If
   // there
   // are no arguments, then the GDB server will start up and wait for an 'A'