From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id Ee3KJRiwgWDYYQAAWB0awg (envelope-from ) for ; Thu, 22 Apr 2021 13:19:20 -0400 Received: by simark.ca (Postfix, from userid 112) id 89BFD1F104; Thu, 22 Apr 2021 13:19:20 -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 03A5C1E54D for ; Thu, 22 Apr 2021 13:19:19 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4D11B3894435; Thu, 22 Apr 2021 17:19:18 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4D11B3894435 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1619111958; bh=mKSFyNFD8XpRseAjoSbQsMUos6A5xvrJ3JnzfHgG/0k=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=CIpwUvDyk1m3kOzlDcWyfjgiu532V5ts6HVHsaa30ejk7YECHiE3dpac8tghxvsMA AQl7rIcbAHT7JtVTMeJwKMLDu0MX3Iml8afVLF/ZLdY9AMRuK66NryKZwnAKPJQ546 cL+ImKgWROh7osVe8wvDmEmJ1fpQh4oAb6A9a70Y= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id A039C3844036 for ; Thu, 22 Apr 2021 17:19:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A039C3844036 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 13MHJ8U9023295 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 22 Apr 2021 13:19:13 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 13MHJ8U9023295 Received: from [10.0.0.11] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 4009A1E54D; Thu, 22 Apr 2021 13:19:08 -0400 (EDT) Subject: Re: [PATCH master + gdb-10-branch] gdb: fix getting range of flexible array member in Python To: Andrew Burgess References: <20210422155950.3520038-1-simon.marchi@polymtl.ca> <20210422164830.GQ2610@embecosm.com> Message-ID: Date: Thu, 22 Apr 2021 13:19:07 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: <20210422164830.GQ2610@embecosm.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Thu, 22 Apr 2021 17:19:08 +0000 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: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" 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. 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. + + 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