[lldb] [Platform] Move common ::DebugProcess() to PlatformPOSIX

[mgorny]

Summary

Move common ::DebugProcess() implementation shared by Linux and NetBSD
(and to be shared by FreeBSD shortly) into PlatformPOSIX, and move
the old base implementation used only by Darwin to PlatformDarwin.

[mgorny]

@labath, I've tested it on Linux so far. I'm currently building on NetBSD.

@JDevlieghere, could you test this on Darwin? Alternatively, it's quite simple so I can wait for buildbots to complain after pushing ;-).

[labath]

This looks good, thanks. I don't think we need to do any special kind of testing on macos for this.

lldb/source/Plugins/Platform/Linux/PlatformLinux.h

public:
   void GetStatus(Stream &strm) override;
 
   bool GetSupportedArchitectureAtIndex(uint32_t idx, ArchSpec &arch) override;
 
   int32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) override;
 
   bool CanDebugProcess() override;
 
-  lldb::ProcessSP DebugProcess(ProcessLaunchInfo &launch_info,
-                               Debugger &debugger, Target *target,
-                               Status &error) override;
-
   void CalculateTrapHandlerSymbolNames() override;
 
   MmapArgList GetMmapArgumentList(const ArchSpec &arch, lldb::addr_t addr,
                                   lldb::addr_t length, unsigned prot,
                                   unsigned flags, lldb::addr_t fd,
                                   lldb::addr_t offset) override;
 
 private:
   PlatformLinux(const PlatformLinux &) = delete;
   const PlatformLinux &operator=(const PlatformLinux &) = delete;
 };
 
 } // namespace platform_linux
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_PLATFORM_LINUX_PLATFORMLINUX_H

lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp

bool PlatformLinux::CanDebugProcess() {
   if (IsHost()) {
     return true;
   } else {
     // If we're connected, we can debug.
     return IsConnected();
   }
 }
 
