Re: Frame sniffers in Python/Guile/*

[Andy Wingo <wingo at igalia dot com>]

Greets :)

On Tue 10 Mar 2015 17:35, Pedro Alves <palves@redhat.com> writes:

> On 03/09/2015 07:18 PM, Andy Wingo wrote:
>
>> On Mon 09 Mar 2015 16:38, Pedro Alves <palves@redhat.com> writes:
>> 
>>> The sniffer callback is only supposed to identify whether the
>>> unwinder can unwind, not do any unwinding at all.  :-/
>> 
>> Unfortunately you have to do some unwinding in order to compute a frame
>> ID, and the "unwind->this_id" callback runs in approximately the same
>> environment as the sniffer.
>
> Can you give an example, so we can understand the requirements better?

Yes.  For example we are unwinding the innermost frame, frame 0, from
the sentinel.  The Guile sniffer is called to see if any Guile unwinder
can handle the frame.  I have a Guile unwinder that checks if the PC is
within any JIT-compiled page in V8.  It has to traverse the V8 heap,
basically, to see if it can handle the page.

Once the Guile V8 sniffer has enough information to know whether it can
unwind the frame or not, it _also_ has enough information to compute the
frame ID for frame 0 -- namely, since it knows what V8 object contains
the code, it knows how to interpret the registers to find the frame
pointer and it knows about the stack layout, so it can compute the start
PC and the start SP easily, which is the frame ID.

For the same reason it also knows how to unwind the PC, SP, and FP.

I guess by "you have to do some unwinding in the sniffer" I just meant
that users (ignorant ones like me :) of the Guile/Python interface to
unwinders will have to do some computation over the state of the
inferior in the sniffer callback to know whether to handle the frame or
not -- and that relatively speaking the other parts of the unwinder
interface are trivial.

> The main problem would be unwind-past-this_frame: that is, something doing
> get_prev_frame(this_frame), get_frame_id (this_frame) or something like that
> while inside the sniffer.

This is impossible in the proposed Guile or Python patches because the
wrappers for this_frame are not frames.  You can't actually even get to
any linked frame from the "ephemeral frame object" (in Guile) or the
"sniffer info object" (in Python).

What can happen is that someone could inadvertently cause recursion to
this_frame's unwinder -- for example via get_prev_frame on
get_selected_frame(), or even just by calling get_selected_frame() while
unwinding frame 0.

In this case -- somehow recursing to unwind this_frame -- my patch makes
get_prev_frame(next_frame) return NULL.  But you'd have to have a
next_frame to begin with, so this is OK.  It doesn't add strangeness.
True, this can make get_selected_frame() return the sentinel frame if
recursion happens while unwinding frame 0, but that case appears to be
already reachable otherwise -- see unwind_to_current_frame.

> Unwinding registers from the next frame to build a cache to compute
> the frame id or figure out where registers were saved is ok.

Yep, no problem here.

>>> See here for example:
>>>
>>>   https://www.sourceware.org/ml/gdb-patches/2013-11/msg00602.html
>> 
>> Is it important to use frame_unwind_got_memory et al for results?
>
> I don't think I understand this question.

I think I misunderstood the issue, please ignore the question.

(I thought it was that because you used frame_unwind_got_memory to
produce GDB values for saved registers, and those values linked to a
frame by ID, that somehow caused recursion when computing the frame_id
for this_frame.  I was wondering if it were somehow necessary to build
register values by frame_unwind_got_memory and not the value-building
API in Python or Guile.  It seems that is not the case.)

>> In my unwinders I am just returning values using e.g.
>
>> 
>>   (value-dereference
>>    (value-cast (ephemeral-frame-read-register ef "rbp") void**-type))
>> 
>> or something similar.  If this is important, we will need to provide an
>> interface that looks much more like the unwind interface instead of a
>> simple Maybe<UnwindInfo> :(
>
> I'm afraid I don't know what "Maybe<UnwindInfo>" is.

My apologies!

The Python patch calls the return value of the sniffer the "unwind
info".  If the sniffer returns None, then the sniffer is considered to
fail, and otherwise the return value is the "unwind info".  A Maybe<T>
is just an idiom for a data type that is either an instance of T or
None.  Although I'm a schemer at heart I have enjoyed thinking about
program design in types recently, but I see I have added to the
confusion.

What I meant was: in both the Python and the Guile patches, the unwinder
interface is essentially a function that either punts, or it does all of
the unwinding at once.  It doesn't reflect the nuances that the sniffer,
the this_id callback, and the prev_register callbacks are all called at
different times.  I like the simplification but I do see why the
low-level GDB interface is different (avoiding unnecessary prev_register
computation, chiefly).

The question was, is there a problem with the all-at-a-time approach
that the Guile and Python patches use?  I am thinking that no, that it's
OK to do it all at once.

>> [Should the Guile bindings avoid assuming that] it's sensible to get
>> the current architecture, or the selected or current frame?
>
> It probably makes sense to refer to those.  As long as e.g.,
> can e.g., make get_current_frame() really return the current frame.
> But from what I understood, we end up instead falling back to NULL?
> If we're going to give the wrong answer, it makes me nervous to have
> to support doing it going forward.  :-/

If I understand the concern, I think I answered it above when referring
to the new way that get_prev_frame could return NULL.

>> I was pleased that $pc, $sp, and $fp
>> "worked" otherwise, so I hoped to avoid encoding architecture-specific
>> things.  Alas.  (Incidentally, my patch still has the problem that
>> passing in a pseudo register will barf.)
>
> Speaking of which, it occurred to me that we'll likely have problems
> on architectures that hide all raw registers (by making them unnamed)
> behind pseudo registers, like MIPS.

What's the solution here then?  Perhaps Alexander was right that
register numbers have a place :)

Thank you again for the review and for thinking this through, Pedro.  I
know that it takes time and I really do appreciate it.

Cheers,

Andy