[PATCH] [PR python/18089] Fix.

[Doug Evans <dje at google dot com>]

Hi

This patch address PR 18089.

The problem here is that if the pretty-printer returns a bogus
result from the "children" iterator, the user is left scratching
his/her head wondering where the problem is.
This patch adds more error checking and improves the error message.

Regression tested on amd64-linux.

2015-04-22  Doug Evans  <dje@google.com>

	PR python/18089
	* python/py-prettyprint.c (print_children): Verify result of children
	iterator.  Provide better error message.
	* python/python-internal..h (gdbpy_print_python_errors_p): Declare.
	* python/python.c (gdbpy_print_python_errors_p): New function.

	testsuite/
	* gdb.python/py-bad-printers.c: New file.
	* gdb.python/py-bad-printers.py: New file.
	* gdb.python/py-bad-printers.exp: New file.

diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index d8579fa..274ac6c 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -554,8 +554,22 @@ print_children (PyObject *printer, const char *hint,
 	  break;
 	}
 
+      if (! PyTuple_Check (item) || PyTuple_Size (item) != 2)
+	{
+	  PyErr_SetString (PyExc_TypeError,
+			   _("Result of children iterator not a tuple"
+			     " of two elements."));
+	  gdbpy_print_stack ();
+	  Py_DECREF (item);
+	  continue;
+	}
       if (! PyArg_ParseTuple (item, "sO", &name, &py_v))
 	{
+	  /* The user won't necessarily get a stack trace here, so provide
+	     more context.  */
+	  if (gdbpy_print_python_errors_p ())
+	    fprintf_unfiltered (gdb_stderr,
+				_("Bad result from children iterator.\n"));
 	  gdbpy_print_stack ();
 	  Py_DECREF (item);
 	  continue;
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 0581b33..55af10e 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -527,6 +527,7 @@ extern const struct language_defn *python_language;
 	}								\
     } while (0)
 
+int gdbpy_print_python_errors_p (void);
 void gdbpy_print_stack (void);
 
 PyObject *python_string_to_unicode (PyObject *obj);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 1da63fd..ee86680 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1182,6 +1182,14 @@ gdbpy_flush (PyObject *self, PyObject *args, PyObject *kw)
   Py_RETURN_NONE;
 }
 
+/* Return non-zero if print-stack is not "none".  */
+
+int
+gdbpy_print_python_errors_p (void)
+{
+  return gdbpy_should_print_stack != python_excp_none;
+}
+
 /* Print a python exception trace, print just a message, or print
    nothing and clear the python exception, depending on
    gdbpy_should_print_stack.  Only call this if a python exception is
diff --git a/gdb/testsuite/gdb.python/py-bad-printers.c b/gdb/testsuite/gdb.python/py-bad-printers.c
new file mode 100644
index 0000000..1949467
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-bad-printers.c
@@ -0,0 +1,57 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2008-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 lets us avoid malloc.  */
+int array[100];
+
+struct container
+{
+  const char *name;
+  int len;
+  int *elements;
+};
+
+struct container
+make_container (const char *name)
+{
+  struct container result;
+
+  result.name = name;
+  result.len = 0;
+  result.elements = 0;
+
+  return result;
+}
+
+void
+add_item (struct container *c, int val)
+{
+  if (c->len == 0)
+    c->elements = array;
+  c->elements[c->len] = val;
+  ++c->len;
+}
+
+int
+main ()
+{
+  struct container c = make_container ("foo");
+
+  add_item (&c, 23);
+
+  return 0; /* break here */
+}
diff --git a/gdb/testsuite/gdb.python/py-bad-printers.exp b/gdb/testsuite/gdb.python/py-bad-printers.exp
new file mode 100644
index 0000000..b93db01
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-bad-printers.exp
@@ -0,0 +1,54 @@
+# Copyright (C) 2008-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
+# pretty-printing for the CLI.
+
+load_lib gdb-python.exp
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
+    return -1
+}
+
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
+if ![runto_main ] then {
+    fail "Can't run to main"
+    return -1
+}
+
+set remote_python_file [gdb_remote_download host \
+			    ${srcdir}/${subdir}/${testfile}.py]
+
+gdb_test_no_output "python exec (open ('${remote_python_file}').read ())" \
+    "load python file"
+
+gdb_breakpoint [gdb_get_line_number "break here"]
+gdb_continue_to_breakpoint "break here" ".* break here .*"
+
+gdb_test "enable pretty-printer global bad-printers;container1" \
+    "printers enabled"
+gdb_test "disable pretty-printer global bad-printers;container2" \
+    "printers enabled"
+gdb_test "print c" "Result of children iterator not a tuple of two elements.*"
+
+gdb_test "enable pretty-printer global bad-printers;container2" \
+    "printers enabled"
+gdb_test "disable pretty-printer global bad-printers;container1" \
+    "printers enabled"
+gdb_test "print c" "Bad result from children iterator.*"
diff --git a/gdb/testsuite/gdb.python/py-bad-printers.py b/gdb/testsuite/gdb.python/py-bad-printers.py
new file mode 100644
index 0000000..37c818b
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-bad-printers.py
@@ -0,0 +1,80 @@
+# Copyright (C) 2008-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 GDB's handling of
+# bad python pretty printers.
+
+# Test a printer with a bad children iterator.
+
+import re
+import gdb.printing
+
+
+class BadChildrenContainerPrinter1(object):
+    """Children iterator doesn't return a tuple of two elements."""
+
+    def __init__(self, val):
+        self.val = val
+
+    def to_string(self):
+        return 'container %s with %d elements' % (self.val['name'], self.val['len'])
+
+    @staticmethod
+    def _bad_iterator(pointer, len):
+        start = pointer
+        end = pointer + len
+        while pointer != end:
+            yield 'intentional violation of children iterator protocol'
+            pointer += 1
+
+    def children(self):
+        return self._bad_iterator(self.val['elements'], self.val['len'])
+
+
+class BadChildrenContainerPrinter2(object):
+    """Children iterator returns a tuple of two elements with bad values."""
+
+    def __init__(self, val):
+        self.val = val
+
+    def to_string(self):
+        return 'container %s with %d elements' % (self.val['name'], self.val['len'])
+
+    @staticmethod
+    def _bad_iterator(pointer, len):
+        start = pointer
+        end = pointer + len
+        while pointer != end:
+            # The first argument is supposed to be a string.
+            yield (42, 'intentional violation of children iterator protocol')
+            pointer += 1
+
+    def children(self):
+        return self._bad_iterator(self.val['elements'], self.val['len'])
+
+
+def build_pretty_printer():
+    pp = gdb.printing.RegexpCollectionPrettyPrinter("bad-printers")
+
+    pp.add_printer('container1', '^container$',
+                   BadChildrenContainerPrinter1)
+    pp.add_printer('container2', '^container$',
+                   BadChildrenContainerPrinter2)
+
+    return pp
+
+
+my_pretty_printer = build_pretty_printer()
+gdb.printing.register_pretty_printer(gdb, my_pretty_printer)