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

Doug Evans dje@google.com
Wed Feb 4 22:36:00 GMT 2015


Alexander Smundak writes:
 > Python frame filters provide the ability to display non-native frames in
 > a mixed-language application (say, a backtrace of an application written
 > in C and embedding Java Virtual Machine can be displayed as a mix of
 > native frames and Java frames). However, GDB cannot always unwind
 > non-native frames. The proposed patch adds the ability to write frame
 > unwinders in Python.
 > 
 > 2014-12-12  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/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.
 >         * python/lib/gdb/__init__.py (packages): add frame_sniffers.
 > 
 > 2014-12-12  Sasha Smundak  <asmundak@google.com>
 > 
 >         * gdb.python/py-unwind.c: Test program for the py-unwind test.
 >         * gdb.python/py-unwind.exp: Python frame sniffers test.
 >         * gdb.python/py-unwind.py: Frame sniffer in Python tested by
 >         py-unwind test.

Hi.
Sorry for the delay, and THANKS for the patience.

grep for >>>> to find comments

High level comments:

Is it possible to see the code, and example usage, of a real-life use-case
of this? That will help folks not familiar with this project to understand
the problem we are trying to solve.

I'm still not sure what kind of performance cost we're looking at here as
it scales up, I can imagine most times there'll be no Python sniffers,
or at most one or two.  But it would be good to collect some perf data
(e.g., install 1,10,100 no-op sniffers and see if there's any measurable
difference in backtrace performance).

Exposing frame id implementation details (sp,pc,special), and the
form of how to do that, is something the community needs to decide on.
I think we can come up with something suitable, though perhaps not
the current form.

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 2f69eb2..ba396ea 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
@@ -2593,6 +2595,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 a6789bd..cfced73 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -29,6 +29,7 @@
      selecting a new file to debug.
   ** You can now add attributes to gdb.Objfile and gdb.Progspace objects.
   ** New function gdb.lookup_objfile.
+  ** You can now write frame unwinders in Python.
 
 * New Python-based convenience functions:
 

>>>>
NEWS file entry will need to be updated now that we've branched 7.9.

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index 00c70bb..ff6e8d2 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -62,6 +62,7 @@ PYTHON_FILE_LIST = \
 	gdb/FrameDecorator.py \
 	gdb/types.py \
 	gdb/printing.py \
+	gdb/sniffers.py \
 	gdb/prompt.py \
 	gdb/xmethod.py \
 	gdb/command/__init__.py \
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 234ce5c..45b38d9 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.
+* Writing a Frame Unwinder::    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.

>>>>
We may want a couple of sections, akin to pretty-printers
and frame-filters.  E.g., one section describing the API,
and another section being a tutorial on how to write an unwinder,
akin to "Frame Filter API" vs "Writing a Frame Filter".
I don't have a strong opinion on whether to combine them,
other than if there's no compelling reason to be different
than how things are currently done then don't be.

and one section
@@ -2178,6 +2179,57 @@ printed hierarchically.  Another approach would be to combine the
 marker in the inlined frame, and also show the hierarchical
 relationship.
 
+@node Writing a Frame Unwinder
+@subsubsection Writing a Frame Unwinder in Python
+@cindex Writing a frame unwinder in Python.
+
+You can tell GDB how to unwind certain types of frames by writing a
+sniffer function and adding it to the list of the sniffers. 

>>>>
.... list of sniffers. ?

+
+@subheading Frame Sniffer API
+
+A sniffer function receives a single argument describing the frame to
+be examined (a @code{gdb.SnifferInfo} object). It returns frame unwind
+information described below if it knows how to unwind the frame, or
+@code{None} otherwise.
+

>>>>
extra blank line

+
+The @code{gdb.SnifferInfo} class has a single method:
+
+@defun SnifferInfo.read_register (self, regnum)
+

>>>>
no blank line here

+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.
+@end defun
+
+The frame unwind information returned by your sniffer should be a pair
+(@var{registers}, @var{id_register_numbers}), where
+@var{registers} describe the registers that can be unwound (i.e.,
+the registers from the previous frame that have been saved in this
+frame), and @var{id_register_numbers} provides the data for the
+construction of the frame ID of the previous frame.
+
+The @var{registers} is a list of (@var{regnum}, @var{regdata})

