Host::LaunchProcess vs. ProcessPlugin::DoLaunch

Why are there different code paths for launching a process through these two methods? Shouldn’t there just be one codepath for launching a process?

I’ve implemented a very primitive process launch for Windows in ProcessPluginWindows::DoLaunch, but now I’m running into cases where tests are failing because Host::LaunchProcess isn’t implemented yet on Windows. What do I need to understand about the differences between these two codepaths to make sure I implement the two correctly? And is there perhaps a way to refactor some of this code so that all of the process-spawning code lives in the same place, and the launch args are flexible enough to support all of the different use cases?

Here is a discussion that may give you some answers:

http://comments.gmane.org/gmane.comp.debugging.lldb.devel/2805

For the record, I wrote a process plugin, and end-up not having to implements the Process:DoLaunch method at all.
On OS X, all processes are launched by the platform which then call DoAttach. So the Process::DoLaunch code path is never use.

Thanks. So what I take away from this is that they basically do the same thing, and Process::DoLaunch() can simply call Host::LaunchProcess.

So one thing I’m still unclear about, is exactly when I will receive a launch request through the Process plugin versus when I will receive the request directly through the Host without first going through the Process plugin.

You should be implementing the launching in your Host layer one time:

    static Error
    Host::LaunchProcess (ProcessLaunchInfo &launch_info);

Then you should be using this in your ProcessPluginWindows plug-in.

This is mostly cruft from when we first brought up LLDB without having the Platform or the Host layer fully implemented.

The preferred option is to have your PlatformWindows do the launching by using the Host::LaunchProcess():

    static Error
    Host::LaunchProcess (ProcessLaunchInfo &launch_info);

The ProcessLaunchInfo contains a flag to indicate when the process wants to be debugged:

    if (launch_info.GetFlags().Test (eLaunchFlagDebug))
    {
        // Do something to make the process stop at entry point...
    }

So your PlatformWindows should implement LaunchProcess:

Error
PlatformWindows::LaunchProcess (ProcessLaunchInfo &launch_info)
{
    if (IsHost())
    {
        error = Platform::LaunchProcess (launch_info);
    }
}

Platform::LaunchProcess() already uses the Host::LaunchProcess(). Other code will then use the Process::Attach() to attach to that process by process ID.

The best part about implementing the Host::LaunchProcess() is you implement your launching code once, and it gets used by the platforms, by the lldb-platform (to allow remote debugging to a windows machine via lldb-gdbserver). You should then be implementing a ProcessNative subclass and using Todd Fiala's lldb-gdbserver code to get debugging working. This will get you remote debugging for free and you won't need a ProcessWindows subclass at all, you will just use the ProcessGDBRemote.

Greg Clayton

Thanks, that makes sense. The problem is that Host::LaunchProcess doesn’t really return anything useful. When I create a process on Windows, I get a HANDLE back that I can use to later manipulate the process. Note that this isn’t the same as a Pid. I guess Host::LaunchProcess will return the pid in the launch_info structure, which I could then use to get another handle, but that would involve closing the first handle before I get the second handle, which I’m a little wary of, and I would feel better just using the original handle. But there’s no obvious way to pass it back.

I’m not sure what the right abstraction is here that would make sense on all platforms, but I feel like what is needed is some kind of NativeProcess class, and LaunchProcess’s signature could change to this:

static Error
Host::LaunchProcess (ProcessLaunchInfo &launch_info, NativeProcess **process);

If the user passes in NULL, then we just clean up the process handle (or do nothing, depending on platform), and if it’s not NULL, then Host allocates the appropriate type of Platform specific Process object. For example, it allocates a NativeProcessLinux, or a NativeProcessMacOSX, or a NativeProcessWindows, etc.

Do you think something like this could work? Or maybe have other ideas?

Thanks, that makes sense. The problem is that Host::LaunchProcess doesn't really return anything useful. When I create a process on Windows, I get a HANDLE back that I can use to later manipulate the process. Note that this isn't the same as a Pid. I guess Host::LaunchProcess will return the pid in the launch_info structure,

Indeed it does.

which I could then use to get another handle, but that would involve closing the first handle before I get the second handle, which I'm a little wary of, and I would feel better just using the original handle. But there's no obvious way to pass it back.

We can have the LaunchInfo contain the new "host" version of the process handle which would need to be added to the lldb-types.h. We had spoke about this before about possibly adding a:

namespace lldb {
  namespace host {
    typedef pid_t process_t;
  }
}

On windows you could make this a process handle and add lldb_private::ProcessInfo could have a new ivar added:

namespace lldb_private
{
    class ProcessInfo
    {

        FileSpec m_executable;
        std::string m_arg0; // argv[0] if supported. If empty, then use m_executable.
        // Not all process plug-ins support specifying an argv[0]
        // that differs from the resolved platform executable
        // (which is in m_executable)
        Args m_arguments; // All program arguments except argv[0]
        Args m_environment;
        uint32_t m_uid;
        uint32_t m_gid;
        ArchSpec m_arch;
        lldb::pid_t m_pid;
  lldb::host::process_t m_process_handle; // <<<<< new ivar
    };

I'm not sure what the right abstraction is here that would make sense on all platforms, but I feel like what is needed is some kind of NativeProcess class, and LaunchProcess's signature could change to this:

I believe the lldb::host::process_t should be enough. It can be a class if needed for your system, or it can just be a quick typedef like above.

static Error
Host::LaunchProcess (ProcessLaunchInfo &launch_info, NativeProcess **process);

If the user passes in NULL, then we just clean up the process handle (or do nothing, depending on platform), and if it's not NULL, then Host allocates the appropriate type of Platform specific Process object. For example, it allocates a NativeProcessLinux, or a NativeProcessMacOSX, or a NativeProcessWindows, etc.

Do you think something like this could work? Or maybe have other ideas?

I would update the ProcessInfo (ProcessInstanceInfo inherits from it) since this also ties into the platform functions that find and get info for processes:

        virtual uint32_t
        Platform::FindProcesses (const ProcessInstanceInfoMatch &match_info,
                                 ProcessInstanceInfoList &proc_infos);

        virtual bool
        Platform::GetProcessInfo (lldb::pid_t pid, ProcessInstanceInfo &proc_info);

And it would be nice to get these native process handles there as well.

Greg

Makes sense. I think the handle makes more sense in the ProcessInstanceInfo than the ProcessInfo, since by definition a HANDLE implies that you’re referring to an instance of the process that is currently running, which is what ProcessInstanceInfo is for.

Still though, I wonder if it might be better to have LaunchProcess take a ProcessInstanceInfo* argument that could be filled out by the function. I agree that just putting the lldb::host::process_t directly into the ProcessLaunchInfo alongside the pid would be easier, but I don’t mind doing the extra work for a more robust solution, if you agree that there’s value in doing it this way.