diff mbox

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

Message ID CAHQ51u7sUkGhkmvTaaO_Jo6Jn+kojfiMWHmc2=7OWHThAq6EKw@mail.gmail.com
State New
Headers show

Commit Message

Alexander Smundak March 11, 2015, 2:22 a.m. UTC
This take revises the API by introducing UnwindInfo class. The main difference
with the proposed Guile interface is that previous frame registers are passed to
the constructor as a tuple and cannot be revised later.

gdb/ChangeLog:
2015-02-30  Sasha Smundak  <asmundak@google.com>

        * Makefile.in (SUBDIR_PYTHON_OBJS): Add py-unwind.o.
        (SUBDIR_PYTHON_SRCS): Add py-unwind.c.
        (py-unwind.o): New recipe.
        * NEWS: mention Python frame unwinding.
        * data-directory/Makefile.in (PYTHON_FILE_LIST):  Add sniffers.py.
        * doc/python.texi (Writing a Frame Unwinder in Python): Add
        section.
        * python/lib/gdb/__init__.py (packages): Add frame_sniffers list.
        * python/lib/gdb/command/sniffers.py: New file, implements GDB
        commands to list/enable/disable Python sniffers.
        * python/lib/gdb/function/sniffers.py: New file, implements
        execute_sniffers function.
        * python/lib/gdb/sniffer.py: New file, contains Sniffer class and
        register_sniffer function.
        * python/py-objfile.c (objfile_object): Add frame_sniffers field.
        (objfpy_dealloc): Decrement frame_sniffers reference count.
        (objfpy_initialize): Create frame_sniffers list.
        (objfpy_get_frame_sniffers): Implement Objfile.frame_sniffers
        getter.
        (objfpy_set_frame_sniffers): Implement Objfile.frame_sniffers
        setter.
        (objfile_getset): Add frame_sniffers attribute to Objfile.
        * python/py-progspace.c (pspace_object): Add frame_sniffers field.
        (pspy_dealloc): Decrement frame_sniffers reference count.
        (pspy_initialize): Create frame_sniffers list.
        (pspy_get_frame_sniffers): Implement gdb.Progspace.frame_sniffers
        getter.
        (pspy_set_frame_sniffers): Implement gdb.Progspace.frame_sniffers
        setter.
        (pspy_getset): Add frame_sniffers attribute to gdb.Progspace.
        * python/py-unwind.c: New file, implements Python frame sniffers
        interface.
        * python/python-internal.h (pspy_get_name_sniffers): New prototype.
        (objpy_get_frame_sniffers): New prototype.
        (gdbpy_initialize_unwind): New prototype.
        * python/python.c (gdbpy_apply_type_printers): Call
        gdbpy_initialize_unwind.

gdb/testsuite/ChangeLog:
2014-02-30  Sasha Smundak  <asmundak@google.com>

        * gdb.python/py-unwind-maint.c: Test program for py-unwind-maint.
        * gdb.python/py-unwind-maint.exp: Tests sniffer-related GDB
        commands.
        * gdb.python/py-unwind-maint.py: Pythons sniffers for the test.

On Mon, Mar 9, 2015 at 4:02 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> On 04/03/15 07:49, Andy Wingo wrote:
>> Howdy :)
>>
>> Reordering some of the replies.
>>
>> On Wed 04 Mar 2015 03:36, Alexander Smundak <asmundak@google.com> writes:
>
> Just a note. I am generally satisfied with the patch and your answers
> to my questions. Just now need to get the nod off Doug, and coordinate
> with Andy and/or other guile people to harmonious the design.
>
> Cheers
>
> Phil
>

Comments

Andy Wingo March 11, 2015, 8:49 a.m. UTC | #1
Hi,

On Wed 11 Mar 2015 03:22, Alexander Smundak <asmundak@google.com> writes:

> +@defun SnifferInfo.read_register (self, regnum)
> +This method returns the contents of the register @var{regnum} in the
> +frame as a @code{gdb.Value} object. @var{regnum} values are
> +platform-specific. They are usually defined in the corresponding
> +xxx-@code{tdep.h} file in the gdb source tree.
> +@end defun

I note that the patch has no interface to specify registers by name.
I still wouldn't do numbers but I see we disagree :)

> +          /* Call `deprecated_frame_register_read' -- calling
> +             `value_of_register' would an assert in `get_frame_id'
> +             because our frame is incomplete.  */
> +          if (deprecated_frame_register_read (frame, regnum, buffer))
> +            val = value_from_contents (register_type (gdbarch, regnum),
> +                                       buffer);

As mentioned in a previous comment, can be replaced with:

      val = get_frame_register_value (frame, regnum);

> +/* Initialize new UnwindInfo object.  */
> +static int
> +unwind_infopy_init (PyObject *self, PyObject *args, PyObject *kwargs)
> +{
> +  PyObject *pyo_sniffer_info;
> +  PyObject *pyo_regs;
> +
> +  if (PyArg_UnpackTuple (args, "__init__", 2, 2, &pyo_sniffer_info, &pyo_regs)
> +      && (pyo_sniffer_info != NULL
> +      && PyObject_IsInstance (pyo_sniffer_info,
> +                              (PyObject *) &sniffer_info_object_type) > 0)
> +      && pyo_regs != NULL
> +      && PyTuple_Check (pyo_regs))
> +    {
> +      unwind_info_object *unwind_info = (unwind_info_object *) self;
> +      unwind_info->frame_id = null_frame_id;
> +      unwind_info->sniffer_info = pyo_sniffer_info;
> +      Py_INCREF (pyo_sniffer_info);
> +      unwind_info->previous_frame_registers = pyo_regs;

Probably better to check types and values here, so the user gets a good backtrace.

What do you think about merging the SnifferInfo and UnwindInfo objects
into an EphemeralFrame object?  It would be nice to use the same nouns
on the Guile and Python sides.

Andy
Doug Evans March 11, 2015, 5:34 p.m. UTC | #2
On Wed, Mar 11, 2015 at 1:49 AM, Andy Wingo <wingo@igalia.com> wrote:
> [...]
> What do you think about merging the SnifferInfo and UnwindInfo objects
> into an EphemeralFrame object?  It would be nice to use the same nouns
> on the Guile and Python sides.

Agreed.
I'd like to see the two implementations be as identical as is
reasonably possible.
Consistency Is Good.

[Which isn't to advocate for one implementation or the other yet.
You guys are writing both the API side and the client side, so you're
more up to speed on what you need.]
Alexander Smundak March 11, 2015, 6:48 p.m. UTC | #3
On Wed, Mar 11, 2015 at 1:49 AM, Andy Wingo <wingo@igalia.com> wrote:
>I note that the patch has no interface to specify registers by name.
>I still wouldn't do numbers but I see we disagree :)
Will do in the next revision.

> What do you think about merging the SnifferInfo and UnwindInfo objects
> into an EphemeralFrame object?  It would be nice to use the same nouns
> on the Guile and Python sides.

I prefer the separation of the input (SnifferInfo) and output (UnwindInfo).
I am surprised someone from Guile side is advocating stateful interface :-)

In fact, I'd rather avoid having modifiable UnwindInfo at all -- a sniffer can
return a 2-tuple of (<previous frame registers>, <FrameId instance>),
where FrameId instance can be created by a class method
gdb.SnifferInfo.frame_id_build_xxx(). Can we agree on that?
Andy Wingo March 16, 2015, 11:29 a.m. UTC | #4
Hi :)

[-pmuldoon as he has already given an LGTM]

On Wed 11 Mar 2015 19:48, Alexander Smundak <asmundak@google.com> writes:

>> What do you think about merging the SnifferInfo and UnwindInfo objects
>> into an EphemeralFrame object?  It would be nice to use the same nouns
>> on the Guile and Python sides.
>
> I prefer the separation of the input (SnifferInfo) and output (UnwindInfo).
> I am surprised someone from Guile side is advocating stateful interface :-)

I know what you mean, but because there should be a link between them --
in that saved register set should be checked for validity against the
architecture of the incoming frame -- then given that, joining them
makes sense to me.  Otherwise you'd have to pass the SnifferInfo to the
UnwindInfo constructor, which would be fine but what if we could avoid
it by joining them...

And then there's the question of what do you call the two things --
SnifferInfo is a bad name, for starters.  It's a frame, albeit some
ephemeral kind of frame.  It's literally a struct frame_info* under the
hood.  Currently you can read a register, but you might want to do other
frame things on it -- get_next_frame, get_frame_arch,
frame_relative_level, etc.  WDYT about the name EphemeralFrame ?

> In fact, I'd rather avoid having modifiable UnwindInfo at all -- a sniffer can
> return a 2-tuple of (<previous frame registers>, <FrameId instance>),
> where FrameId instance can be created by a class method
> gdb.SnifferInfo.frame_id_build_xxx(). Can we agree on that?

Regarding the return value, I think returning a 2-tuple is a bad idea
for all of the reasons I have previously mentioned and which you have
not responded to :)

  * bad error reporting (e.g. gdb.Value width doesn't match register
    width, how to map that back to some source location or program
    state)

  * bad extensibility (e.g. what about stop_reason?  what about other
    register states?)

  * bad ergonomics (is the frame id first, or is it the saved
    registers?)

On the other hand to have to give a name the result object is painful
too -- "this is an UnwindInfo and it is composed of blah blah".  For
that reason I joined the result with the ephemeral frame object, to
avoid introducing another concept.  But checking "did this unwinder
succeed" via "did the unwinder set the frame ID on this ephemeral frame"
is not nice either.

How about let's meet somewhat halfway.

  * We rename SnifferInfo to EphemeralFrame.

  * Unwinders return UnwindInfo (better name welcome) on success or
    None/#f otherwise

    * UnwindInfo takes EphemeralFrame as constructor arg

      - the EphemeralFrame must be valid

      - in practice there is only ever one EphemeralFrame alive because
        unwinding is not recursive

    * UnwindInfo also takes frame ID as positional constructor args

      - setting frame_id is the only thing an unwinder *must* do, so
        this makes an invariant "if return value is an UnwindInfo, then
        it is valid and has all necessary info"

    * UnwindInfo has add_saved_register() API (see discussion with Pedro
      here: http://article.gmane.org/gmane.comp.gdb.patches/105538)

  * After accepting an UnwindInfo as an unwinder return result,
    py-unwinders.c / scm-frame-unwinders.c marks the UnwindInfo as
    frozen so that add_saved_register() etc can't alter the state

  * continuation of unwinder call also checks that the ephemeral frame
    on the unwindinfo is valid

Example of use:

   def unwind(frame):
       if we_can_handle(frame):
           var ret = UnwindInfo(frame, fp, pc)
           ret.add_saved_register(r0)
           return ret

I will rework the Guile patch along these lines, and then hopefully I am
done reworking :)

Andy
Andy Wingo March 16, 2015, 12:01 p.m. UTC | #5
Responding with a thinko:

On Mon 16 Mar 2015 12:29, Andy Wingo <wingo@igalia.com> writes:

>   * After accepting an UnwindInfo as an unwinder return result,
>     py-unwinders.c / scm-frame-unwinders.c marks the UnwindInfo as
>     frozen so that add_saved_register() etc can't alter the state

No need -- marking the associated EphemeralFrame as invalid will prevent
add_saved_register() from working.

Andy
Alexander Smundak March 16, 2015, 5:25 p.m. UTC | #6
> How about let's meet somewhat halfway.
>
>   * We rename SnifferInfo to EphemeralFrame.
>
>   * Unwinders return UnwindInfo (better name welcome) on success or
>     None/#f otherwise
>
>     * UnwindInfo takes EphemeralFrame as constructor arg
>
>       - the EphemeralFrame must be valid
>
>       - in practice there is only ever one EphemeralFrame alive because
>         unwinding is not recursive
>
>     * UnwindInfo also takes frame ID as positional constructor args
>
>       - setting frame_id is the only thing an unwinder *must* do, so
>         this makes an invariant "if return value is an UnwindInfo, then
>         it is valid and has all necessary info"
>
>     * UnwindInfo has add_saved_register() API (see discussion with Pedro
>       here: http://article.gmane.org/gmane.comp.gdb.patches/105538)
>
>   * After accepting an UnwindInfo as an unwinder return result,
>     py-unwinders.c / scm-frame-unwinders.c marks the UnwindInfo as
>     frozen so that add_saved_register() etc can't alter the state
>
>   * continuation of unwinder call also checks that the ephemeral frame
>     on the unwindinfo is valid
>
> Example of use:
>
>    def unwind(frame):
>        if we_can_handle(frame):
>            var ret = UnwindInfo(frame, fp, pc)
>            ret.add_saved_register(r0)
>            return ret
>
> I will rework the Guile patch along these lines, and then hopefully I am
> done reworking :)

