Reverted with 241688. Let me know if you have a new patch you’d like to test. I’m also happy to help you add any files you need to the Xcode build if that’s the appropriate way to fix this.
Sean > On Jul 8, 2015, at 9:30 AM, Sean Callanan <scalla...@apple.com> wrote: > > 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 _______________________________________________ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
