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

Message ID 20210422155950.3520038-1-simon.marchi@polymtl.ca
State New
Headers show
Series
  • [master,+,gdb-10-branch] gdb: fix getting range of flexible array member in Python
Related show

Commit Message

Konstantin Kharlamov via Gdb-patches April 22, 2021, 3:59 p.m.
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

-- 
2.30.1

Comments

Andrew Burgess April 22, 2021, 4:48 p.m. | #1
* 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

>
Konstantin Kharlamov via Gdb-patches April 22, 2021, 5:19 p.m. | #2
On 2021-04-22 12:48 p.m., Andrew Burgess wrote:
> * 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.


Thanks, you are right.  I added a test too (although the most
minimalistic one that would trigger the bug).  See updated patch below.

> Also I wonder if, now we have defined this behaviour, should we add

> this to the docs?


As you can see in the Python test, we get two different results for the
high bound (0 and -1) for cases where I think we should get the same
result.  So before formalizing it in the doc, I think we should address
that, make it always 0 or always -1.  I don't know if that would be an
unacceptable backwards-incompatible breakage though.  To be clear, I
would suggest changing the behavior in master, not gdb-10-branch.

Simon


From edf11b9916c87a67fb1c6aa662c4c5eaed271445 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>

Date: Thu, 22 Apr 2021 11:59:50 -0400
Subject: [PATCH] gdb: fix getting range of flexible array member in Python

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.
	* guile/scm-type.c (gdbscm_type_range): Likewise.

gdb/testsuite/ChangeLog:

	PR gdb/27757
	* gdb.python/flexible-array-member.c: New test.
	* gdb.python/flexible-array-member.exp: New test.
	* gdb.guile/scm-type.exp (test_range): Add test for flexible
	array member.
	* gdb.guile/scm-type.c (struct flex_member): New.
	(main): Use it.

Change-Id: Ibef92ee5fd871ecb7c791db2a788f203dff2b841
---
 gdb/guile/scm-type.c                          | 11 ++-
 gdb/python/py-type.c                          | 11 ++-
 gdb/testsuite/gdb.guile/scm-type.c            | 12 +++
 gdb/testsuite/gdb.guile/scm-type.exp          | 12 +++
 .../gdb.python/flexible-array-member.c        | 70 ++++++++++++++++
 .../gdb.python/flexible-array-member.exp      | 81 +++++++++++++++++++
 6 files changed, 193 insertions(+), 4 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/guile/scm-type.c b/gdb/guile/scm-type.c
index dd45d8e8b35c..11693844edcb 100644
--- a/gdb/guile/scm-type.c
+++ b/gdb/guile/scm-type.c
@@ -823,8 +823,15 @@ gdbscm_type_range (SCM self)
     case TYPE_CODE_ARRAY:
     case TYPE_CODE_STRING:
     case TYPE_CODE_RANGE:
-      low = type->bounds ()->low.const_val ();
-      high = type->bounds ()->high.const_val ();
+      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/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 ();;
+      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.guile/scm-type.c b/gdb/testsuite/gdb.guile/scm-type.c
index 782b8fe70cf9..64f5f02efedc 100644
--- a/gdb/testsuite/gdb.guile/scm-type.c
+++ b/gdb/testsuite/gdb.guile/scm-type.c
@@ -15,6 +15,8 @@
    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 s
 {
   int a;
@@ -53,6 +55,12 @@ enum E
 struct s vec_data_1 = {1, 1};
 struct s vec_data_2 = {1, 2};
 
+struct flex_member
+{
+  int n;
+  int items[];
+};
+
 int
 main ()
 {
@@ -72,6 +80,10 @@ main ()
   st.b = 5;
 
   e = v2;
+
+  struct flex_member *f = (struct flex_member *) malloc (100);
+  f->items[0] = 111;
+  f->items[1] = 222;
   
   return 0;      /* break to inspect struct and array.  */
 }
diff --git a/gdb/testsuite/gdb.guile/scm-type.exp b/gdb/testsuite/gdb.guile/scm-type.exp
index 8778cdb36cb1..517c99f8369d 100644
--- a/gdb/testsuite/gdb.guile/scm-type.exp
+++ b/gdb/testsuite/gdb.guile/scm-type.exp
@@ -261,6 +261,18 @@ proc test_range {} {
 		"ERROR: .*: Wrong type argument in position 1 \\(expecting ranged type\\): .*" \
 		"check range for non ranged type"
 	}
+
+	with_test_prefix "on flexible array member" {
+	    gdb_scm_test_silent_cmd "print f" "print value (f)"
+	    gdb_scm_test_silent_cmd "guile (define f (history-ref 0))" \
+		"get value (f) from history"
+	    gdb_test "guile (print (type-range (field-type (type-field (value-type (value-dereference f)) \"items\"))))" \
+		"= \\(0 0\\)"
+	    gdb_test "guile (print (value-subscript (value-field (value-dereference f) \"items\") 0))" \
+		"= 111"
+	    gdb_test "guile (print (value-subscript (value-field (value-dereference f) \"items\") 1))" \
+		"= 222"
+	}
     }
 }
 
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
Andrew Burgess April 22, 2021, 5:50 p.m. | #3
* Simon Marchi <simon.marchi@polymtl.ca> [2021-04-22 13:19:07 -0400]:

