[Lldb-commits] [lldb] r241672 - Avoid going through Platform when creating a NativeProcessProtocol instance
Sean Callanan
scallanan at apple.com
Wed Jul 8 09:30:48 PDT 2015
Pavel,
this breaks the OS X build, which does not implement these statics. I am preparing to revert this patch.
Sean
> On Jul 8, 2015, at 2:08 AM, Pavel Labath <labath at google.com> wrote:
>
> 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)
> {
>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
More information about the lldb-commits
mailing list