Avoid going through Platform when creating a NativeProcessProtocol instance

[labath]

Summary

This commit avoids the Platform instance when spawning or attaching to a process in lldb-server.
Instead, I have the server call a (static) method of NativeProcessProtocol directly. The reason
for this is that I believe that NativeProcessProtocol should be decoupled from the Platform
(after all, it always knows which platform it is running on, unlike the rest of lldb).
Additionally, the kind of platform actions a NativeProcessProtocol instance is likely to differ
greatly from the platform actions of the lldb client, so I think the separation makes sense.

After this, the only dependency NativeProcessLinux has on PlatformLinux is the ResolveExecutable
method, which needs additional refactoring.

[clayborg]

So the reason we originally involved the platform is that you might want to select a different platform (like a simulator platform) and have it create its own NativeProcess (I concede that the "NativeProcess" is not quite right for this assumption). Fine to remove it for now and we can change the names around if we ever need this functionality.

[ovyalov]

lgtm

[labath]

In D10996#200419, @clayborg wrote:

So the reason we originally involved the platform is that you might want to select a different platform (like a simulator platform) and have it create its own NativeProcess (I concede that the "NativeProcess" is not quite right for this assumption). Fine to remove it for now and we can change the names around if we ever need this functionality.

Thanks for the explanation. I was not aware of these plans, but I will keep them in mind for the future.

include/lldb/Host/common/NativeProcessProtocol.h

namespace lldb_private
     // NativeProcessProtocol
     //------------------------------------------------------------------
     class NativeProcessProtocol :
         public std::enable_shared_from_this<NativeProcessProtocol>
     {
         friend class SoftwareBreakpoint;
 
     public:
-        static NativeProcessProtocol *
-        CreateInstance (lldb::pid_t pid);
 
         // 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);
 
-    public:
         virtual ~NativeProcessProtocol ()
         {
         }
 
         virtual Error
         Resume (const ResumeActionList &resume_actions) = 0;
 
         virtual Error
public:
         Terminate ();
 
         virtual Error
         GetLoadedModuleFileSpec(const char* module_path, FileSpec& file_spec) = 0;
 
         virtual Error
         GetFileLoadAddress(const llvm::StringRef& file_name, lldb::addr_t& load_addr) = 0;
 
+        //------------------------------------------------------------------
+        /// 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.)
+        ///
+        /// @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[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.
+        //------------------------------------------------------------------
+        static Error
+        Launch (ProcessLaunchInfo &launch_info,
+                NativeDelegate &native_delegate,
+                NativeProcessProtocolSP &process_sp);
+
+        //------------------------------------------------------------------
+        /// 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.)
+        ///
+        /// @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[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.
+        //------------------------------------------------------------------
+        static Error
+        Attach (lldb::pid_t pid,
+                NativeDelegate &native_delegate,
+                NativeProcessProtocolSP &process_sp);
+
     protected:
         lldb::pid_t m_pid;
 
         std::vector<NativeThreadProtocolSP> m_threads;
         lldb::tid_t m_current_thread_id;
         mutable Mutex m_threads_mutex;
 
         lldb::StateType m_state;

include/lldb/Target/Platform.h

//                Error &error) = 0;
         /// signal handler routines as needed.  For most Unix platforms, add _sigtramp.
         ///
         /// @return
         ///     A list of symbol names.  The list may be empty.
         //------------------------------------------------------------------
         virtual const std::vector<ConstString> &
         GetTrapHandlerSymbolNames ();
 
-        //------------------------------------------------------------------
-        /// Launch a process for debugging.
-        ///
-        /// This differs from Launch in that it returns a NativeProcessProtocol.
-        /// Currently used by lldb-gdbserver.
-        ///
-        /// @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[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.
-        //------------------------------------------------------------------
-        virtual Error
-        LaunchNativeProcess (
-            ProcessLaunchInfo &launch_info,
-            lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
-            NativeProcessProtocolSP &process_sp);
-
-        //------------------------------------------------------------------
-        /// Attach to an existing process on the given platform.
-        ///
-        /// This method differs from Attach() in that it returns a
-        /// NativeProcessProtocol.  Currently this is used by lldb-gdbserver.
-        ///
-        /// @param[in] pid
-        ///     pid of the process locatable by the platform.
-        ///
-        /// @param[in] native_delegate
-        ///     The delegate that will receive messages regarding the
-        ///     inferior.  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.
-        //------------------------------------------------------------------
-        virtual Error
-        AttachNativeProcess (lldb::pid_t pid,
-                             lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
-                             NativeProcessProtocolSP &process_sp);
-
     protected:
         bool m_is_host;
         // Set to true when we are able to actually set the OS version while 
         // being connected. For remote platforms, we might set the version ahead
         // of time before we actually connect and this version might change when
         // we actually connect to a remote platform. For the host platform this
         // will be set to the once we call HostInfo::GetOSVersion().
         bool m_os_version_set_while_connected;

source/Plugins/Platform/Kalimba/PlatformKalimba.h

public:
         bool
         CanDebugProcess() override
         {
             return false;
         }
 
         void CalculateTrapHandlerSymbolNames() override;
 
-        Error
-        LaunchNativeProcess (
-            ProcessLaunchInfo &launch_info,
-            lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
-            NativeProcessProtocolSP &process_sp) override;
-
-        Error
-        AttachNativeProcess (lldb::pid_t pid,
-                             lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
-                             NativeProcessProtocolSP &process_sp) override;
-
     protected:
         lldb::PlatformSP m_remote_platform_sp;
 
     private:
         DISALLOW_COPY_AND_ASSIGN (PlatformKalimba);
     };
 } // namespace lldb_private
 
 #endif  // liblldb_PlatformKalimba_h_