-// For local debugging, Linux will override the debug logic to use llgs-launch
-// rather than lldb-launch, llgs-attach.  This differs from current lldb-
-// launch, debugserver-attach approach on MacOSX.
-lldb::ProcessSP
-PlatformLinux::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger,
-                            Target *target, // Can be NULL, if NULL create a new
-                                            // target, else use existing one
-                            Status &error) {
-  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM));
-  LLDB_LOG(log, "target {0}", target);
-
-  // If we're a remote host, use standard behavior from parent class.
-  if (!IsHost())
-    return PlatformPOSIX::DebugProcess(launch_info, debugger, target, error);
-
-  //
-  // For local debugging, we'll insist on having ProcessGDBRemote create the
-  // process.
-  //
-
-  ProcessSP process_sp;
-
-  // Make sure we stop at the entry point
-  launch_info.GetFlags().Set(eLaunchFlagDebug);
-
-  // We always launch the process we are going to debug in a separate process
-  // group, since then we can handle ^C interrupts ourselves w/o having to
-  // worry about the target getting them as well.
-  launch_info.SetLaunchInSeparateProcessGroup(true);
-
-  // Ensure we have a target.
-  if (target == nullptr) {
-    LLDB_LOG(log, "creating new target");
-    TargetSP new_target_sp;
-    error = debugger.GetTargetList().CreateTarget(
-        debugger, "", "", eLoadDependentsNo, nullptr, new_target_sp);
-    if (error.Fail()) {
-      LLDB_LOG(log, "failed to create new target: {0}", error);
-      return process_sp;
-    }
-
-    target = new_target_sp.get();
-    if (!target) {
-      error.SetErrorString("CreateTarget() returned nullptr");
-      LLDB_LOG(log, "error: {0}", error);
-      return process_sp;
-    }
-  }
-
-  // Mark target as currently selected target.
-  debugger.GetTargetList().SetSelectedTarget(target);
-
-  // Now create the gdb-remote process.
-  LLDB_LOG(log, "having target create process with gdb-remote plugin");
-  process_sp =
-      target->CreateProcess(launch_info.GetListener(), "gdb-remote", nullptr);
-
-  if (!process_sp) {
-    error.SetErrorString("CreateProcess() failed for gdb-remote process");
-    LLDB_LOG(log, "error: {0}", error);
-    return process_sp;
-  }
-
-  LLDB_LOG(log, "successfully created process");
-  // Adjust launch for a hijacker.
-  ListenerSP listener_sp;
-  if (!launch_info.GetHijackListener()) {
-    LLDB_LOG(log, "setting up hijacker");
-    listener_sp =
-        Listener::MakeListener("lldb.PlatformLinux.DebugProcess.hijack");
-    launch_info.SetHijackListener(listener_sp);
-    process_sp->HijackProcessEvents(listener_sp);
-  }
-
-  // Log file actions.
-  if (log) {
-    LLDB_LOG(log, "launching process with the following file actions:");
-    StreamString stream;
-    size_t i = 0;
-    const FileAction *file_action;
-    while ((file_action = launch_info.GetFileActionAtIndex(i++)) != nullptr) {
-      file_action->Dump(stream);
-      LLDB_LOG(log, "{0}", stream.GetData());
-      stream.Clear();
-    }
-  }
-
-  // Do the launch.
-  error = process_sp->Launch(launch_info);
-  if (error.Success()) {
-    // Handle the hijacking of process events.
-    if (listener_sp) {
-      const StateType state = process_sp->WaitForProcessToStop(
-          llvm::None, nullptr, false, listener_sp);
-
-      LLDB_LOG(log, "pid {0} state {0}", process_sp->GetID(), state);
-    }
-
-    // Hook up process PTY if we have one (which we should for local debugging
-    // with llgs).
-    int pty_fd = launch_info.GetPTY().ReleasePrimaryFileDescriptor();
-    if (pty_fd != PseudoTerminal::invalid_fd) {
-      process_sp->SetSTDIOFileDescriptor(pty_fd);
-      LLDB_LOG(log, "hooked up STDIO pty to process");
-    } else
-      LLDB_LOG(log, "not using process STDIO pty");
-  } else {
-    LLDB_LOG(log, "{0}", error);
-    // FIXME figure out appropriate cleanup here.  Do we delete the target? Do
-    // we delete the process?  Does our caller do that?
-  }
-
-  return process_sp;
-}
-
 void PlatformLinux::CalculateTrapHandlerSymbolNames() {
   m_trap_handlers.push_back(ConstString("_sigtramp"));
   m_trap_handlers.push_back(ConstString("__kernel_rt_sigreturn"));
   m_trap_handlers.push_back(ConstString("__restore_rt"));
 }
 
 MmapArgList PlatformLinux::GetMmapArgumentList(const ArchSpec &arch,
                                                addr_t addr, addr_t length,

lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h

 //===-- PlatformDarwin.h ----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMDARWIN_H
 #define LLDB_SOURCE_PLUGINS_PLATFORM_MACOSX_PLATFORMDARWIN_H
 
 #include "Plugins/Platform/POSIX/PlatformPOSIX.h"
 #include "lldb/Host/FileSystem.h"
+#include "lldb/Host/ProcessLaunchInfo.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/StructuredData.h"
 #include "lldb/Utility/XcodeSDK.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/FileSystem.h"
 
 #include <string>
bool ARMGetSupportedArchitectureAtIndex(uint32_t idx,
                                           lldb_private::ArchSpec &arch);
 
   bool x86GetSupportedArchitectureAtIndex(uint32_t idx,
                                           lldb_private::ArchSpec &arch);
 
   int32_t GetResumeCountForLaunchInfo(
       lldb_private::ProcessLaunchInfo &launch_info) override;
 
+  lldb::ProcessSP DebugProcess(lldb_private::ProcessLaunchInfo &launch_info,
+                               lldb_private::Debugger &debugger,
+                               lldb_private::Target *target,
+                               lldb_private::Status &error) override;
+
   void CalculateTrapHandlerSymbolNames() override;
 
   llvm::VersionTuple
   GetOSVersion(lldb_private::Process *process = nullptr) override;
 
   bool SupportsModules() override { return true; }
 
   lldb_private::ConstString

lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

return 1;
              strcmp(shell_name, "tcsh") == 0 ||
              strcmp(shell_name, "zsh") == 0) {
     // csh and tcsh always seem to re-exec themselves.
     return 2;
   } else
     return 1;
 }
 
