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

[Jan Kratochvil <jan.kratochvil@redhat.com>]

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