From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id yNuZCJmdlGKbMwkAWB0awg (envelope-from ) for ; Mon, 30 May 2022 06:34:01 -0400 Received: by simark.ca (Postfix, from userid 112) id 1F1891E221; Mon, 30 May 2022 06:34:01 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=KRQ9YNSv; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 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 085221E143 for ; Mon, 30 May 2022 06:34:00 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 70A1838F8600 for ; Mon, 30 May 2022 10:33:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 70A1838F8600 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1653906839; bh=r8RI0CVrESl0yH4SSH9ooDC4AqbE7GwfFHXUzI7HWHs=; h=To:Subject:In-Reply-To:References:Date:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=KRQ9YNSvEvhkr8HgkGLczdP7JF4o2qIcWjPl3BiMTieASitbWzrt470Ylv+GadlJ6 5j48TkIIAW+sxPxwM9yHBUo3kPU7n5QROmmxMdGP8cuQjXBRDYIBERV/qfzZqb+l77 /7XM8wCsWscTHXxQzfHDDSycr+JjZfSoOcR1wOYg= Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 574C638133E4 for ; Mon, 30 May 2022 10:33:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 574C638133E4 Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-596-_RG2da9UM6SDLJhDB1JSzA-1; Mon, 30 May 2022 06:33:37 -0400 X-MC-Unique: _RG2da9UM6SDLJhDB1JSzA-1 Received: by mail-qt1-f200.google.com with SMTP id c1-20020ac81101000000b002f9219952f0so9844870qtj.15 for ; Mon, 30 May 2022 03:33:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:subject:in-reply-to:references:date :message-id:mime-version; bh=r8RI0CVrESl0yH4SSH9ooDC4AqbE7GwfFHXUzI7HWHs=; b=BMJm8IgGOO1eWHi+We5B6oRoEMzttbN0J+UOuP+qflsublR/tOo03K/cs7b9XOechC KLwGHB7E9NaXYwm4Skgu194elrKWggpAjyMZjQmqfq+Gsnv88NomAVWJF8mdzQVsEwZH lueBhdtC7V4g5y2q2B0itnUW/CTLoLrZ85KUpQ9cOYMQKA56Aornj3NhW0VwMOe89YxB E2bIKH9O80+9Us/sNge503YSUScjkqi9T/EAukyawCRMuV9mRFcPTEB6qiQTBHFjo5kA bzJYkVl4AlLd19zLhhqxAqsE0hjIHAqvhGID7IwAeZ52H35d+uUJJ/SmVO2ULjVYXRDk HPBw== X-Gm-Message-State: AOAM531NlVEN9CPavAK3vvs0GVWXnReIgNHjge4NtSDW8rcIQwiKhjA9 RIaiU2rueNp4x+hAj3k+9JUefO2zXyXQJXMvmjLB+401Ez1YPOfS5oIPBzQGo+qLYy8FCVo6Na2 fSNSiXi3Aps6BoCmIZ7uE/w== X-Received: by 2002:a37:9dd3:0:b0:6a3:52fa:5859 with SMTP id g202-20020a379dd3000000b006a352fa5859mr31656878qke.332.1653906816412; Mon, 30 May 2022 03:33:36 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyfISbilqknv8Deru5nqKRx0ct5ZUITV39YWoqQdy208hmLTX83KoSyVxnqcyUAJPxTjYVuUw== X-Received: by 2002:a37:9dd3:0:b0:6a3:52fa:5859 with SMTP id g202-20020a379dd3000000b006a352fa5859mr31656869qke.332.1653906816105; Mon, 30 May 2022 03:33:36 -0700 (PDT) Received: from localhost (host109-152-215-36.range109-152.btcentralplus.com. [109.152.215.36]) by smtp.gmail.com with ESMTPSA id cp8-20020a05622a420800b002f39b99f6a4sm7081300qtb.62.2022.05.30.03.33.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 30 May 2022 03:33:35 -0700 (PDT) To: "Kempke\, Nils-Christian" , "Kempke\, Nils-Christian" , "gdb-patches\@sourceware.org" Subject: RE: [PATCH 13/18] testsuite, fortran: fix info-types for intel compilers In-Reply-To: References: <20220510142437.1397399-1-nils-christian.kempke@intel.com> <20220510142437.1397399-14-nils-christian.kempke@intel.com> <87zgjojmt8.fsf@redhat.com> Date: Mon, 30 May 2022 11:33:34 +0100 Message-ID: <87mteze2dd.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain 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: Andrew Burgess via Gdb-patches Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" "Kempke, Nils-Christian" writes: >> -----Original Message----- >> From: Gdb-patches > 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 ; gdb- >> patches@sourceware.org >> Subject: RE: [PATCH 13/18] testsuite, fortran: fix info-types for intel >> compilers >> >> > -----Original Message----- >> > From: Andrew Burgess >> > Sent: Wednesday, May 11, 2022 2:06 PM >> > To: Kempke, Nils-Christian ; gdb- >> > patches@sourceware.org >> > Cc: JiniSusan.George@amd.com; Kempke, Nils-Christian > > christian.kempke@intel.com> >> > Subject: Re: [PATCH 13/18] testsuite, fortran: fix info-types for intel >> > compilers >> > >> > Nils-Christian Kempke 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. Thanks for all the work you've done looking into this. Given what you've said, then I'm happy with your original change. Thanks, Andrew