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 ? >>> 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
