From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25999 invoked by alias); 8 Jul 2013 22:18:16 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 25988 invoked by uid 89); 8 Jul 2013 22:18:15 -0000 X-Spam-SWARE-Status: No, score=-6.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 08 Jul 2013 22:18:12 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r68MIBUH019576 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 8 Jul 2013 18:18:11 -0400 Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r68MI9IS014382 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO) for ; Mon, 8 Jul 2013 18:18:11 -0400 Message-ID: <51DB3AA1.2060005@redhat.com> Date: Mon, 08 Jul 2013 22:18:00 -0000 From: Keith Seitz User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 MIME-Version: 1.0 To: "gdb-patches@sourceware.org ml" Subject: [RFA] Fix varobj/15166 Content-Type: multipart/mixed; boundary="------------020404000903080802020804" X-Virus-Found: No X-SW-Source: 2013-07/txt/msg00237.txt.bz2 This is a multi-part message in MIME format. --------------020404000903080802020804 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 1200 Hi, $SUBJECT concerns an assertion failure that is triggered when using pretty-printing with varobj. The cause of the problem is actually pretty simple: cplus_describe_child knows nothing about pretty printing. As a result, it strictly enforces that the immediate children of a root varobj whose type is a class/struct must be one of the "fake" children. When asking for an index greater than 2 (CPLUS_FAKE_CHILDren are indices 0, 1, 2), the code asserts because of an unknown fake child. The solution is to teach cplus_describe_child how to fetch the N'th child from the pretty-printed varobj. No regressions on x86_64 native and native-gdbserver. Keith ChangeLog 2013-07-08 Keith Seitz PR varobj/15166 * varobj.c (get_pretty_printed_element_index): New function. (cplus_describe_child): Add handling for pretty-printed varobjs. testsuite/ChangeLog 2013-07-08 Keith Seitz PR varobj/15166 * lib/mi-support.exp (mi_list_varobj_children_range): Ignore a display hint in the expected output. * gdb.python/py-pp-varobj-update.cc: New file. * gdb.python/py-pp.varobj-update.exp: New file. * gdb.python/py-pp-varobj-update.py: New file. --------------020404000903080802020804 Content-Type: text/x-patch; name="pp-varobj-update.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="pp-varobj-update.patch" Content-length: 13488 diff --git a/gdb/varobj.c b/gdb/varobj.c index d4fa6ba..540b440 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -3697,6 +3697,60 @@ match_accessibility (struct type *type, int index, enum accessibility acc) return 0; } +#if HAVE_PYTHON +/* Return the INDEX'th child element from the pretty-printed varobj VAR. + The return value must be decref'd by the caller (if non-NULL). */ + +static PyObject * +get_pretty_printed_element_index (struct varobj *var, int index) +{ + struct cleanup *back_to; + PyObject *children, *iter, *item; + + /* This function should not be called with a non-pretty-printed varobj. */ + gdb_assert (varobj_pretty_printed_p (var)); + + if (!gdb_python_initialized) + return NULL; + + if (!PyObject_HasAttr (var->pretty_printer, gdbpy_children_cst)) + return NULL; + + children = PyObject_CallMethodObjArgs (var->pretty_printer, + gdbpy_children_cst, NULL); + if (children == NULL) + { + gdbpy_print_stack (); + error (_("Null value for children.")); + } + + back_to = make_cleanup_py_decref (children); + + iter = PyObject_GetIter (children); + if (iter == NULL) + { + gdbpy_print_stack (); + error (_("Could not get children iterator")); + } + + make_cleanup_py_decref (iter); + + item = NULL; + while (index-- >= 0) + { + Py_XDECREF (item); + item = PyIter_Next (iter); + + /* We should not be able to ask for an index for which we do not + have a child varobj already! */ + gdb_assert (item != NULL); + } + + do_cleanups (back_to); + return item; +} +#endif /* HAVE_PYTHON */ + static void cplus_describe_child (struct varobj *parent, int index, char **cname, struct value **cvalue, struct type **ctype, @@ -3737,7 +3791,52 @@ cplus_describe_child (struct varobj *parent, int index, { char *join = was_ptr ? "->" : "."; - if (CPLUS_FAKE_CHILD (parent)) + if (varobj_pretty_printed_p (var)) + { +#if HAVE_PYTHON + struct cleanup *back_to; + PyObject *py_v, *item; + const char *name; + struct value *v = NULL; + + if (cfull_expression != NULL) + error (_("Cannot get path expression for dynamic varobj.")); + + back_to = varobj_ensure_python_env (var); + item = get_pretty_printed_element_index (var, index); + if (item != NULL) + { + if (!PyArg_ParseTuple (item, "sO", &name, &py_v)) + { + gdbpy_print_stack (); + error (_("Invalid item from the child list")); + } + + if (cname != NULL) + *cname = xstrdup (name); + + if (cvalue != NULL || ctype != NULL) + { + v = convert_value_from_python (py_v); + if (v == NULL) + gdbpy_print_stack (); + } + + if (cvalue != NULL && value != NULL) + *cvalue = v; + + if (ctype != NULL && v != NULL) + *ctype = value_type (v); + } + + Py_XDECREF (item); + do_cleanups (back_to); +#else /* !HAVE_PYTHON */ + gdb_assert_not_reached + ("should never be called if Python is not enabled"); +#endif /* !HAVE_PYTHON */ + } + else if (CPLUS_FAKE_CHILD (parent)) { /* The fields of the class type are ordered as they appear in the class. We are given an index for a diff --git a/gdb/testsuite/gdb.python/py-pp-varobj-update.cc b/gdb/testsuite/gdb.python/py-pp-varobj-update.cc new file mode 100644 index 0000000..1b2552f --- /dev/null +++ b/gdb/testsuite/gdb.python/py-pp-varobj-update.cc @@ -0,0 +1,53 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2013 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 . */ + +// This class is used alongside its pretty-printer to test varobj/15166. + +class hole +{ +public: + hole () : a (1), b (2), c (3), d (4), e (5), f (6), g (7), + x (10), y (20), z (30) {} + int sum (void) const; + + int a; + int b; + int c; + int d; + int e; + int f; + int g; + +private: + int x; + int y; + int z; +}; + +int +hole::sum (void) const +{ + return a + b + c + d + e + f + g + y - x - z - 8; +} + +int +main (void) +{ + hole h; + + return h.sum (); // break here +} diff --git a/gdb/testsuite/gdb.python/py-pp-varobj-update.exp b/gdb/testsuite/gdb.python/py-pp-varobj-update.exp new file mode 100644 index 0000000..e130657 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-pp-varobj-update.exp @@ -0,0 +1,172 @@ +# Copyright (C) 2013 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 . + +# This file is part of the GDB testsuite. +# +# Test varobj/15166 +# +# This test creates a pretty-printer which inserts a "hole" in the +# layout for the class. The hole is intentionally put after the +# indices for the CPLUS_FAKE_CHILD (0, 1, 2), which caused an assert +# in cplus_describe_child. +# +# For reference, these "holes" are: +# +# index 0 1 2 3 4 5 6 7 8 9 +# gdb a b c d e f g x y z +# PP a c e g +# +# So, for example, the fourth value child (#7 when keys are taken into +# account) of the pretty-printed varobj is actually index six in the real +# type information for the class. + +load_lib mi-support.exp +load_lib cp-support.exp +set MIFLAGS "-i=mi2" + +standard_testfile .cc + +# Start gdb so that we can query supported MI features. +gdb_exit +if {[mi_gdb_start]} { + continue +} + +# Skip these tests if either Python scripting is disabled or C++ is +# verboten. +if {[lsearch -exact [mi_get_features] python] < 0 + || [skip_cplus_tests]} { continue } + +if {[gdb_compile $srcdir/$subdir/$srcfile $binfile executable \ + {debug c++}] != ""} { + untested "Could not compile $srcfile" + return -1 +} + +mi_delete_breakpoints +mi_gdb_reinitialize_dir $srcdir/$subdir +mi_gdb_load $binfile +mi_runto main + +# The test is actually pretty simple: for a given child (index +# must be greater than CPLUS_FAKE_CHILD, ie, 2): +# We will attempt to modify and evaluate the child's value both +# with MI and without, making sure the values are in sync. + +mi_continue_to_line [gdb_get_line_number "break here" $srcfile] \ + "step to `break here'" + +# First, run the test without pretty-printing. + +# Create the raw (no pretty-printing) varobj. +mi_create_dynamic_varobj raw h \ + "create raw varobj, no pretty-printing" + +# Get the list of public children +mi_list_varobj_children raw { + {raw.public public 7} + {raw.private private 3} +} "children of `raw'" + +mi_list_varobj_children raw.public { + {raw.public.a a 0 int} + {raw.public.b b 0 int} + {raw.public.c c 0 int} + {raw.public.d d 0 int} + {raw.public.e e 0 int} + {raw.public.f f 0 int} + {raw.public.g g 0 int} +} "children of `raw.public'" + +# Check the value of `h.g' +mi_check_varobj_value raw.public.g 7 "value of `raw.public.g'" + +# Set the value and double-check the varobj's value. The value will +# not change until the varobj is updated. +mi_gdb_test "set var h.g = 77" "" +mi_check_varobj_value raw.public.g 7 \ + "value of `raw.public.g' after external set" +mi_varobj_update raw.public.g raw.public.g "update `raw.public.g'" + +# Check the value of this varobj +mi_check_varobj_value raw.public.g 77 \ + "value of `raw.public.f' after update" + +# Now change the value of this varobj using MI +mi_gdb_test "-var-assign raw.public.g 123" \ + "\\^done,value=\"123\"" "assign new value to `raw.public.g'" + +# Finally, check the CLI's value of this varobj +mi_gdb_test "print h.g" ".* = 123.*\\^done" "new CLI value of h.g" + +# Delete the root varobj and try again using pretty-printing +mi_delete_varobj raw "delete `raw'" + +# Load the pretty-printer +set remote_python_file [remote_download host \ + "$srcdir/$subdir/${testfile}.py"] +mi_gdb_test "python exec (open ('$remote_python_file').read ())" "" + +# Enable pretty-printing +mi_gdb_test "-enable-pretty-printing" "" + +# Create the new pretty-printed varobj root +mi_create_dynamic_varobj pp h \ + "create pp varobj, pretty-printing enabled" + +# Get the list of children, which will be alternating keys and values +mi_list_varobj_children pp { + {pp.\\[0\\] \\[0\\] 4 "char \\[4\\]"} + {pp.\\[1\\] \\[1\\] 0 int} + {pp.\\[2\\] \\[2\\] 4 "char \\[4\\]"} + {pp.\\[3\\] \\[3\\] 0 int} + {pp.\\[4\\] \\[4\\] 4 "char \\[4\\]"} + {pp.\\[5\\] \\[5\\] 0 int} + {pp.\\[6\\] \\[6\\] 4 "char \\[4\\]"} + {pp.\\[7\\] \\[7\\] 0 int} +} "children of `pp'" + +# Check the value of `h.g' +mi_check_varobj_value pp.\[7\] 123 "value of `pp.\[7\]'" + +# Set the value and double-check the varobj's value. The value will +# not change until the varobj is updated. +mi_gdb_test "set var h.g = 77" "" +mi_check_varobj_value pp.\[7\] 123 \ + "value of `pp.\[7\]' after external set" + +# Update the child varobj and check the new value +mi_varobj_update pp.\[7\] { + pp.\\[7\\] +} "update `pp.\[7\]'" +mi_check_varobj_value pp.\[7\] 77 \ + "value of `pp.\[7\]' after update" + +# Now change the value of this varobj using MI +mi_gdb_test "-var-assign pp.\[7\] 321" \ + "\\^done,value=\"321\"" "assign new value to `pp.\[7\]'" + +# Check the CLI's value of this varobj +mi_gdb_test "print h.g" ".* = 321.*\\^done" "new CLI value of h.g" + +# One last test: attempt to modify a key's value +mi_gdb_test "-var-assign pp.\[0\] 1" \ + "\\^error,msg=\"-var-assign: Variable object is not editable\"" \ + "attempt to modify a key" + +# Delete the root varobj and try again using pretty-printing +mi_delete_varobj pp "delete `pp'" + +remote_file host delete $remote_python_file diff --git a/gdb/testsuite/gdb.python/py-pp-varobj-update.py b/gdb/testsuite/gdb.python/py-pp-varobj-update.py new file mode 100644 index 0000000..c1624b2 --- /dev/null +++ b/gdb/testsuite/gdb.python/py-pp-varobj-update.py @@ -0,0 +1,88 @@ +# Copyright (C) 2013 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 . + +# This file is part of the GDB test suite. + +# This file defines a pretty printer that is used as a test case for +# varobj/15166. + +import gdb + +# This iterator method is used to create "holes" in the `hole' class +# layout so that the indices of the type used by gdb (internally) are +# intentionally skewed from the pretty-printer, e.g., +# +# index 0 1 2 3 4 5 6 7 8 9 +# gdb a b c d e f g x y z +# PP a c e g + +def myiterator (var): + p = 0 + while p != 2 * 4: + if ((p % 2) == 0): + # key + value = "key" + str (p / 2) + else: + # value + i = ((p - 1) / 2) % 4 + if (i == 0): + name = 'a' + elif (i == 1): + name = 'c' + elif (i == 2): + name = 'e' + else: + name = 'g' + value = var.val[name] + + yield ("[" + str (p) + "]", value) + p += 1 + +class HolePrinter (object): + def __init__ (self, val): + self.val = val + + def to_string (self): + return "<" + str (self.val['a']) + "," + str (self.val['c']) + "," \ + + str (self.val['e']) + "," + str (self.val['g']) + ">" + + def display_hint (self): + return 'map' + + def children (self): + return myiterator (self) + +def hole_lookup_function (val): + "Look-up and return a pretty-printer that can print val." + + # Get the type. + type = val.type + + # If it points to a reference, get the reference. + if type.code == gdb.TYPE_CODE_REF: + type = type.target () + + # Get the unqualified type, stripped of typedefs. + type = type.unqualified ().strip_typedefs () + + # Get the type name. + typename = type.tag + + if typename == "hole": + return HolePrinter (val) + + return None + +gdb.pretty_printers.append (hole_lookup_function) --------------020404000903080802020804--