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

["Kempke, Nils-Christian via Gdb-patches" <gdb-patches@sourceware.org>]

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