From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24239 invoked by alias); 12 May 2012 18:37:58 -0000 Received: (qmail 24082 invoked by uid 22791); 12 May 2012 18:37:56 -0000 X-SWARE-Spam-Status: No, hits=-6.1 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD 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; Sat, 12 May 2012 18:37:31 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q4CIbVtN002383 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sat, 12 May 2012 14:37:31 -0400 Received: from host2.jankratochvil.net (ovpn-116-17.ams2.redhat.com [10.36.116.17]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q4CIbNa1018106 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Sat, 12 May 2012 14:37:26 -0400 Date: Sat, 12 May 2012 18:37:00 -0000 From: Jan Kratochvil To: John Steele Scott Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [patch] PR symtab/13277: Resolving opaque structures in ICC generated binaries. Message-ID: <20120512183722.GA20606@host2.jankratochvil.net> References: <4E9A6F3C.6010400@toojays.net> <20111019084011.GA9326@host1.jankratochvil.net> <4EA3E995.8040206@toojays.net> <20111026221057.GA24628@host1.jankratochvil.net> <4EBFB451.8030503@toojays.net> <4FA4912E.9050709@toojays.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4FA4912E.9050709@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: 2012-05/txt/msg00480.txt.bz2 On Sat, 05 May 2012 04:32:14 +0200, John Steele Scott wrote: > I ran the dwarf2 and base tests and saw no new failures. Please apply. BTW did you run the testsuite with icc or gcc or both? > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -8727,6 +8727,23 @@ quirk_gcc_member_function_pointer (struct type *type, struct objfile *objfile) > smash_to_methodptr_type (type, new_type); > } > > +/* Return non-zero if the supplied PRODUCER string matches the Intel C/C++ > + compiler (icc). */ > + > +static int > +producer_is_icc (const char *producer) > +{ > + static const char *const icc_ident = "Intel(R) C Intel(R) 64 Compiler XE"; I believe the same problem applies to 32-bit compiler while this string will not match it. Also isn't the same problem for example with some Intel Fortran compiler? That maybe some "Intel(R)" match would be enough. > + > + if (producer == NULL) > + return 0; > + > + if (strncmp (producer, icc_ident, strlen (icc_ident)) == 0) > + return 1; > + > + return 0; > +} > + > /* Called when we find the DIE that starts a structure or union scope > (definition) to create a type for the structure or union. Fill in > the type's name and general properties; the members will not be > @@ -8837,6 +8854,11 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu) > /* RealView does not output the required DW_AT_declaration > on incomplete types. */ > TYPE_STUB (type) = 1; > + else if (attr != NULL && die->child == NULL && TYPE_LENGTH (type) == 0 > + && producer_is_icc (cu->producer)) > + /* ICC does not output the required DW_AT_declaration > + on incomplete types, but gives them a size of zero. */ > + TYPE_STUB (type) = 1; braces; it is just said so in gdb/doc/gdbint.texinfo: Any two or more lines in code should be wrapped in braces, even if they are comments, as they look like separate statements: { /* ICC does not output the required DW_AT_declaration on incomplete types, but gives them a size of zero. */ TYPE_STUB (type) = 1; } > > /* We need to add the type field to the die immediately so we don't > infinitely recurse when dealing with pointers to the structure > @@ -8849,6 +8871,30 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu) > return type; > } > > +/* Return non-zero if the DIE from the compilation unit CU is an incomplete > + type. "An incomplete structure, union or class type is represented by a > + structure, union or class entry that does not have a byte size attribute and > + that has a DW_AT_declaration attribute." */ > + > +static int > +die_is_incomplete_type (struct die_info *die, struct dwarf2_cu *cu) > +{ > + struct attribute *attr = dwarf2_attr (die, DW_AT_byte_size, cu); > + > + if (dwarf2_flag_true_p (die, DW_AT_declaration, cu) && attr == NULL) This conditional should use die_is_declaration like before, as you no longer check DW_AT_specification == NULL, as die_is_declaration does check. But in general I do not understand why did not you put the ICC exception to existing die_is_declaration instead. Even with your patch still various parts of code call die_is_declaration which may be a problem for ICC. I do not have a countercase at hand. I would try to patch die_is_declaration and see if it will make any difference with icc on the testsuite, I do not have icc ready now to make this simple test. > + return 1; > + else if (producer_is_icc (cu->producer) > + && attr != NULL && DW_UNSND (attr) == 0 && die->child == NULL > + && (die->tag == DW_TAG_structure_type > + || die->tag == DW_TAG_class_type > + || die->tag == DW_TAG_union_type)) > + /* ICC does not output the required DW_AT_declaration on incomplete > + structure, union and class types, but gives them a size of zero. */ > + return 1; braces: { /* ICC does not output the required DW_AT_declaration on incomplete structure, union and class types, but gives them a size of zero. */ return 1; } > + > + return 0; > +} > + > /* Finish creating a structure or union type, including filling in > its members and creating a symbol for it. */ > > @@ -9053,11 +9099,7 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu) > child_die = sibling_die (child_die); > } > > - /* Do not consider external references. According to the DWARF standard, > - these DIEs are identified by the fact that they have no byte_size > - attribute, and a declaration attribute. */ > - if (dwarf2_attr (die, DW_AT_byte_size, cu) != NULL > - || !die_is_declaration (die, cu)) > + if (!die_is_incomplete_type (die, cu)) > new_symbol (die, type, cu); > } > > @@ -10991,8 +11033,23 @@ read_partial_die (const struct die_reader_specs *reader, > part_die->sibling = buffer + dwarf2_get_ref_die_offset (&attr).sect_off; > break; > case DW_AT_byte_size: > - part_die->has_byte_size = 1; > - break; > + if (DW_UNSND (&attr) == 0 && producer_is_icc (cu->producer) > + && (part_die->tag == DW_TAG_structure_type > + || part_die->tag == DW_TAG_union_type > + || part_die->tag == DW_TAG_class_type)) > + { > + /* ICC ddoes not output DW_AT_declaration on incomplete types, > + instead giving them a size of zero. Fix that up so that we Two spaces after dot. > + treat this as an incomplete type. Ideally we would do this in Again. > + fixup_partial_die(), but that would mean re-reading the Remove those "()" for GNU Coding Standards compliance. > + DW_AT_byte_size attribute. */ > + part_die->is_declaration = 1; > + } > + else > + { > + part_die->has_byte_size = 1; > + } Needless braces but I do not mind. > + 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 Thanks, Jan