[Lldb-commits] [lldb] r241672 - Avoid going through Platform when creating a NativeProcessProtocol instance
Sean Callanan
scallanan at apple.com
Wed Jul 8 09:36:12 PDT 2015
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 <scallanan at 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 <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
>
>
> _______________________________________________
> 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