I'd like to propose one improvement on the Python side: UnwinderInfo
is constructed by a frame method instead of an implicit constructor.
I.e., frame.create_frame_with_id(sp, pc) returns UnwindInfo instance
whose ID is the result of calling GDB's frame_id_build(sp, pc),
frame.create_frame_with_id_wild(sp) returns UnwindInfo instance
whose ID is the results of calling frame_id_build_wild(sp), etc.

The example above would then look as follows:
  def unwind(frame):
    if we_can_handle(frame):
      unwind_info = frame.create_frame_with_id(sp, pc)
      unwind_info.set_previous_frame_register("r0", r0)
      unwind_info.set_previous_frame_register(...)
      return unwind_info
    else
      return None

Would this work for Guile?

As to the class of an object passed to a sniffer, how about calling it
FrameData? Note that it's not very important from the user's point of
view as sniffer code does not ever reference it by name.
Andy Wingo March 17, 2015, 8:57 a.m. UTC | #7
On Mon 16 Mar 2015 18:25, Alexander Smundak <asmundak@google.com> writes:

> I'd like to propose one improvement on the Python side: UnwinderInfo
> is constructed by a frame method instead of an implicit constructor.
> I.e., frame.create_frame_with_id(sp, pc) returns UnwindInfo instance
> whose ID is the result of calling GDB's frame_id_build(sp, pc),
> frame.create_frame_with_id_wild(sp) returns UnwindInfo instance
> whose ID is the results of calling frame_id_build_wild(sp), etc.
>
> The example above would then look as follows:
>   def unwind(frame):
>     if we_can_handle(frame):
>       unwind_info = frame.create_frame_with_id(sp, pc)
>       unwind_info.set_previous_frame_register("r0", r0)
>       unwind_info.set_previous_frame_register(...)
>       return unwind_info
>     else
>       return None

Looks great to me :)  Thank you for the consideration!

I might consider naming "set_previous_frame_register" as
"add_saved_register", in anticipation of a possible
add_unavailable_register(), add_unmodified_register(), etc, but that is
just a minor nit.

> As to the class of an object passed to a sniffer, how about calling it
> FrameData? Note that it's not very important from the user's point of
> view as sniffer code does not ever reference it by name.

It's true that from user code it barely matters to Python, but Scheme's
monomorphic flavor makes these things more apparent:

  (frame-data-read-register frame "r0")

This doesn't read so well to me -- is it "read-register" on a
"frame-data", or is it "data-read-register" on a "frame" ?  A weak point
but "ephemeral-frame-read-register" avoids the question.

I also think that "ephemeral frame" is easier to document as a concept
than the more generic "frame data", and corresponds better to what is
happening in GDB.  YMMV, though.

As an aside, it seems to me that if we can avoid it, the word "sniffer"
should not enter the documentation or the API.  The Python and Guile
APIs don't just sniff, they do the whole of the unwinding operation, so
it's more clear to call them "unwinders".

Andy
Alexander Smundak March 17, 2015, 7:48 p.m. UTC | #8
On Tue, Mar 17, 2015 at 1:57 AM, Andy Wingo <wingo@igalia.com> wrote:
> On Mon 16 Mar 2015 18:25, Alexander Smundak <asmundak@google.com> writes:
>
>> I'd like to propose one improvement on the Python side: UnwinderInfo
>> is constructed by a frame method instead of an implicit constructor.
>> I.e., frame.create_frame_with_id(sp, pc) returns UnwindInfo instance
>> whose ID is the result of calling GDB's frame_id_build(sp, pc),
>> frame.create_frame_with_id_wild(sp) returns UnwindInfo instance
>> whose ID is the results of calling frame_id_build_wild(sp), etc.
>>
>> The example above would then look as follows:
>>   def unwind(frame):
>>     if we_can_handle(frame):
>>       unwind_info = frame.create_frame_with_id(sp, pc)
>>       unwind_info.set_previous_frame_register("r0", r0)
>>       unwind_info.set_previous_frame_register(...)
>>       return unwind_info
>>     else
>>       return None
>
> Looks great to me :)  Thank you for the consideration!
I realized after writing my proposal that 'create_frame_xxx'
misguides about the type of the returned object. The new
name candidate is `unwind_info_with_id' (omitting 'create'
prefix, too).

> I might consider naming "set_previous_frame_register" as
> "add_saved_register", in anticipation of a possible
> add_unavailable_register(), add_unmodified_register(), etc, but that is
> just a minor nit.
It does not reflect that we are setting previous frame's register.

> I also think that "ephemeral frame" is easier to document as a concept
> than the more generic "frame data", and corresponds better to what is
> happening in GDB.  YMMV, though.
EphemeralFrame emphasizes that the object is short-lived. Maybe it's
better to put forward what this object is for and name it InspectedFrame?

> As an aside, it seems to me that if we can avoid it, the word "sniffer"
> should not enter the documentation or the API.  The Python and Guile
> APIs don't just sniff, they do the whole of the unwinding operation, so
> it's more clear to call them "unwinders".
'Sniffer' comes from the "GDB Internals" document; I am not too fond of
it, but I thought it's desirable to use similar terminology.

IMHO we reached an agreement on API.

I would really like the reviewers to express their opinions about the naming
at this point. To summarize:
* We need a name for the entity passed to a sniffer. This entity provides
access to the registers of the frame being sniffed, and has factory methods
to create instances returned by a sniffer.
* We need a name for the entity returned by a sniffer.
* Should the term 'sniffer' be used in the API and documentation describing
implementing frame unwinding in GDB extensions?
Doug Evans March 17, 2015, 10:21 p.m. UTC | #9
[+ guile-devel, in case they have an opinion on the spelling of
frame-data-read-register vs frame-data:read-register]

On Tue, Mar 17, 2015 at 1:57 AM, Andy Wingo <wingo@igalia.com> wrote:
>> As to the class of an object passed to a sniffer, how about calling it
>> FrameData? Note that it's not very important from the user's point of
>> view as sniffer code does not ever reference it by name.
>
> It's true that from user code it barely matters to Python, but Scheme's
> monomorphic flavor makes these things more apparent:
>
>   (frame-data-read-register frame "r0")
>
> This doesn't read so well to me -- is it "read-register" on a
> "frame-data", or is it "data-read-register" on a "frame" ?  A weak point
> but "ephemeral-frame-read-register" avoids the question.

As food for discussion,
I know some people use foo:bar in Scheme to separate
the object "foo" from the operation on it "bar".
-> frame-data:read-register
I like having some separator, but I went with what
I thought was the preferred spelling (all -'s).
It's not too late to change gdb/guile to use foo:bar throughout (IMO),
but the door is closing.
Andy Wingo March 18, 2015, 8:57 a.m. UTC | #10
Hi,

[-asmundak, as he probably doesn't care :)]

On Tue 17 Mar 2015 23:21, Doug Evans <dje@google.com> writes:

> On Tue, Mar 17, 2015 at 1:57 AM, Andy Wingo <wingo@igalia.com> wrote:
>>> As to the class of an object passed to a sniffer, how about calling it
>>> FrameData? Note that it's not very important from the user's point of
>>> view as sniffer code does not ever reference it by name.
>>
>> It's true that from user code it barely matters to Python, but Scheme's
>> monomorphic flavor makes these things more apparent:
>>
>>   (frame-data-read-register frame "r0")
>>
>> This doesn't read so well to me -- is it "read-register" on a
>> "frame-data", or is it "data-read-register" on a "frame" ?  A weak point
>> but "ephemeral-frame-read-register" avoids the question.
>
> As food for discussion,
> I know some people use foo:bar in Scheme to separate
> the object "foo" from the operation on it "bar".
> -> frame-data:read-register

This convention is not often used in Guile.  When it is used, it often
denotes field access rather than some more involved procedure call --
similar to the lowercase "foo_bar()" versus camel-cased "FooBar()" in
Google C++ guidelines.

> I like having some separator, but I went with what
> I thought was the preferred spelling (all -'s).
> It's not too late to change gdb/guile to use foo:bar throughout (IMO),
> but the door is closing.

FWIW, I prefer "-".

Andy
Doug Evans March 18, 2015, 4:48 p.m. UTC | #11
On Wed, Mar 18, 2015 at 1:57 AM, Andy Wingo <wingo@igalia.com> wrote:
> Hi,
>
> [-asmundak, as he probably doesn't care :)]
>
> On Tue 17 Mar 2015 23:21, Doug Evans <dje@google.com> writes:
>
>> On Tue, Mar 17, 2015 at 1:57 AM, Andy Wingo <wingo@igalia.com> wrote:
>>>> As to the class of an object passed to a sniffer, how about calling it
>>>> FrameData? Note that it's not very important from the user's point of
>>>> view as sniffer code does not ever reference it by name.
>>>
>>> It's true that from user code it barely matters to Python, but Scheme's
>>> monomorphic flavor makes these things more apparent:
>>>
>>>   (frame-data-read-register frame "r0")
>>>
>>> This doesn't read so well to me -- is it "read-register" on a
>>> "frame-data", or is it "data-read-register" on a "frame" ?  A weak point
>>> but "ephemeral-frame-read-register" avoids the question.
>>
>> As food for discussion,
>> I know some people use foo:bar in Scheme to separate
>> the object "foo" from the operation on it "bar".
>> -> frame-data:read-register
>
> This convention is not often used in Guile.  When it is used, it often
> denotes field access rather than some more involved procedure call --
> similar to the lowercase "foo_bar()" versus camel-cased "FooBar()" in
> Google C++ guidelines.
>
>> I like having some separator, but I went with what
>> I thought was the preferred spelling (all -'s).
>> It's not too late to change gdb/guile to use foo:bar throughout (IMO),
>> but the door is closing.
>
> FWIW, I prefer "-".

Even though a different character solves a problem?
What problem does it introduce?

The comparison with _ vs CamelCase is apples and oranges.
They don't separate object/class name from method name.
If I were to invoke static method read_register on class
ephemeral_frame it would be ephemeral_frame::read_register().
The problem of the readability of frame-data-read-register
that ephemeral-frame-read-register attempts to solve
just doesn't arise. Same with frame-data:read-register.
Doug Evans March 18, 2015, 11:25 p.m. UTC | #12
On Tue, Mar 17, 2015 at 12:48 PM, Alexander Smundak <asmundak@google.com> wrote:
> IMHO we reached an agreement on API.
>
> I would really like the reviewers to express their opinions about the naming
> at this point. To summarize:
> * We need a name for the entity passed to a sniffer. This entity provides
> access to the registers of the frame being sniffed, and has factory methods
> to create instances returned by a sniffer.
> * We need a name for the entity returned by a sniffer.
> * Should the term 'sniffer' be used in the API and documentation describing
> implementing frame unwinding in GDB extensions?

I'm not sure I'm going to be of much help here.

I don't mind "sniffing" as the name of the act of deciding whether
a frame as recognized, and I do like "unwinder" as the name of
the object doing the sniffing. But then "sniffer" instead of
"unwinder" seems fine too.

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.

Would it make sense to speak of an Unwinder taking
an EphemeralFrame as input and returning a Frame as output?
Alexander Smundak March 19, 2015, 12:36 a.m. UTC | #13
> 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.

> Would it make sense to speak of an Unwinder taking
> an EphemeralFrame as input and returning a Frame as output?
Existing gdb.Frame interface wraps GDB core's `frame_info',
providing read-only capability and no instance construction.
An object returned by a Python unwinder is quite different.
Andy Wingo March 19, 2015, 8:03 a.m. UTC | #14
Hi :)

On Wed 18 Mar 2015 17:48, Doug Evans <dje@google.com> writes:

> On Wed, Mar 18, 2015 at 1:57 AM, Andy Wingo <wingo@igalia.com> wrote:
>>
>> On Tue 17 Mar 2015 23:21, Doug Evans <dje@google.com> writes:
>>
>>> On Tue, Mar 17, 2015 at 1:57 AM, Andy Wingo <wingo@igalia.com> wrote:
>>>>> As to the class of an object passed to a sniffer, how about calling it
>>>>> FrameData? Note that it's not very important from the user's point of
>>>>> view as sniffer code does not ever reference it by name.
>>>>
>>>> It's true that from user code it barely matters to Python, but Scheme's
>>>> monomorphic flavor makes these things more apparent:
>>>>
>>>>   (frame-data-read-register frame "r0")
>>>>
>>>> This doesn't read so well to me -- is it "read-register" on a
>>>> "frame-data", or is it "data-read-register" on a "frame" ?  A weak point
>>>> but "ephemeral-frame-read-register" avoids the question.
>>>
>>> As food for discussion,
>>> I know some people use foo:bar in Scheme to separate
>>> the object "foo" from the operation on it "bar".
>>> -> frame-data:read-register
>>
>> This convention is not often used in Guile.  When it is used, it often
>> denotes field access rather than some more involved procedure call --
>> similar to the lowercase "foo_bar()" versus camel-cased "FooBar()" in
>> Google C++ guidelines.
>>
>>> I like having some separator, but I went with what
>>> I thought was the preferred spelling (all -'s).
>>> It's not too late to change gdb/guile to use foo:bar throughout (IMO),
>>> but the door is closing.
>>
>> FWIW, I prefer "-".
>
> Even though a different character solves a problem?
> What problem does it introduce?

I find it to be uncommon style in Guile and so it doesn't appeal to me.
YMMV.

Of course, what appeals to me is a function of what is common, and I'm
sure anything can work well as long as it's consistent.

Adding a convention for using colons also makes me wonder when to use
colons or dashes.  Perhaps that is my real objection.

Note that actually my original concern was not valid in this case;
"frame-data-read-register" is unlikely to be misread, because people
tend to break the name at the verb, if present.  "read-register" on a
"frame-data".

> The comparison with _ vs CamelCase is apples and oranges.

At least in V8 there is a similarity; there are two conventions for
writing method names: underscores if it's a simple field access or
camel-case for more complicated things.  Same decision criteria as colon
versus dash, as I understand the convention.  You don't have the
subject/verb punctuation problem but you do have different ways to write
the verb based on what's going on underneath.

Doug if you want a specific coding style do let me know and I'm happy to
change.  Please consider these comments to be just gut reactions /
reflections.

Andy
Andy Wingo March 19, 2015, 8:12 a.m. UTC | #15
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.

Andy
Doug Evans March 20, 2015, 12:15 a.m. UTC | #16
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.
Alexander Smundak March 20, 2015, 2:27 a.m. UTC | #17
> In python, let's use EphemeralFrame instead of SnifferInfo.

I'd rather have InspectedFrame than EphemeralFrame,
but it's fine with me if it expedites the review.

> 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.]

It isn't a big issue for the JVM unwinder to do the lookups
twice (first in unwinder, then in the frame filter), but it might
be expensive in other languages. Not sure there is a need for
multiple attributes -- wouldn't a single attribute whose value is
an object suffice?