+lldb::ProcessSP
+PlatformDarwin::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger,
+                             Target *target, // Can be NULL, if NULL create
+                                             // a new target, else use existing
+                                             // one
+                             Status &error) {
+  ProcessSP process_sp;
+
+  if (IsHost()) {
+    // We are going to hand this process off to debugserver which will be in
+    // charge of setting the exit status.  However, we still need to reap it
+    // from lldb. So, make sure we use a exit callback which does not set exit
+    // status.
+    const bool monitor_signals = false;
+    launch_info.SetMonitorProcessCallback(
+        &ProcessLaunchInfo::NoOpMonitorCallback, monitor_signals);
+    process_sp = Platform::DebugProcess(launch_info, debugger, target, error);
+  } else {
+    if (m_remote_platform_sp)
+      process_sp = m_remote_platform_sp->DebugProcess(launch_info, debugger,
+                                                      target, error);
+    else
+      error.SetErrorString("the platform is not currently connected");
+  }
+  return process_sp;
+}
+
 void PlatformDarwin::CalculateTrapHandlerSymbolNames() {
   m_trap_handlers.push_back(ConstString("_sigtramp"));
 }
 
 static FileSpec GetCommandLineToolsLibraryPath() {
   static FileSpec g_command_line_tools_filespec;
 
   if (!g_command_line_tools_filespec) {

lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.h

public:
   void GetStatus(Stream &strm) override;
 
   bool GetSupportedArchitectureAtIndex(uint32_t idx, ArchSpec &arch) override;
 
   int32_t GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) override;
 
   bool CanDebugProcess() override;
 
-  lldb::ProcessSP DebugProcess(ProcessLaunchInfo &launch_info,
-                               Debugger &debugger, Target *target,
-                               Status &error) override;
-
   void CalculateTrapHandlerSymbolNames() override;
 
   MmapArgList GetMmapArgumentList(const ArchSpec &arch, lldb::addr_t addr,
                                   lldb::addr_t length, unsigned prot,
                                   unsigned flags, lldb::addr_t fd,
                                   lldb::addr_t offset) override;
 
 private:
   PlatformNetBSD(const PlatformNetBSD &) = delete;
   const PlatformNetBSD &operator=(const PlatformNetBSD &) = delete;
 };
 
 } // namespace platform_netbsd
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_PLATFORM_NETBSD_PLATFORMNETBSD_H

lldb/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp

bool PlatformNetBSD::CanDebugProcess() {
   if (IsHost()) {
     return true;
   } else {
     // If we're connected, we can debug.
     return IsConnected();
   }
 }
 
