[Lldb-commits] [lldb] r241672 - Avoid going through Platform when creating a NativeProcessProtocol instance

Pavel Labath labath at google.com
Wed Jul 8 02:08:54 PDT 2015


Author: labath
Date: Wed Jul  8 04:08:53 2015
New Revision: 241672

URL: http://llvm.org/viewvc/llvm-project?rev=241672&view=rev
Log:
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.

Reviewers: ovyalov, clayborg

Subscribers: lldb-commits

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

Modified:
    lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
    lldb/trunk/include/lldb/Target/Platform.h
    lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
    lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.h
    lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
    lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.h
    lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
    lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
    lldb/trunk/source/Target/Platform.cpp

Modified: lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h?rev=241672&r1=241671&r2=241672&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h (original)
+++ lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h Wed Jul  8 04:08:53 2015
@@ -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;
 

Modified: lldb/trunk/include/lldb/Target/Platform.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/Platform.h?rev=241672&r1=241671&r2=241672&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Target/Platform.h (original)
+++ lldb/trunk/include/lldb/Target/Platform.h Wed Jul  8 04:08:53 2015
@@ -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 

Modified: lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp?rev=241672&r1=241671&r2=241672&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.cpp Wed Jul  8 04:08:53 2015
@@ -322,21 +322,3 @@ PlatformKalimba::CalculateTrapHandlerSym
 {   
     // 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();
-}
-

Modified: lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.h?rev=241672&r1=241671&r2=241672&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.h (original)
+++ lldb/trunk/source/Plugins/Platform/Kalimba/PlatformKalimba.h Wed Jul  8 04:08:53 2015
@@ -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;
 

Modified: lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp?rev=241672&r1=241671&r2=241672&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp Wed Jul  8 04:08:53 2015
@@ -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::CalculateTrapHandlerSymbo
     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)
 {

Modified: lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.h?rev=241672&r1=241671&r2=241672&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.h (original)
+++ lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.h Wed Jul  8 04:08:53 2015
@@ -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;
 

Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp?rev=241672&r1=241671&r2=241672&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Wed Jul  8 04:08:53 2015
@@ -824,15 +824,22 @@ NativeProcessLinux::LaunchArgs::~LaunchA
 // -----------------------------------------------------------------------------
 
 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)

Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h?rev=241672&r1=241671&r2=241672&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h Wed Jul  8 04:08:53 2015
@@ -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.

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp?rev=241672&r1=241671&r2=241672&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp Wed Jul  8 04:08:53 2015
@@ -218,7 +218,7 @@ GDBRemoteCommunicationServerLLGS::Launch
     {
         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::Attach
         }
 
         // 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 ());

Modified: lldb/trunk/source/Target/Platform.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Platform.cpp?rev=241672&r1=241671&r2=241672&view=diff
==============================================================================
--- lldb/trunk/source/Target/Platform.cpp (original)
+++ lldb/trunk/source/Target/Platform.cpp Wed Jul  8 04:08:53 2015
@@ -1536,27 +1536,6 @@ Platform::CalculateMD5 (const FileSpec&
         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)
 {





More information about the lldb-commits mailing list