RE: [PING][PATCH 13/18] testsuite, fortran: fix info-types for intel compilers

RE: [PING][PATCH 13/18] testsuite, fortran: fix info-types for intel compilers

[Kempke, Nils-Christian]

Kindly pinging.

> -----Original Message-----
> From: Kempke, Nils-Christian <nils-christian.kempke@intel.com>
> Sent: Wednesday, May 11, 2022 6:44 PM
> To: Kempke, Nils-Christian <nils-christian.kempke@intel.com>; Andrew
> Burgess <aburgess@redhat.com>; gdb-patches@sourceware.org
> Subject: RE: [PATCH 13/18] testsuite, fortran: fix info-types for intel
> compilers
> 
> 
> 
> > -----Original Message-----
> > From: Gdb-patches <gdb-patches-bounces+nils-
> > christian.kempke=intel.com@sourceware.org> On Behalf Of Kempke, Nils-
> > Christian via Gdb-patches
> > Sent: Wednesday, May 11, 2022 5:20 PM
> > To: Andrew Burgess <aburgess@redhat.com>; gdb-
> > patches@sourceware.org
> > Subject: RE: [PATCH 13/18] testsuite, fortran: fix info-types for intel
> > compilers
> >
> > > -----Original Message-----
> > > From: Andrew Burgess <aburgess@redhat.com>
> > > Sent: Wednesday, May 11, 2022 2:06 PM
> > > To: Kempke, Nils-Christian <nils-christian.kempke@intel.com>; gdb-
> > > patches@sourceware.org
> > > Cc: JiniSusan.George@amd.com; Kempke, Nils-Christian <nils-
> > > christian.kempke@intel.com>
> > > Subject: Re: [PATCH 13/18] testsuite, fortran: fix info-types for intel
> > > compilers
> > >
> > > Nils-Christian Kempke <nils-christian.kempke@intel.com> writes:
> > >
> > > > First, the emitted symbol character*1 which is checked in the test
> > > > is not even referenced as a type in the compiled examples.  It seems
> > > > to be a gfortran specific check for some type that gets emitted always.
> > > > I changed the test to use check_optional_entry here to allow the
> > > > symbol's absence.
> > > >
> > > > Second, the line checked for s1 was hardcoded in the test.  Given that
> > > > the type is actually defined on line 41 (which is what is emitted by
> > > > ifx) it even seems wrong.  I changed the line check for s1 to actually
> > > > check for 41 and a gfortran bug has been filed here
> > > >
> > > >    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105454
> > > >
> > > > The test is now marked as xfail for gfortran.
> > > >
> > > > Third, the test was checking for s1 to be emitted by info types.  This
> > > > would mean that the type is put into compilation unit scope in the
> > DWARF
> > > > but, as it is local to the main program this is actually not expected
> > > > and gfortran specific.
> > > > Since I already added the xfail for gfortran here, I opted to also make
> > > > this check gfortran specific.
> > > > ---
> > > >  gdb/testsuite/gdb.fortran/info-types.exp | 10 +++++++---
> > > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/gdb/testsuite/gdb.fortran/info-types.exp
> > > b/gdb/testsuite/gdb.fortran/info-types.exp
> > > > index 67fe4d79c5..06770aada1 100644
> > > > --- a/gdb/testsuite/gdb.fortran/info-types.exp
> > > > +++ b/gdb/testsuite/gdb.fortran/info-types.exp
> > > > @@ -41,12 +41,16 @@ set real4 [fortran_real4]
> > > >  GDBInfoSymbols::run_command "info types"
> > > >  GDBInfoSymbols::check_header "All defined types:"
> > > >
> > > > -GDBInfoSymbols::check_entry "${srcfile}" "" "${character1}"
> > > > +GDBInfoSymbols::check_optional_entry "${srcfile}" "" "${character1}"
> > >
> > > Could we not just add a reference to character*1 type?  I'm happy to
> > > take this change, but just adding a use might make a stronger test?
> >
> > Yes, I'll do that. It is true, there will be a bit more coverage.
> >
> > > >  GDBInfoSymbols::check_entry "${srcfile}" "" "${integer4}"
> > > >  GDBInfoSymbols::check_entry "${srcfile}" "" "${logical4}"
> > > >  GDBInfoSymbols::check_entry "${srcfile}" "$decimal" "Type m1t1;"
> > > >  GDBInfoSymbols::check_entry "${srcfile}" "" "${real4}"
> > > > -GDBInfoSymbols::check_entry "${srcfile}" "37" "Type s1;"
> > > > +
> > > > +if { [test_compiler_info {gfortran-*} f90] } {
> > > > +    setup_xfail *-*-* gcc/105454
> > > > +    GDBInfoSymbols::check_entry "${srcfile}" "41" "Type s1;"
> > > > +}
> > >
> > > Shouldn't the GDBInfoSymbols::check_entry call be outside of the if
> > > block?  I think, with your change, the test will _only_ be run on
> > > gfortran, which is not what you wanted.
> >
> > Mh - so actually this is what I wanted.  In my opinion emitting s1 here
> > is actually not expected.  The type s1 is defined inside the Fortran program.
> > E.g. ifx also emits it as a child of the program - limiting its scope to that.
> >
> > Gfortran on the other hand emits it at global CU scope (so not as a child of
> > the program info_types_test).  I think the fact that this type is visible via
> info
> > types is not correct here.  But, since this emission is also buggy I did not
> want
> > to delete the test in order to somehow keep track of this line bug.
> >
> > So I changed the test to only test for this type when ran with gfortran.
> >
> > My baseline assumption here is, that gdb in the test only prints types in the
> > 'info types' command that are defined at CU DWARF scope.  At least it looks
> > like this when compiling with ifx (which, as said, emits the Type s1 as a child
> > of the program info_types_test).
> >
> > When checking this compiled with ifx I see in the DWARF
> >
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > <1><1d5>: Abbrev Number: 12 (DW_TAG_subprogram)
> >     <1d6>   DW_AT_low_pc      : 0x4055b0
> >     <1de>   DW_AT_high_pc     : 0x42
> >     <1e2>   DW_AT_frame_base  : 1 byte block: 56        (DW_OP_reg6 (rbp))
> >     <1e4>   DW_AT_linkage_name: (indirect string, offset: 0x224): MAIN__
> >     <1e8>   DW_AT_name        : (indirect string, offset: 0x22b):
> info_types_test
> >     <1ec>   DW_AT_decl_file   : 1
> >     <1ed>   DW_AT_decl_line   : 37
> >     <1ee>   DW_AT_external    : 1
> >     <1ee>   DW_AT_main_subprogram: 1
> > ...
> > <2><239>: Abbrev Number: 14 (DW_TAG_structure_type)
> >     <23a>   DW_AT_name        : (indirect string, offset: 0x1d4): s1
> >     <23e>   DW_AT_byte_size   : 4
> >     <23f>   DW_AT_decl_file   : 1
> >     <240>   DW_AT_decl_line   : 41
> >  <3><241>: Abbrev Number: 15 (DW_TAG_member)
> >     <242>   DW_AT_name        : (indirect string, offset: 0x207): a
> >     <246>   DW_AT_type        : <0x1ce>
> >     <24a>   DW_AT_decl_file   : 1
> >     <24b>   DW_AT_decl_line   : 41
> >     <24c>   DW_AT_data_member_location: 0
> >     <24d>   DW_AT_accessibility: 1      (public)
> >  <3><24e>: Abbrev Number: 0
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > So s1 is being emitted but as a child of MAIN__. With gfortran on the other
> > hand I get
> >
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > <1><2ec>: Abbrev Number: 18 (DW_TAG_structure_type)
> >     <2ed>   DW_AT_name        : s1
> >     <2f0>   DW_AT_byte_size   : 4
> >     <2f1>   DW_AT_decl_file   : 1
> >     <2f2>   DW_AT_decl_line   : 37
> >     <2f3>   DW_AT_sibling     : <0x302>
> >  <2><2f7>: Abbrev Number: 4 (DW_TAG_member)
> >     <2f8>   DW_AT_name        : a
> >     <2fa>   DW_AT_decl_file   : 1
> >     <2fb>   DW_AT_decl_line   : 42
> >     <2fc>   DW_AT_type        : <0x2bf>
> >     <300>   DW_AT_data_member_location: 0
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > emitted as a child of the whole compile unit.
> >
> > It might be, that this is actually not the expected behavior of GDB here.
> > But it seems, that types defined as children of subroutines will not be
> > displayed by 'info types'.
> >
> > Similarly, I looked at this in c++ and we have the same here: Compiling
> >
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > int main (int argc, char *argv[])
> > {
> >   struct test {};
> >
> >   test a;
> >   return 0;
> > }
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > With gcc -O0 -g (version 9.4.0) will emit DWARF like:
> >
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > <1><2d>: Abbrev Number: 2 (DW_TAG_subprogram)
> >     <2e>   DW_AT_external    : 1
> >     <2e>   DW_AT_name        : (indirect string, offset: 0xb4): main
> >     <32>   DW_AT_decl_file   : 1
> >     <33>   DW_AT_decl_line   : 1
> >     <34>   DW_AT_decl_column : 5
> >     <35>   DW_AT_type        : <0xf7>
> >     <39>   DW_AT_low_pc      : 0x1129
> >     <41>   DW_AT_high_pc     : 0x16
> >     <49>   DW_AT_frame_base  : 1 byte block: 9c
> (DW_OP_call_frame_cfa)
> >     <4b>   DW_AT_GNU_all_call_sites: 1
> >     <4b>   DW_AT_sibling     : <0xf7>
> >  <2><4f>: Abbrev Number: 3 (DW_TAG_formal_parameter)
> > ...
> > <2><6d>: Abbrev Number: 4 (DW_TAG_structure_type)
> >     <6e>   DW_AT_name        : (indirect string, offset: 0xf): test
> >     <72>   DW_AT_byte_size   : 1
> >     <73>   DW_AT_decl_file   : 1
> >     <74>   DW_AT_decl_line   : 3
> >     <75>   DW_AT_decl_column : 10
> >     <76>   DW_AT_sibling     : <0xe9>
> >  <3><7a>: Abbrev Number: 5 (DW_TAG_subprogram)
> > ...
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > So the type of test is not emitted at CU level, but as a child of main.
> > Doing
> >
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > (gdb) info types
> > All defined types:
> >
> > File ./c.cpp:
> >         char
> >         int
> > (gdb) start
> > ...
> > 6         return 0;
> > (gdb) info types test
> > All types matching regular expression "test":
> > (gdb)
> > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > will also not emit the type.
> >
> > So either this is wrong in GDB - or the emission of the type at global level
> > is not quite correct here and should not even be tested.
> >
> > I see that my commit message does not quite cover all this though.
> > But what do you think?
> >
> > Thanks,
> > Nils
> >
> 
> I dug into this a bit more .. It seems indeed that the symbols are only
> searched at a global level.  In symtab.c:add_matching_symbols which is
> called as a result of e.g. 'info types' we only search the symbols put into
> either the block GLOBAL_BLOCK, or STATIC_BLOCK.
> 
> According to block.h
> 
>    The GLOBAL_BLOCK contains all the symbols defined in this compilation
>    whose scope is the entire program linked together.
>    The STATIC_BLOCK contains all the symbols whose scope is the
>    entire compilation excluding other separate compilations.
> 
> so indeed, I would not expect these local symbols to appear when typing
> 'info symbols'/'info types' in GDB.
> 
> So, I think the emission of s1 in 'info types' when compiled with gfortran
> Is wrong (and likely this should also become a GCC/gfortran bug). It does not
> happen with ifx or flang.
> 
> About the second comment on this patch:
> 
> > > > -GDBInfoSymbols::check_entry "${srcfile}" "" "${character1}"
> > > > +GDBInfoSymbols::check_optional_entry "${srcfile}" "" "${character1}"
> > >
> > > Could we not just add a reference to character*1 type?  I'm happy to
> > > take this change, but just adding a use might make a stronger test?
> >
> > Yes, I'll do that. It is true, there will be a bit more coverage.
> 
> It is actually difficult to do this.  I added this line to the test (in this case to
> the program info_types_test):
> 
>   character(kind=1) :: d = 'c'
> 
> Adding a character to the executable and compiling it with ifx or gfortran
> will not trigger the emission of an additional type though.  In fact, neither,
> gfortran, nor ifx emit the type of variable d as ${character1}, e.g. character*1:
> 
> gfortran this type is described as
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> <2><32e>: Abbrev Number: 21 (DW_TAG_variable)
>     <32f>   DW_AT_name        : d
>     <331>   DW_AT_decl_file   : 1
>     <332>   DW_AT_decl_line   : 45
>     <333>   DW_AT_type        : <0x365>
> ...
> <1><365>: Abbrev Number: 25 (DW_TAG_string_type)
>     <366>   DW_AT_byte_size   : 1
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> and IFX
> will describe the character type not with a
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> <2><24f>: Abbrev Number: 13 (DW_TAG_variable)
>     <250>   DW_AT_name        : (indirect string, offset: 0x1f7): d
>     <254>   DW_AT_type        : <0x286>
>     <258>   DW_AT_decl_file   : 1
>     <259>   DW_AT_decl_line   : 45
>     <25a>   DW_AT_location    : 9 byte block: 3 a0 f3 48 0 0 0 0 0
> (DW_OP_addr: 48f3a0)
>     <264>   DW_AT_linkage_name: (indirect string, offset: 0x205):
> info_types_test_$D
> ...
> <1><286>: Abbrev Number: 18 (DW_TAG_string_type)
>     <287>   DW_AT_name        : (indirect string, offset: 0x1f9): CHARACTER_0
>     <28b>   DW_AT_byte_size   : 1
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> I also tried lines like
> 
>   character*1 :: d = 'c'
> 
> and
> 
>   character :: d = 'c'
> 
> but all three were emitted as the same string_type in DWARF.
> So, to conclude, I do not even know whether it is possible to
> actually get gfortran to emit a type called character*1 naturally..
> 
> The only place within the Fortran testsuite where a check for
> the compiler dependent type fortran_character1 exists is info-types.
> The problem with the emission as DW_TAG_string_type is that this will not
> make GDB generate a symbol for the type - in read.c it says
> 
>     /* These dies have a type, but processing them does not create
>        a symbol or recurse to process the children.  Therefore we can
>        read them on-demand through read_type_die.  */
> 
> So for this part, I think the type should not be emitted at all.  I do also not
> think
> that it is wrong DWARF to emit the character*1 as a string_type of length 1.
> Btw. when printing the type in gdb this is hidden as Fortran string types
> are printed as character*DW_AT_byte_size, so for ifx or gfortran we get
> 
>    (gdb) ptype d
>    type = character*1
> 
> I lastly checked this with flang and here, finally, we get a character base type
> emitted as
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> <2><29b>: Abbrev Number: 16 (DW_TAG_variable)
>     <29c>   DW_AT_name        : (indirect string, offset: 0x1b9): d
>     <2a0>   DW_AT_type        : <0x372>
> ...
> <1><372>: Abbrev Number: 10 (DW_TAG_base_type)
>     <373>   DW_AT_name        : (indirect string, offset: 0x1ad): character
>     <377>   DW_AT_encoding    : 5       (signed)
>     <378>   DW_AT_byte_size   : 1
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> which, rightfully, made GDB emit a type called 'character' in the 'info types'.
> 
> I think we should keep the check for the character type optional, even when
> adding a line that actually uses it to the test (as only flang DWARF emits it).
> The other alternative is to remove this check completely which I also think is
> ok.
> 
> There should be a gfortran bug filed about the emission of this character*1,
> too though.
> 
> Cheers,
> Nils
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928