[lldb-dev] Update on Linux work.

[Stephen Wilson]

Greg Clayton <gclayton at apple.com> writes:
> I got a chance to checkout the start of your Process plug-in.
>
> A few things:
>
> LinuxThread::BreakNotify()
>
> 	You might want to call into your architecture specific register context and let it
> 	know it hit a breakpoint so you can have it do what it needs to. That way 
> 	for i386 and x86_64 you can backup the PC, but for other architectures
> 	you don't have to.

Ah, good idea.  Will do.

> Are you reaping your process after you launch/attach to it with waitpid anywhere?
> There is a lldb_private::Host abstraction call to do this if you need that service. 
>
> Checkout the Host.h:
> 	Host::StartMonitoringChildProcess(...) 
>
> and search for its use in ProcessMacOSX for an example.

Yes, I am calling waitpid from the so-called
ProcessMonitor::SignalThread.  StartMonitoringChildProcess looks like it
will help with the job nicely.  Thanks for pointing that one out!

> ProcessLinux.cpp:74
>
> 	Is there a reason for the g_process global? You can have more than one process
> 	at a time in LLDB, so it seems like a dangerous variable to keep around. If you
> 	need a global way to locate your process by pid, you can use a static function
> 	in lldb_private::Debugger:
>
> 	static lldb::TargetSP FindTargetWithProcessID (lldb::pid_t pid); 
>
> 	If you need to lookup a target by any other means globally, let me know and
> 	we can add the needed functionality to a static call in Debugger.

Ah, no.  There is no reason for g_process anymore -- just leftover cruft from
previous work.  Will remove.

Regarding multiple processes: Is it guaranteed that a new Process
instance is created for every process launched, or is it possible that
the same instance be called upon to manage a different inferior (say via
a call sequence of the form Launch(process-1), Destroy(process-1),
Launch(process-2)) ?


> ProcessLinux.cpp:79
>
> 	Your ProcessLinux constructor shouldn't need to call UpdateLoadedSections()
> 	since when a process is created it isn't alive yet, nor does it have
> 	any connection to a valid live process. 

This is just a temporary hack to work around the lack of a DynamicLoader
plugin.  But even if it is temporary I should still be calling it from
the right spot.  Will fix.

>       DoLaunch will need to be called,
> 	or DoAttach before you would need to call this function. Also there are
> 	functions that get called prior to, and after DoLaunch and DoAttach:
>
> 	When launching the follwing functions will be called:
>
> 	virtual Error WillLaunch (...);
> 	virtual Error DoLaunch (...);
> 	virtual void DidLaunch (...);
>
> 	Likewise with DoAttach:
>
> 	virtual Error WillAttachToProcessWithID (lldb::pid_t pid);
> 	virtual Error DoAttachToProcessWithID (lldb::pid_t pid);
> 	virtual void DidAttach ();
> 	
> 	virtual Error WillAttachToProcessWithName (const char *process_name, bool wait_for_launch)
> 	virtual Error DoAttachToProcessWithName (const char *process_name, bool wait_for_launch) 
> 	virtual void DidAttach ();
>
> 	If any of the Will* functions return an error, the process launch/attach
> 	will stop. If they return succes, then the Do* functions will be called.
> 	If the Do* functions return success, then the Did* functions will be called.
>
> 	So a good place to do your call to "UpdateLoadedSections()" is in the
> 	DidLaunch() or DidAttach() functions.

OK.  Will move the UpdateLoadedSections call.  And thanks for the
clarification about the role of these methods.

> 	There are similar Will*/Do*/Did* functions for detaching, and a few
> 	other things. Many of them have default implementations that do nothing,
> 	but are designed to be overridden so you can do just this kind of stuff.
>
> 	You will also want to make a DYLD plug-in at some point to take care of
> 	this if you plan to re-use the code that can locate where shared libraries
> 	are loaded in another linux process plug-in (like for remote debugging
> 	using "debugserver"). This way you can just plug your Linux dynamic loader
> 	plug-in into ProcessGDBRemote and all should work (after "debugserver" has
> 	been modified to run on linux that is).

There is another developer who is putting effort into a DYLD plugin, so
plans are in the works for adding this support.


> RegisterContextLinux_x86_64:
> 	You will want to fill in a static RegisterInfo array and return valid
> 	values for your registers (See how the other RegisterContext subclasses
> 	in ProcessMacOSX do this). 

This is on my todo list.  Does not seem critical for the simple debug
sessions I can run with the plugin as is, but I will add this very soon.


> Other than that, overall it looks pretty good. Feel free to commit your
> "source/Plugins/Process/Linux" whenever you can!

Thanks so much for looking this over and letting me contribute to the
project!   I should be able to get the plugin ready for commit within a
few days.


Thanks again!
Steve