This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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 <case DW_AT_byte_size> 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  <jan.kratochvil@redhat.com>

	* 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]