> On 2021-04-22 12:48 p.m., Andrew Burgess wrote:

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

> 

> Thanks, you are right.  I added a test too (although the most

> minimalistic one that would trigger the bug).  See updated patch below.

> 

> > Also I wonder if, now we have defined this behaviour, should we add

> > this to the docs?

> 

> As you can see in the Python test, we get two different results for the

> high bound (0 and -1) for cases where I think we should get the same

> result.  So before formalizing it in the doc, I think we should address

> that, make it always 0 or always -1.  I don't know if that would be an

> unacceptable backwards-incompatible breakage though.  To be clear, I

> would suggest changing the behavior in master, not gdb-10-branch.

> 

> Simon

> 

> 

> From edf11b9916c87a67fb1c6aa662c4c5eaed271445 Mon Sep 17 00:00:00 2001

> From: Simon Marchi <simon.marchi@polymtl.ca>

> Date: Thu, 22 Apr 2021 11:59:50 -0400

> Subject: [PATCH] gdb: fix getting range of flexible array member in Python

> 

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

> 	* guile/scm-type.c (gdbscm_type_range): Likewise.

> 

> gdb/testsuite/ChangeLog:

> 

> 	PR gdb/27757

> 	* gdb.python/flexible-array-member.c: New test.

> 	* gdb.python/flexible-array-member.exp: New test.

> 	* gdb.guile/scm-type.exp (test_range): Add test for flexible

> 	array member.

> 	* gdb.guile/scm-type.c (struct flex_member): New.

> 	(main): Use it.


Looks good, just one minor nit...

> 

> Change-Id: Ibef92ee5fd871ecb7c791db2a788f203dff2b841

> ---

>  gdb/guile/scm-type.c                          | 11 ++-

>  gdb/python/py-type.c                          | 11 ++-

>  gdb/testsuite/gdb.guile/scm-type.c            | 12 +++

>  gdb/testsuite/gdb.guile/scm-type.exp          | 12 +++

>  .../gdb.python/flexible-array-member.c        | 70 ++++++++++++++++

>  .../gdb.python/flexible-array-member.exp      | 81 +++++++++++++++++++

>  6 files changed, 193 insertions(+), 4 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/guile/scm-type.c b/gdb/guile/scm-type.c

> index dd45d8e8b35c..11693844edcb 100644

> --- a/gdb/guile/scm-type.c

> +++ b/gdb/guile/scm-type.c

> @@ -823,8 +823,15 @@ gdbscm_type_range (SCM self)

>      case TYPE_CODE_ARRAY:

>      case TYPE_CODE_STRING:

>      case TYPE_CODE_RANGE:

> -      low = type->bounds ()->low.const_val ();

> -      high = type->bounds ()->high.const_val ();

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

> +      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.guile/scm-type.c b/gdb/testsuite/gdb.guile/scm-type.c

> index 782b8fe70cf9..64f5f02efedc 100644

> --- a/gdb/testsuite/gdb.guile/scm-type.c

> +++ b/gdb/testsuite/gdb.guile/scm-type.c

> @@ -15,6 +15,8 @@

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

>  {

>    int a;

> @@ -53,6 +55,12 @@ enum E

>  struct s vec_data_1 = {1, 1};

>  struct s vec_data_2 = {1, 2};

>  

> +struct flex_member

> +{

> +  int n;

> +  int items[];

> +};

> +

>  int

>  main ()

>  {

> @@ -72,6 +80,10 @@ main ()

>    st.b = 5;

>  

>    e = v2;

> +

> +  struct flex_member *f = (struct flex_member *) malloc (100);

> +  f->items[0] = 111;

> +  f->items[1] = 222;

>    

>    return 0;      /* break to inspect struct and array.  */

>  }

> diff --git a/gdb/testsuite/gdb.guile/scm-type.exp b/gdb/testsuite/gdb.guile/scm-type.exp

> index 8778cdb36cb1..517c99f8369d 100644

> --- a/gdb/testsuite/gdb.guile/scm-type.exp

> +++ b/gdb/testsuite/gdb.guile/scm-type.exp

> @@ -261,6 +261,18 @@ proc test_range {} {

>  		"ERROR: .*: Wrong type argument in position 1 \\(expecting ranged type\\): .*" \

>  		"check range for non ranged type"

>  	}

> +

> +	with_test_prefix "on flexible array member" {

> +	    gdb_scm_test_silent_cmd "print f" "print value (f)"

> +	    gdb_scm_test_silent_cmd "guile (define f (history-ref 0))" \

> +		"get value (f) from history"

> +	    gdb_test "guile (print (type-range (field-type (type-field (value-type (value-dereference f)) \"items\"))))" \

> +		"= \\(0 0\\)"

> +	    gdb_test "guile (print (value-subscript (value-field (value-dereference f) \"items\") 0))" \

> +		"= 111"

> +	    gdb_test "guile (print (value-subscript (value-field (value-dereference f) \"items\") 1))" \

> +		"= 222"

> +	}

>      }

