Re: [PATCH master + gdb-10-branch] gdb: fix getting range of flexible array member in Python

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> [2021-04-22 11:59:50 -0400]:

> As reported in bug 27757, we get an internal error when doing:
> 
>     $ cat test.c
>     struct foo {
>         int len;
>         int items[];
>     };
> 
>     struct foo *p;
> 
>     int main() {
>         return 0;
>     }
>     $ gcc test.c -g -O0 -o test
>     $ ./gdb -q -nx --data-directory=data-directory ./test -ex 'python gdb.parse_and_eval("p").type.target()["items"].type.range()'
>     Reading symbols from ./test...
>     /home/simark/src/binutils-gdb/gdb/gdbtypes.h:435: internal-error: LONGEST dynamic_prop::const_val() const: Assertion `m_kind == PROP_CONST' failed.
>     A problem internal to GDB has been detected,
>     further debugging may prove unreliable.
>     Quit this debugging session? (y or n)
> 
> This is because the Python code (typy_range) blindly reads the high
> bound of the type of `items` as a constant value.  Since it is a
> flexible array member, it has no high bound, the property is undefined.
> Since commit 8c2e4e0689 ("gdb: add accessors to struct dynamic_prop"),
> the getters check that you are not getting a property value of the wrong
> kind, so this causes a failed assertion.
> 
> Fix it by checking if the property is indeed a constant value before
> accessing it as such.  Otherwise, use 0.  This restores the previous GDB
> behavior: because the structure was zero-initialized, this is what was
> returned before.  But now this behavior is explicit and not accidental.
> 
> Add a test, gdb.python/flexible-array-member.exp, that is derived from
> gdb.base/flexible-array-member.exp.  It tests the same things, but
> through the Python API.  It also specifically tests getting the range
> from the various kinds of flexible array member types (AFAIK it wasn't
> possible to do the equivalent through the CLI).
> 
> gdb/ChangeLog:
> 
> 	PR gdb/27757
> 	* python/py-type.c (typy_range): Check that bounds are constant
> 	before accessing them as such.
> 
> gdb/testsuite/ChangeLog:
> 
> 	PR gdb/27757
> 	* gdb.python/flexible-array-member.c: New test.
> 	* gdb.python/flexible-array-member.exp: New test.
> 
> Change-Id: Ibef92ee5fd871ecb7c791db2a788f203dff2b841
> ---
>  gdb/python/py-type.c                          | 11 ++-
>  .../gdb.python/flexible-array-member.c        | 70 ++++++++++++++++
>  .../gdb.python/flexible-array-member.exp      | 81 +++++++++++++++++++
>  3 files changed, 160 insertions(+), 2 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.python/flexible-array-member.c
>  create mode 100644 gdb/testsuite/gdb.python/flexible-array-member.exp
> 
> diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
> index bf707078841f..148e4a6aa3a0 100644
> --- a/gdb/python/py-type.c
> +++ b/gdb/python/py-type.c
> @@ -592,8 +592,15 @@ typy_range (PyObject *self, PyObject *args)
>      case TYPE_CODE_ARRAY:
>      case TYPE_CODE_STRING:
>      case TYPE_CODE_RANGE:
> -      low = type->bounds ()->low.const_val ();
> -      high = type->bounds ()->high.const_val ();;

I think the same bug exists in guile/scm-type.c, even if we don't add
a test it is probably worth duplicating the fix there too.

Also I wonder if, now we have defined this behaviour, should we add
this to the docs?

Thanks,
Andrew


> +      if (type->bounds ()->low.kind () == PROP_CONST)
> +	low = type->bounds ()->low.const_val ();
> +      else
> +	low = 0;
> +
> +      if (type->bounds ()->high.kind () == PROP_CONST)
> +	high = type->bounds ()->high.const_val ();
> +      else
> +	high = 0;
>        break;
>      }
>  
> diff --git a/gdb/testsuite/gdb.python/flexible-array-member.c b/gdb/testsuite/gdb.python/flexible-array-member.c
> new file mode 100644
> index 000000000000..79815e2d38e0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/flexible-array-member.c
> @@ -0,0 +1,70 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2020-2021 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/>.  */
> +
> +#include <stdlib.h>
> +
> +struct no_size
> +{
> +  int n;
> +  int items[];
> +};
> +
> +struct zero_size
> +{
> +  int n;
> +  int items[0];
> +};
> +
> +struct zero_size_only
> +{
> +  int items[0];
> +};
> +
> +struct no_size *ns;
> +struct zero_size *zs;
> +struct zero_size_only *zso;
> +
> +static void
> +break_here (void)
> +{
> +}
> +
> +int
> +main (void)
> +{
> +  ns = (struct no_size *) malloc (sizeof (*ns) + 3 * sizeof (int));
> +  zs = (struct zero_size *) malloc (sizeof (*zs) + 3 * sizeof (int));
> +  zso = (struct zero_size_only *) malloc (sizeof (*zso) + 3 * sizeof (int));
> +
> +  ns->n = 3;
> +  ns->items[0] = 101;
> +  ns->items[1] = 102;
> +  ns->items[2] = 103;
> +
> +  zs->n = 3;
> +  zs->items[0] = 201;
> +  zs->items[1] = 202;
> +  zs->items[2] = 203;
> +
> +  zso->items[0] = 301;
> +  zso->items[1] = 302;
> +  zso->items[2] = 303;
> +
> +  break_here ();
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.python/flexible-array-member.exp b/gdb/testsuite/gdb.python/flexible-array-member.exp
> new file mode 100644
> index 000000000000..3739c9a9e5c0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/flexible-array-member.exp
> @@ -0,0 +1,81 @@
> +# Copyright 2020-2021 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/>.
> +
> +# Test getting the range of flexible array members in Python.
> +
> +standard_testfile
> +
> +if { [prepare_for_testing "failed to prepare" \
> +	${testfile} ${srcfile}] } {
> +    return
> +}
> +
> +if { ![runto break_here] } {
> +    untested "could not run to break_here"
> +    return
> +}
> +
> +# The various cases are:
> +#
> +#  - ns: flexible array member with no size
> +#  - zs: flexible array member with size 0 (GNU C extension that predates the
> +#    standardization of the feature, but widely supported)
> +#  - zso: zero-size only, a corner case where the array is the sole member of
> +#    the structure
> +
> +gdb_test "python ns = gdb.parse_and_eval('ns').dereference()"
> +gdb_test "python zs = gdb.parse_and_eval('zs').dereference()"
> +gdb_test "python zso = gdb.parse_and_eval('zso').dereference()"
> +
> +# Print the whole structure.
> +
> +gdb_test "python print(ns)" "{n = 3, items = $hex}"
> +gdb_test "python print(zs)" "{n = 3, items = $hex}"
> +gdb_test "python print(zso)" "{items = $hex}"
> +
> +# Print all items.
> +
> +gdb_test "python print(ns\['items'\])" "$hex"
> +gdb_test "python print(ns\['items'\]\[0\])" "101"
> +gdb_test "python print(ns\['items'\]\[1\])" "102"
> +gdb_test "python print(ns\['items'\]\[2\])" "103"
> +
> +gdb_test "python print(zs\['items'\])" "$hex"
> +gdb_test "python print(zs\['items'\]\[0\])" "201"
> +gdb_test "python print(zs\['items'\]\[1\])" "202"
> +gdb_test "python print(zs\['items'\]\[2\])" "203"
> +
> +gdb_test "python print(zso\['items'\])" "$hex"
> +gdb_test "python print(zso\['items'\]\[0\])" "301"
> +gdb_test "python print(zso\['items'\]\[1\])" "302"
> +gdb_test "python print(zso\['items'\]\[2\])" "303"
> +
> +# Check taking the address of array elements (how PR 28675 was originally
> +# reported).
> +
> +gdb_test "python print(ns\['items'\] == ns\['items'\]\[0\].address)" "True"
> +gdb_test "python print(ns\['items'\]\[0\].address + 1 == ns\['items'\]\[1\].address)" "True"
> +gdb_test "python print(zs\['items'\] == zs\['items'\]\[0\].address)" "True"
> +gdb_test "python print(zs\['items'\]\[0\].address + 1 == zs\['items'\]\[1\].address)" "True"
> +gdb_test "python print(zso\['items'\] == zso\['items'\]\[0\].address)" "True"
> +gdb_test "python print(zso\['items'\]\[0\].address + 1 == zso\['items'\]\[1\].address)" "True"
> +
> +# Verify the range attribute.  It looks a bit inconsistent that the high bound
> +# is sometimes 0, sometimes -1, but that's what GDB produces today, so that's
> +# what we test.
> +
> +gdb_test "python print(ns\['items'\].type.range())" "\\(0, 0\\)"
> +gdb_test "python print(zs\['items'\].type.range())" "\\(0, -1\\)"
> +gdb_test "python print(zso\['items'\].type.range())" "\\(0, -1\\)"
> -- 
> 2.30.1
>