>>>>
This doesn't read well to me but I'm not sure how to change it.
The @var{registers} value is a ... ?

+pairs, where @var{regnum} is a (platform-specific) register number,

>>>>
Are "pairs" a common name for python tuples of two elements?
Alas, I'm not sure whether it's ok to use "pairs" here or not.
Let's leave it as is until someone else speaks up.

Plus I think you can remove the parens around "platform-specific".

+and @var{regdata} is register contents (a @code{gdb.Value} object).

>>>>
.... is the register's contents as a @code{gdb.Value} object.

+
+The @var{id_register_numbers} is either (@var{sp}), (@var{sp}, @var{pc}),

>>>>
The @var{id_register_numbers} value is either ... ?

+or (@var{sp}, @var{pc}, @var{special}) tuple, where @var{sp},
+@var{pc}, @var{special} are register numbers, and the referenced
+registers should be present in @var{registers}. The frame ID is
+constructed by calling
+@code{make_id_build_wild}(@var{ValueOf}(@var{sp}),
+@code{make_id_build}(@var{ValueOf}(@var{sp}), @var{ValueOf}(@var{pc}),
+or @code{make_id_build}(@var{ValueOf}(@var{sp}),
+@var{ValueOf}(@var{pc}, @var{ValueOf}(@var{special}) respectively.

>>>>
This paragraph will need elaboration.
E.g., "special" will be confusing to readers not familiar
with gdb implementation details.
Which of course raises the question: How much of gdb implementation
details do we want to expose here? And in what form?
I don't have a good answer to that yet, but hopefully the review process
will shake this out.

Where do make_id_build* come from? I can't find them anywhere.
Also, the names are too generic, there's nothing in the name that
suggests "frame id". Are these in fact frame_id_build*?
Those are internal gdb function names which we won't want to expose
in the documentation. One could just say something generic like
"The frame ID is an opaque object that is constructed from
@var{sp}, @var{pc}, and @var{special}." or some such.
[setting aside the issues raised in the previous paragraph]

+
+@subheading Registering a Sniffer
+
+@code{gdb.Objfile}, @code{gdb.Progspace}, and @code{gdb} itself have
+@code{frame_sniffers} attribute containing a list of the
+sniffers. Register your sniffer by adding it to one of them.
+

>>>>
This will need elaboration of course, e.g., mimicing what is done for
pretty-printers. E.g., grep for "Progspace.pretty_printers" and
"Objfile.pretty_printers" in python.texi. There is no specific entry for
"gdb.pretty_printers", that's probably an oversight.

OTOH I totally understand not wanting to invest too much time in
documentation until the details of what to document are worked out. :-)

We'll also want all of the machinery that pretty-printers, et.al.,
have for enabling, disabling, and listing them.

 @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 8c6eee2..ffc0d1d 100644
--- a/gdb/python/lib/gdb/__init__.py
+++ b/gdb/python/lib/gdb/__init__.py
@@ -28,7 +28,7 @@ class _GdbFile (object):
     # These two are needed in Python 3
     encoding = "UTF-8"
     errors = "strict"
-    
+

>>>>
unnecessary whitespace change

     def close(self):
         # Do nothing.
         return None
@@ -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/sniffers.py b/gdb/python/lib/gdb/sniffers.py
new file mode 100644
index 0000000..0533642
--- /dev/null
+++ b/gdb/python/lib/gdb/sniffers.py
@@ -0,0 +1,64 @@
+# Frame unwinding support.
+# Copyright (C) 2013-2014 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
+import collections

>>>>
do we need to import collections?

+
+
+def execute_sniffers(sniffer_info):
+    """Internal function called from GDB that executes sniffers
+    implemented in Python. A sniffer able to unwind the frame returns
+    a tuple containing unwind information.
+
+    Arguments:
+        sniffer_info: an instance of gdb.SnifferInfo.
+
+    Returns:
+        unwind_info: a pair (REG_DATA, FRAME_ID_REGNUMS). REG_DATA is
+        tuple of (REG_NUM, REG_VALUE) pairs, where REG_NUM is
+        (platform-specific) register number, and REG_VALUE is Value
+        object with register value. FRAME_ID_REGNUM can be a (SP,),
+        (SP, PC), or (SP, PC, SPECIAL) tuple, where SP, PC, and
+        SPECIAL are (platform specific) register numbers.
+        The frame ID is built in each case as follows:
+          (SP,)                 make_id_build_wild (Value(SP))
+          (SP, PC)              make_id_build (Value(SP), Value(PC))
+          (SP, PC, SPECIAL)     make_id_build_special (Value(SP),
+                                   Value(PC), Value(SPECIAL)
+        The registers present in FRAME_ID_REGNUM should be among those
+        returned by REG_DATA.
+    """
+
+    current_progspace = gdb.current_progspace()

>>>>
Move this assignment to current_progspace down to where it is used.

+    for objfile in gdb.objfiles():
+        for sniffer in objfile.frame_sniffers:
+            unwind_info = sniffer(sniffer_info)
+            if unwind_info is not None:
+                return unwind_info
+
+    for sniffer in current_progspace.frame_sniffers:
+        unwind_info = sniffer(sniffer_info)
+        if unwind_info is not None:
+            return unwind_info
+
+    for sniffer in gdb.frame_sniffers:
+        unwind_info = sniffer(sniffer_info)
+        if unwind_info is not None:
+            return unwind_info
+
+    return None

>>>>
I'm tempted to have this implemented this in C,
but let's leave it as is for now.

diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index e78ceba..bbe68f8 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;
 
@@ -162,6 +166,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);
@@ -184,6 +189,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;
@@ -291,6 +300,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 *
@@ -618,6 +669,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 96339b1..0972498 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..47a99f7
--- /dev/null
+++ b/gdb/python/py-unwind.c
@@ -0,0 +1,514 @@
+/* Python frame unwinder interface
+
+   Copyright (C) 2013-2014 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 "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 a frame we can unwind: frame_id and an array of
+   (register_number, register_value) pairs.  */
+
+typedef struct
+{
+  struct frame_id frame_id;
+  struct gdbarch *gdbarch;
+  int reg_count;
+  struct reg_info
+  {
+    int number;
+    gdb_byte *data;
+  } reg[];
+} cached_frame_info;
+
+static PyTypeObject sniffer_info_object_type
+    CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("sniffer_info_object");
+
+static unsigned int pyuw_debug = 0;
+
+static struct gdbarch_data *pyuw_gdbarch_data;
+
+/* 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;
+
+  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 a register as pointer.  */

>>>>
Returns the value of register REGNUM (but see below: do we want to pass
register names here?).

+
+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;
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      /* Cannot call `value_of_register' as frame_info is not ready yet, so
+         use deprecated call instead.  */

>>>>
It would be good to expand on what "frame_info is not ready yet" means.
The reader sees it being used anyway so what about it isn't ready?

+      struct frame_info *frame = ((sniffer_info_object *)self)->frame_info;

>>>>
space before "self"

Question: What if a sniffer_info object is saved in python,
(e.g., a registered sniffer could save it in a global),
and is later used when frame_info is invalid?
It would be good to have a testcase exercising this.

+      struct gdbarch *gdbarch = get_frame_arch (frame);
+      gdb_byte buffer[sizeof (CORE_ADDR)];

>>>>
Some registers are bigger than CORE_ADDR.
Use MAX_REGISTER_SIZE (from defs.h) instead.

+
+      gdb_assert (register_size (gdbarch, regnum) <= ARRAY_SIZE (buffer));

>>>>
register_size has an assert that regnum is valid,
but we don't know that yet (the user could pass -1 or whatever).
Plus, I see that gdb.Frame.read_register takes register names
as arguments (and uses user_reg_map_name_to_regnum to validate its
argument). How about passing register names instead of numbers here too?

+      if (deprecated_frame_register_read (frame, regnum, buffer))
+        {
+          struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
+
+          val = value_from_pointer (ptr_type, unpack_pointer (ptr_type, buffer));

>>>>
We're assuming all registers (that one would be interested in) are pointers
here. That may be ok for the task at hand, but we'll need to be clear about
that in the documentation. Plus, "read_register" doesn't really tell me, the
reader, that it can only return regs that can be used as pointers.
Can read_register just return the register in its normal type
(e.g., whatever gdb.Frame.read_register returns)?
I know we can't use value_of_register here because we don't have a
complete frame yet (that's what we're trying to build),
but can we still do the equivalent? One can get a register's type
with register_type (gdbarch, regnum).

+        }
+
+      if (val == NULL)
+        PyErr_SetString (PyExc_ValueError, _("Unknown register."));

>>>>
I think val can be NULL for other reasons (e.g.,
deprecated_frame_register_read checks !optimized && !unavailable).
We'll be handling "unknown register" earlier anyway so we'll need
to change this.

+    }
+  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);
+
+  if (sniffer_info != NULL)
+    sniffer_info->frame_info = frame;
+
+  return (PyObject *) sniffer_info;
+}
+
+/* Parse given tuple of Python Ints into an array. Returns the number of
+   items in the tuple, or -1 if it is not a tuple. If tuple has
+   more elements than array size, these elements are ignored.  */

