[Lldb-commits] [lldb] r128547 - in /lldb/trunk/source/Plugins/Process/Linux: LinuxSignals.cpp LinuxSignals.h LinuxStopInfo.cpp LinuxStopInfo.h LinuxThread.cpp LinuxThread.h ProcessLinux.cpp ProcessLinux.h ProcessMessage.cpp ProcessMessage.h ProcessMonitor.cpp ProcessMonitor.h

[Greg Clayton]

On Mar 30, 2011, at 10:32 AM, Stephen Wilson wrote:

> On Wed, Mar 30, 2011 at 09:51:56AM -0700, Greg Clayton wrote:
>> Well done! I am glad to see this works well for Linux.
> 
> So far, LLDB has proven to be very easy to "port" onto linux.  
> 
>> One question: are all of the signals you added actually signals? It
>> looked like it from what I can tell. One of the hacks we did in GDB
>> for mach exceptions was to make up new fake signal numbers because the
>> debugger wanted to deal with signals.
> 
> They are all real signals.  I have not needed to add anything extra yet,
> but see below.
> 
Great.

> 
>> One thing I just want to make sure happens in LLDB is that we never
>> have to make up anything. For mach exceptions, we have a StopInfo
>> subclass that is not part of the built in StopInfo stuff:
>> StopInfoMachException. This class takes care of doing all of the
>> special mach things that no other platform really cares about. 
>> 
>> So if there are things that aren't signals in your recent checkin, you
>> might consider making a StopInfoLinux to take care of any special
>> things that aren't signal related.
> 
> We do have a couple of linux specific stop info's now.  For example, we
> have a LinuxLimboStopInfo that is generated when a process is just about
> to exit (in such a state, you cannot stop the exit from happening, but
> you can inspect registers and probe memory -- helpful when you
> mistakenly press 'n' one too many times and terminate the process after
> a SIGSEGV!).  We also have a LinuxCrashStopInfo that contains a
> description of the event in question.
> 
> 
> Another thing we do in the Linux plugin is propagate information about
> state changes using so-called ProcessMessage's.  These are *very* similar
> to StopInfo objects, but can be tailored easily to the needs of the
> plugin.  The basic flow is:
> 
>    - ProcessMonitor gets an event as a result of wait4().
> 
>    - The event is packaged into a ProcessMessage and sent to a
>      ProcessLinux instance.  The events trigger state changes in the
>      process as appropriate, and are queued up for delivery to
>      individual threads.
> 
>    - When the process state change event has propagated the
>      corresponding ProcessMessage is delivered to the appropriate
>      LinuxThread via LinuxThread::Notify.
> 
>    - The thread converts the message into a StopInfo object to
>      advertise to the rest of LLDB.
> 
> The above seems to work well for the single threaded case -- we will
> need to tag ProcessMessages with a "stop ID" when we support
> multithreading.  Basic mutithreading support is the next big item on my
> TODO list.
> 
> 
> It is possible that, over time, the ProcessMessage class could be
> replaced by StopInfo's completely.  However, having a little internal
> class like ProcessMessage seems to be a flexible way to keep linux
> specific semantics internal to the plugin.
> 
> 
> Of course, any feedback on how I am doing this is very much appreciated!

Your design makes sense and I don't think I would change much. You can integrate with the StopInfo class if you want and we will need to make sure that the StopInfo base class has all the virtual functions needed. The current virtual functions for StopInfo are:

    virtual lldb::StopReason
    GetStopReason () const = 0;
    
    // Perform any action that is associated with this stop.  This is done as the
    // Event is removed from the event queue.
    virtual void
    PerformAction (Event *event_ptr)
    {
    }

    // Stop the thread by default. Subclasses can override this to allow
    // the thread to continue if desired.
    virtual bool
    ShouldStop (Event *event_ptr)
    {
        return true;
    }

    // If should stop returns false, check if we should notify of this event
    virtual bool
    ShouldNotify (Event *event_ptr)
    {
        return false;
    }

    virtual void
    WillResume (lldb::StateType resume_state)
    {
        // By default, don't do anything
    }
    
    virtual const char *
    GetDescription () = 0;

Are these callbacks enough to allow you to integrate ProcessMessage into? Is there anything missing? No rush on the integration, but it  might be something to think about.

Greg