Le 10 déc. 2013 à 00:06, Jean-Daniel Dupas <[email protected]> a écrit :
> > Le 9 déc. 2013 à 20:33, Greg Clayton <[email protected]> a écrit : > >> >> On Dec 9, 2013, at 11:11 AM, Jean-Daniel Dupas <[email protected]> >> wrote: >> >>> >>> Le 9 déc. 2013 à 19:55, Greg Clayton <[email protected]> a écrit : >>> >>>> >>>> On Dec 9, 2013, at 10:41 AM, Jean-Daniel Dupas <[email protected]> >>>> 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. > > > Done in r196837 > > >> 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... > > Using the default Host reaper should not be an issue as it defaults to > calling Process::SetProcessExitStatus() which is what we want when debugging > a process isn't it ? > And now that I think about it, this is probably not what we want when doing ptrace based debugging, as the waitpid in the reaper thread will conflict with the ptrace debugging thread. > >>>> 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 > > -- Jean-Daniel > > > > > > _______________________________________________ > lldb-dev mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev -- Jean-Daniel
_______________________________________________ lldb-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
