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

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

Jan, 

Thanks for your review.

Unfortunately I have found that I needed to extend this patch, and probably
further work is required before this can be committed. While the previous patch
worked for my simple test case, it was failing with a larger program: from some
contexts the opaque structure type could still not be resolved.

It turns out that the bogus stubs which ICC creates were getting into the
psymbol table: one entry per compilation unit which referenced the type, in
addition to the psymbol from the compilation unit where the complete structure
was defined. basic_lookup_transparent_type will search through all symbol tables
which contain a matching type, but I didn't see a way to iterate through all
psymbols---so basic_lookup_transparent_type_quick will only examine a single
psymbol. This means that the opaque type will only be guaranteed to be resolved
if the transparent type is already in the symbol table. If the transparent type
is only in psymbols, it may or may not be resolved depending on order.

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: the byte size attribute of ffffffff
is translated to zero in read_attribute_value, then the new check I've added in
read_partial_die translates this to "structure is a stub", so it never gets into
the psymbol table, thus the test breaks. A producer check would get around that,
but cu->producer is NULL in read_partial_die.

A different approach would be to do something like Ada's remove_extra_symbols,
which discards stubs which have the same name as a transparent type. That seems
like a more far-reaching change than what I'm trying to do here. I much prefer
to get these ICC-produced symbols looking like the GCC ones as early as
possible.

I appreciate your comments as to how this could be made suitable for commit.

cheers,

John

Patch follows:

    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.
    
    Changelog:
    2011-10-23  John Steele Scott  <toojays@toojays.net>
    
    	PR symtab/13277: Resolving opaque structures in ICC generated binaries.
    	* dwarf2read.c (die_is_incomplete_type): New forward declaration.
    	(producer_is_icc): New function.
    	(read_structure_type): Set TYPE_STUB on structures
    	with a byte size of zero, if the binary was produced by ICC.
    	(process_structure_scope): Extract "external reference" check into
    	die_is_incomplete_type.
    	(die_is_incomplete_type): New function.
    	(read_partial_die): New variable byte_size, set to the value of a
    	DW_AT_byte_size attribute if we find one. If a structure has a
    	byte_size of zero, clear part_die->has_byte_size and set
    	part_die->is_declaration.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 99f67d9..fcd18b0 100644
--- 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);
+
 static int die_is_declaration (struct die_info *, struct dwarf2_cu *cu);
 
 static struct die_info *die_specification (struct die_info *die,
@@ -7526,6 +7529,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
@@ -7636,6 +7656,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
@@ -7852,11 +7877,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);
 }
 
@@ -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;
+    }
+
   return info_ptr;
 }
 
@@ -10733,6 +10769,28 @@ die_is_declaration (struct die_info *die, struct dwarf2_cu *cu)
 	  && dwarf2_attr (die, DW_AT_specification, cu) == NULL);
 }
 
+/* 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 (die->tag == DW_TAG_structure_type && die->child == NULL
+	   && attr != NULL && DW_UNSND (attr) == 0
+	   && producer_is_icc (cu->producer))
+    /* ICC does not output the required DW_AT_declaration
+       on incomplete structure types, but gives them a size of zero.  */
+    /* TODO: Check if this is also required for union and class declarations. */
+    return 1;
+
+  return 0;
+}
+
 /* Return the die giving the specification for DIE, if there is
    one.  *SPEC_CU is the CU containing DIE on input, and the CU
    containing the return value on output.  If there is no