source/Plugins/Platform/Kalimba/PlatformKalimba.cpp

PlatformKalimba::Attach(ProcessAttachInfo &attach_info,
     return process_sp;
 }
 
 void
 PlatformKalimba::CalculateTrapHandlerSymbolNames ()
 {   
     // TODO Research this sometime.
 }   
-
-Error
-PlatformKalimba::LaunchNativeProcess (
-    ProcessLaunchInfo &,
-    lldb_private::NativeProcessProtocol::NativeDelegate &,
-    NativeProcessProtocolSP &)
-{
-    return Error();
-}
-
-Error
-PlatformKalimba::AttachNativeProcess (lldb::pid_t,
-                                    lldb_private::NativeProcessProtocol::NativeDelegate &,
-                                    NativeProcessProtocolSP &)
-{
-    return Error();
-}
-

source/Plugins/Platform/Linux/PlatformLinux.h

public:
         DebugProcess (ProcessLaunchInfo &launch_info,
                       Debugger &debugger,
                       Target *target,
                       Error &error) override;
 
         void
         CalculateTrapHandlerSymbolNames () override;
 
-        Error
-        LaunchNativeProcess (
-            ProcessLaunchInfo &launch_info,
-            NativeProcessProtocol::NativeDelegate &native_delegate,
-            NativeProcessProtocolSP &process_sp) override;
-
-        Error
-        AttachNativeProcess (lldb::pid_t pid,
-                             NativeProcessProtocol::NativeDelegate &native_delegate,
-                             NativeProcessProtocolSP &process_sp) override;
-
         uint64_t
         ConvertMmapFlagsToPlatform(const ArchSpec &arch, unsigned flags) override;
 
     private:
         DISALLOW_COPY_AND_ASSIGN (PlatformLinux);
     };
 
 } // namespace platform_linux
 } // namespace lldb_private
 
 #endif  // liblldb_PlatformLinux_h_

source/Plugins/Platform/Linux/PlatformLinux.cpp

 #include "lldb/Core/StreamString.h"
 #include "lldb/Host/FileSpec.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Interpreter/OptionValueProperties.h"
 #include "lldb/Interpreter/Property.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Process.h"
 
-#if defined(__linux__)
-#include "../../Process/Linux/NativeProcessLinux.h"
-#endif
-
 // Define these constants from Linux mman.h for use when targeting
 // remote linux systems even when host has different values.
 #define MAP_PRIVATE 2
 #define MAP_ANON 0x20
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::platform_linux;

 }
 
 void
 PlatformLinux::CalculateTrapHandlerSymbolNames ()
 {
     m_trap_handlers.push_back (ConstString ("_sigtramp"));
 }
 