> 5) The docs don't make any mention of target endianness
> in the saved register values. They probably should.
Register values passed are gdb.Value instances, so I thought
that if they are created using the official interface (get inspected
frame's register, call its `cast' method to cast it to inferior's memory
pointer. then call `dereference' we wolud be fine.

> 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?
We can (I believe penultimate patch had UnwindInfo constructor and frame_id
methods). The advantage of the factory method is that it's impossible to create
an UnwindInfo which lacks frame ID.

> 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).
This way the error checking happens on return. If we have several
unwinders, it will not be obvious which one sniffer the frame and
returned bad UnwindInfo.

> 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.
This will loose the association with the underlying GDB API,
but I am fine with this.
Andy Wingo March 20, 2015, 8:26 a.m. UTC | #18
Hi,

Thanks for taking the time to look at the patches, Doug!

On Fri 20 Mar 2015 01:15, Doug Evans <dje@google.com> writes:

> 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.

Just to throw out some other names -- PendingFrame.  Alexander mentioned
InspectedFrame.  IntermediateFrame.  FrameTemplate.  SkeletonFrame.
MinimalFrame.  FrameStub.

Dunno, just some ideas :)

> 3) We need to allow sniffers to record anything they
> want in gdb.UnwindInfo/<gdb:unwind-info>.

I see from your point (4) later that you are thinking that if we add
some kind of callback interface to UnwindInfo that e.g. Guile code might
need to fetch arbitrary data from the UnwindInfo.

You also note correctly that this is not currently something that the
patches require -- the UnwindInfo contains all of the information, and
the C prev_register/this_id callbacks just look into the UnwindInfo
data.

If we did add a callback interface I don't think there would be a
problem.

Let's say we want to add a way for prev_register to call back into
Guile.  If the prev-register callback is added to the unwind info via
unwind-info-set-prev-register-callback! or something then the callback
can be a closure that captures the data it needs.  Or it can use an
external weak hash table.  Or when we add the
set-prev-register-callback! interface, we can add some other associated
interface to store data in the unwind-info.  There are multiple fine
options here.

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

+1 to Alexander's answer -- as they are GDB values, there shouldn't be
an issue, should there?

> 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)

This would be fine; and actually at this point the kwarg is unnecessary.

  (make-unwind-info ephem-frame frame-id)

> but then I'm wondering if there should be an
> unwind-info-set-frame-id! function and move sp,pc there.

I agree with Alexander that this isn't great.  The only thing that an
unwinder *must* do is set a frame ID.  It must produce a frame_id at the
same time as the sniffer returns TRUE.  (They are different callbacks
but always called one after the other.)  Therefore it makes sense to
have the invariant:

  Is it an UnwindInfo?  Then it definitely has a frame ID.

That way it takes away one possible error case.  Also you wouldn't want
to set the frame ID on an UnwindInfo twice, it doesn't make sense.

> 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?

In Guile you have to give it a name of course -- make-unwind-info.  But
in Python I liked Alexander's use of a factory method.  But I dunno,
I'll leave the Python discussion to you all :)

> 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.

If you don't have such an interest on being consistent for this piece
then I guess the easy way is to leave these as they are then.  Avoids
multiple-day awkward backs and forths :)  But if you decide they need to
be the same let us know.

Speaking for myself I prefer the Guile one of course, but I prefer
executive decisions over having to endure more days of naming
discussions over email :)

Thank you again Doug for the time!

Andy
Doug Evans March 20, 2015, 5:48 p.m. UTC | #19
Alexander Smundak writes:
 > > In python, let's use EphemeralFrame instead of SnifferInfo.
 > 
 > I'd rather have InspectedFrame than EphemeralFrame,
 > but it's fine with me if it expedites the review.

Andy suggested PendingFrame.
Let's go with that.
It is a frame, but not a "real" one yet.

 > > 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.]
 > 
 > It isn't a big issue for the JVM unwinder to do the lookups
 > twice (first in unwinder, then in the frame filter), but it might
 > be expensive in other languages. Not sure there is a need for
 > multiple attributes -- wouldn't a single attribute whose value is
 > an object suffice?

I was looking through all the internal unwinders in gdb.
Some of them cache more than just registers.
The extension languages don't export the stop_reason, this_id
or prev_register API calls, but some day they might,
and I want to make sure that we don't make it difficult
to do so later. But (3) was written early, I should have
edited it out so disregard.

 > > 5) The docs don't make any mention of target endianness
 > > in the saved register values. They probably should.
 > Register values passed are gdb.Value instances, so I thought
 > that if they are created using the official interface (get inspected
 > frame's register, call its `cast' method to cast it to inferior's memory
 > pointer. then call `dereference' we wolud be fine.

We can't control how the user will construct the value.
We can only provide guidance/rules on what will work.
The value has to be the same size as the register,
but we don't say that anywhere either (we 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?
 > We can (I believe penultimate patch had UnwindInfo constructor and frame_id
 > methods). The advantage of the factory method is that it's impossible to create
 > an UnwindInfo which lacks frame ID.

The constructor could take a frame ID too.

One problem I have is that while frame IDs are just sp,pc,special
today who knows what they will have tomorrow. Hopefully we could
just use special, but I wouldn't assume that.
If we separate out frame ID construction from UnwindInfo construction
then we can grow frame IDs without having to complicate UnwindInfo.
The complexity lives where it should live: in the FrameID constructor.

 > > 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).
 > This way the error checking happens on return. If we have several
 > unwinders, it will not be obvious which one sniffer the frame and
 > returned bad UnwindInfo.

Good point, but there's a general problem here: E.g., What if
the sniffer created a frame id with sp and pc swapped?
Or what if the pc got errantly elided?

I recognize the goal of detecting errors at construction time,
but in this case I think you're only solving one piece of a
larger problem. If you feel it's important in this case I
don't make making the frame id a required argument to the
UnwindInfo constructor, but let's pass it an object of type FrameID.

I have similar problems with pretty-printers btw: sometimes
I don't always know which printer got selected.
We need to provide ways to debug these kinds of problems.

For debugging purposes IWBN if the result had a backlink to
its creator. Then any error messages could reference the creator.
Thoughts?

Also, I was thinking of using "set debug foo" options, but those are for
maintainers and these aren't gdb bugs. OTOH "set debug pretty-printer"
is appealing. It's harder for unwinders because we need a unique name
for extension language provided unwinders: we don't want the option
to also turn on debugging of gdb's internal unwinders (it's likely
just noise to the user).

set debug python unwinder?
set python debug unwinder?

I kinda like leaving "set debug foo" for internal gdb usage,
and we do already have set python foo, so "set python debug unwinder"
is what I'm proposing.
I thought of names not containing the word python/guile
but in this case we know which extension language we're working in.
The code that looped over all unwinders could check this parameter
and if set print the name of winning unwinder before it returns.
There's no need to complicate the current patch with this
additional functionality though, we can add it as a follow-on
patch if you want.

 > > 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.
 > This will loose the association with the underlying GDB API,
 > but I am fine with this.

Cool.
Doug Evans March 20, 2015, 6:32 p.m. UTC | #20
Andy Wingo writes:
 > Hi,
 > 
 > Thanks for taking the time to look at the patches, Doug!
 > 
 > On Fri 20 Mar 2015 01:15, Doug Evans <dje@google.com> writes:
 > 
 > > 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.
 > 
 > Just to throw out some other names -- PendingFrame.  Alexander mentioned
 > InspectedFrame.  IntermediateFrame.  FrameTemplate.  SkeletonFrame.
 > MinimalFrame.  FrameStub.
 > 
 > Dunno, just some ideas :)

I like PendingFrame.
It's a frame, but not a real one yet.

 > > 3) We need to allow sniffers to record anything they
 > > want in gdb.UnwindInfo/<gdb:unwind-info>.
 > 
 > I see from your point (4) later that you are thinking that if we add
 > some kind of callback interface to UnwindInfo that e.g. Guile code might
 > need to fetch arbitrary data from the UnwindInfo.
 > 
 > You also note correctly that this is not currently something that the
 > patches require -- the UnwindInfo contains all of the information, and
 > the C prev_register/this_id callbacks just look into the UnwindInfo
 > data.
 > 
 > If we did add a callback interface I don't think there would be a
 > problem.

Cool.

 > Let's say we want to add a way for prev_register to call back into
 > Guile.  If the prev-register callback is added to the unwind info via
 > unwind-info-set-prev-register-callback! or something then the callback
 > can be a closure that captures the data it needs.  Or it can use an
 > external weak hash table.  Or when we add the
 > set-prev-register-callback! interface, we can add some other associated
 > interface to store data in the unwind-info.  There are multiple fine
 > options here.
 > 
 > > 5) The docs don't make any mention of target endianness
 > > in the saved register values. They probably should.
 > 
 > +1 to Alexander's answer -- as they are GDB values, there shouldn't be
 > an issue, should there?

Well, as I wrote in my reply to Alexander,
we can't control how the user will create these values.
They *might* want to do something off the beaten path, so to speak.
We should specify what's required.

 > > 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)
 > 
 > This would be fine; and actually at this point the kwarg is unnecessary.

I'd have make-frame-id take kwargs though.

 >   (make-unwind-info ephem-frame frame-id)
 > 
 > > but then I'm wondering if there should be an
 > > unwind-info-set-frame-id! function and move sp,pc there.
 > 
 > I agree with Alexander that this isn't great.  The only thing that an
 > unwinder *must* do is set a frame ID.  It must produce a frame_id at the
 > same time as the sniffer returns TRUE.  (They are different callbacks
 > but always called one after the other.)  Therefore it makes sense to
 > have the invariant:
 > 
 >   Is it an UnwindInfo?  Then it definitely has a frame ID.
 > 
 > That way it takes away one possible error case.

I recognize these points, but I guess in this particular case
I don't feel as strongly. But I don't mind making
frame-id a required argument to make-unwind-info.
So go for it.

 > Also you wouldn't want
 > to set the frame ID on an UnwindInfo twice, it doesn't make sense.

One could say the same of saved registers.

 > > 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?
 > 
 > In Guile you have to give it a name of course -- make-unwind-info.  But
 > in Python I liked Alexander's use of a factory method.  But I dunno,
 > I'll leave the Python discussion to you all :)

A factory method is a layer of abstraction
which is nice (e.g., if later we need to change/extend things
it'd be easier to add a new method or whatever than to play
with the constructor). So ok let's use a factory method.

 > > 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.
 > 
 > If you don't have such an interest on being consistent for this piece
 > then I guess the easy way is to leave these as they are then.  Avoids
 > multiple-day awkward backs and forths :)  But if you decide they need to
 > be the same let us know.
 > 
 > Speaking for myself I prefer the Guile one of course, but I prefer
 > executive decisions over having to endure more days of naming
 > discussions over email :)

I'll make an executive decision to let you guys decide.  1/2 :-)

"But seriously, ..."
Let's go with add_saved_register / unwind-info-add-saved-register!
Sorry Alexander. :-)
I have to pause whenever I read "previous" vs "next": the meaning
is clear in frame-unwind.h, but it just doesn't stick.
E.g., The next frame to unwind is the previous frame on the stack.
Thank goodness frame.c uses all of next/down/inner/younger
and prev/up/outer/older.
diff mbox

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 8addef4..3773e2c 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -394,6 +394,7 @@  SUBDIR_PYTHON_OBS = \
 	py-symtab.o \
 	py-threadevent.o \
 	py-type.o \
+	py-unwind.o \
 	py-utils.o \
 	py-value.o \
 	py-varobj.o
@@ -433,6 +434,7 @@  SUBDIR_PYTHON_SRCS = \
 	python/py-symtab.c \
 	python/py-threadevent.c \
 	python/py-type.c \
+	python/py-unwind.c \
 	python/py-utils.c \
 	python/py-value.c \
 	python/py-varobj.c
@@ -2608,6 +2610,10 @@  py-type.o: $(srcdir)/python/py-type.c
 	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-type.c
 	$(POSTCOMPILE)
 
+py-unwind.o: $(srcdir)/python/py-unwind.c
+	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-unwind.c
+	$(POSTCOMPILE)
+
 py-utils.o: $(srcdir)/python/py-utils.c
 	$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-utils.c
 	$(POSTCOMPILE)
diff --git a/gdb/NEWS b/gdb/NEWS
index f19577a..89157a4 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -12,6 +12,7 @@ 
   ** gdb.Objfile objects have a new attribute "username",
      which is the name of the objfile as specified by the user,
      without, for example, resolving symlinks.
+  ** You can now write frame unwinders in Python.
 
 * New commands
 
diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index c01b86d..47b4957 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -62,17 +62,20 @@  PYTHON_FILE_LIST = \
 	gdb/FrameDecorator.py \
 	gdb/types.py \
 	gdb/printing.py \
+	gdb/sniffer.py \
 	gdb/prompt.py \
 	gdb/xmethod.py \
 	gdb/command/__init__.py \
 	gdb/command/xmethods.py \
 	gdb/command/frame_filters.py \
+	gdb/command/sniffers.py \
 	gdb/command/type_printers.py \
 	gdb/command/pretty_printers.py \
 	gdb/command/prompt.py \
 	gdb/command/explore.py \
 	gdb/function/__init__.py \
 	gdb/function/caller_is.py \
+        gdb/function/sniffers.py \
 	gdb/function/strfns.py \
 	gdb/printer/__init__.py \
 	gdb/printer/bound_registers.py
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index d725eb0..44683bd 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -144,6 +144,7 @@  optional arguments while skipping others.  Example:
 * Frame Filter API::            Filtering Frames.
 * Frame Decorator API::         Decorating Frames.
 * Writing a Frame Filter::      Writing a Frame Filter.
+* Unwinding Frames in Python::  Writing a frame unwinder in Python.
 * Xmethods In Python::          Adding and replacing methods of C++ classes.
 * Xmethod API::                 Xmethod types.
 * Writing an Xmethod::          Writing an xmethod.
@@ -2178,6 +2179,111 @@  printed hierarchically.  Another approach would be to combine the
 marker in the inlined frame, and also show the hierarchical
 relationship.
 
+@node Unwinding Frames in Python
+@subsubsection Unwinding Frames in Python
+@cindex Unwinding frames in Python.
+
+In GDB terminology ``unwinding'' is the process of finding the
+previous frame (that is, caller's) from the current one. A running GDB
+mantains a list of the unwinders and calls each unwinder's sniffer in
+turn until it finds the one that recognizes the current frame. There
+is an API to register an unwinder.
+
+The unwinders that come with GDB handle standard frames for each
+platform where GDB is running. However, mixed language applications
+(for example, and application running Java Virtual Machine) sometimes
+use frame layouts that cannot be handled by the GDB unwinders. You
+can write Python code that can handle such custom frames.
+
+You implement a sniffer as a class with which has two attributes,
+@code{name} and @code{enabled}, with obvious meanings, and a single
+method @code{__call__}, which examines a given frame and returns the data
+describing it (that is, when it recognizes it). GDB comes with the module
+containing the base @code{Sniffer} class for that. The sniffer looks as
+follows:
+@smallexample
+from gdb.sniffers import Sniffer
+
+class MySniffer(Sniffer):
+    def __init__(....):
+        super(MySniffer, self).__init___(<expects sniffer name argument>)
+    def __call__(sniffer_info):
+        if not <we recognize frame>:
+            return None
+        <find previous frame registers, create UnwindInfo instance from it>
+        <save frame's ID in UnwindInfo instance by calling one of
+         frame_id_build_xxx methods>
+        return <UnwindInfo instance>
+@end smallexample
+
+@subheading Examining The Current Frame
+
+@value{GDBN} passes a @code{gdb.SnifferInfo} instance when it calls
+sniffer's @code{__call__} method. This class has a single method:
+
+@defun SnifferInfo.read_register (self, regnum)
+This method returns the contents of the register @var{regnum} in the
+frame as a @code{gdb.Value} object. @var{regnum} values are
+platform-specific. They are usually defined in the corresponding
+xxx-@code{tdep.h} file in the gdb source tree.
+@end defun
+
+@subheading Sniffer return: UnwindInfo
+
+If sniffer's @code{__call__} method recognizes the frame, it should
+return an instance of @code{gdb.UnwindInfo} class describing it. 
+
+@code{UnwindInfo} has the following methods:
+
+@defun gdb.UnwindInfo.__init__ (sniffer_info, previous_frame_registers)
+@var{sniffer_info} is the SnifferInfo instance that has been passed to
+a sniffer on invocation.
+@var{previous_frame_registers} describes the registers that can be
+unwound (i.e., the registers from the previous frame that have been
+saved in the current frame described by @var{sniffer_info}). It is a
+tuple where each element is a (@var{regnum}, @var{regdata}) 2-tuple.
+@var{regnum} is a register number, and @var{regdata} is register
+contents (a @code{gdb.Value} object).
+@end defun
+
+@defun gdb.UnwindInfo.frame_id_build(sp, pc)
+Builds frame ID from the given @var{sp} and @var{pc} values.
+@end defun
+
+@defun gdb.UnwindInfo.frame_id_build_special(sp, pc, special)
+Builds frame ID from the given @var{sp}, @var{pc}, and @var{special}
+values.
+@end defun
+
+@defun gdb.UnwindInfo.frame_id_build_wild(sp)
+Builds frame ID from the given @var{sp} value.
+@end defun
+
+@subheading Registering a Sniffer
+
+An object file, a program space, and the @value{GDBN} proper can have
+sniffers registered with it.
+
+The @code{gdb.sniffers} module provides the function to register a
+sniffer:
+
+@defun gdb.sniffer.register_sniffer (locus, sniffer, replace=False)
+@var{locus} is specifies an object file or a program space to which
+@var{sniffer} is added. Passing @code{None} or @code{gdb} adds
+@var{sniffer} to the @value{GDBN}'s global sniffer list.  The newly
+added @var{sniffer} will be called before any other sniffer from the
+same locus.  Two sniffers in the same locus cannot have the same
+name. An attempt to add a sniffer with already existing name raises an
+exception unless @var{replace} is @code{True}, in which case the old
+sniffer is deleted.
+@end defun
+
+@subheading Sniffer Precedence
+
+@value{GDBN} first calls the sniffers from all the object files in no
+particular order, then the sniffers from the current program space,
+and finally the sniffers from @value{GDBN}.
+
 @node Xmethods In Python
 @subsubsection Xmethods In Python
 @cindex xmethods in Python
diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
index 92b06f2..8d7f651 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -71,6 +71,8 @@  type_printers = []
 xmethods = []
 # Initial frame filters.
 frame_filters = {}
+# Initial frame sniffers.
+frame_sniffers = []
 
 # Convenience variable to GDB's python directory
 PYTHONDIR = os.path.dirname(os.path.dirname(__file__))
diff --git a/gdb/python/lib/gdb/command/sniffers.py b/gdb/python/lib/gdb/command/sniffers.py
new file mode 100644
index 0000000..ad7f693
--- /dev/null
+++ b/gdb/python/lib/gdb/command/sniffers.py
@@ -0,0 +1,198 @@ 
+# Sniffer commands.
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import gdb
+import re
+
+
+def validate_regexp(exp, idstring):
+    try:
+        return re.compile(exp)
+    except SyntaxError:
+        raise SyntaxError("Invalid %s regexp: %s." % (idstring, exp))
+
+
+def parse_sniffer_command_args(arg):
+    """Internal utility to parse sniffer command argv.
+
+    Arguments:
+        arg: The arguments to the command. The format is:
+             [locus-regexp [name-regexp]]
+
+    Returns:
+        A 2-tuple of compiled regular expressions.
+
+    Raises:
+        SyntaxError: an error processing ARG
+    """
+
+    argv = gdb.string_to_argv(arg)
+    argc = len(argv)
+    if argc > 2:
+        raise SyntaxError("Too many arguments.")
+    locus_regexp = ""
+    name_regexp = ""
+    if argc >= 1:
+        locus_regexp = argv[0]
+        if argc >= 2:
+            name_regexp = argv[1]
+    return (validate_regexp(locus_regexp, "locus"),
+            validate_regexp(name_regexp, "sniffer"))
+
+
+class InfoSniffer(gdb.Command):
+    """GDB command to list sniffers.
+
+    Usage: info sniffer [locus-regexp [name-regexp]]
+
+    LOCUS-REGEXP is a regular expression matching the location of the
+    sniffer.  If it is omitted, all registered sniffers from all loci
+    are listed.  A locus could be 'global', a regular expression
+    matching the current program space's filename, or a regular
+    expression matching filenames of objfiles.  Locus could be
+    'progspace' to specify that only sniffers from the current
+    progspace should be listed.
+
+    NAME-REGEXP is a regular expression to filter sniffer names.
+    If this omitted for a specified locus, then all registered
+    sniffers in the locus are listed.
+    """
+
+    def __init__(self):
+        super(InfoSniffer, self).__init__("info sniffer",
+                                          gdb.COMMAND_DATA)
+
+    def list_sniffers(self, title, sniffers, name_re):
+        """Lists the sniffers whose name matches regexp.
+
+        Arguments:
+            title: The line to print before the list.
+            sniffers: The list of the sniffers.
+            name_re: sniffer name filter.
+        """
+        if not sniffers:
+            return
+        print title
+        for sniffer in sniffers:
+            if name_re.match(sniffer.name):
+                print("  %s%s" % (sniffer.name,
+                                  "" if sniffer.enabled else "[disabled]"))
+
+    def invoke(self, arg, from_tty):
+        locus_re, name_re = parse_sniffer_command_args(arg)
+        if locus_re.match("global"):
+            self.list_sniffers("global sniffers:", gdb.frame_sniffers,
+                               name_re)
+        if locus_re.match("progspace"):
+            cp = gdb.current_progspace()
+            self.list_sniffers("progspace %s sniffers:" % cp.filename,
+                               cp.frame_sniffers, name_re)
+        for objfile in gdb.objfiles():
+            if locus_re.match(objfile.filename):
+                self.list_sniffers("objfile %s sniffers:" % objfile.filename,
+                                   objfile.frame_sniffers, name_re)
+
+
+def do_enable_sniffer1(sniffers, name_re, flag):
+    """Enable/disable sniffers whose names match given regex.
+
+    Arguments:
+        sniffers: The list of sniffers.
+        name_re: Sniffer name filter.
+        flag: Enable/disable.
+
+    Returns:
+        The number of sniffers affected.
+    """
+    total = 0
+    for sniffer in sniffers:
+        if name_re.match(sniffer.name):
+            sniffer.enabled = flag
+            total += 1
+    return total
+
+
+def do_enable_sniffer(arg, flag):
+    """Enable/disable sniffer(s)."""
+    (locus_re, name_re) = parse_sniffer_command_args(arg)
+    total = 0
+    if locus_re.match("global"):
+        total += do_enable_sniffer1(gdb.frame_sniffers, name_re, flag)
+    if locus_re.match("progspace"):
+        total += do_enable_sniffer1(gdb.current_progspace().frame_sniffers,
+                                    name_re, flag)
+    for objfile in gdb.objfiles():
+        if locus_re.match(objfile.filename):
+            total += do_enable_sniffer1(objfile.frame_sniffers, name_re,
+                                        flag)
+    print("%d sniffer%s %s" % (total, "" if total == 1 else "s",
+                               "enabled" if flag else "disabled"))
+
+
+class EnableSniffer(gdb.Command):
+    """GDB command to enable sniffers.
+
+    Usage: enable sniffer [locus-regexp [name-regexp]]
+
+    LOCUS-REGEXP is a regular expression matching the objects to examine.
+    Loci are "global", the program space's file, and the objfiles within
+    that program space.
+
+    NAME_REGEXP is a regular expression to filter sniffer names.
+    If this omitted for a specified locus, then all registered
+    sniffers in the locus are affected.
+    """
+
+    def __init__(self):
+        super(EnableSniffer, self).__init__("enable sniffer",
+                                            gdb.COMMAND_DATA)
+
+    def invoke(self, arg, from_tty):
+        """GDB calls this to perform the command."""
+        do_enable_sniffer(arg, True)
+
+
+class DisableSniffer(gdb.Command):
+    """GDB command to disable the specified sniffer.
+
+    Usage: disable sniffer [locus-regexp [name-regexp]]
+
+    LOCUS-REGEXP is a regular expression matching the objects to examine.
+    Loci are "global", the program space's file, and the objfiles within
+    that program space.
+
+    NAME_REGEXP is a regular expression to filter sniffer names.
+    If this omitted for a specified locus, then all registered
+    sniffers in the locus are affected.
+    """
+
+    def __init__(self):
+        super(DisableSniffer, self).__init__("disable sniffer",
+                                             gdb.COMMAND_DATA)
+
+    def invoke(self, arg, from_tty):
+        """GDB calls this to perform the command."""
+        do_enable_sniffer(arg, False)
+
+
+def register_sniffer_commands():
+    """Installs the sniffer commands."""
+    InfoSniffer()
+    EnableSniffer()
+    DisableSniffer()
+
+
+register_sniffer_commands()
diff --git a/gdb/python/lib/gdb/function/sniffers.py b/gdb/python/lib/gdb/function/sniffers.py
new file mode 100644
index 0000000..9bdca62
--- /dev/null
+++ b/gdb/python/lib/gdb/function/sniffers.py
@@ -0,0 +1,53 @@ 
+# Copyright (C) 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Internal functions for working with frame sniffers."""
+
+import gdb
+
+
+def execute_sniffers(sniffer_info):
+    """Internal function called from GDB to execute all sniffers.
+
+    Runs each currently enabled sniffer until it finds the one that can
+    unwind given frame.
+
+    Arguments:
+        sniffer_info: an instance of gdb.SnifferInfo.
+    Returns:
+        Unwind info or None. See the description of the value returned
+        by Sniffer.__call__ below.
+    """
+    for objfile in gdb.objfiles():
+        for sniffer in objfile.frame_sniffers:
+            if sniffer.enabled:
+                unwind_info = sniffer.__call__(sniffer_info)
+                if unwind_info is not None:
+                    return unwind_info
+
+    current_progspace = gdb.current_progspace()
+    for sniffer in current_progspace.frame_sniffers:
+        if sniffer.enabled:
+            unwind_info = sniffer.__call__(sniffer_info)
+            if unwind_info is not None:
+                return unwind_info
+
+    for sniffer in gdb.frame_sniffers:
+        if sniffer.enabled:
+            unwind_info = sniffer.__call__(sniffer_info)
+            if unwind_info is not None:
+                return unwind_info
+
+    return None
diff --git a/gdb/python/lib/gdb/sniffer.py b/gdb/python/lib/gdb/sniffer.py
new file mode 100644
index 0000000..fbd519d
--- /dev/null
+++ b/gdb/python/lib/gdb/sniffer.py
@@ -0,0 +1,89 @@ 
+# Copyright (C) 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Sniffer class and register_sniffer function."""
+
+import gdb
+
+
+class Sniffer(object):
+    """Base class (or a template) for frame sniffers written in Python.
+
+    A sniffer has a single method __call__ and the attributes described below.
+
+    Attributes:
+        name: The name of the sniffer.
+        enabled: A boolean indicating whether the sniffer is enabled.
+    """
+
+    def __init__(self, name):
+        """Constructor.
+
+        Args:
+            name: An identifying name for the sniffer.
+        """
+        self.name = name
+        self.enabled = True
+
+    def __call__(self, sniffer_info):
+        """GDB calls this method to unwind a frame.
+
+        Arguments:
+            sniffer_info: An instance of gdb.SnifferInfo describing the frame.
+
+        Returns:
+	    gdb.UnwindInfo instance.
+        """
+        raise NotImplementedError("Sniffer __call__.")
+
+
+def register_sniffer(locus, sniffer, replace=False):
+    """Register sniffer in given locus.
+
+    The sniffer is prepended to the locus's sniffers list. Sniffer
+    name should be unique.
+
+    Arguments:
+        locus: Either an objfile, progspace, or None (in which case
+               the sniffer is registered globally).
+        sniffer: An object of a gdb.Sniffer subclass
+        replace: If True, replaces existing sniffer with the same name.
+                 Otherwise, raises exception if sniffer with the same
+                 name already exists.
+
+    Returns:
+        Nothing.
+
+    Raises:
+        RuntimeError: Sniffer name is not unique.
+
+    """
+    if locus is None:
+        if gdb.parameter("verbose"):
+            gdb.write("Registering global %s sniffer ...\n" % sniffer.name)
+        locus = gdb
+    else:
+        if gdb.parameter("verbose"):
+            gdb.write("Registering %s sniffer for %s ...\n" %
+                      (sniffer.name, locus.filename))
+    i = 0
+    for needle in locus.frame_sniffers:
+        if needle.name == sniffer.name:
+            if replace:
+                del locus.frame_sniffers[i]
+            else:
+                raise RuntimeError("Sniffer %s already exists." % sniffer.name)
+        i += 1
+    locus.frame_sniffers.insert(0, sniffer)
diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index 0aecaf6..a7bb7cd 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -42,6 +42,10 @@  typedef struct
 
   /* The frame filter list of functions.  */
   PyObject *frame_filters;
+
+  /* The frame sniffers list of functions.  */
+  PyObject *frame_sniffers;
+
   /* The type-printer list.  */
   PyObject *type_printers;
 
@@ -181,6 +185,7 @@  objfpy_dealloc (PyObject *o)
   Py_XDECREF (self->dict);
   Py_XDECREF (self->printers);
   Py_XDECREF (self->frame_filters);
+  Py_XDECREF (self->frame_sniffers);
   Py_XDECREF (self->type_printers);
   Py_XDECREF (self->xmethods);
   Py_TYPE (self)->tp_free (self);
@@ -203,6 +208,10 @@  objfpy_initialize (objfile_object *self)
   if (self->frame_filters == NULL)
     return 0;
 
+  self->frame_sniffers = PyList_New (0);
+  if (self->frame_sniffers == NULL)
+    return 0;
+
   self->type_printers = PyList_New (0);
   if (self->type_printers == NULL)
     return 0;
@@ -310,6 +319,48 @@  objfpy_set_frame_filters (PyObject *o, PyObject *filters, void *ignore)
   return 0;
 }
 
+/* Return the frame sniffers attribute for this object file.  */
+
+PyObject *
+objfpy_get_frame_sniffers (PyObject *o, void *ignore)
+{
+  objfile_object *self = (objfile_object *) o;
+
+  Py_INCREF (self->frame_sniffers);
+  return self->frame_sniffers;
+}
+
+/* Set this object file's frame sniffers list to SNIFFERS.  */
+
+static int
+objfpy_set_frame_sniffers (PyObject *o, PyObject *sniffers, void *ignore)
+{
+  PyObject *tmp;
+  objfile_object *self = (objfile_object *) o;
+
+  if (!sniffers)
+    {
+      PyErr_SetString (PyExc_TypeError,
+		       _("Cannot delete the frame sniffers attribute."));
+      return -1;
+    }
+
+  if (!PyList_Check (sniffers))
+    {
+      PyErr_SetString (PyExc_TypeError,
+		       _("The frame_sniffers attribute must be a list."));
+      return -1;
+    }
+
+  /* Take care in case the LHS and RHS are related somehow.  */
+  tmp = self->frame_sniffers;
+  Py_INCREF (sniffers);
+  self->frame_sniffers = sniffers;
+  Py_XDECREF (tmp);
+
+  return 0;
+}
+
 /* Get the 'type_printers' attribute.  */
 
 static PyObject *
@@ -645,6 +696,8 @@  static PyGetSetDef objfile_getset[] =
     "Pretty printers.", NULL },
   { "frame_filters", objfpy_get_frame_filters,
     objfpy_set_frame_filters, "Frame Filters.", NULL },
+  { "frame_sniffers", objfpy_get_frame_sniffers,
+    objfpy_set_frame_sniffers, "Frame Sniffers", NULL },
   { "type_printers", objfpy_get_type_printers, objfpy_set_type_printers,
     "Type printers.", NULL },
   { "xmethods", objfpy_get_xmethods, NULL,
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index 29b9f96..ce85b1a 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -41,6 +41,10 @@  typedef struct
 
   /* The frame filter list of functions.  */
   PyObject *frame_filters;
+
+  /* The frame sniffer list.  */
+  PyObject *frame_sniffers;
+
   /* The type-printer list.  */
   PyObject *type_printers;
 
@@ -82,6 +86,7 @@  pspy_dealloc (PyObject *self)
   Py_XDECREF (ps_self->dict);
   Py_XDECREF (ps_self->printers);
   Py_XDECREF (ps_self->frame_filters);
+  Py_XDECREF (ps_self->frame_sniffers);
   Py_XDECREF (ps_self->type_printers);
   Py_XDECREF (ps_self->xmethods);
   Py_TYPE (self)->tp_free (self);
@@ -104,6 +109,10 @@  pspy_initialize (pspace_object *self)
   if (self->frame_filters == NULL)
     return 0;
 
+  self->frame_sniffers = PyList_New (0);
+  if (self->frame_sniffers == NULL)
+    return 0;
+
   self->type_printers = PyList_New (0);
   if (self->type_printers == NULL)
     return 0;
@@ -211,6 +220,48 @@  pspy_set_frame_filters (PyObject *o, PyObject *frame, void *ignore)
   return 0;
 }
 
+/* Return the list of the frame sniffers for this program space.  */
+
+PyObject *
+pspy_get_frame_sniffers (PyObject *o, void *ignore)
+{
+  pspace_object *self = (pspace_object *) o;
+
+  Py_INCREF (self->frame_sniffers);
+  return self->frame_sniffers;
+}
+
+/* Set this program space's list of the sniffers to SNIFFERS.  */
+
+static int
+pspy_set_frame_sniffers (PyObject *o, PyObject *sniffers, void *ignore)
+{
+  PyObject *tmp;
+  pspace_object *self = (pspace_object *) o;
+
+  if (!sniffers)
+    {
+      PyErr_SetString (PyExc_TypeError,
+		       "cannot delete the frame sniffers list");
+      return -1;
+    }
+
+  if (!PyList_Check (sniffers))
+    {
+      PyErr_SetString (PyExc_TypeError,
+		       "the frame sniffers attribute must be a list");
+      return -1;
+    }
+
+  /* Take care in case the LHS and RHS are related somehow.  */
+  tmp = self->frame_sniffers;
+  Py_INCREF (sniffers);
+  self->frame_sniffers = sniffers;
+  Py_XDECREF (tmp);
+
+  return 0;
+}
+
 /* Get the 'type_printers' attribute.  */
 
 static PyObject *
@@ -345,6 +396,8 @@  static PyGetSetDef pspace_getset[] =
     "Pretty printers.", NULL },
   { "frame_filters", pspy_get_frame_filters, pspy_set_frame_filters,
     "Frame filters.", NULL },
+  { "frame_sniffers", pspy_get_frame_sniffers, pspy_set_frame_sniffers,
+    "Frame sniffers.", NULL },
   { "type_printers", pspy_get_type_printers, pspy_set_type_printers,
     "Type printers.", NULL },
   { "xmethods", pspy_get_xmethods, NULL,
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
new file mode 100644
index 0000000..3e49766
--- /dev/null
+++ b/gdb/python/py-unwind.c
@@ -0,0 +1,743 @@ 
+/* Python frame unwinder interface.
+
+   Copyright (C) 2015 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include "arch-utils.h"
+#include "frame-unwind.h"
+#include "gdb_obstack.h"
+#include "gdbcmd.h"
+#include "language.h"
+#include "observer.h"
+#include "python-internal.h"
+#include "regcache.h"
+#include "valprint.h"
+#include "user-regs.h"
+
+#define TRACE_PY_UNWIND(level, args...) if (pyuw_debug >= level)  \
+  { fprintf_unfiltered (gdb_stdlog, args); }
+
+typedef struct
+{
+  PyObject_HEAD
+  struct frame_info *frame_info;
+} sniffer_info_object;
+
+/* The data we keep for the PyUnwindInfo: sniffer_info, previous
+ * frame's register set and frame ID.  */
+
+typedef struct
+{
+  PyObject_HEAD
+
+  /* gdb.SnifferInfo for the frame we are unwinding.  */
+  PyObject *sniffer_info;
+
+  /* Previous frame registers tuple (2-tuple per register).  */
+  PyObject *previous_frame_registers;
+
+  /* Its ID.  */
+  struct frame_id frame_id;
+} unwind_info_object;
+
+/* The data we keep for a frame we can unwind: frame ID and an array of
+   (register_number, register_value) pairs.  */
+
+typedef struct
+{
+  /* Frame ID.  */
+  struct frame_id frame_id;
+
+  /* GDB Architecture.  */
+  struct gdbarch *gdbarch;
+
+  /* Length of the `reg' array below.  */
+  int reg_count;
+
+  struct reg_info
+  {
+    /* Register number.  */
+    int number;
+
+    /* Register data bytes pointer.  */
+    gdb_byte *data;
+  } reg[];
+} cached_frame_info;
+
+static PyTypeObject sniffer_info_object_type
+    CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("sniffer_info_object");
+
+static PyTypeObject unwind_info_object_type
+    CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("unwind_info_object");
+
+static unsigned int pyuw_debug = 0;
+
+static struct gdbarch_data *pyuw_gdbarch_data;
+
+/* Parse Python Int, saving it at the given address. Returns 1 on success,
+   0 otherwise.  */
+
+static int
+pyuw_parse_int (PyObject *pyo_int, int *valuep)
+{
+  long long_value;
+
+  if (pyo_int == NULL)
+    return 0;
+  /* Make a long logic check first.  In Python 3.x, internally, all
+     integers are represented as longs.  In Python 2.x, there is still
+     a differentiation internally between a PyInt and a PyLong.
+     Explicitly do this long check conversion first. In GDB, for
+     Python 3.x, we #ifdef PyInt = PyLong.  This check has to be done
+     first to ensure we do not lose information in the conversion
+     process.  */
+  else if (PyLong_Check (pyo_int))
+    {
+      LONGEST l = PyLong_AsLongLong (pyo_int);
+
+      if (PyErr_Occurred ())
+        return 0;
+      long_value = (long)l;
+      if (l != long_value)
+        return 0;
+    }
+  else if (PyInt_Check (pyo_int))
+    {
+      long_value = PyInt_AsLong (pyo_int);
+      if (PyErr_Occurred ())
+        return 0;
+    }
+  else
+    return 0;
+  if (long_value != (int) long_value)
+    return 0;
+  *valuep = (int) long_value;
+  return 1;
+}
+
+/* Parses (REGNUM, REGVALUE) tuple.  Returns 1 on success, 0 otherwise.  */
+static int
+pyuw_parse_register_tuple(PyObject *pyo_reg,
+                              int *reg_num, struct value **reg_value)
+{
+
+  if (pyo_reg != NULL && PyTuple_Check (pyo_reg)
+      && PyTuple_Size (pyo_reg) == 2
+      && pyuw_parse_int (PyTuple_GetItem (pyo_reg, 0), reg_num))
+    {
+      PyObject *pyo_reg_value = PyTuple_GetItem (pyo_reg, 1);
+      if (pyo_reg_value != NULL
+          && (*reg_value = value_object_to_value (pyo_reg_value)) != NULL)
+        return 1;
+    }
+  return 0;
+}
+
+/* Called by the Python interpreter to obtain string representation
+   of the SnifferInfo object.  */
+
+static PyObject *
+sniffer_infopy_str (PyObject *self)
+{
+  char *s;
+  PyObject *result;
+  struct frame_info *frame = ((sniffer_info_object *)self)->frame_info;
+
+  if (frame == NULL)
+    return PyString_FromString ("Stale SnifferInfo instance");
+  s = xstrprintf ("SP=%s,PC=%s", core_addr_to_string_nz (get_frame_sp (frame)),
+        core_addr_to_string_nz (get_frame_pc (frame)));
+  result = PyString_FromString (s);
+  xfree (s);
+
+  return result;
+}
+
+/* Implementation of gdb.SnifferInfo.read_register (self, regnum) -> gdb.Value.
+   Returns the value of register REGNUM as gdb.Value instance.  */
+
+static PyObject *
+sniffer_infopy_read_register (PyObject *self, PyObject *args)
+{
+  volatile struct gdb_exception except;
+  int regnum;
+  struct value *val = NULL;
+
+  if (!PyArg_ParseTuple (args, "i", &regnum))
+    return NULL;
+  if (((sniffer_info_object *) self)->frame_info == NULL)
+    error (_("Attempting to read register from stale SnifferInfo"));
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      struct frame_info *frame = ((sniffer_info_object *) self)->frame_info;
+      struct gdbarch *gdbarch = get_frame_arch (frame);
+      char *error_text = NULL;
+
+      /* Validate regnum to prevent assert in `regcache_cooked_read_value'.  */
+      if (user_reg_map_regnum_to_name (gdbarch, regnum) == NULL)
+        error_text = xstrprintf (_("Bad register number: %d."), regnum);
+      else
+        {
+          gdb_byte buffer[MAX_REGISTER_SIZE];
+
+          /* Call `deprecated_frame_register_read' -- calling
+             `value_of_register' would an assert in `get_frame_id'
+             because our frame is incomplete.  */
+          if (deprecated_frame_register_read (frame, regnum, buffer))
+            val = value_from_contents (register_type (gdbarch, regnum),
+                                       buffer);
+          if (val == NULL)
+            error_text = xstrprintf (_("Cannot read register %d from frame."),
+                                     regnum);
+        }
+      if (error_text != NULL)
+        {
+          PyErr_SetString (PyExc_ValueError, error_text);
+          xfree (error_text);
+        }
+    }
+  GDB_PY_HANDLE_EXCEPTION (except);
+
+  return val == NULL ? NULL : value_to_value_object (val);
+}
+
+/* Create Python SnifferInfo object.  */
+
+static PyObject *
+frame_info_to_sniffer_info_object (struct frame_info *frame)
+{
+  sniffer_info_object *sniffer_info
+      = PyObject_New (sniffer_info_object, &sniffer_info_object_type);
+
+  sniffer_info->frame_info = frame;
+  return (PyObject *) sniffer_info;
+}
+
+/* Invalidate SnifferInfo object.  */
+static void
+sniffer_info_invalidate (PyObject *pyo_sniffer_info)
+{
+  if (pyo_sniffer_info == NULL)
+    return;
+  ((sniffer_info_object *) pyo_sniffer_info)->frame_info = NULL;
+}
+
+/* frame_unwind.this_id method.  */
+
+static void
+pyuw_this_id (struct frame_info *this_frame, void **cache_ptr,
+              struct frame_id *this_id)
+{
+  *this_id = ((cached_frame_info *) *cache_ptr)->frame_id;
+  if (pyuw_debug >= 1)
+    {
+      fprintf_unfiltered (gdb_stdlog, "%s: frame_id: ", __FUNCTION__);
+      fprint_frame_id (gdb_stdlog, *this_id);
+      fprintf_unfiltered (gdb_stdlog, "\n");
+    }
+}
+
+/* frame_unwind.prev_register.  */
+
+static struct value *
+pyuw_prev_register (struct frame_info *this_frame, void **cache_ptr,
+                    int regnum)
+{
+  cached_frame_info *cached_frame = *cache_ptr;
+  struct reg_info *reg_info = cached_frame->reg;
+  struct reg_info *reg_info_end = reg_info + cached_frame->reg_count;
+
+  TRACE_PY_UNWIND (1, "%s(frame=%p,...,reg=%d)\n", __FUNCTION__, this_frame,
+                   regnum);
+  for (; reg_info < reg_info_end; ++reg_info)
+    {
+      if (regnum == reg_info->number)
+        return frame_unwind_got_bytes (this_frame, regnum, reg_info->data);
+    }
+
+  return frame_unwind_got_optimized (this_frame, regnum);
+}
+
+/* Called by the Python interpreter to obtain string representation
+   if the UnwindInfo object.  */
+static PyObject *
+unwind_infopy_str (PyObject *self)
+{
+  PyObject *result;
+  struct ui_file *strfile = mem_fileopen ();
+  unwind_info_object *pyo_unwind_info = (unwind_info_object *) self;
+
+  fprintf_unfiltered (strfile, "Frame ID: ");
+  fprint_frame_id (strfile, pyo_unwind_info->frame_id);
+  {
+    int reg_count = PyTuple_Size (pyo_unwind_info->previous_frame_registers);
+    int i;
+    char *sep = "";
+    struct value_print_options opts;
+
+    get_user_print_options (&opts);
+    fprintf_unfiltered (strfile, "\nPrevious frame registers: (");
+    for (i = 0; i < reg_count; i++)
+      {
+        int reg_num;
+        struct value *reg_value;
+        PyObject *pyo_reg = PyTuple_GetItem (
+            pyo_unwind_info->previous_frame_registers,i);
+
+        if (pyuw_parse_register_tuple (pyo_reg, &reg_num, &reg_value))
+          {
+            fprintf_unfiltered (strfile, "%s(%d, ", sep, reg_num);
+            value_print (reg_value, strfile, &opts);
+            fprintf_unfiltered (strfile, ")");
+          }
+        else
+          fprintf_unfiltered (strfile, "(BAD)");
+        sep = ", ";
+      }
+    fprintf_unfiltered (strfile, ")");
+  }
+  {
+    char *s = ui_file_xstrdup (strfile, NULL);
+
+    result = PyString_FromString (s);
+    xfree (s);
+  }
+  ui_file_delete (strfile);
+  return result;
+}
+
+/* Convert gdb.Value object to COREADDR.  */
+static int
+pyuw_value_obj_to_pointer (PyObject *pyo_value, CORE_ADDR *addr)
+{
+  struct value *value = value_object_to_value (pyo_value);
+
+  if (value == NULL)
+    return 0;
+  *addr = unpack_pointer (value_type (value), value_contents (value));
+  return 1;
+}
+
+/* Implementation of gdb.UnwindInfo.frame_id_build (self, SP, PC) -> None.  */
+
+static PyObject *
+unwind_infopy_frame_id_build (PyObject *self, PyObject *args)
+{
+  PyObject *pyo_sp;
+  PyObject *pyo_pc;
+  CORE_ADDR sp;
+  CORE_ADDR pc;
+
+  if (!PyArg_ParseTuple (args, "OO:frame_id_build", &pyo_sp, &pyo_pc)
+       || !pyuw_value_obj_to_pointer (pyo_sp, &sp)
+       || !pyuw_value_obj_to_pointer (pyo_pc, &pc))
+    return NULL;
+
+  ((unwind_info_object *) self)->frame_id = frame_id_build (sp, pc);
+  Py_INCREF (Py_None);
+  return Py_None;
+}
+
+/* Implementation of
+   gdb.UnwindInfo.frame_id_build_special (self, SP, PC, SPECIAL) -> None.  */
+
+static PyObject *
+unwind_infopy_frame_id_build_special (PyObject *self, PyObject *args)
+{
+  PyObject *pyo_sp;
+  PyObject *pyo_pc;
+  PyObject *pyo_special;
+  CORE_ADDR sp;
+  CORE_ADDR pc;
+  CORE_ADDR special;
+
+  if (!PyArg_ParseTuple (args, "OOO:frame_id_build_special",
+                          &pyo_sp, &pyo_pc, &pyo_special)
+      || !pyuw_value_obj_to_pointer (pyo_sp, &sp)
+      || !pyuw_value_obj_to_pointer (pyo_pc, &pc)
+      || !pyuw_value_obj_to_pointer (pyo_special, &special))
+    return NULL;
+
+  ((unwind_info_object *) self)->frame_id
+              = frame_id_build_special (sp, pc, special);
+  Py_INCREF (Py_None);
+  return Py_None;
+}
+
+/* Implementation of gdb.UnwindInfo.frame_build_id_wild (self, SP) -> None.  */
+
+static PyObject *
+unwind_infopy_frame_id_build_wild (PyObject *self, PyObject *args)
+{
+  PyObject *pyo_sp;
+  CORE_ADDR sp;
+
+  if (!PyArg_ParseTuple (args, "O:frame_id_build_wild", &pyo_sp)
+      || !pyuw_value_obj_to_pointer (pyo_sp, &sp))
+    return NULL;
+
+  ((unwind_info_object *) self)->frame_id = frame_id_build_wild (sp);
+  Py_INCREF (Py_None);
+  return Py_None;
+}
+
+/* Initialize new UnwindInfo object.  */
+static int
+unwind_infopy_init (PyObject *self, PyObject *args, PyObject *kwargs)
+{
+  PyObject *pyo_sniffer_info;
+  PyObject *pyo_regs;
+
+  if (PyArg_UnpackTuple (args, "__init__", 2, 2, &pyo_sniffer_info, &pyo_regs)
+      && (pyo_sniffer_info != NULL
+      && PyObject_IsInstance (pyo_sniffer_info,
+                              (PyObject *) &sniffer_info_object_type) > 0)
+      && pyo_regs != NULL
+      && PyTuple_Check (pyo_regs))
+    {
+      unwind_info_object *unwind_info = (unwind_info_object *) self;
+      unwind_info->frame_id = null_frame_id;
+      unwind_info->sniffer_info = pyo_sniffer_info;
+      Py_INCREF (pyo_sniffer_info);
+      unwind_info->previous_frame_registers = pyo_regs;
+      Py_INCREF (pyo_regs);
+      return 0;
+    }
+  return -1;
+}
+
+static void
+unwind_infopy_dealloc (PyObject *self)
+{
+  unwind_info_object *unwind_info = (unwind_info_object *) self;
+  Py_XDECREF (unwind_info->sniffer_info);
+  Py_XDECREF (unwind_info->previous_frame_registers);
+  Py_TYPE (self)->tp_free (self);
+}
+
+/* Frame sniffer dispatch.  */
+
+static int
+pyuw_sniffer (const struct frame_unwind *self, struct frame_info *this_frame,
+              void **cache_ptr)
+{
+  struct gdbarch *gdbarch;
+  struct cleanup *cleanups;
+  struct cleanup *cached_frame_cleanups;
+  PyObject *pyo_module;
+  PyObject *pyo_execute;
+  PyObject *pyo_sniffer_info;
+  PyObject *pyo_unwind_info;
+  cached_frame_info *cached_frame = NULL;
+
+  gdbarch = (struct gdbarch *) (self->unwind_data);
+  cleanups = ensure_python_env (gdbarch, current_language);
+  TRACE_PY_UNWIND (3, "%s(SP=%s, PC=%s)\n", __FUNCTION__,
+                   paddress (gdbarch, get_frame_sp (this_frame)),
+                   paddress (gdbarch, get_frame_pc (this_frame)));
+  pyo_sniffer_info = frame_info_to_sniffer_info_object (this_frame);
+  if (pyo_sniffer_info == NULL)
+    goto error;
+  make_cleanup_py_decref (pyo_sniffer_info);
+
+  if ((pyo_module = PyImport_ImportModule ("gdb.function.sniffers")) == NULL)
+    goto error;
+  make_cleanup_py_decref (pyo_module);
+
+  pyo_execute = PyObject_GetAttrString (pyo_module, "execute_sniffers");
+  if (pyo_execute == NULL)
+    goto error;
+  make_cleanup_py_decref (pyo_execute);
+
+  pyo_unwind_info
+      = PyObject_CallFunctionObjArgs (pyo_execute, pyo_sniffer_info, NULL);
+  if (pyo_unwind_info == NULL)
+    goto error;
+  make_cleanup_py_decref (pyo_unwind_info);
+  if (pyo_unwind_info == Py_None)
+    goto error;
+  if (PyObject_IsInstance (pyo_unwind_info,
+                           (PyObject *) &unwind_info_object_type) <= 0)
+    error (_("A Sniffer should return gdb.UnwindInfo instance."));
+
+  {
+    unwind_info_object *unwind_info = (unwind_info_object *) pyo_unwind_info;
+    PyObject *pyo_registers = unwind_info->previous_frame_registers;
+    int i;
+    int reg_count;
+    size_t cached_frame_size;
+    size_t gdb_bytes_count;
+    gdb_byte *gdb_data_free, *gdb_data_end;
+
+    if (frame_id_eq (unwind_info->frame_id, null_frame_id))
+      error (_("Sniffer should call one of the frame_id_build methods "
+               "on UnwindInfo instance before returning it."));
+    if (pyo_registers == NULL)
+      goto error;
+    if (!PyTuple_Check (pyo_registers))
+      error (_("Registers passed to UnwindInfo constructor should be a tuple."));
+
+    /* Figure out how much space we need to allocate.  */
+    reg_count = PyTuple_Size (pyo_registers);
+    if (reg_count <= 0)
+      error (_("Register list should not be empty."));
+    /* We might overestimate `gdb_bytes_count', but it's not worth
+       parsing register numbers twice to calculate the exact number of
+       bytes needed.  */
+    gdb_bytes_count = reg_count * MAX_REGISTER_SIZE;
+    cached_frame_size = sizeof (*cached_frame) +
+        reg_count * sizeof (cached_frame->reg[0]) +
+        gdb_bytes_count * sizeof (gdb_byte);
+
+    cached_frame = xmalloc (cached_frame_size);
+    cached_frame_cleanups = make_cleanup (xfree, cached_frame);
+    /* Allocations after this point will be discarded!  */
+
+    gdb_data_end = (gdb_byte *)((char *) cached_frame + cached_frame_size);
+    gdb_data_free = gdb_data_end - gdb_bytes_count;
+
+    cached_frame->gdbarch = gdbarch;
+    cached_frame->frame_id = unwind_info->frame_id;
+    cached_frame->reg_count = reg_count;
+
+    /* Populate registers array.  */
+    for (i = 0; i < reg_count; i++)
+      {
+        struct reg_info *reg = &(cached_frame->reg[i]);
+        struct value *value;
+        size_t data_size;
+
+        if (!pyuw_parse_register_tuple (PyTuple_GetItem (pyo_registers, i),
+                                        &reg->number, &value))
+          error (_("Python sniffer returned bad register tuple: "
+                   "item #%d is not a (reg_no, reg_data) tuple "
+                   "with integer reg_no and gdb.Value reg_data."), i);
+
+        data_size = register_size (gdbarch, reg->number);
+        if (data_size != TYPE_LENGTH (value_enclosing_type (value)))
+          error (_("The value of the register #%d returned by the "
+                   "Python sniffer has unexpected size: %u instead "
+                   "of %u."), reg->number,
+                 (unsigned)(TYPE_LENGTH (value_enclosing_type (value))),
+                 (unsigned)data_size);
+
+        /* Should not overflow  the buffer.  */
+        gdb_assert ((gdb_data_free + data_size) <= gdb_data_end);
+        memcpy (gdb_data_free, value_contents (value), data_size);
+        reg->data = gdb_data_free;
+        gdb_data_free += data_size;
+      }
+  }
+
+  *cache_ptr = cached_frame;
+  discard_cleanups (cached_frame_cleanups);
+  do_cleanups (cleanups);
+  sniffer_info_invalidate (pyo_sniffer_info);
+  return 1;
+
+error:
+  do_cleanups (cleanups);
+  sniffer_info_invalidate (pyo_sniffer_info);
+  return 0;
+}
+
+/* Frame cache release shim.  */
+
+static void
+pyuw_dealloc_cache (struct frame_info *this_frame, void *cache)
+{
+  TRACE_PY_UNWIND (3, "%s: enter", __FUNCTION__);
+  xfree (cache);
+}
+
+struct pyuw_gdbarch_data_type
+{
+  /* Has the unwinder shim been prepended? */
+  int unwinder_registered;
+};
+
+static void *
+pyuw_gdbarch_data_init (struct gdbarch *gdbarch)
+{
+  return GDBARCH_OBSTACK_ZALLOC (gdbarch, struct pyuw_gdbarch_data_type);
+}
+
+/* New inferior architecture callback: register the Python sniffers
+   intermediary.  */
+
+static void
+pyuw_on_new_gdbarch (struct gdbarch *newarch)
+{
+  struct pyuw_gdbarch_data_type *data =
+      gdbarch_data (newarch, pyuw_gdbarch_data);
+
+  if (!data->unwinder_registered)
+    {
+      struct frame_unwind *unwinder
+          = GDBARCH_OBSTACK_ZALLOC (newarch, struct frame_unwind);
+
+      unwinder->type = NORMAL_FRAME;
+      unwinder->stop_reason = default_frame_unwind_stop_reason;
+      unwinder->this_id = pyuw_this_id;
+      unwinder->prev_register = pyuw_prev_register;
+      unwinder->unwind_data = (void *) newarch;
+      unwinder->sniffer = pyuw_sniffer;
+      unwinder->dealloc_cache = pyuw_dealloc_cache;
+      frame_unwind_prepend_unwinder (newarch, unwinder);
+      TRACE_PY_UNWIND (1, "%s: registered unwinder for %s\n", __FUNCTION__,
+                       gdbarch_bfd_arch_info (newarch)->printable_name);
+      data->unwinder_registered = 1;
+    }
+}
+
+/* Initialize unwind machinery.  */
+
+int
+gdbpy_initialize_unwind (void)
+{
+  int rc;
+  add_setshow_zuinteger_cmd
+      ("py-unwind", class_maintenance, &pyuw_debug,
+        _("Set Python unwinder debugging."),
+        _("Show Python unwinder debugging."),
+        _("When non-zero, Pythin unwinder debugging is enabled."),
+        NULL,
+        NULL,
+        &setdebuglist, &showdebuglist);
+  pyuw_gdbarch_data
+      = gdbarch_data_register_post_init (pyuw_gdbarch_data_init);
+  observer_attach_architecture_changed (pyuw_on_new_gdbarch);
+  if (PyType_Ready (&sniffer_info_object_type) < 0)
+    return -1;
+  rc = gdb_pymodule_addobject (gdb_module, "SnifferInfo",
+      (PyObject *) &sniffer_info_object_type);
+  if (rc)
+    return rc;
+
+  unwind_info_object_type.tp_new = PyType_GenericNew;
+  if (PyType_Ready (&unwind_info_object_type) < 0)
+    return -1;
+  return gdb_pymodule_addobject (gdb_module, "UnwindInfo",
+      (PyObject *) &unwind_info_object_type);
+}
+
+static PyMethodDef sniffer_info_object_methods[] =
+{
+  { "read_register", sniffer_infopy_read_register, METH_VARARGS,
+    "read_register (REGNUM) -> gdb.Value\n\
+Return the value of the REGNUM in the frame." },
+  {NULL}  /* Sentinel */
+};
+
+static PyTypeObject sniffer_info_object_type =
+{
+  PyVarObject_HEAD_INIT (NULL, 0)
+  "gdb.SnifferInfo",              /* tp_name */
+  sizeof (sniffer_info_object),   /* tp_basicsize */
+  0,                              /* tp_itemsize */
+  0,                              /* tp_dealloc */
+  0,                              /* tp_print */
+  0,                              /* tp_getattr */
+  0,                              /* tp_setattr */
+  0,                              /* tp_compare */
+  0,                              /* tp_repr */
+  0,                              /* tp_as_number */
+  0,                              /* tp_as_sequence */
+  0,                              /* tp_as_mapping */
+  0,                              /* tp_hash  */
+  0,                              /* tp_call */
+  sniffer_infopy_str,             /* tp_str */
+  0,                              /* tp_getattro */
+  0,                              /* tp_setattro */
+  0,                              /* tp_as_buffer */
+  Py_TPFLAGS_DEFAULT,             /* tp_flags */
+  "GDB snifferInfo object",       /* tp_doc */
+  0,                              /* tp_traverse */
+  0,                              /* tp_clear */
+  0,                              /* tp_richcompare */
+  0,                              /* tp_weaklistoffset */
+  0,                              /* tp_iter */
+  0,                              /* tp_iternext */
+  sniffer_info_object_methods,    /* tp_methods */
+  0,                              /* tp_members */
+  0,                              /* tp_getset */
+  0,                              /* tp_base */
+  0,                              /* tp_dict */
+  0,                              /* tp_descr_get */
+  0,                              /* tp_descr_set */
+  0,                              /* tp_dictoffset */
+  0,                              /* tp_init */
+  0,                              /* tp_alloc */
+};
+
+
+static PyMethodDef unwind_info_object_methods[] =
+{
+  { "frame_id_build", unwind_infopy_frame_id_build, METH_VARARGS,
+    "frame_id_build (SP, PC) -> None\n"
+    "Build frame ID from given SP and PC." },
+  { "frame_id_build_special", unwind_infopy_frame_id_build_special,
+    METH_VARARGS,
+    "frame_id_build_special (SP, PC, SPECIAL) -> None\n"
+    "Build frame ID from given SP, PC and SPECIAL." },
+  { "frame_id_build_wild", unwind_infopy_frame_id_build_wild, METH_VARARGS,
+    "frame_id_build_wild (SP) -> None\nBuild frame ID from given SP." },
+  { NULL }  /* Sentinel */
+};
+
+static PyTypeObject unwind_info_object_type =
+{
+  PyVarObject_HEAD_INIT (NULL, 0)
+  "gdb.UnwindInfo",               /* tp_name */
+  sizeof (unwind_info_object),    /* tp_basicsize */
+  0,                              /* tp_itemsize */
+  unwind_infopy_dealloc,          /* tp_dealloc */
+  0,                              /* tp_print */
+  0,                              /* tp_getattr */
+  0,                              /* tp_setattr */
+  0,                              /* tp_compare */
+  0,                              /* tp_repr */
+  0,                              /* tp_as_number */
+  0,                              /* tp_as_sequence */
+  0,                              /* tp_as_mapping */
+  0,                              /* tp_hash  */
+  0,                              /* tp_call */
+  unwind_infopy_str,              /* tp_str */
+  0,                              /* tp_getattro */
+  0,                              /* tp_setattro */
+  0,                              /* tp_as_buffer */
+  Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,  /* tp_flags */
+  "GDB UnwindInfo object",        /* tp_doc */
+  0,                              /* tp_traverse */
+  0,                              /* tp_clear */
+  0,                              /* tp_richcompare */
+  0,                              /* tp_weaklistoffset */
+  0,                              /* tp_iter */
+  0,                              /* tp_iternext */
+  unwind_info_object_methods,     /* tp_methods */
+  0,                              /* tp_members */
+  0,                              /* tp_getset */
+  0,                              /* tp_base */
+  0,                              /* tp_dict */
+  0,                              /* tp_descr_get */
+  0,                              /* tp_descr_set */
+  0,                              /* tp_dictoffset */
+  unwind_infopy_init,             /* tp_init */
+  0,                              /* tp_alloc */
+};
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index a77f5a6..34179de 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -390,12 +390,14 @@  PyObject *pspace_to_pspace_object (struct program_space *)
     CPYCHECKER_RETURNS_BORROWED_REF;
 PyObject *pspy_get_printers (PyObject *, void *);
 PyObject *pspy_get_frame_filters (PyObject *, void *);
