From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 0tSlFWu3gWCQYgAAWB0awg (envelope-from ) for ; Thu, 22 Apr 2021 13:50:35 -0400 Received: by simark.ca (Postfix, from userid 112) id 4D03B1F104; Thu, 22 Apr 2021 13:50:35 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id BE9F11E54D for ; Thu, 22 Apr 2021 13:50:33 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 320363894434; Thu, 22 Apr 2021 17:50:33 +0000 (GMT) Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by sourceware.org (Postfix) with ESMTPS id 7697B385482D for ; Thu, 22 Apr 2021 17:50:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 7697B385482D Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x429.google.com with SMTP id c15so36724476wro.13 for ; Thu, 22 Apr 2021 10:50:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=9tg+Gj2yf3NupKD/yiJM+PDPDV1ignSSWQFXOseCT0g=; b=aayJm+EqG/AhPk0kIxx7N9Irn4hlr2b86O44RGgOIQpC7lGLETHT7Q0AN2D4P7eNog z59mq2GEkZgcHFCm0TWRVAZ8SS5pq5QZOkidyw7H7ri04p/wgMqdBjmKTOWiemAf8c7u REkN4bbssnlbkaAiEJ6s02uBDPZOPLEgskhdSbwPhnzVrh4kPOE+1lO0uq9HBK5dId6c kByj+xBV5X3td0nkQJTwaLyZ7CE9+GoIiOZnvAEYcjT4ZZt5AyAcTP+QUliHddCNnkUc mspfhMJfWoWmJmoBpzw4ipSNm8PEurBUc2Dv3JqqRIb4hMMI2xpkvPn0WTobvOLgT4Q9 Gn7w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=9tg+Gj2yf3NupKD/yiJM+PDPDV1ignSSWQFXOseCT0g=; b=HIy8WzgKoxbWDuyUDP8/EGgNi6+giIPT6Ju4PFJS178qcEe8MoafRO1g6WRy5piOqk 8hN0wDkJ0w7YPg06SyHpHGgVwFofpPpzJdkYKIEfWdsGKFMCuU1hgceFriHKacjAVbDu QerVnFmw83FHgPNcSwBA+Yo/UuHq1I4KbQrJCV4gukhPz/naU+esBPXvcjaPzV1xvcSs T+701rUnWWeYAn2QC4qGkPNA+qIVrGd5ZUNvKnMzgZ8zLannzseoiEBwvBZXkijH1TRM w2ECmxX0zTa41a80SAl2ckuZg+T/E4eWilaS3SUsGEx5ZBbIGmyMBRPU4xjivZMXU7re QBUQ== X-Gm-Message-State: AOAM531NHkWbctOgWtTjl39J7EVUleDHvQO3KBiacQfvl4FNXMys8LxA Doq1D7PG369WKlSJ8Dp3o65ZKO26VSsG8Q== X-Google-Smtp-Source: ABdhPJx3O6WkJ52TTb6Ze6Q+03SA1uT3rdyX21redyvwO/TO3SGOKgECKT+/AxmRgFLOwzMDD4APeg== X-Received: by 2002:a5d:45ce:: with SMTP id b14mr5489372wrs.357.1619113828563; Thu, 22 Apr 2021 10:50:28 -0700 (PDT) Received: from localhost (host109-151-46-70.range109-151.btcentralplus.com. [109.151.46.70]) by smtp.gmail.com with ESMTPSA id u4sm8219950wml.0.2021.04.22.10.50.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 22 Apr 2021 10:50:28 -0700 (PDT) Date: Thu, 22 Apr 2021 18:50:27 +0100 From: Andrew Burgess To: Simon Marchi Subject: Re: [PATCH master + gdb-10-branch] gdb: fix getting range of flexible array member in Python Message-ID: <20210422175027.GR2610@embecosm.com> References: <20210422155950.3520038-1-simon.marchi@polymtl.ca> <20210422164830.GQ2610@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 18:48:55 up 12 days, 4:36, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" * Simon Marchi [2021-04-22 13:19:07 -0400]: > On 2021-04-22 12:48 p.m., Andrew Burgess wrote: > > * Simon Marchi via Gdb-patches [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 > 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 . */ > > +#include > + > 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 . */ > + > +#include > + > +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 . > + > +# 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 >