From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id brbuHD0BJmEYdwAAWB0awg (envelope-from ) for ; Wed, 25 Aug 2021 04:37:17 -0400 Received: by simark.ca (Postfix, from userid 112) id 62D061EE1A; Wed, 25 Aug 2021 04:37:17 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (ip-8-43-85-97.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 963251EDF0 for ; Wed, 25 Aug 2021 04:37:15 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 22B453857011 for ; Wed, 25 Aug 2021 08:37:15 +0000 (GMT) Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by sourceware.org (Postfix) with ESMTPS id 4DB823857825 for ; Wed, 25 Aug 2021 08:36:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4DB823857825 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com Received: by mail-wr1-x434.google.com with SMTP id z4so19511977wrr.6 for ; Wed, 25 Aug 2021 01:36:53 -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:content-transfer-encoding:in-reply-to; bh=r1bfaDMYp1/ZUVlenmPGNhda/IjLYfsLvvoTc/IvMOc=; b=BADcJLYk3ryPKjUBIlmrSib5rD+ULl0yBCTpyq/rCf1CUuc8v5C39XCLpFyfLhZZWp VpjqB6nVZzCQWSRRVFNLS5irw+0tpGQooS4XacqaxlYdTUAJhdIGRAglAGofdhn7q9KO 7WOURlpmdciDG1oTVwOCo/VVOlHS7iG3TJGLkib5lLMf86DkqApfHb5+9vOqGI9RxfRq Y9ZFT/MXDoK6LSoAWE/pa5b1m+NVZm2C3A2IgaMhnpTlCxEQEc2iavZmaRqYzJ88dljv ARynUZ23F+25wp0NA35PP0rychlDkLOqH1jm2OmiCsXHfAgmfVlOSBJVq/ugLCEs/x8x 5iTA== 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:content-transfer-encoding :in-reply-to; bh=r1bfaDMYp1/ZUVlenmPGNhda/IjLYfsLvvoTc/IvMOc=; b=mRRXQMlXC4xbKlxUjIplWlHjFT9KpYVPgvA3qYmUxxRLx/KzjVHzdXzLQHcP2KpCpO DbNX6u1eBfcluzBlbhHMW4pBfOZ7j8KnZde1AOu0pj1XIiT5/caDDZRfp8QoXpqhJ7nx 31/yHyVvT6i8vJSex+ZIGfp8Fj5JPwMOzvNZ3NNb73YUXorl2yXT/lZtrMtSe3E5imJu f0tjzW7me6eR5eWk8G2PHmIHa4mx3eAxz/nw3wbZQo/mABw/epe+Sz/hpCutEbhKVt6Y 7Ut64ClZHKYYEsm5toXO7L13k9Miw8p33/FgsTXhJXkiJnEo/FGB+lh47Gnxo1a8KL/B EfVA== X-Gm-Message-State: AOAM530VlGYvESNLPmnWbly8fcm4w1ZIVbVF+0RSZriTJ74z7ULLer17 OCJEqjVIk5ETubZJ2zBV021TkA== X-Google-Smtp-Source: ABdhPJzDRe44iR9D30v15sBEV2pTwiz1WX/0kZ3v5NyuC1nYIy7+lczIKhEeMAaw2Clak92bc1A/TQ== X-Received: by 2002:a5d:5908:: with SMTP id v8mr23197308wrd.8.1629880612195; Wed, 25 Aug 2021 01:36:52 -0700 (PDT) Received: from localhost (host86-188-49-44.range86-188.btcentralplus.com. [86.188.49.44]) by smtp.gmail.com with ESMTPSA id z137sm4835804wmc.14.2021.08.25.01.36.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Aug 2021 01:36:51 -0700 (PDT) Date: Wed, 25 Aug 2021 09:36:50 +0100 From: Andrew Burgess To: "Ijaz, Abdul B" Subject: Re: [PATCH 1/1] gdb: Fix arrays of variable length strings for FORTRAN Message-ID: <20210825083650.GF2581@embecosm.com> References: <20210820110638.26648-1-abdul.b.ijaz@intel.com> <20210820110638.26648-2-abdul.b.ijaz@intel.com> <20210820155247.GA2581@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 09:27:31 up 7 days, 21:23, 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: "abdul.b.ijaz" , "gdb-patches@sourceware.org" Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" * Ijaz, Abdul B [2021-08-23 08:47:46 +0000]: > > GDB is not able to print arrays of FORTRAN variable length strings=20 > > when dwarf info contains DW_AT_string_length only regarding the string= =20 > > length information. So handing of dynamic array is updated to handle= =20 > > such cases. >=20 > Thanks for working on this. I have some feedback, see below... > >> Thanks Andrew for the feedback. I added my response below. Can you ple= ase review if it looks fine to you so I will update it accordingly.=20 >=20 > >=20 > > Suppose we have > >=20 > > subroutine vla_array (arr1, arr2) > > character (len=3D*):: arr1 (:) > > character (len=3D5):: arr2 (:) > >=20 > > print *, arr1 ! break-here > > print *, arr2 > > end subroutine vla_array > >=20 > > The "print arr1" and "print arr2" command at the "break-here" line=20 > > gives the following output: > >=20 > > (gdb) print arr1 > > $1 =3D ( > > (gdb) print arr2 > > $2 =3D ('abcde', 'abcde', 'abcde') > > (gdb) ptype arr1 > > type =3D character*(*) (5) > > (gdb) ptype arr2 > > type =3D character*5 (3) > >=20 > > So GDB is able to print the array of string with static length but for= =20 > > variable length it fail to do so. For this case now improve handling= =20 > > of the TYPE_CODE_STRING code in dynamic array length resolving=20 > > function to set the static length of strings as well when only=20 > > DW_AT_string_length is given in the dwarf info. > >=20 > > Dwarf info using Intel=AE Fortran Compiler for such case contains follo= wing: > > <1>: Abbrev Number: 12 (DW_TAG_string_type) > > DW_AT_name : (indirect string, offset: 0xd2): .str.AR= R1 > > <102> DW_AT_string_length: 3 byte block: 97 23 8 (DW_OP_push_obj= ect_address; DW_OP_plus_uconst: 8) >=20 > I don't think the extra indentation before "DW_AT_string_length" is corre= ct here, this attribute is a child of the DW_TAG_string_type, right? Not a= child of the DW_AT_name. >=20 > >> This is output of readelf also there is no extra indentation > >> before "DW_AT_string_length". DW_AT_name has 2 digits in front > >> while DW_AT_string_lengt has 3 "<102>" so this is 1 more to > >> right. Yes "DW_AT_string_length" is a child of the > >> DW_TAG_string_type. Ah, OK that makes sense. No need to change anything then. >=20 >=20 > >=20 > > After fixing it and compiling using Intel=AE Fortran Compiler now gdb= =20 > > shows > > following: > >=20 > > (gdb) p arr1 > > $1 =3D ('abddefghij', 'abddefghij', 'abddefghij', 'abddefghij',=20 > > 'abddefghij') > > (gdb) p arr2 > > $2 =3D ('abcde', 'abcde', 'abcde') > > (gdb) ptype arr1 > > type =3D character*10 (5) > > (gdb) ptype arr2 > > type =3D character*5 (3) > >=20 > > gdb/ChangeLog: > > 2021-08-20 Abdul Basit Ijaz > >=20 > > * gdbtypes.c (resolve_dynamic_array_or_string): Improve handling > > of TYPE_CODE_STRING code to use return value of create_string_type > > outcome for this case. > > * c-valprint.c (c_value_print_inner): Handle String type code > > in the same way as the Array type code. > >=20 > > gdb/testsuite/ChangeLog: > > 2021-08-20 Abdul Basit Ijaz > >=20 > > * gdb.fortran/vla-array.f90: New fie. > > * gdb.fortran/vla-array.exp: New fie. > >=20 > > 2021-08-20 Abdul Basit Ijaz > > --- > > gdb/c-valprint.c | 1 + > > gdb/gdbtypes.c | 12 ++++-- > > gdb/testsuite/gdb.fortran/vla-array.exp | 57=20 > > +++++++++++++++++++++++++ > > gdb/testsuite/gdb.fortran/vla-array.f90 | 44 +++++++++++++++++++ > > 4 files changed, 110 insertions(+), 4 deletions(-) create mode=20 > > 100644 gdb/testsuite/gdb.fortran/vla-array.exp > > create mode 100644 gdb/testsuite/gdb.fortran/vla-array.f90 > >=20 > > diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c index=20 > > 9c82869525f..e5a8e122676 100644 > > --- a/gdb/c-valprint.c > > +++ b/gdb/c-valprint.c > > @@ -426,6 +426,7 @@ c_value_print_inner (struct value *val, struct ui_f= ile *stream, int recurse, > > switch (type->code ()) > > { > > case TYPE_CODE_ARRAY: > > + case TYPE_CODE_STRING: > > c_value_print_array (val, stream, recurse, options); > > break; >=20 > I don't understand what part this change plays in this patch. I can see = below how you're now creating values with TYPE_CODE_STRING instead of TYPE_= CODE_ARRAY, but then I'd expect these to be covered by the existing handlin= g of TYPE_CODE_STRING in f_language::value_print_inner. >=20 > Of course, if you forced the language to C while inside the Fortran frame= and tried to print the string value then I guess maybe you'd hit this case= , but I don't think your test does this, and if that is the case you're cov= ering here it might be worth splitting this into a separate commit to make = the split crystal clear. >=20 > >> Actually this part was only modified since after fixig the issue > >> test "gdb.fortran/mixed-lang-stack.exp" has shown regression for > >> exactly the same scenario you mentioned "forced the language to C > >> while inside the Fortran frame" so that is why it was updated in > >> the same patch and this change is cover by this test > >> "gdb.fortran/mixed-lang-stack.exp" already. So please let me > >> know shall we move it to separate patch. You should definitely mention this in the commit message I think, ideally, just mention which test showed the regression and why (e.g. printing a Fortran variable while the language is forced to C). >=20 >=20 >=20 > > =20 > > diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index=20 > > 74ad5d6f7fe..1444c5feb6c 100644 > > --- a/gdb/gdbtypes.c > > +++ b/gdb/gdbtypes.c > > @@ -2312,8 +2312,9 @@ resolve_dynamic_array_or_string (struct type *typ= e, > > range_type =3D resolve_dynamic_range (range_type, addr_stack,=20 > > resolve_p); > > =20 > > ary_dim =3D check_typedef (TYPE_TARGET_TYPE (type)); > > - if (ary_dim !=3D NULL && ary_dim->code () =3D=3D TYPE_CODE_ARRAY) > > - elt_type =3D resolve_dynamic_array_or_string (ary_dim, addr_stack,= resolve_p); > > + if (ary_dim !=3D NULL && (ary_dim->code () =3D=3D TYPE_CODE_ARRAY > > + || ary_dim->code () =3D=3D TYPE_CODE_STRING)) >=20 > >> Will update the indentation like you pointed out in second email. >=20 >=20 > > + elt_type =3D resolve_dynamic_array_or_string (ary_dim, addr_stack); I noticed you dropped `resolve_p` from the end of the argument list here. Is this intentional? > > else > > elt_type =3D TYPE_TARGET_TYPE (type); > > =20 > > @@ -2337,8 +2338,11 @@ resolve_dynamic_array_or_string (struct type *ty= pe, > > else > > bit_stride =3D TYPE_FIELD_BITSIZE (type, 0); > > =20 > > - return create_array_type_with_stride (type, elt_type, range_type, NU= LL, > > - bit_stride); > > + if (type->code () =3D=3D TYPE_CODE_STRING) > > + return create_string_type (type, elt_type, range_type); >=20 > I wonder if we should be doing anything with the bit_stride here? I'm no= t sure if it makes sense for the bit_stride to be anything other than zero,= but at the very least it feels like we should throw an error if the stride= is not zero .... >=20 > >> Regarding bit_stride it is not needed for string type. So shall > >> we can put a check to try reading it only for array type only > >> since it will not be needed for string type instead of throwing > >> error. I think we should do _something_ with the stride, given how closely related arrays and strings seem to be, maybe just this would do: prop =3D type->dyn_prop (DYN_PROP_BYTE_STRIDE); if (prop !=3D nullptr && type->code () =3D=3D TYPE_CODE_STRING) { prop =3D nullptr; warning (_("byte stride property ignored on string type")); } if this ever does crop up we can figure out what's going on then. Thanks, Andrew >=20 >=20 > > + else > > + return create_array_type_with_stride (type, elt_type, range_type, = NULL, > > + bit_stride); > > } > > =20 > > /* Resolve dynamic bounds of members of the union TYPE to static diff= =20 > > --git a/gdb/testsuite/gdb.fortran/vla-array.exp=20 > > b/gdb/testsuite/gdb.fortran/vla-array.exp > > new file mode 100644 > > index 00000000000..a9223576bbd > > --- /dev/null > > +++ b/gdb/testsuite/gdb.fortran/vla-array.exp > > @@ -0,0 +1,57 @@ > > +# Copyright 2021 Free Software Foundation, Inc. > > + > > +# This program is free software; you can redistribute it and/or=20 > > +modify # it under the terms of the GNU General Public License as=20 > > +published by # the Free Software Foundation; either version 3 of the= =20 > > +License, or # (at your option) any later version. > > +# > > +# This program is distributed in the hope that it will be useful, #=20 > > +but WITHOUT ANY WARRANTY; without even the implied warranty of #=20 > > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU=20 > > +General Public License for more details. > > +# > > +# You should have received a copy of the GNU General Public License #= =20 > > +along with this program. If not, see . > > + > > +standard_testfile ".f90" > > +load_lib "fortran.exp" > > + > > +if {[skip_fortran_tests]} { return -1 } > > + > > +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \ > > + {debug f90 quiet}] } { > > + return -1 > > +} > > + > > +if ![fortran_runto_main] { > > + untested "could not run to main" > > + return -1 > > +} > > + > > +# Try to access vla string / vla string array / string array values >=20 > Full stop at the end of the comment. > >> Will do >=20 > Thanks & Best Regards, > Abdul Basit >=20 >=20 > > +gdb_breakpoint [gdb_get_line_number "arr_vla1-print"]=20 > > +gdb_continue_to_breakpoint "arr_vla1-print" > > + > > +# GFortran does not emit DW_TAG_string_type for array of variable=20 > > +length # string. > > +if [test_compiler_info "gcc*"] { setup_xfail *-*-* gcc/101826 }=20 > > +gdb_test "print arr_vla1" \ > > + " =3D \\\('vlaaryvlaary', 'vlaaryvlaary', 'vlaaryvlaary', 'vlaaryv= laary', 'vlaaryvlaary'\\\)" \ > > + "print vla string array" > > + > > +if [test_compiler_info "gcc*"] { setup_xfail *-*-* gcc/101826 }=20 > > +gdb_test "ptype arr_vla1" \ > > + "type =3D character\\*12 \\(5\\)" \ > > + "print variable length string array type" > > +gdb_test "print arr_vla2" \ > > + " =3D 'vlaary'" \ > > + "print variable length string" > > +gdb_test "ptype arr_vla2" \ > > + "type =3D character\\*6" \ > > + "print variable length string type" > > +gdb_test "print arr2" \ > > + " =3D \\\('vlaaryvla', 'vlaaryvla', 'vlaaryvla'\\\)" \ > > + "print string array" > > +gdb_test "ptype arr2" \ > > + "type =3D character\\*9 \\(3\\)" \ > > + "print string array type" > > diff --git a/gdb/testsuite/gdb.fortran/vla-array.f90=20 > > b/gdb/testsuite/gdb.fortran/vla-array.f90 > > new file mode 100644 > > index 00000000000..612e84fe213 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.fortran/vla-array.f90 > > @@ -0,0 +1,44 @@ > > +! Copyright 2021 Free Software Foundation, Inc. > > +! > > +! This program is free software; you can redistribute it and/or=20 > > +modify ! it under the terms of the GNU General Public License as=20 > > +published by ! the Free Software Foundation; either version 3 of the= =20 > > +License, or ! (at your option) any later version. > > +! > > +! This program is distributed in the hope that it will be useful, !=20 > > +but WITHOUT ANY WARRANTY; without even the implied warranty of !=20 > > +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the ! GNU=20 > > +General Public License for more details. > > +! > > +! You should have received a copy of the GNU General Public License != =20 > > +along with this program. If not, see . > > + > > +subroutine vla_array_func (arr_vla1, arr_vla2, arr2) > > + character (len=3D*):: arr_vla1 (:) > > + character (len=3D*):: arr_vla2 > > + character (len=3D9):: arr2 (:) > > + > > + print *, arr_vla1 ! arr_vla1-print > > + print *, arr_vla2 > > + print *, arr2 > > +end subroutine vla_array_func > > + > > +program vla_array_main > > +interface > > + subroutine vla_array_func (arr_vla1, arr_vla2, arr2) > > + character (len=3D*):: arr_vla1 (:) > > + character (len=3D*):: arr_vla2 > > + character (len=3D9):: arr2 (:) > > + end subroutine vla_array_func > > +end interface > > + character (len=3D9) :: arr1 (3) > > + character (len=3D6) :: arr2 > > + character (len=3D12) :: arr3 (5) > > + > > + arr1 =3D 'vlaaryvla' > > + arr2 =3D 'vlaary' > > + arr3 =3D 'vlaaryvlaary' > > + > > + call vla_array_func (arr3, arr2, arr1) > > + > > +end program vla_array_main > > -- > > 2.31.1 > > > =20 > Intel Deutschland GmbH > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany > Tel: +49 89 99 8853-0, www.intel.de > Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva= =20 > Chairperson of the Supervisory Board: Nicole Lau > Registered Office: Munich > Commercial Register: Amtsgericht Muenchen HRB 186928