+PyObject *pspy_get_frame_sniffers (PyObject *, void *);
 PyObject *pspy_get_xmethods (PyObject *, void *);
 
 PyObject *objfile_to_objfile_object (struct objfile *)
     CPYCHECKER_RETURNS_BORROWED_REF;
 PyObject *objfpy_get_printers (PyObject *, void *);
 PyObject *objfpy_get_frame_filters (PyObject *, void *);
+PyObject *objfpy_get_frame_sniffers (PyObject *, void *);
 PyObject *objfpy_get_xmethods (PyObject *, void *);
 PyObject *gdbpy_lookup_objfile (PyObject *self, PyObject *args, PyObject *kw);
 
@@ -490,6 +492,8 @@  int gdbpy_initialize_arch (void)
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 int gdbpy_initialize_xmethods (void)
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
+int gdbpy_initialize_unwind (void)
+  CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 
 struct cleanup *make_cleanup_py_decref (PyObject *py);
 struct cleanup *make_cleanup_py_xdecref (PyObject *py);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 344d8d2..3e079b5 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1790,7 +1790,8 @@  message == an error message without a stack will be printed."),
       || gdbpy_initialize_new_objfile_event ()  < 0
       || gdbpy_initialize_clear_objfiles_event ()  < 0
       || gdbpy_initialize_arch () < 0