-Error
-PlatformLinux::LaunchNativeProcess (ProcessLaunchInfo &launch_info,
-                                    NativeProcessProtocol::NativeDelegate &native_delegate,
-                                    NativeProcessProtocolSP &process_sp)
-{
-#if !defined(__linux__)
-    return Error("Only implemented on Linux hosts");
-#else
-    if (!IsHost ())
-        return Error("PlatformLinux::%s (): cannot launch a debug process when not the host", __FUNCTION__);
-
-    // Retrieve the exe module.
-    lldb::ModuleSP exe_module_sp;
-    ModuleSpec exe_module_spec(launch_info.GetExecutableFile(), launch_info.GetArchitecture());
-
-    Error error = ResolveExecutable (
-        exe_module_spec,
-        exe_module_sp,
-        NULL);
-
-    if (!error.Success ())
-        return error;
-
-    if (!exe_module_sp)
-        return Error("exe_module_sp could not be resolved for %s", launch_info.GetExecutableFile ().GetPath ().c_str ());
-
-    // Launch it for debugging
-    error = process_linux::NativeProcessLinux::LaunchProcess (
-        exe_module_sp.get (),
-        launch_info,
-        native_delegate,
-        process_sp);
-
-    return error;
-#endif
-}
-
-Error
-PlatformLinux::AttachNativeProcess (lldb::pid_t pid,
-                                    NativeProcessProtocol::NativeDelegate &native_delegate,
-                                    NativeProcessProtocolSP &process_sp)
-{
-#if !defined(__linux__)
-    return Error("Only implemented on Linux hosts");
-#else
-    if (!IsHost ())
-        return Error("PlatformLinux::%s (): cannot attach to a debug process when not the host", __FUNCTION__);
-
-    // Launch it for debugging
-    return process_linux::NativeProcessLinux::AttachToProcess (pid, native_delegate, process_sp);
-#endif
-}
-
 uint64_t
 PlatformLinux::ConvertMmapFlagsToPlatform(const ArchSpec &arch, unsigned flags)
 {
     uint64_t flags_platform = 0;
     uint64_t map_anon = MAP_ANON;
 
     // To get correct flags for MIPS Architecture
     if (arch.GetTriple ().getArch () == llvm::Triple::mips64

source/Plugins/Process/Linux/NativeProcessLinux.h

namespace process_linux {
     /// @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
     {
-    public:
-
-        static Error
-        LaunchProcess (
-            Module *exe_module,
-            ProcessLaunchInfo &launch_info,
-            NativeProcessProtocol::NativeDelegate &native_delegate,
-            NativeProcessProtocolSP &native_process_sp);
+        friend Error
+        NativeProcessProtocol::Launch (ProcessLaunchInfo &launch_info,
+                NativeDelegate &native_delegate,
+                NativeProcessProtocolSP &process_sp);
 
-        static Error
-        AttachToProcess (
-            lldb::pid_t pid,
+        friend Error
+        NativeProcessProtocol::Attach (lldb::pid_t pid,
             NativeProcessProtocol::NativeDelegate &native_delegate,
             NativeProcessProtocolSP &native_process_sp);
 
+    public:
         //------------------------------------------------------------------------------
         /// @class Operation
         /// @brief Represents a NativeProcessLinux operation.
         ///
         /// Under Linux, it is not possible to ptrace() from any other thread but the
         /// one that spawned or attached to the process from the start.  Therefore, when
         /// a NativeProcessLinux is asked to deliver or change the state of an inferior
         /// process the operation must be "funneled" to a specific thread to perform the

source/Plugins/Process/Linux/NativeProcessLinux.cpp

 NativeProcessLinux::LaunchArgs::~LaunchArgs()
 { }
 
 // -----------------------------------------------------------------------------
 // Public Static Methods
 // -----------------------------------------------------------------------------
 
 Error
-NativeProcessLinux::LaunchProcess (
-    Module *exe_module,
+NativeProcessProtocol::Launch (
     ProcessLaunchInfo &launch_info,
     NativeProcessProtocol::NativeDelegate &native_delegate,
     NativeProcessProtocolSP &native_process_sp)
 {
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
 
-    Error error;
+    lldb::ModuleSP exe_module_sp;
+    PlatformSP platform_sp (Platform::GetHostPlatform ());
+    Error error = platform_sp->ResolveExecutable(
+            ModuleSpec(launch_info.GetExecutableFile(), launch_info.GetArchitecture()),
+            exe_module_sp,
+            nullptr);
+
+    if (! error.Success())
+        return error;
 
     // Verify the working directory is valid if one was specified.
     FileSpec working_dir{launch_info.GetWorkingDirectory()};
     if (working_dir &&
             (!working_dir.ResolvePath() ||
              working_dir.GetFileType() != FileSpec::eFileTypeDirectory))
     {
         error.SetErrorStringWithFormat ("No such file or directory: %s",
NativeProcessProtocol::Launch (
     if (!native_process_sp->RegisterNativeDelegate (native_delegate))
     {
         native_process_sp.reset ();
         error.SetErrorStringWithFormat ("failed to register the native delegate");
         return error;
     }
 
     std::static_pointer_cast<NativeProcessLinux> (native_process_sp)->LaunchInferior (
-            exe_module,
+            exe_module_sp.get(),
             launch_info.GetArguments ().GetConstArgumentVector (),
             launch_info.GetEnvironmentEntries ().GetConstArgumentVector (),
             stdin_file_spec,
             stdout_file_spec,
             stderr_file_spec,
             working_dir,
             launch_info,
             error);
 
     if (error.Fail ())
     {
         native_process_sp.reset ();
         if (log)
             log->Printf ("NativeProcessLinux::%s failed to launch process: %s", __FUNCTION__, error.AsCString ());
         return error;
     }
 
     launch_info.SetProcessID (native_process_sp->GetID ());
 
     return error;
 }
 
 Error
-NativeProcessLinux::AttachToProcess (
+NativeProcessProtocol::Attach (
     lldb::pid_t pid,
     NativeProcessProtocol::NativeDelegate &native_delegate,
     NativeProcessProtocolSP &native_process_sp)
 {
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
     if (log && log->GetMask ().Test (POSIX_LOG_VERBOSE))
         log->Printf ("NativeProcessLinux::%s(pid = %" PRIi64 ")", __FUNCTION__, pid);
 

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

GDBRemoteCommunicationServerLLGS::LaunchProcess ()
 
     if (!m_process_launch_info.GetArguments ().GetArgumentCount ())
         return Error ("%s: no process command line specified to launch", __FUNCTION__);
 
     Error error;
     {
         Mutex::Locker locker (m_debugged_process_mutex);
         assert (!m_debugged_process_sp && "lldb-gdbserver creating debugged process but one already exists");
-        error = m_platform_sp->LaunchNativeProcess (
+        error = NativeProcessProtocol::Launch(
             m_process_launch_info,
             *this,
             m_debugged_process_sp);
     }
 
     if (!error.Success ())
     {
         fprintf (stderr, "%s: failed to launch executable %s", __FUNCTION__, m_process_launch_info.GetArguments ().GetArgumentAtIndex (0));
// Scope for mutex locker.
         Mutex::Locker locker (m_spawned_pids_mutex);
         if (!m_spawned_pids.empty ())
         {
             error.SetErrorStringWithFormat ("cannot attach to a process %" PRIu64 " when another process with pid %" PRIu64 " is being debugged.", pid, *m_spawned_pids.begin());
             return error;
         }
 
         // Try to attach.
-        error = m_platform_sp->AttachNativeProcess (pid, *this, m_debugged_process_sp);
+        error = NativeProcessProtocol::Attach(pid, *this, m_debugged_process_sp);
         if (!error.Success ())
         {
             fprintf (stderr, "%s: failed to attach to process %" PRIu64 ": %s", __FUNCTION__, pid, error.AsCString ());
             return error;
         }
 
         // Setup stdout/stderr mapping from inferior.
         auto terminal_fd = m_debugged_process_sp->GetTerminalFileDescriptor ();

source/Target/Platform.cpp

Platform::CalculateMD5 (const FileSpec& file_spec,
                         uint64_t &high)
 {
     if (IsHost())
         return FileSystem::CalculateMD5(file_spec, low, high);
     else
         return false;
 }
 
-Error
-Platform::LaunchNativeProcess (
-    ProcessLaunchInfo &launch_info,
-    lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
-    NativeProcessProtocolSP &process_sp)
-{
-    // Platforms should override this implementation if they want to
-    // support lldb-gdbserver.
-    return Error("unimplemented");
-}
-
-Error
-Platform::AttachNativeProcess (lldb::pid_t pid,
-                               lldb_private::NativeProcessProtocol::NativeDelegate &native_delegate,
-                               NativeProcessProtocolSP &process_sp)
-{
-    // Platforms should override this implementation if they want to
-    // support lldb-gdbserver.
-    return Error("unimplemented");
-}
-
 void
 Platform::SetLocalCacheDirectory (const char* local)
 {
     m_local_cache_directory.assign(local);
 }
 
 const char*
 Platform::GetLocalCacheDirectory ()