Re: [patch] PR symtab/13277: Resolving opaque structures in ICC generated binaries.

[John Steele Scott <toojays at toojays dot net>]

On 27/10/11 08:40, Jan Kratochvil wrote:
> 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.

Okay, now we always check DW_AT_producer when applying this workaround. This
depends on your patch which reads the producer in prepare_one_comp_unit(), which
was included in your review but has not yet been committed.
 
> 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.

I don't follow this paragraph. Did you mean something other than icc? I'm
using ICC 12.0.4, and it's definitely setting DW_AT_producer. I don't see it
outputting any .debug_types section though, it seems to only emit dwarf2.

I have submitted a testcase for this in a fork of this thread. Below is the
updated GDB patch which addresses your previous comments. As noted in the other
mail, I'm still waiting for my employer to produce my disclaimer paperwork.

Cheers,

John

commit 6c3b19b3cc118b9a5195c4013cd80cfdce137f9b
Author: John Steele Scott <toojays@toojays.net>
Date:   Mon Oct 17 08:27:18 2011

    ICC does not set DW_AT_declaration on opaque structure types, but does set their
    DW_AT_byte_size to zero. This patch adds checks for this, allowing gdb to
    resolve opaque structures in binaries which were built with ICC.
    
    read_structure_type now contains a special case to recognize such structures and
    mark them as TYPE_STUB.
    
    The logic used in process_structure_scope to determine whether to add a
    structure to the symbol table has been extracted out into a new function
    die_is_incomplete_type. This new function includes the ICC/zero-byte-size check.
    
    Some fixup code is added to read_partial_die() to avoid adding bogus structure
    stubs to the partial symbol table. This was causing problems when looking up a
    type which was only in the psymbols, because lookup_symbol_aux_psymtabs (called
    via basic_lookup_transparent_type_quick) only returns the first psymbol found,
    which would not always be the complete type.
    
    Union and class types are similarly affected and so have the same workarounds
    applied to them.
    
    gdb/Changelog:
    2011-11-13  John Steele Scott  <toojays@toojays.net>
    
    	PR symtab/13277: Resolving opaque structures in ICC generated binaries.
    	* dwarf2read.c (producer_is_icc): New function.
    	(read_structure_type): Set TYPE_STUB on structures/unions/classes
    	with a byte size of zero, if they were produced by ICC.
    	(process_structure_scope): Extract "external reference" check into
    	die_is_incomplete_type.
    	(die_is_incomplete_type): New function.
    	(read_partial_die): If a structure/union/class has a byte_size of zero,
    	and it was produced by ICC, set part_die->is_declaration instead of
    	part_die->has_byte_size.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index c8227c9..d2b7313 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -7585,6 +7585,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";
+
+  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
@@ -7695,6 +7712,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;
 
   /* We need to add the type field to the die immediately so we don't
      infinitely recurse when dealing with pointers to the structure
@@ -7707,6 +7729,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)
+    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;
+
+  return 0;
+}
+
 /* Finish creating a structure or union type, including filling in
    its members and creating a symbol for it.  */
 
@@ -7911,11 +7957,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);
 }
 
@@ -9861,8 +9903,23 @@ 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;
+	  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
+		 treat this as an incomplete type. Ideally we would do this in
+		 fixup_partial_die(), but that would mean re-reading the
+		 DW_AT_byte_size attribute.  */
+	      part_die->is_declaration = 1;
+	    }
+	  else
+	    {
+	      part_die->has_byte_size = 1;
+	    }
+	  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