>>>>
It's easier to relax restrictions than it is to impose them after the fact.
Thus I prefer being strict in what I accept as valid input here.
So instead of ignoring extra elements I would flag it as an error.

+
+static Py_ssize_t
+pyuw_parse_ints (PyObject *pyo_sequence, int *values, Py_ssize_t max_values)
+{
+  Py_ssize_t size;
+  Py_ssize_t i;
+
+  if (! PyTuple_Check (pyo_sequence))
+    return -1;
+  size = PyTuple_Size (pyo_sequence);
+  if (size < 0)
+    return -1;
+  if (size < max_values)
+    max_values = size;

>>>>
Instead of modifying max_values here, I'd set size and use that in the for()
loop.

+  for (i = 0; i < max_values; ++i)
+    {
+      PyObject *pyo_item = PyTuple_GetItem (pyo_sequence, i);
+
+      if (pyo_item == NULL || !PyInt_Check (pyo_item))
+        return -1;
+      values[i] = (int)PyInt_AsLong (pyo_item);

>>>>
It would be good to check that there's no loss of precision here.
I.e., first assign to a long and then check val == (int) val.

+    }
+  return i;
+}
+
+/* Retrieve register value for the cached unwind info as target pointer.
+   Return 1 on success, 0 on failure.  */
+
+static int
+pyuw_reg_value (cached_frame_info *cached_frame, int regnum, CORE_ADDR *value)
+{
+  struct reg_info *reg_info = cached_frame->reg;
+  struct reg_info *reg_info_end = reg_info + cached_frame->reg_count;
+
+  for (; reg_info < reg_info_end; ++reg_info)
+    {
+      if (reg_info->number == regnum)
+        {
+          *value = unpack_pointer
+              (register_type (cached_frame->gdbarch, regnum), reg_info->data);
+          return 1;
+        }
+    }
+
+  error (_("Python sniffer uses register #%d for this_id, "
+           "but this register is not available"), regnum);
+}
+
+/* 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;

>>>>
space before *cache_ptr.

+  if (pyuw_debug >= 1)
+    {
+      fprintf_unfiltered (gdb_stdlog, "%s: frame_id: ", __FUNCTION__);
+      fprint_frame_id (gdb_stdlog, *this_id);
+      fprintf_unfiltered (gdb_stdlog, "\n");
+    }
+}
+
+/* Register unwind shim.  */