-      || gdbpy_initialize_xmethods () < 0)
+      || gdbpy_initialize_xmethods () < 0
+      || gdbpy_initialize_unwind () < 0)
     goto fail;
 
   gdbpy_to_string_cst = PyString_FromString ("to_string");
diff --git a/gdb/testsuite/gdb.python/py-unwind-maint.c b/gdb/testsuite/gdb.python/py-unwind-maint.c
new file mode 100644
index 0000000..8c1d935
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-unwind-maint.c
@@ -0,0 +1,24 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main (void)
+{
+  int i = 0;
+
+  return i; /* next-line */
+}
diff --git a/gdb/testsuite/gdb.python/py-unwind-maint.exp b/gdb/testsuite/gdb.python/py-unwind-maint.exp
new file mode 100644
index 0000000..a10e1aa
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-unwind-maint.exp
@@ -0,0 +1,64 @@ 
+# Copyright (C) 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the GDB testsuite.  It tests Python-based
+# unwinding CLI.
+
+load_lib gdb-python.exp
+
+standard_testfile
+
+if {[prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+    return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]
+
+if ![runto_main ] then {
+    fail "Can't run to main"
+    return -1
+}
+
+gdb_test "source ${pyfile}" "Python script imported" "import python scripts"
+
+gdb_test_sequence "info sniffer" "Show all sniffers" {
+    "global sniffers:"
+    "  global_sniffer"
+    "progspace.*sniffers:"
+    "py_unwind_maint_ps_sniffer"
+}
+
+gdb_breakpoint ${srcfile}:[gdb_get_line_number "next-line"]
+
+gdb_test_sequence "continue" "Sniffers called" {
+    "py_unwind_maint_ps_sniffer called"
+    "global_sniffer called"
+}
+
+gdb_test "disable sniffer global .*" "1 sniffer disabled" "Sniffer disabled"
+
+gdb_test_sequence "info sniffer" "Show with global sniffer disabled" {
+    "global sniffers:"
+    "  global_sniffer\\[disabled\\]"
+    "progspace.*sniffers:"
+    "  py_unwind_maint_ps_sniffer"
+}
+
+gdb_test_sequence "where" "Global sniffer disabled" {
+    "py_unwind_maint_ps_sniffer called\r\n#0  main"
+}
diff --git a/gdb/testsuite/gdb.python/py-unwind-maint.py b/gdb/testsuite/gdb.python/py-unwind-maint.py
new file mode 100644
index 0000000..74b702c
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-unwind-maint.py
@@ -0,0 +1,59 @@ 
+# Copyright (C) 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the GDB testsuite.  It tests python sniffers.
+
+import re
+import gdb.types
+from gdb.sniffer import Sniffer, register_sniffer
+
+class TestGlobalSniffer(Sniffer):
+    def __init__(self):
+        super(TestGlobalSniffer, self).__init__("global_sniffer")
+
+    def __call__(self, sniffer_info):
+        print "%s called" % self.name
+        return None
+
+class TestProgspaceSniffer(Sniffer):
+    def __init__(self, name):
+        super(TestProgspaceSniffer, self).__init__("%s_ps_sniffer" % name)
+
+    def __call__(self, sniffer_info):
+        print "%s called" % self.name
+        return None
+
+class TestObjfileSniffer(Sniffer):
+    def __init__(self, name):
+        super(TestObjfileSniffer, self).__init__("%s_obj_sniffer" % name)
+
+    def __call__(self, sniffer_info):
+        print "%s called" % self.name
+        return None
+
+
+
+gdb.sniffer.register_sniffer(gdb, TestGlobalSniffer())
+saw_runtime_error = False
+try:
+    gdb.sniffer.register_sniffer(gdb, TestGlobalSniffer(), replace=False)
+except RuntimeError:
+    saw_runtime_error = True
+if not saw_runtime_error:
+    raise RuntimeError("Missing runtime error from register_sniffer.")
+gdb.sniffer.register_sniffer(gdb, TestGlobalSniffer(), replace=True)
+gdb.sniffer.register_sniffer(gdb.current_progspace(),
+                              TestProgspaceSniffer("py_unwind_maint"))
+print "Python script imported"
diff --git a/gdb/testsuite/gdb.python/py-unwind.c b/gdb/testsuite/gdb.python/py-unwind.c
new file mode 100644
index 0000000..cf41d78
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-unwind.c
@@ -0,0 +1,81 @@ 
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2015 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* This is the test program loaded into GDB by the py-unwind test.  */
+
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+static void *
+swap_value (void **location, void *new_value)
+{
+  void *old_value = *location;
+  *location = new_value;
+  return old_value;
+}
+
+static void
+bad_layout(void **variable_ptr, void *fp)
+{
+  fprintf (stderr, "First variable should be allocated one word below "
+           "the frame.  Got variable's address %p, frame at %p instead.\n",
+           variable_ptr, fp);
+  abort();
+}
+
+#define MY_FRAME (__builtin_frame_address (0))
+
+static void
+corrupt_frame_inner (void)
+{
+  /* Save outer frame address, then corrupt the unwind chain by
+     setting the outer frame address in it to self.  This is
+     ABI-specific: the first word of the frame contains previous frame
+     address in amd64.  */
+  void *previous_fp = swap_value ((void **) MY_FRAME, MY_FRAME);
+
+  /* Verify the compiler allocates the first local variable one word
+     below frame.  This is where the test unwinder expects to find the
+     correct outer frame address.  */
+  if (&previous_fp + 1 != (void **) MY_FRAME)
+    bad_layout (&previous_fp + 1, MY_FRAME);
+
+  /* Now restore it so that we can return.  The test sets the
+     breakpoint just before this happens, and GDB will not be able to
+     show the backtrace without JIT reader.  */
+  swap_value ((void **) MY_FRAME, previous_fp); /* break backtrace-broken */
+}
+
+static void
+corrupt_frame_outer (void)
+{
+  /* See above for the explanation of the code here.  This function
+     corrupts its frame, too, and then calls the inner one.  */
+  void *previous_fp = swap_value ((void **) MY_FRAME, MY_FRAME);
+  if (&previous_fp + 1 != (void **) MY_FRAME)
+    bad_layout (&previous_fp, MY_FRAME);
+  corrupt_frame_inner ();
+  swap_value ((void **) MY_FRAME, previous_fp);
+}
+
+int
+main ()
+{
+  corrupt_frame_outer ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.python/py-unwind.exp b/gdb/testsuite/gdb.python/py-unwind.exp
new file mode 100644
index 0000000..4bf09b3
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-unwind.exp
@@ -0,0 +1,54 @@ 
+# Copyright (C) 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the GDB testsuite.  It verifies that frame
+# sniffers can be implemented in Python.
+
+load_lib gdb-python.exp
+
+standard_testfile
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile}] } {
+    return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+# This test runs on a specific platform.
+if { ! [istarget x86_64-*]} { continue }
+
+# The following tests require execution.
+
+if ![runto_main] then {
+    fail "Can't run to main"
+    return 0
+}
+
+set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]
+
+gdb_breakpoint [gdb_get_line_number "break backtrace-broken"]
+
+gdb_test "source ${pyfile}" "Python script imported" \
+         "import python scripts"
+
+gdb_continue_to_breakpoint "break backtrace-broken"
+gdb_test_sequence "where"  "Backtrace restored by sniffer" {
+    "\\r\\n#0 .* corrupt_frame_inner \\(\\) at "
+    "\\r\\n#1 .* corrupt_frame_outer \\(\\) at "
+    "\\r\\n#2 .* main \\(.*\\) at"
+}
+
+
diff --git a/gdb/testsuite/gdb.python/py-unwind.py b/gdb/testsuite/gdb.python/py-unwind.py
new file mode 100644
index 0000000..a8d591f
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-unwind.py
@@ -0,0 +1,81 @@ 
+# Copyright (C) 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import gdb
+from gdb.sniffer import Sniffer
+
+class TestSniffer(Sniffer):
+    AMD64_RBP = 6
+    AMD64_RSP = 7
+    AMD64_RIP = 16
+
+    def __init__(self):
+        Sniffer.__init__(self, "test sniffer")
+        self.char_ptr_t = gdb.lookup_type("unsigned char").pointer()
+        self.char_ptr_ptr_t = self.char_ptr_t.pointer()
+
+    def _read_word(self, address):
+        return address.cast(self.char_ptr_ptr_t).dereference()
+
+    def __call__(self, sniffer_info):
+        """Test sniffer written in Python.
+
+        This sniffer can unwind the frames that have been deliberately
+        corrupted in a specific way (functions in the accompanying
+        py-unwind.c file do that.)
+        This code is only on AMD64.
+        On AMD64 $RBP points to the innermost frame (unless the code
+        was compiled with -fomit-frame-pointer), which contains the
+        address of the previous frame at offset 0. The functions
+        deliberately corrupt their frames as follows:
+                     Before                 After
+                   Corruption:           Corruption:
+                +--------------+       +--------------+
+        RBP-8   |              |       | Previous RBP |
+                +--------------+       +--------------+
+        RBP     + Previous RBP |       |    RBP       |
+                +--------------+       +--------------+
+        RBP+8   | Return RIP   |       | Return  RIP  |
+                +--------------+       +--------------+
+        Old SP  |              |       |              |
+
+        This sniffer recognizes the corrupt frames by checking that
+        *RBP == RBP, and restores previous RBP from the word above it.
+        """
+        try:
+            bp = sniffer_info.read_register(
+                TestSniffer.AMD64_RBP).cast(self.char_ptr_t)
+            if self._read_word(bp) != bp:
+                return None
+            # Found the frame that the test program has corrupted for us.
+            # The correct BP for the outer frame has been saved one word
+            # above, previous IP and SP are at the expected places.
+            previous_bp = self._read_word(bp - 8)
+            previous_ip = self._read_word(bp + 8)
+            previous_sp = bp + 16
+            unwind_info = gdb.UnwindInfo(
+                sniffer_info,
+                ((TestSniffer.AMD64_RBP, previous_bp),
+                 (TestSniffer.AMD64_RIP, previous_ip),
+                 (TestSniffer.AMD64_RSP, previous_sp)))
+            sp = sniffer_info.read_register(TestSniffer.AMD64_RSP)
+            ip = sniffer_info.read_register(TestSniffer.AMD64_RIP)
+            unwind_info.frame_id_build(sp, ip)
+            return unwind_info
+        except (gdb.error, RuntimeError):
+            return None
+
+gdb.sniffer.register_sniffer(None, TestSniffer(), True)
+print("Python script imported")