Avoid going through Platform when creating a NativeProcessProtocol instance

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.

This is a resubmit of r241672, after it was reverted due to build failueres on non-linux
platforms.

Reviewers: ovyalov, clayborg

Subscribers: lldb-commits

Differential Revision: http://reviews.llvm.org/D10996

llvm-svn: 241796

11 files changed, 99 insertions, 198 deletions

diff --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
index f6a685aae14..4f0f3a962d3 100644
--- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -35,8 +35,6 @@ namespace lldb_private
         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
@@ -44,7 +42,6 @@ namespace lldb_private
         // and then this function should be called.
         NativeProcessProtocol (lldb::pid_t pid);
 
-    public:
         virtual ~NativeProcessProtocol ()
         {
         }
@@ -297,6 +294,62 @@ namespace lldb_private
         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;
 
diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index 8f89e9b3cf5..cd9b8723aef 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -939,65 +939,6 @@ class ModuleCache;
         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 
diff --git a/lldb/source/Host/common/NativeProcessProtocol.cpp b/lldb/source/Host/common/NativeProcessProtocol.cpp
index 35003f5d020..818d69bdabd 100644
--- a/lldb/source/Host/common/NativeProcessProtocol.cpp
+++ b/lldb/source/Host/common/NativeProcessProtocol.cpp
@@ -441,3 +441,25 @@ NativeProcessProtocol::Terminate ()
 {
     // Default implementation does nothing.
 }
+
+#ifndef __linux__
+// 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.
+
+Error
+NativeProcessProtocol::Launch (ProcessLaunchInfo &launch_info,
+        NativeDelegate &native_delegate,
+        NativeProcessProtocolSP &process_sp)
+{
+    llvm_unreachable("Platform has no NativeProcessProtocol support");
+}
+
+Error
+NativeProcessProtocol::Attach (lldb::pid_t pid,
+        NativeDelegate &native_delegate,
+        NativeProcessProtocolSP &process_sp)
+{
+    llvm_unreachable("Platform has no NativeProcessProtocol support");
+}
+
+#endif
diff --git a/lldb/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp b/lldb/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
index 71d1adc236f..2f1e4d55432 100644
--- a/lldb/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
+++ b/lldb/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
@@ -322,21 +322,3 @@ 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();
-}
-
diff --git a/lldb/source/Plugins/Platform/Kalimba/PlatformKalimba.h b/lldb/source/Plugins/Platform/Kalimba/PlatformKalimba.h
index 92de05cf19a..50028c2609a 100644
--- a/lldb/source/Plugins/Platform/Kalimba/PlatformKalimba.h
+++ b/lldb/source/Plugins/Platform/Kalimba/PlatformKalimba.h
@@ -89,17 +89,6 @@ namespace lldb_private {
 
         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;
 
diff --git a/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp b/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
index df44281d285..782ee92d99b 100644
--- a/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
+++ b/lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
@@ -36,10 +36,6 @@
 #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
@@ -842,59 +838,6 @@ 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)
 {
diff --git a/lldb/source/Plugins/Platform/Linux/PlatformLinux.h b/lldb/source/Plugins/Platform/Linux/PlatformLinux.h
index c9ca326fc69..22a5aab3cd7 100644
--- a/lldb/source/Plugins/Platform/Linux/PlatformLinux.h
+++ b/lldb/source/Plugins/Platform/Linux/PlatformLinux.h
@@ -108,17 +108,6 @@ namespace platform_linux {
         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;
 
diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 41d6b7674a0..ae25fb4b4f1 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -824,15 +824,22 @@ NativeProcessLinux::LaunchArgs::~LaunchArgs()
 // -----------------------------------------------------------------------------
 
 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()};
@@ -906,7 +913,7 @@ NativeProcessLinux::LaunchProcess (
     }
 
     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,
@@ -930,7 +937,7 @@ NativeProcessLinux::LaunchProcess (
 }
 
 Error
-NativeProcessLinux::AttachToProcess (
+NativeProcessProtocol::Attach (
     lldb::pid_t pid,
     NativeProcessProtocol::NativeDelegate &native_delegate,
     NativeProcessProtocolSP &native_process_sp)
diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
index fa8770430ef..e3832d6eab2 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
@@ -40,21 +40,17 @@ namespace process_linux {
     /// Changes in the inferior process state are broadcasted.
     class NativeProcessLinux: public NativeProcessProtocol
     {
-    public:
+        friend Error
+        NativeProcessProtocol::Launch (ProcessLaunchInfo &launch_info,
+                NativeDelegate &native_delegate,
+                NativeProcessProtocolSP &process_sp);
 
-        static Error
-        LaunchProcess (
-            Module *exe_module,
-            ProcessLaunchInfo &launch_info,
-            NativeProcessProtocol::NativeDelegate &native_delegate,
-            NativeProcessProtocolSP &native_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.
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index e8955ddbd6e..5f0233b227b 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -218,7 +218,7 @@ GDBRemoteCommunicationServerLLGS::LaunchProcess ()
     {
         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);
@@ -306,7 +306,7 @@ GDBRemoteCommunicationServerLLGS::AttachToProcess (lldb::pid_t pid)
         }
 
         // 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 ());
diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index 6758ecf7e22..50727b3f03b 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -1536,27 +1536,6 @@ Platform::CalculateMD5 (const FileSpec& file_spec,
         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)
 {