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 <lab...@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@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits _______________________________________________ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
