Re: [PATCH v4] Add Guile frame unwinder interface

[Doug Evans <dje at google dot com>]

Andy Wingo writes:
 > Voici a new version of the Guile frame unwinder patch.  Changes:
 > 
 >   * Separate ephemeral frame object info ephemeral frame + unwind info
 >     objects; see:
 > 
 >       http://article.gmane.org/gmane.comp.gdb.patches/105759
 > 
 >   * Update management of unwinders as in newest frame filter patch
 >     (http://article.gmane.org/gmane.comp.gdb.patches/105616)
 > 
 >      - Specify #:scope when registering, not when creating unwinders
 > 
 >      - s/add-frame-unwinder!/register-frame-unwinder!/
 > 
 >      - s/remove-frame-unwinder!/unregister-frame-unwinder!/
 > 
 >      - Add s/set-frame-unwinder-enabled!/
 > 
 >   * Adapt for new TRY/CATCH/END_CATCH
 > 
 > Regards,
 > 
 > Andy
 > 
 > >From 55dae8b56541ef51a03676ce48ea6c2cdeb25f29 Mon Sep 17 00:00:00 2001
 > From: Andy Wingo <wingo@igalia.com>
 > Date: Thu, 5 Mar 2015 16:40:20 +0100
 > Subject: [PATCH] Add Guile frame unwinder interface
 > 
 > gdb/doc/ChangeLog:
 > 
 > 	* guile.texi (Guile Frame Unwinder API): New section.
 > 
 > gdb/ChangeLog:
 > 
 > 	* guile/scm-symbol.c (gdbscm_lookup_symbol): Don't error if there
 > 	is no selected frame and no block is selected; instead, fall back
 > 	to the current frame.
 > 	* guile/scm-frame-unwinder.c: New file.
 > 	* guile/lib/gdb/frame-unwinders.scm: New file.
 > 	* guile/guile.c (initialize_gdb_module): Call
 > 	gdbscm_initialize_frame_unwinders.
 > 	* guile/guile-internal.h (gdbscm_initialize_frame_unwinders): New
 > 	declaration.
 > 	* frame.c (get_prev_frame): Detect recursive unwinds, returning
 > 	NULL in that case.
 > 	* frame-unwind.h (frame_unwind_got_bytes): Make buf arg const.
 > 	(frame_unwind_is_unwinding): New declaration.
 > 	* frame-unwind.c (is_unwinding): New file-local variable.
 > 	(set_is_unwinding, unset_is_unwinding): New file-local helpers.
 > 	(frame_unwind_is_unwinding): New exported predicate.
 > 	(frame_unwind_try_handler): Arrange for
 > 	frame_unwind_is_unwinding to return true when unwinding the
 > 	innermost frame.
 > 	(frame_unwind_got_bytes): Make buf arg const.
 > 	* data-directory/Makefile.in (GUILE_SOURCE_FILES): Add
 > 	frame-unwinders.scm.
 > 	(GUILE_COMPILED_FILES): Add frame-unwinders.go.
 > 	* Makefile.in (SUBDIR_GUILE_OBS): Add scm-frame-unwinder.o.
 > 	(SUBDIR_GUILE_SRCS): Add scm-frame-unwinder.c
 > 	(scm-frame-unwinder.o): New target.
 > 
 > gdb/testsuite/ChangeLog:
 > 
 > 	* gdb.guile/scm-frame-unwinder.exp:
 > 	* gdb.guile/scm-frame-unwinder.c:
 > 	* gdb.guile/scm-frame-unwinder-gdb.scm.in:
 > 	* gdb.guile/scm-frame-unwinder.scm: Add unwinder tests.
 > ...
 > 
 > diff --git a/gdb/Makefile.in b/gdb/Makefile.in
 > index fed8035..08e08db 100644
 > --- a/gdb/Makefile.in
 > +++ b/gdb/Makefile.in
 > @@ -315,6 +315,7 @@ SUBDIR_GUILE_OBS = \
 >  	scm-exception.o \
 >  	scm-frame.o \
 >  	scm-frame-filter.o \
 > +	scm-frame-unwinder.o \
 >  	scm-gsmob.o \
 >  	scm-iterator.o \
 >  	scm-lazy-string.o \
 > @@ -342,6 +343,7 @@ SUBDIR_GUILE_SRCS = \
 >  	guile/scm-exception.c \
 >  	guile/scm-frame.c \
 >  	guile/scm-frame-filter.c \
 > +	guile/scm-frame-unwinder.c \
 >  	guile/scm-gsmob.c \
 >  	guile/scm-iterator.c \
 >  	guile/scm-lazy-string.c \
 > @@ -2416,6 +2418,10 @@ scm-frame-filter.o: $(srcdir)/guile/scm-frame-filter.c
 >  	$(COMPILE) $(srcdir)/guile/scm-frame-filter.c
 >  	$(POSTCOMPILE)
 >  
 > +scm-frame-unwinder.o: $(srcdir)/guile/scm-frame-unwinder.c
 > +	$(COMPILE) $(srcdir)/guile/scm-frame-unwinder.c
 > +	$(POSTCOMPILE)
 > +
 >  scm-gsmob.o: $(srcdir)/guile/scm-gsmob.c
 >  	$(COMPILE) $(srcdir)/guile/scm-gsmob.c
 >  	$(POSTCOMPILE)
 > diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
 > index b4916b0..14d3653 100644
 > --- a/gdb/data-directory/Makefile.in
 > +++ b/gdb/data-directory/Makefile.in
 > @@ -88,6 +88,7 @@ GUILE_SOURCE_FILES = \
 >  	gdb/boot.scm \
 >  	gdb/experimental.scm \
 >  	gdb/frame-filters.scm \
 > +	gdb/frame-unwinders.scm \
 >  	gdb/init.scm \
 >  	gdb/iterator.scm \
 >  	gdb/printing.scm \
 > @@ -100,6 +101,7 @@ GUILE_NO_UNBOUND_WARNING_COMPILED_FILES = \
 >  GUILE_COMPILED_FILES = \
 >  	gdb/experimental.go \
 >  	gdb/frame-filters.go \
 > +	gdb/frame-unwinders.go \
 >  	gdb/iterator.go \
 >  	gdb/printing.go \
 >  	gdb/support.go \
 > diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
 > index 162ace7..3be1926 100644
 > --- a/gdb/doc/ChangeLog
 > +++ b/gdb/doc/ChangeLog
 > @@ -1,3 +1,7 @@
 > +2015-03-06  Andy Wingo  <wingo@igalia.com>
 > +
 > +	* guile.texi (Guile Frame Unwinder API): New section.
 > +
 >  2015-02-15  Andy Wingo  <wingo@igalia.com>
 >  
 >  	* guile.texi (Guile Frame Filter API)
 > diff --git a/gdb/doc/guile.texi b/gdb/doc/guile.texi
 > index 3045d90..fdcbca3 100644
 > --- a/gdb/doc/guile.texi
 > +++ b/gdb/doc/guile.texi
 > @@ -143,6 +143,7 @@ from the Guile interactive prompt.
 >  * Writing a Guile Pretty-Printer:: Writing a pretty-printer
 >  * Guile Frame Filter API::   Filtering frames.
 >  * Writing a Frame Filter in Guile:: Writing a frame filter.
 > +* Guile Frame Unwinder API:: Programmatically unwinding stack frames
 >  * Commands In Guile::        Implementing new commands in Guile
 >  * Parameters In Guile::      Adding new @value{GDBN} parameters
 >  * Progspaces In Guile::      Program spaces
 > @@ -2137,6 +2138,210 @@ also possible to do the job of an decorator with a filter.  Still,
 >  avoiding the stream interfaces can often be a good reason to use the
 >  simpler decorator layer.
 >  
 > +@node Guile Frame Unwinder API
 > +@subsubsection Unwinding Frames in Guile
 > +@cindex frame unwinder api, guile
 > +
 > +In @value{GDBN} terminology, ``unwinding'' is the process of finding
 > +an older (outer) frame on the stack.  Unwinders form the core of
 > +backtrace computation in @value{GDBN}.  @value{GDBN} comes with
 > +unwinders for each target architecture that it supports, and these
 > +usually suffice to unwind the stack.  However, some target programs
 > +can have non-standard frame layouts that cannot be unwound by the
 > +standard unwinders.  This is often the case when working with
 > +just-in-time compilation environments, for example in JavaScript
 > +implementations.  In such cases, users can define custom code in Guile
 > +to programmatically unwind the problematic stack frames.
 > +
 > +Before getting into the API, we should discuss how unwinders work in
 > +@value{GDBN}.
 > +
 > +As an example, consider a stack in which we have already computed
 > +frame 0 and we want to compute frame 1.  We say that frame 0 is the
 > +``inner'' frame, and frame 1 will be the ``outer'' frame.
 > +
 > +Unwinding starts with a model of the state of all registers in an
 > +inner, already unwound frame.  In our case, we start with frame 0.
 > +@value{GDBN} then constructs a ephemeral frame object for the outer
 > +frame that is being built (frame 1) and links it to the inner frame
 > +(frame 0).  @value{GDBN} then goes through its list of registered
 > +unwinders, searching for one that knows how to unwind the frame.  When
 > +it finds one, @value{GDBN} will ask the unwinder to compute a frame
 > +identifier for the outer frame.  Once the unwinder has done so, the
 > +frame is marked as ``valid'' and can be accessed using the normal
 > +frame API.
 > +
 > +A frame identifier (frame ID) consists of code and data pointers
 > +associated with a frame which will remain valid as long as the frame
 > +is still alive.  Usually a frame ID is a pair of the code and stack
 > +pointers as they were when control entered the function associated
 > +with the frame, though as described below there are other ways to
 > +build a frame ID@.  However as you can see, computing the frame ID
 > +requires some input from the unwinder to determine the start code
 > +address (PC) and the frame pointer (FP), especially on platforms that
 > +don't dedicate a register to the FP.
 > +
 > +(Given this description, you might wonder how the frame ID for the
 > +innermost frame (frame 0) is unwound, given that unwinding requires an
 > +inner frame.  The answer is that internally, @value{GDBN} always has a
 > +``sentinel'' frame that is inner to the innermost frame, and which has
 > +a pre-computed unwinder that just returns the registers as they are,
 > +without unwinding.)
 > +
 > +The Guile frame unwinder API loosely follows this protocol as
 > +described above.  Guile will build a special ``ephemeral frame
 > +object'' corresponding the frame being unwound (in our example, frame
 > +1).  It allows the user to read registers from that ephemeral frame,
 > +which in reality are unwound from the already-existing frame 0.  If
 > +the unwinder decides that it can handle the frame in question, it then
 > +creates and returns an ``unwind info'' object for that frame.  The
 > +unwind info object contains a frame ID for the ephemeral frame.  It
 > +also records the values of any registers saved in the frame, for use
 > +when unwinding its outer frame (frame 2).
 > +
 > +Frame unwinder objects are managed in Guile much in the same way as
 > +frame filters.  Indeed, users will often want to implement both frame
 > +unwinders and frame filters: unwinders will compute the correct
 > +backtrace and register state, and filters can fill in function names,
 > +line numbers, and the like.  @xref{Guile Frame Filter API}, for more
 > +on frame filters.
 > +
 > +As with frame filters, there can be multiple frame unwinders
 > +registered with @value{GDBN}, and each one may be individually enabled
 > +or disabled at will.  The filters will be tried in priority order,
 > +from highest to lowest priority, and the first one that sets the frame
 > +ID will take responsibility for the frame.
 > +
 > +To use frame unwinders, first load the @code{(gdb frame-unwinders)} module
 > +to have access to the procedures that manipulate frame unwinders:
 > +
 > +@example
 > +(use-modules (gdb frame-unwinders))
 > +@end example
 > +
 > +@deffn {Scheme Procedure} make-frame-unwinder name procedure @
 > +       @r{[}#:priority priority@r{]} @r{[}#:enabled? boolean@r{]}
 > +Make a new frame unwinder.
 > +
 > +The unwinder will be identified by the string @var{name}.
 > +@var{procedure} should be a function of one argument, taking an
 > +ephemeral frame object.  If the unwinder procedure decides to handle
 > +the frame, it should return an unwind info object.  Otherwise the
 > +unwinder returns @code{#f}, @value{GDBN} will continue to search its
 > +list for an unwinder.
 > +
 > +The unwinder will be initially enabled, unless the keyword argument
 > +@code{#:enabled? #f} is given.  Even if the unwinder is marked as
 > +enabled, it will need to be registered with @value{GDBN} via
 > +@code{register-frame-unwinder!} in order to take effect.  When
 > +registered, the unwinder will be inserted into the list of registered
 > +unwinders with the given @var{priority}, which should be a number, and
 > +which defaults to 20 if not given.  Higher priority unwinders will be
 > +tried before lower-priority unwinders.
 > +@end deffn

Hi.

It would be good to forcefully limit the values of priority such that
a possible future implementation that referenced priorities
in C(/C++) could just use ints (or unsigned ints).

 > +
 > +@deffn {Scheme Procedure} all-frame-unwinders
 > +Return a list of all frame unwinders.
 > +@end deffn
 > +
 > +@deffn {Scheme Procedure} register-frame-unwinder! unwinder @
 > +       @r{[}#:scope scope@r{]}
 > +Register the frame unwinder @var{unwinder} with @value{GDBN}.
 > +
 > +By default, the scope of the unwinder is global, meaning that it is
 > +associated with all objfiles and progspaces.  Pass an objfile or a
 > +progspace as the @code{#:scope} keyword argument to instead scope the
 > +unwinder into a specific objfile or progspace, respectively.
 > +
 > +The unwinder's name will be checked for uniqueness within its registered
 > +scope.
 > +@end deffn
 > +
 > +@deffn {Scheme Procedure} set-frame-unwinder-enabled! unwinder enabled?
 > +@deffnx {Scheme Procedure} enable-frame-unwinder! unwinder
 > +@deffnx {Scheme Procedure} disable-frame-unwinder! unwinder
 > +Mark a frame unwinder as enabled, if @var{enabled?} is true, or
 > +as disabled otherwise.

If we have {en,dis}able-frame-unwinder! then we should have
them for all things that can be enabled/disabled.
Such redundancy in the lowest level API bothers me,
and if a particular user wants to provide their own wrappers
it's trivial.

 > +
 > +@var{unwinder} can either be a frame unwinder object, or it can be a
 > +string naming an unwinder in the current scope.  If no such unwinder
 > +is found, an error is signalled.

I think these should operate on just an unwinder object.

There is a convention for naming such objects (pretty-printers,
type-printers, frame-filters, xmethods, and so on) which involves
two pieces: scope(/locus) name and object name.
But that feels like something for a higher level API than
these primitives.

For reference sake, there's a further convention for pretty-printers
that split "object name" into two pieces, but it's not relevant
to this API, the name is still one string.

 > +
 > +@code{enable-frame-unwinder!} and @code{disable-frame-unwinder} are simple
 > +wrappers around @code{set-frame-unwinder-enabled!} which pass @code{#t}
 > +or @code{#f} as the @var{enabled?} argument, respectively.
 > +@end deffn
 > +
 > +@deffn {Scheme Procedure} frame-unwinder-name unwinder
 > +@deffnx {Scheme Procedure} frame-unwinder-enabled? unwinder
 > +@deffnx {Scheme Procedure} frame-unwinder-registered? unwinder
 > +@deffnx {Scheme Procedure} frame-unwinder-priority unwinder
 > +@deffnx {Scheme Procedure} frame-unwinder-procedure unwinder
 > +@deffnx {Scheme Procedure} frame-unwinder-scope unwinder
 > +Accessors for a frame unwinder object's fields.  The
 > +@code{registered?}  field indicates whether a unwinder has been added
 > +to @value{GDBN} or not.  @code{scope} is the objfile or progspace in
 > +which the unwinder was registered, or @code{#f} otherwise.
 > +@end deffn
 > +
 > +Frame unwinders operate on ``ephemeral frames''.  Ephemeral frames are
 > +valid only while they are being unwound; any access to an ephemeral
 > +frame outside the extent of their unwind operation will signal an
 > +error.  Currently ephemeral frames can only be used in two limited
 > +ways: to read registers from the frame, and to construct an unwind
 > +info object for a successful unwinder return.
 > +
 > +@deffn {Scheme Procedure} ephemeral-frame-read-register frame register
 > +Return the value of a register in the ephemeral frame @var{frame}.
 > +@var{register} should be given as a string.
 > +@end deffn
 > +
 > +If an unwinder successfully unwinds a frame, it should return an
 > +unwind info object created via the @code{make-unwind-info} procedure.
 > +An unwind info object specifies the frame ID for its associated
 > +ephemeral frame, and also records values of registers that are saved
 > +within the frame.
 > +
 > +@deffn {Scheme Procedure} make-unwind-info frame fp [pc [special]]
 > +Make an unwind info object for the ephemeral frame @var{frame}.

What if we defer adding "special" to a later patch?
It seems like it would be straightforward to add at a later date.
As much as you don't like "sniffer", "special" bugs me 10x more. :-)

 > +
 > +@var{fp}, @var{pc}, and @var{special} are used to build an identifier
 > +(frame ID) for the frame.  A frame ID is a unique name for a frame
 > +that remains valid as long as the frame itself is valid.  Usually the
 > +frame ID is built from from the frame's stack address and code
 > +address.  The stack address @var{fp} should normally be a pointer to
 > +the new end of the stack when the function was called, as a
 > +@value{GDBN} value.  Similarly the code address @var{pc} should be
 > +given as the address of the entry point of the function.
 > +
 > +For most architectures, it is sufficient to just specify just the
 > +stack and code pointers @var{fp} and @var{pc}.  Some architectures
 > +have another stack or some other frame state store, like ia64.  For
 > +these platforms the frame ID needs an additional address, which may be
 > +passed as the @var{special} optional argument.
 > +
 > +It is possible to create a frame ID with just a stack address, but
 > +it's better to specify a code address as well if possible.
 > +@end deffn
 > +
 > +After building an unwind info object for an ephemeral frame, an
 > +unwinder can call @code{unwind-info-add-saved-register!} to record
 > +saved registers.  The values of the saved registers logically belong
 > +to the frame that is older than the ephemeral frame being unwound, not
 > +to the ephemeral frame itself.
 > +
 > +@deffn {Scheme Procedure} unwind-info-add-saved-register! unwind-info register value
 > +Set the saved value of a register in a ephemeral frame.
 > +
 > +@var{register} names a register, and should be a string, for example
 > +@samp{rip}.  @var{value} is the register value, as a @value{GDBN}
 > +value.
 > +@end deffn
 > +
 > +Any register whose value is not recorded as saved via
 > +@code{unwind-info-add-saved-register!} will be marked as ``not saved''
 > +in the outer frame.
 > +
 >  @node Commands In Guile
 >  @subsubsection Commands In Guile
 >  

Bleah.
I need to head out.
I'll get to the code tonight or tomorrow.