From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11343 invoked by alias); 26 Oct 2011 22:11:22 -0000 Received: (qmail 11334 invoked by uid 22791); 26 Oct 2011 22:11:20 -0000 X-SWARE-Spam-Status: No, hits=-6.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_BJ,TW_GD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 26 Oct 2011 22:11:02 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p9QMB1G2018960 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 26 Oct 2011 18:11:01 -0400 Received: from host1.jankratochvil.net (ovpn-116-23.ams2.redhat.com [10.36.116.23]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p9QMAxde020476 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Wed, 26 Oct 2011 18:11:00 -0400 Received: from host1.jankratochvil.net (localhost [127.0.0.1]) by host1.jankratochvil.net (8.14.4/8.14.4) with ESMTP id p9QMAwXE024743; Thu, 27 Oct 2011 00:10:58 +0200 Received: (from jkratoch@localhost) by host1.jankratochvil.net (8.14.4/8.14.4/Submit) id p9QMAvj3024742; Thu, 27 Oct 2011 00:10:57 +0200 Date: Wed, 26 Oct 2011 23:09:00 -0000 From: Jan Kratochvil To: John Steele Scott Cc: gdb-patches@sourceware.org Subject: Re: [patch] PR symtab/13277: Resolving opaque structures in ICC generated binaries. Message-ID: <20111026221057.GA24628@host1.jankratochvil.net> References: <4E9A6F3C.6010400@toojays.net> <20111019084011.GA9326@host1.jankratochvil.net> <4EA3E995.8040206@toojays.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4EA3E995.8040206@toojays.net> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-10/txt/msg00712.txt.bz2 On Sun, 23 Oct 2011 12:16:53 +0200, John Steele Scott wrote: > It turns out that the bogus stubs which ICC creates were getting into the > psymbol table: I see this was a very bad review from me, thanks for catching it. > This updated patch tries harder to make sure that the stubs don't end up in the > psymbol table. This is done by adding a check in read_partial_die. Unfortunately > this breaks the dw2-ada-ffffffff testcase: This work around should be checking DW_AT_producer. There is one problem that -gdwarf-4 (-fdebug-types-section) .debug_types units do not contain DW_AT_producer and GDB currently does not try to inherit it from the referencing .debug_info sections. But latest icc still does not support DW_AT_producer and I am not sure if it would use the declaration form inside .debug_types anyway. The dw2-ada-ffffffff compiler bug work around should be also checking DW_AT_producer but at the place where it is now implemented it is not easy to do. > A producer check would get around that, but cu->producer is NULL in > read_partial_die. So this can be changed. I find the cost negligible, not sure if anyone disagrees. The cu->producer initialization could be also just duplicated in process_psymtab_comp_unit which would save the calls in load_partial_comp_unit and read_signatured_type. It would be really good to have a testcase for this ICC case. > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -1014,6 +1014,9 @@ static struct attribute *dwarf2_attr_no_follow (struct die_info *, > static int dwarf2_flag_true_p (struct die_info *die, unsigned name, > struct dwarf2_cu *cu); > > +static int die_is_incomplete_type (struct die_info *die, > + struct dwarf2_cu *cu); > + Rather move the function definition before its first use, it should be possible in this case. > @@ -9703,6 +9724,7 @@ read_partial_die (struct partial_die_info *part_die, > struct attribute attr; > int has_low_pc_attr = 0; > int has_high_pc_attr = 0; > + ULONGEST byte_size = 0; > > memset (part_die, 0, sizeof (struct partial_die_info)); > > @@ -9802,8 +9824,9 @@ read_partial_die (struct partial_die_info *part_die, > part_die->sibling = buffer + dwarf2_get_ref_die_offset (&attr); > break; > case DW_AT_byte_size: > - part_die->has_byte_size = 1; > - break; > + part_die->has_byte_size = 1; > + byte_size = DW_UNSND (&attr); > + break; > case DW_AT_calling_convention: > /* DWARF doesn't provide a way to identify a program's source-level > entry point. DW_AT_calling_convention attributes are only meant > @@ -9870,6 +9893,19 @@ read_partial_die (struct partial_die_info *part_die, > part_die->has_pc_info = 1; > } > > + /* ICC ddoes not output DW_AT_declaration on incomplete types, instead giving > + them a size of zero. Fix that up so that we treat this as an incomplete > + type. We can't check the producer string here, since it may not be in the > + cu yet. Ideally we would do this in fixup_partial_die(), but that would > + mean re-reading the DW_AT_byte_size attribute. */ > + if (part_die->has_byte_size && byte_size == 0 > + && part_die->tag == DW_TAG_structure_type) > + { > + /* TODO: Check if this is also required for union and class declarations. */ > + part_die->has_byte_size = 0; > + part_die->is_declaration = 1; > + } > + I think it can be moved into the code above, cannot it? And with the patch of mine below here should be a check for cu->producer now. Thanks, Jan gdb/ 2011-10-26 Jan Kratochvil * dwarf2read.c (load_full_comp_unit): Move cu->producer initialization to ... (prepare_one_comp_unit): ... here. --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -4726,16 +4726,10 @@ load_full_comp_unit (struct dwarf2_per_cu_data *per_cu, /* We try not to read any attributes in this function, because not all objfiles needed for references have been loaded yet, and symbol - table processing isn't initialized. But we have to set the CU language, - or we won't be able to build types correctly. */ + table processing isn't initialized. But we have to set the CU language + and DW_AT_producer, or we won't be able to build types correctly. */ prepare_one_comp_unit (cu, cu->dies); - /* Similarly, if we do not read the producer, we can not apply - producer-specific interpretation. */ - attr = dwarf2_attr (cu->dies, DW_AT_producer, cu); - if (attr) - cu->producer = DW_STRING (attr); - if (read_cu) { do_cleanups (free_abbrevs_cleanup); @@ -15901,6 +15895,12 @@ prepare_one_comp_unit (struct dwarf2_cu *cu, struct die_info *comp_unit_die) cu->language = language_minimal; cu->language_defn = language_def (cu->language); } + + /* Similarly, if we do not read the producer, we can not apply + producer-specific interpretation. */ + attr = dwarf2_attr (comp_unit_die, DW_AT_producer, cu); + if (attr) + cu->producer = DW_STRING (attr); } /* Release one cached compilation unit, CU. We unlink it from the tree