[lldb-dev] Platform::DebugProcess vs Target::CreateProcess + Process->Launch

[Greg Clayton]

On Dec 9, 2013, at 11:11 AM, Jean-Daniel Dupas <devlists at shadowlab.org> wrote:

> 
> Le 9 déc. 2013 à 19:55, Greg Clayton <gclayton at apple.com> a écrit :
> 
>> 
>> On Dec 9, 2013, at 10:41 AM, Jean-Daniel Dupas <devlists at shadowlab.org> wrote:
>> 
>>> Hello,
>>> 
>>> While working on a Mach native process, I saw there is actually two code paths to launch a process in the debugger.
>>> 
>>> One that query the platform using CanDebugProcess() and then call Platform::DebugProcess, which end up doing:
>>> - Launch Process.
>>> - Ask target to Create a Process instance.
>>> - Use the process instance to Attach to the freshly launched process.
>>> 
>>> and a second code path that:
>>> - Ask the target to create a process instance.
>>> - Use the process instance to Launch the process to debug.
>>> 
>>> Why do we need two code paths to do basically the same thing, especially as all processes, included the GDBRemote one supports Launch, but no platform but the Darwin one supports the first code path.
>>> 
>>> Wouldn't it be simpler to just let the process take care of the Launch part in all cases ? 
>> 
>> It currently relies on the platform which is good. Why? Because if you are debugging something on a remote system, it will use the same code path as the current host system (the platform does the launching for debugging). Underneath it all, everyone should be using the Host::LaunchProcess(), so the posix_spawn() code should be the same no matter which way things get launched. 
> 
> Ideally Process::DoLaunch should be using Host::LaunchProcess, but the former take a "const ProcessLaunchInfo" while Host requires a non const ProcessLaunchInfo, and as ProcessLaunchInfo is not copyable (due to the m_pty PseudoTerminal member which is not copyable), we can't simply create a non const copy to use the Host method.
> 
> Is there a reason the DoLaunch method takes a const instance ? If not, removing this constraint will simplify the way we can handle it. At least for process plugins that need it.

Yes, please do remove it. When the host launching a process it will fill the new process ID. Also be careful to be sure that you call ProcessLaunchInfo::SetMonitorProcessCallback(...) prior to launching as when the host launches the process, if there isn't a callback in the ProcessLaunchInfo, it will use its own version that will reap the process in the host layer...

> 
>> Another reason to leave things the way they are is if you have a simulator platform, like "ios-simulator", and it uses a native application that runs on the local host using native code, you might want your "simulator" platform to include extra mandatory environment variables when launching and/or do some special handshake with the simulator prior to or after launching, even though it is just a native application. If we don't let the platforms do the launching, then we have to have all sorts of platform functions that would be called prior to and after launching...
>> 
>> This means that the Process::Launch() is probably not needed if we can get all platforms to handle launching the process. I don't believe that any platforms other than the native darwin platform and the iOS platform handle launching right now, so it was done partially to move the darwin side over to the latest and greatest and yet leaving the other native debugging alone so they continue to work.
>> 
> 
> Thanks for the explanation. This is a perfectly good reason to let the platform take care of the process spawning. 
> 
> It works well on darwin as "debug" has always been "launch suspended + attach", but I'm not sure this scheme apply to POSIX ptrace based debugging, so we may have to keep both code path.

We might need to, though I don't think it would be hard to modify the other unix's over to use fork() + posix_spawn() with the exec only flag. Then in the child, it first calls ptrace() to attach to itself and then execs itself into the new process which should cause it to stop at the entry point..

Greg