>  }

>  

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


In this file and the .exp the date range is 2020-2021, is this
correct?

Otherwise, looks good.

Thanks,
Andrew


> +

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

>
Konstantin Kharlamov via Gdb-patches April 22, 2021, 7:07 p.m. | #4
On 2021-04-22 1:50 p.m., Andrew Burgess wrote:
>> 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.

> 

> In this file and the .exp the date range is 2020-2021, is this

> correct?


Since I copied it from gdb.base/flexible-array-member.c, it should
retain the original copyright date.

> Otherwise, looks good.


Thanks, pushed to both branches!

Simon
Konstantin Kharlamov via Gdb-patches April 22, 2021, 7:40 p.m. | #5
Hi Simon,

On 4/22/21 4:07 PM, Simon Marchi via Gdb-patches wrote:
> On 2021-04-22 1:50 p.m., Andrew Burgess wrote:

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

>>

>> In this file and the .exp the date range is 2020-2021, is this

>> correct?

> 

> Since I copied it from gdb.base/flexible-array-member.c, it should

> retain the original copyright date.

> 

>> Otherwise, looks good.

> 

> Thanks, pushed to both branches!

> 

> Simon

> 


I notice this test being executed without python support. I think we 
need this guard:

# Skip all tests if Python scripting is not enabled.
if { [skip_python_tests] } { continue }
Konstantin Kharlamov via Gdb-patches April 22, 2021, 7:47 p.m. | #6
On 2021-04-22 3:40 p.m., Luis Machado wrote:
> Hi Simon,

> 

> On 4/22/21 4:07 PM, Simon Marchi via Gdb-patches wrote:

>> On 2021-04-22 1:50 p.m., Andrew Burgess wrote:

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

>>>

>>> In this file and the .exp the date range is 2020-2021, is this

>>> correct?

>>

>> Since I copied it from gdb.base/flexible-array-member.c, it should

>> retain the original copyright date.

>>

>>> Otherwise, looks good.

>>

>> Thanks, pushed to both branches!

>>

>> Simon

>>

> 

> I notice this test being executed without python support. I think we need this guard:

> 

> # Skip all tests if Python scripting is not enabled.

> if { [skip_python_tests] } { continue }


You're right, thanks for noticing.  I pushed the patch below in the two
branches:

From a87caa6d523e573ab71694233a7501c6b5b5b9f7 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>

Date: Thu, 22 Apr 2021 15:45:34 -0400
Subject: [PATCH] gdb/testsuite: add Python support check in
 gdb.python/flexible-array-member.exp

We don't want to execute this test if Python support is not compiled in
GDB, add the necessary check.

gdb/testsuite/ChangeLog:

	* gdb.python/flexible-array-member.exp: Add check for Python
	support.

Change-Id: I853b937d2a193a0bb216566bef1a35354264b1c5
---
 gdb/testsuite/ChangeLog                            | 5 +++++
 gdb/testsuite/gdb.python/flexible-array-member.exp | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 9730658299bf..266645bd6ad6 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2021-04-22  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* gdb.python/flexible-array-member.exp: Add check for Python
+	support.
+
 2021-04-22  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	PR gdb/27757
diff --git a/gdb/testsuite/gdb.python/flexible-array-member.exp b/gdb/testsuite/gdb.python/flexible-array-member.exp
index 3739c9a9e5c0..349670cb7e72 100644
--- a/gdb/testsuite/gdb.python/flexible-array-member.exp
+++ b/gdb/testsuite/gdb.python/flexible-array-member.exp
@@ -22,6 +22,9 @@ if { [prepare_for_testing "failed to prepare" \
     return
 }
 
+# Skip all tests if Python scripting is not enabled.
+if { [skip_python_tests] } { continue }
+
 if { ![runto break_here] } {
     untested "could not run to break_here"
     return
-- 
2.30.1

Patch

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 ();;
+      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\\)"