>>>>
Misplaced comment.
/* frame_unwind.prev_register method.  */ ?

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

>>>>
This for loop needs to be wrapped in {} (the body isn't more than one line).

+    if (regnum == reg_info->number)
+      return frame_unwind_got_bytes (this_frame, regnum, reg_info->data);
+
+  return frame_unwind_got_optimized (this_frame, regnum);
+}
+
+/* Parse frame ID tuple returned by the sniffer info GDB's frame_id and
+   saved it in the cached frame.  */
+
+static void
+pyuw_parse_frame_id (cached_frame_info *cached_frame, 
+                     PyObject *pyo_frame_id_regs)
+{
+  int regno[3];
+  CORE_ADDR sp, pc, special;
+
+  if (!PyTuple_Check (pyo_frame_id_regs))
+    error (_("The second element of the pair returned by a Python "
+             "sniffer should be a tuple"));
+
+  switch (pyuw_parse_ints (pyo_frame_id_regs, regno, ARRAY_SIZE (regno))) {

>>>>
opening { goes on next line, and cases are indented.
grep for other uses of switch() in the sources.

+  case 1:
+    if (pyuw_reg_value (cached_frame, regno[0], &sp))
+      {
+        cached_frame->frame_id = frame_id_build_wild (sp);
+        return;
+      }
+  case 2:
+    if (pyuw_reg_value (cached_frame, regno[0], &sp)
+        || pyuw_reg_value (cached_frame, regno[1], &pc))
+      {
+        cached_frame->frame_id = frame_id_build (sp, pc); 
+        return;
+      }
+  case 3:
+    if (pyuw_reg_value (cached_frame, regno[0], &sp)
+        || pyuw_reg_value (cached_frame, regno[1], &pc)
+        || pyuw_reg_value (cached_frame, regno[2], &special))
+      {
+        cached_frame->frame_id = frame_id_build_special (sp, pc, special);
+        return;
+      }
+  }
+  error (_("Unwinder should return a tuple of ints in the second item"));
+}
+
+/* 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;
+
+  gdb_assert (*cache_ptr == NULL);
+  gdbarch = (void *)(self->unwind_data);
+  cleanups = ensure_python_env (gdbarch, current_language);
+  TRACE_PY_UNWIND (3, "%s(SP=%lx, PC=%lx)\n", __FUNCTION__,
+      get_frame_sp (this_frame), get_frame_pc (this_frame));

>>>>
Alas, one can't use %lx to print CORE_ADDR.
Use paddress().

+  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.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;
+  if (pyo_unwind_info == Py_None)
+    goto error;

>>>>
Technically, Py_None needs to be decref'd too.

+  make_cleanup_py_decref (pyo_unwind_info);
+
+  /* Unwind_info is a pair (REGISTERS, FRAME_ID_REGNUMS).  REGISTERS
+   * is a list of the (REG_NR, REG_VALUE) pairs. FRAME_ID_REGNUMS is
+   * the list of REGNO values.  */
+  if (!(PyTuple_Check (pyo_unwind_info) && PyTuple_Size (pyo_unwind_info) == 2))
+    error (_("Sniffer should return a pair (REGISTERS, FRAME_ID_REGNUMS)"));
+
+  {
+    PyObject *pyo_registers = PyTuple_GetItem (pyo_unwind_info, 0);
+    int i;
+    int reg_count;
+    size_t cached_frame_size;
+    size_t gdb_bytes_count;
+    gdb_byte *gdb_data_free, *gdb_data_end;
+
+    if (pyo_registers == NULL)
+      goto error;
+    if (!PyTuple_Check (pyo_registers))
+      error (_("The first element of the returned pair should be a tuple"));

>>>>
Lists and tuples are different in Python.
The above comment says REGISTERS is a list but here we're checking for tuples.

+
+    /* 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"));
+    gdb_bytes_count = reg_count * sizeof (CORE_ADDR);

>>>>
s/sizeof (CORE_ADDR)/MAX_REGISTER_SIZE/
Or you could loop over all the registers first collecting their sizes.

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

>>>>
Best add a comment here saying no more cleanups may be added,
as they will be discarded along with cached_frame_cleanups.
A more robust way would be to allocate cached_frame and its cleanup
first, and discard it last.

+    gdb_data_end = (gdb_byte *)((char *)cached_frame + cached_frame_size);

>>>>
space between cast and value

+    gdb_data_free = gdb_data_end - gdb_bytes_count;
+
+    cached_frame->gdbarch = gdbarch;
+    cached_frame->reg_count = reg_count;
+
+    /* Populate registers array.  */
+    for (i = 0; i < reg_count; i++)
+    {

>>>>
indent { two more spaces (grep for examples in the sources)

+      PyObject *pyo_reg = PyTuple_GetItem (pyo_registers, i);
+      struct reg_info *reg = &(cached_frame->reg[i]);
+
+      if (pyo_reg == NULL)
+        goto error;
+
+      if (!(PyTuple_Check (pyo_reg) && PyTuple_Size (pyo_reg) == 2))
+        error (_("Python sniffer returned bad register list: "
+                 "item #%d is not a (reg_no, reg_data) pair"), i);
+
+      {
+        PyObject *pyo_reg_number =  PyTuple_GetItem (pyo_reg, 0);
+
+        if (pyo_reg_number == NULL)
+          goto error;
+        if (!PyInt_Check (pyo_reg_number))
+          error (_("Python sniffer returned bad register list: "
+                   "item #%d contains non-integer register number"), i);
+        reg->number = (int)PyInt_AsLong (pyo_reg_number);
+      }
+
+      {
+        PyObject *pyo_reg_value = PyTuple_GetItem (pyo_reg, 1);
+        struct value *value;
+        size_t data_size;
+
+        if (pyo_reg_value == NULL)
+          goto error;
+
+        if ((value = value_object_to_value (pyo_reg_value)) == NULL)
+          error (_("Python sniffer returned bad register list: item #%d, "
+                   "register value should have type gdb.Value type"), i);
+        data_size = register_size (gdbarch, reg->number);
+        gdb_assert ((gdb_data_free + data_size) <= gdb_data_end);
+        memcpy (gdb_data_free, value_contents (value), data_size);

>>>>
value could have fewer bytes than the register's size,
will need to check for that.

+        cached_frame->reg[i].data = gdb_data_free;
+        gdb_data_free += data_size;
+      }
+    }
+  }
+
+  {
+    PyObject *pyo_frame_id_regs = PyTuple_GetItem (pyo_unwind_info, 1);
+    if (pyo_frame_id_regs == NULL)
+      goto error;
+    pyuw_parse_frame_id (cached_frame, pyo_frame_id_regs);
+  }
+
+  *cache_ptr = cached_frame;
+  discard_cleanups (cached_frame_cleanups);
+  do_cleanups (cleanups);
+  return 1;
+
+error:
+  do_cleanups (cleanups);
+  xfree (cached_frame);

>>>>
Remove the call to xfree as do_cleanups (cleanups) will run all cleanups
created after it was created (unless you reorganize things and allocate
cached_frame and its cleanup first).

+  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;

>>>>
extra space after =

+      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;

>>>>
space before 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)
+{
+  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);

>>>>
"set/show debug py-unwind" needs to be documented in the manual.
There isn't a good place that stands out though.
[I can quibble with every choice I can think of.]
But I do like mentioning all the debug options in one place.
grep for "@node Debugging Output" in gdb.texinfo.
Some are documented in multiple places (there and elsewhere), but I think
we can just document this in one place.

+  pyuw_gdbarch_data
+      = gdbarch_data_register_post_init (pyuw_gdbarch_data_init);
+  observer_attach_architecture_changed (pyuw_on_new_gdbarch);
+  sniffer_info_object_type.tp_new = PyType_GenericNew;

>>>>
Delete the setting of tp_new.
ref: https://sourceware.org/ml/gdb-patches/2014-09/msg00718.html

+  if (PyType_Ready (&sniffer_info_object_type) < 0)
+    return -1;
+  return gdb_pymodule_addobject (gdb_module, "SnifferInfo",
+      (PyObject *) &sniffer_info_object_type);
+}
+
+static PyMethodDef sniffer_info_object_methods[] =
+{
+  { "read_register", sniffer_infopy_read_register, METH_VARARGS,
+    "read_register (register_name) -> gdb.Value\n\

>>>>
Heh, we're using register names here so this part won't have to change. :-)

+Return the value of the register 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 */
+};
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 544fe93..c83830f 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -386,12 +386,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);
 
