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

[Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>]

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

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

Sorry for the delay in replying.

Thanks for the excellent description, this makes it much clearer what's
going on, and I'm happy with the test remaining gfortran only.

It would probably be worth adding at least some of this description to
the commit message, and an abridged summary as a comment in the .exp
file.  My thinking is, that at some point, gfortran might be "fixed" so
the type is not emitted at the global scope.  When that happens, and the
test starts to fail, it will save someone time if there's a comment
saying that the emission of this type is probably not correct, and that
future gfortran versions might decide not to emit this at all.

Thanks,
Andrew



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