-// For local debugging, NetBSD will override the debug logic to use llgs-launch
-// rather than lldb-launch, llgs-attach.  This differs from current lldb-
-// launch, debugserver-attach approach on MacOSX.
-lldb::ProcessSP
-PlatformNetBSD::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger,
-                             Target *target, // Can be NULL, if NULL create a new
-                                             // target, else use existing one
-                             Status &error) {
-  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM));
-  LLDB_LOG(log, "target {0}", target);
-
-  // If we're a remote host, use standard behavior from parent class.
-  if (!IsHost())
-    return PlatformPOSIX::DebugProcess(launch_info, debugger, target, error);
-
-  //
-  // For local debugging, we'll insist on having ProcessGDBRemote create the
-  // process.
-  //
-
-  ProcessSP process_sp;
-
-  // Make sure we stop at the entry point
-  launch_info.GetFlags().Set(eLaunchFlagDebug);
-
-  // We always launch the process we are going to debug in a separate process
-  // group, since then we can handle ^C interrupts ourselves w/o having to
-  // worry about the target getting them as well.
-  launch_info.SetLaunchInSeparateProcessGroup(true);
-
-  // Ensure we have a target.
-  if (target == nullptr) {
-    LLDB_LOG(log, "creating new target");
-    TargetSP new_target_sp;
-    error = debugger.GetTargetList().CreateTarget(
-        debugger, "", "", eLoadDependentsNo, nullptr, new_target_sp);
-    if (error.Fail()) {
-      LLDB_LOG(log, "failed to create new target: {0}", error);
-      return process_sp;
-    }
-
-    target = new_target_sp.get();
-    if (!target) {
-      error.SetErrorString("CreateTarget() returned nullptr");
-      LLDB_LOG(log, "error: {0}", error);
-      return process_sp;
-    }
-  }
-
-  // Mark target as currently selected target.
-  debugger.GetTargetList().SetSelectedTarget(target);
-
-  // Now create the gdb-remote process.
-  LLDB_LOG(log, "having target create process with gdb-remote plugin");
-  process_sp =
-      target->CreateProcess(launch_info.GetListener(), "gdb-remote", nullptr);
-
-  if (!process_sp) {
-    error.SetErrorString("CreateProcess() failed for gdb-remote process");
-    LLDB_LOG(log, "error: {0}", error);
-    return process_sp;
-  }
-
-  LLDB_LOG(log, "successfully created process");
-  // Adjust launch for a hijacker.
-  ListenerSP listener_sp;
-  if (!launch_info.GetHijackListener()) {
-    LLDB_LOG(log, "setting up hijacker");
-    listener_sp =
-        Listener::MakeListener("lldb.PlatformNetBSD.DebugProcess.hijack");
-    launch_info.SetHijackListener(listener_sp);
-    process_sp->HijackProcessEvents(listener_sp);
-  }
-
-  // Log file actions.
-  if (log) {
-    LLDB_LOG(log, "launching process with the following file actions:");
-    StreamString stream;
-    size_t i = 0;
-    const FileAction *file_action;
-    while ((file_action = launch_info.GetFileActionAtIndex(i++)) != nullptr) {
-      file_action->Dump(stream);
-      LLDB_LOG(log, "{0}", stream.GetData());
-      stream.Clear();
-    }
-  }
-
-  // Do the launch.
-  error = process_sp->Launch(launch_info);
-  if (error.Success()) {
-    // Handle the hijacking of process events.
-    if (listener_sp) {
-      const StateType state = process_sp->WaitForProcessToStop(
-          llvm::None, nullptr, false, listener_sp);
-
-      LLDB_LOG(log, "pid {0} state {0}", process_sp->GetID(), state);
-    }
-
-    // Hook up process PTY if we have one (which we should for local debugging
-    // with llgs).
-    int pty_fd = launch_info.GetPTY().ReleasePrimaryFileDescriptor();
-    if (pty_fd != PseudoTerminal::invalid_fd) {
-      process_sp->SetSTDIOFileDescriptor(pty_fd);
-      LLDB_LOG(log, "hooked up STDIO pty to process");
-    } else
-      LLDB_LOG(log, "not using process STDIO pty");
-  } else {
-    LLDB_LOG(log, "{0}", error);
-    // FIXME figure out appropriate cleanup here.  Do we delete the target? Do
-    // we delete the process?  Does our caller do that?
-  }
-
-  return process_sp;
-}
-
 void PlatformNetBSD::CalculateTrapHandlerSymbolNames() {
   m_trap_handlers.push_back(ConstString("_sigtramp"));
 }
 
 MmapArgList PlatformNetBSD::GetMmapArgumentList(const ArchSpec &arch,
                                                 addr_t addr, addr_t length,
                                                 unsigned prot, unsigned flags,
                                                 addr_t fd, addr_t offset) {

lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp

lldb::ProcessSP PlatformPOSIX::Attach(ProcessAttachInfo &attach_info,
   return process_sp;
 }
 
 lldb::ProcessSP
 PlatformPOSIX::DebugProcess(ProcessLaunchInfo &launch_info, Debugger &debugger,
                             Target *target, // Can be NULL, if NULL create a new
                                             // target, else use existing one
                             Status &error) {
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PLATFORM));
+  LLDB_LOG(log, "target {0}", target);
+
   ProcessSP process_sp;
 