@@ -486,6 +488,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 b1d8283..12e4d56 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1762,7 +1762,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.c b/gdb/testsuite/gdb.python/py-unwind.c
new file mode 100644
index 0000000..bb267d7
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-unwind.c
@@ -0,0 +1,70 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2011-2014 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;
+}
+
+#define MY_FRAME (__builtin_frame_address (0))
+
+static void
+break_backtrace ()

>>>>
break_backtrace (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 *outer_fp = swap_value ((void **)MY_FRAME, MY_FRAME);

>>>>
I'm not sure what type __builtin_frame_address returns off hand,
but there's no cast in the call to swap_value below.
Either it's needed in both places or we can remove it here.

+
+  /* Verify the compiler allocates the first local variable one word
+     below frame.  This is where test JIT reader expects to find the
+     correct outer frame address.  */
+  if (&outer_fp + 1 != (void **)MY_FRAME)
+    {
+      fprintf (stderr, "First variable should be allocated one word below "
+               "the frame, got variable's address %p, frame at %p instead\n",
+               &outer_fp, MY_FRAME);
+      abort();
+    }
+
+  /* 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 (MY_FRAME, outer_fp); /* break backtrace-broken */
+}
+
+static void
+break_backtrace_caller ()

>>>>
break_backtrace_caller (void)

