Re: [RFC] [PATCH] Provide the ability to write the frame unwinder in Python

[Doug Evans <dje at google dot com>]

Andy Wingo writes:
 > On Thu 19 Mar 2015 01:36, Alexander Smundak <asmundak@google.com> writes:
 > 
 > >> Regarding the result of an unwinder/sniffer,
 > >> If I approach this from the point of view of what's
 > >> easy to explain, it feels like the result of an Unwinder
 > >> is a Frame.
 > > It is logical, but that's not what GDB core expects from the unwinder.
 > > I hesitate to have Python frame unwinder API differ too much from
 > >  the underlying GDB core API.
 > 
 > I agree with Alexander.  If it were really a frame you'd have to expose
 > a frame constructor to Python/GDB, and then would the resulting frame be
 > interned?  What would happen if you built a frame but then returned
 > None, or threw an exception?  Would there be side effects to the frame
 > cache?  And if not, could you hold on to the frame?  Would it be equal
 > to frame.newer().older() ?  Better to return data instead, which GDB
 > uses to build the actual frame.

Yeah.

Having read everything through last night,
here are some thoughts in no particular order.
Don't rush off and resubmit another patch (unless you want to).
I think there's enough on the table now to get the
high level details decided on. I do see a few
implementation issues in the patches but I
don't want to discuss them prematurely.

1) I like the idea of the input to the unwinder being
a frame (of some kind, ephemeral or whatever).

In python, let's use EphemeralFrame instead of SnifferInfo.

But the only real output of the sniffing action
is a boolean saying "I recognize this frame".
ref: frame-unwind.c:frame_unwind_try_unwinder().
If the unwinder wants to cache something during sniffing
that's its business, and it can cache whatever is appropriate.
IIUC, What the patches do is use the result
of the sniffing action to both say "I do/don't recognize
this frame" and to return an object that is the cache
of random data (which it calls "UnwindInfo").
Thus "UnwindInfo" is as good a name as anything.

2) IIUC, setting aside hitting the outermost frame and such,
it's a rule that some unwinder will recognize the frame.
Therefore "ephemeral" isn't quite right, even "real"
frames are ephemeral since we toss the frame cache when
the inferior resumes. OTOH, we need to make progress
and I'm just throwing this out there. "ephemeral" is
fine with me: we don't use that term for anything else
which is nice.

3) We need to allow sniffers to record anything they
want in gdb.UnwindInfo/<gdb:unwind-info>. In Python
I'm guessing one can just add extra attributes to the
object or subclass. In Guile I guess one could use
an external container (weakly?) indexed by the <gdb:unwind_info>
object. In both cases the documentation should
make recommendations to the reader.
[If it does and I missed it apologies.]

4) I gather the UnwindInfo/<gdb:unwind-info> objects provide
direct support for recording registers so that one doesn't
have to go back out to the extension language to implement
frame_unwind.prev_register.

Plus, they also create the frame id when creating the UnwindInfo
object so that frame_unwind.this_id doesn't have to call back
out to the extension language.

That's fine. Just wanted to write this down to confirm.
Both implementations allow for adding future support for
having frame_unwind.prev_register/this_id/stop_reason
calling back out to the extension language if we need it.

5) The docs don't make any mention of target endianness
in the saved register values. They probably should.

6) I noticed several routines for building frame ids in Python.
Both Python and Guile support keyword based arguments to functions.
Can we just have one frame id builder function and each take
sp and pc (and special if we need to) as keyword arguments?
E.g. (make-unwind-info ephem-frame #:sp sp #:pc pc)
But see (7) below.

7) If we ever need to use frame ids in the extension language
IWBN if they were their own object, in which case we might have
(make-unwind-info ephem-frame #:frame-id frame-id)
but then I'm wondering if there should be an
unwind-info-set-frame-id! function and move sp,pc there.

IOW change make-unwind-info to this so that the various
ways of creating a frame idea are don't complicate make-unwind-info.
[I realize one might want to not let make-unwind-info return
an object without a frame id, but I don't see this as a problem.
uwscm_sniffer could verify a frame id is present.]

(let ((unwind-info (make-unwind-info ephem-frame)))
  (unwind-info-set-frame-id! #:sp sp #:pc pc)
  (unwind-info-add-saved-register! unwind-info "rip" rip)
  ...)

And for python:

I'm not sure where to put the UnwindInfo creator (factory method).
Do we need one?  Can we just create them via normal object construction?

unwind_info = gdb.UnwindInfo(ephem_frame)
unwind_info.set_frame_id(...)
unwind_info.set_previous_frame_register(...)

This could all be done in pure Python (I think), and then
pyuw_sniffer could process the object looking for expected
members(attributes) with expected types (and throw an error
if there's a problem).

At the least let's combine unwind_info_with_id, frame_id_build_special,
and frame_id_build_wild into one function that takes keyword
arguments for each of sp, pc, special.

8) Re: set_previous_frame_register vs unwind-info-add-saved-register!

Dunno. On this particular point I guess I don't have a strong
enough interesting in being consistent.
I'll let you guys decide.

---

That's it for now.