-  if (IsHost()) {
-    // We are going to hand this process off to debugserver which will be in
-    // charge of setting the exit status.  However, we still need to reap it
-    // from lldb. So, make sure we use a exit callback which does not set exit
-    // status.
-    const bool monitor_signals = false;
-    launch_info.SetMonitorProcessCallback(
-        &ProcessLaunchInfo::NoOpMonitorCallback, monitor_signals);
-    process_sp = Platform::DebugProcess(launch_info, debugger, target, error);
-  } else {
+  if (!IsHost()) {
     if (m_remote_platform_sp)
       process_sp = m_remote_platform_sp->DebugProcess(launch_info, debugger,
                                                       target, error);
     else
       error.SetErrorString("the platform is not currently connected");
+    return process_sp;
   }
+
+  //
+  // For local debugging, we'll insist on having ProcessGDBRemote create the
+  // process.
+  //
+
+  // Make sure we stop at the entry point
+  launch_info.GetFlags().Set(eLaunchFlagDebug);
+
+  // We always launch the process we are going to debug in a separate process
+  // group, since then we can handle ^C interrupts ourselves w/o having to
+  // worry about the target getting them as well.
+  launch_info.SetLaunchInSeparateProcessGroup(true);
+
+  // Ensure we have a target.
+  if (target == nullptr) {
+    LLDB_LOG(log, "creating new target");
+    TargetSP new_target_sp;
+    error = debugger.GetTargetList().CreateTarget(
+        debugger, "", "", eLoadDependentsNo, nullptr, new_target_sp);
+    if (error.Fail()) {
+      LLDB_LOG(log, "failed to create new target: {0}", error);
+      return process_sp;
+    }
+
+    target = new_target_sp.get();
+    if (!target) {
+      error.SetErrorString("CreateTarget() returned nullptr");
+      LLDB_LOG(log, "error: {0}", error);
+      return process_sp;
+    }
+  }
+
+  // Mark target as currently selected target.
+  debugger.GetTargetList().SetSelectedTarget(target);
+
+  // Now create the gdb-remote process.
+  LLDB_LOG(log, "having target create process with gdb-remote plugin");
+  process_sp =
+      target->CreateProcess(launch_info.GetListener(), "gdb-remote", nullptr);
+
+  if (!process_sp) {
+    error.SetErrorString("CreateProcess() failed for gdb-remote process");
+    LLDB_LOG(log, "error: {0}", error);
+    return process_sp;
+  }
+
+  LLDB_LOG(log, "successfully created process");
+  // Adjust launch for a hijacker.
+  ListenerSP listener_sp;
+  if (!launch_info.GetHijackListener()) {
+    LLDB_LOG(log, "setting up hijacker");
+    listener_sp =
+        Listener::MakeListener("lldb.PlatformLinux.DebugProcess.hijack");
+    launch_info.SetHijackListener(listener_sp);
+    process_sp->HijackProcessEvents(listener_sp);
+  }
+
+  // Log file actions.
+  if (log) {
+    LLDB_LOG(log, "launching process with the following file actions:");
+    StreamString stream;
+    size_t i = 0;
+    const FileAction *file_action;
+    while ((file_action = launch_info.GetFileActionAtIndex(i++)) != nullptr) {
+      file_action->Dump(stream);
+      LLDB_LOG(log, "{0}", stream.GetData());
+      stream.Clear();
+    }
+  }
+
+  // Do the launch.
+  error = process_sp->Launch(launch_info);
+  if (error.Success()) {
+    // Handle the hijacking of process events.
+    if (listener_sp) {
+      const StateType state = process_sp->WaitForProcessToStop(
+          llvm::None, nullptr, false, listener_sp);
+
+      LLDB_LOG(log, "pid {0} state {0}", process_sp->GetID(), state);
+    }
+
+    // Hook up process PTY if we have one (which we should for local debugging
+    // with llgs).
+    int pty_fd = launch_info.GetPTY().ReleasePrimaryFileDescriptor();
+    if (pty_fd != PseudoTerminal::invalid_fd) {
+      process_sp->SetSTDIOFileDescriptor(pty_fd);
+      LLDB_LOG(log, "hooked up STDIO pty to process");
+    } else
+      LLDB_LOG(log, "not using process STDIO pty");
+  } else {
+    LLDB_LOG(log, "{0}", error);
+    // FIXME figure out appropriate cleanup here.  Do we delete the target? Do
+    // we delete the process?  Does our caller do that?
+  }
+
   return process_sp;
 }
 
 void PlatformPOSIX::CalculateTrapHandlerSymbolNames() {
   m_trap_handlers.push_back(ConstString("_sigtramp"));
 }
 
 Status PlatformPOSIX::EvaluateLibdlExpression(