+{
+  break_backtrace ();
+}
+
+int
+main (int argc, char *argv[])

>>>>
For simplicity I'd leave out argc,argv here.

+{
+  break_backtrace_caller ();
+}
diff --git a/gdb/testsuite/gdb.python/py-unwind.exp b/gdb/testsuite/gdb.python/py-unwind.exp
new file mode 100644
index 0000000..2fc5284
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-unwind.exp
@@ -0,0 +1,54 @@
+# Copyright (C) 2009-2014 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.*" \

>>>>
Remove the trailing ".*" in the regexp.
ref: https://sourceware.org/gdb/wiki/GDBTestcaseCookbook
Could also remove the leading ".*".

+         "import python scripts"
+
+gdb_continue_to_breakpoint "break backtrace-broken"
+gdb_test_sequence "where"  "Bad backtrace" {
+    "\[\r\n\]+#0 .* break_backtrace \\(\\) at "
+    "\[\r\n\]+#1 .* break_backtrace_caller \\(\\) 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..8976e55
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-unwind.py
@@ -0,0 +1,49 @@
+# Copyright (C) 2013-2014 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
+
+def _read_word(address):
+    return address.cast(_char_ptr_ptr_t).dereference()
+
+
+def sniff (sniffer_info):
+    "Sniffer written in Python."
+    bp = sniffer_info.read_register(_amd64_rbp).cast(_char_ptr_t)
+    try:
+        if (_read_word(bp) == bp):
+            # Found the frame that the test program fudged 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 = _read_word(bp - 8)
+            previous_ip = _read_word(bp + 8)
+            previous_sp = bp + 16
+            return (((_amd64_rbp, previous_bp),
+                     (_amd64_rip, previous_ip),
+                     (_amd64_rsp, previous_sp)),
+                    (_amd64_rsp, _amd64_rip))
+
+    except (gdb.error, RuntimeError):
+        return None
+
+
+_char_ptr_t = gdb.lookup_type("unsigned char").pointer()
+_char_ptr_ptr_t = _char_ptr_t.pointer()
+_uint_ptr_t = gdb.lookup_type("unsigned long long")
+_amd64_rbp = 6
+_amd64_rsp = 7
+_amd64_rip = 16
+gdb.frame_sniffers=[sniff]

>>>>
spaces around =

Looking at this I realize this is just copying from the early
pretty-printer implementation where they were just functions.
But then we needed names and an enabled flag, and then it
made more sense to record them as objects. And down the road,
who knows. Thus I'm thinking it'd be better to just start out
with recording these as objects.
I.e., create a Sniffer object akin to what gdb/python/lib/gdb/printing.py
does for class PrettyPrinter.  The object will have two attributes:
name and enabled, and one method: __call__.
I'd also copy gdb/python/lib/gdb/printing.py:register_pretty_printer,
and duplicate gdb/python/lib/gdb/command/pretty_printers.py though
you won't need all the "subprinter" complexity,
gdb/python/lib/gdb/command/xmethods.py will be easier to copy from
(similarly: gdb/python/lib/gdb/xmethod.py).

+print("Python script imported")



More information about the Gdb-patches mailing list