Re: [patch] Fix duplicate types for single DIE

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Sat, 05 Jun 2010 00:47:35 +0200, Tom Tromey wrote:
> I don't have a problem with an internal_error if it can only trigger
> when there is a bug in GDB.

While it is only a GDB internal problem it has been changed in the meantime to
complaint() agreed (not sure if approved) by Joel.
	Re: [patch] Fix duplicate types for single DIE
	http://sourceware.org/ml/gdb-patches/2010-05/msg00477.html


> This patch is ok.

Checked-in.


> I did not look closely to see if this conflicts with Keith's
> obstack-leak.patch.

Keith's patchset is still not in so checking-in mine first.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2010-06/msg00052.html

--- src/gdb/ChangeLog	2010/06/05 05:55:59	1.11877
+++ src/gdb/ChangeLog	2010/06/05 14:11:53	1.11878
@@ -1,3 +1,23 @@
+2010-06-05  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	Fix duplicate types for single DIE.
+	* dwarf2read.c (read_structure_type): Move set_descriptive_type after
+	set_die_type.
+	(read_array_type): Remove type initialization.  Recheck get_die_type
+	after initial die_type.  Move set_die_type before set_descriptive_type.
+	(read_set_type): New variable domain_type.  Recheck get_die_type after
+	initial die_type.  Move attr initialization later.
+	(read_tag_pointer_type, read_tag_reference_type): New variable
+	target_type.  Recheck get_die_type after initial die_type.
+	(read_tag_ptr_to_member_type): Recheck get_die_type after initial
+	die_type and die_containing_type.
+	(read_tag_const_type, read_tag_volatile_type, read_subroutine_type):
+	Recheck get_die_type after initial die_type.
+	(read_subrange_type): Recheck get_die_type after initial die_type.
+	Move set_die_type before set_descriptive_type.
+	(set_die_type): Extend the function comment.  Call complaint if DIE has
+	some type already set.
+
 2010-06-05  Vladimir Prus  <vladimir@codesourcery.com>
 
 	* mi/mi-main.c (mi_cmd_list_thread_groups): Adjust
--- src/gdb/dwarf2read.c	2010/06/03 15:39:43	1.394
+++ src/gdb/dwarf2read.c	2010/06/05 14:11:54	1.395
@@ -5138,13 +5138,14 @@
        on incomplete types.  */
     TYPE_STUB (type) = 1;
 
-  set_descriptive_type (type, die, cu);
-
   /* We need to add the type field to the die immediately so we don't
      infinitely recurse when dealing with pointers to the structure
      type within the structure itself. */
   set_die_type (die, type, cu);
 
+  /* set_die_type should be already done.  */
+  set_descriptive_type (type, die, cu);
+
   if (die->child != NULL && ! die_is_declaration (die, cu))
     {
       struct field_info fi;
@@ -5433,7 +5434,7 @@
 {
   struct objfile *objfile = cu->objfile;
   struct die_info *child_die;
-  struct type *type = NULL;
+  struct type *type;
   struct type *element_type, *range_type, *index_type;
   struct type **range_types = NULL;
   struct attribute *attr;
@@ -5443,6 +5444,11 @@
 
   element_type = die_type (die, cu);
 
+  /* The die_type call above may have already set the type for this DIE.  */
+  type = get_die_type (die, cu);
+  if (type)
+    return type;
+
   /* Irix 6.2 native cc creates array types without children for
      arrays with unspecified length.  */
   if (die->child == NULL)
@@ -5511,12 +5517,15 @@
   if (name)
     TYPE_NAME (type) = name;
   
+  /* Install the type in the die. */
+  set_die_type (die, type, cu);
+
+  /* set_die_type should be already done.  */
   set_descriptive_type (type, die, cu);
 
   do_cleanups (back_to);
 
-  /* Install the type in the die. */
-  return set_die_type (die, type, cu);
+  return type;
 }
 
 static enum dwarf_array_dim_ordering
@@ -5559,11 +5568,22 @@
 static struct type *
 read_set_type (struct die_info *die, struct dwarf2_cu *cu)
 {
-  struct type *set_type = create_set_type (NULL, die_type (die, cu));
-  struct attribute *attr = dwarf2_attr (die, DW_AT_byte_size, cu);
+  struct type *domain_type, *set_type;
+  struct attribute *attr;
 
+  domain_type = die_type (die, cu);
+
+  /* The die_type call above may have already set the type for this DIE.  */
+  set_type = get_die_type (die, cu);
+  if (set_type)
+    return set_type;
+
+  set_type = create_set_type (NULL, domain_type);
+
+  attr = dwarf2_attr (die, DW_AT_byte_size, cu);
   if (attr)
     TYPE_LENGTH (set_type) = DW_UNSND (attr);
+
   return set_die_type (die, set_type, cu);
 }
 
@@ -5783,8 +5803,16 @@
   struct attribute *attr_byte_size;
   struct attribute *attr_address_class;
   int byte_size, addr_class;
+  struct type *target_type;
+
+  target_type = die_type (die, cu);
+
+  /* The die_type call above may have already set the type for this DIE.  */
+  type = get_die_type (die, cu);
+  if (type)
+    return type;
 
-  type = lookup_pointer_type (die_type (die, cu));
+  type = lookup_pointer_type (target_type);
 
   attr_byte_size = dwarf2_attr (die, DW_AT_byte_size, cu);
   if (attr_byte_size)
@@ -5840,6 +5868,11 @@
   to_type = die_type (die, cu);
   domain = die_containing_type (die, cu);
 
+  /* The calls above may have already set the type for this DIE.  */
+  type = get_die_type (die, cu);
+  if (type)
+    return type;
+
   if (TYPE_CODE (check_typedef (to_type)) == TYPE_CODE_METHOD)
     type = lookup_methodptr_type (to_type);
   else
@@ -5855,10 +5888,17 @@
 read_tag_reference_type (struct die_info *die, struct dwarf2_cu *cu)
 {
   struct comp_unit_head *cu_header = &cu->header;
-  struct type *type;
+  struct type *type, *target_type;
   struct attribute *attr;
 
-  type = lookup_reference_type (die_type (die, cu));
+  target_type = die_type (die, cu);
+
+  /* The die_type call above may have already set the type for this DIE.  */
+  type = get_die_type (die, cu);
+  if (type)
+    return type;
+
+  type = lookup_reference_type (target_type);
   attr = dwarf2_attr (die, DW_AT_byte_size, cu);
   if (attr)
     {
@@ -5877,6 +5917,12 @@
   struct type *base_type, *cv_type;
 
   base_type = die_type (die, cu);
+
+  /* The die_type call above may have already set the type for this DIE.  */
+  cv_type = get_die_type (die, cu);
+  if (cv_type)
+    return cv_type;
+
   cv_type = make_cv_type (1, TYPE_VOLATILE (base_type), base_type, 0);
   return set_die_type (die, cv_type, cu);
 }
@@ -5887,6 +5933,12 @@
   struct type *base_type, *cv_type;
 
   base_type = die_type (die, cu);
+
+  /* The die_type call above may have already set the type for this DIE.  */
+  cv_type = get_die_type (die, cu);
+  if (cv_type)
+    return cv_type;
+
   cv_type = make_cv_type (TYPE_CONST (base_type), 1, base_type, 0);
   return set_die_type (die, cv_type, cu);
 }
@@ -5951,6 +6003,12 @@
   struct attribute *attr;
 
   type = die_type (die, cu);
+
+  /* The die_type call above may have already set the type for this DIE.  */
+  ftype = get_die_type (die, cu);
+  if (ftype)
+    return ftype;
+
   ftype = lookup_function_type (type);
 
   /* All functions in C++, Pascal and Java have prototypes.  */
@@ -6164,6 +6222,11 @@
 
   base_type = die_type (die, cu);
 
+  /* The die_type call above may have already set the type for this DIE.  */
+  range_type = get_die_type (die, cu);
+  if (range_type)
+    return range_type;
+
   if (cu->language == language_fortran)
     { 
       /* FORTRAN implies a lower bound of 1, if not given.  */
@@ -6270,9 +6333,12 @@
   if (attr)
     TYPE_LENGTH (range_type) = DW_UNSND (attr);
 
+  set_die_type (die, range_type, cu);
+
+  /* set_die_type should be already done.  */
   set_descriptive_type (range_type, die, cu);
 
-  return set_die_type (die, range_type, cu);
+  return range_type;
 }
   
 static struct type *
@@ -12094,7 +12160,22 @@
 }
 
 /* Set the type associated with DIE to TYPE.  Save it in CU's hash
-   table if necessary.  For convenience, return TYPE.  */
+   table if necessary.  For convenience, return TYPE.
+
+   The DIEs reading must have careful ordering to:
+    * Not cause infite loops trying to read in DIEs as a prerequisite for
+      reading current DIE.
+    * Not trying to dereference contents of still incompletely read in types
+      while reading in other DIEs.
+    * Enable referencing still incompletely read in types just by a pointer to
+      the type without accessing its fields.
+
+   Therefore caller should follow these rules:
+     * Try to fetch any prerequisite types we may need to build this DIE type
+       before building the type and calling set_die_type.
+     * After building typer call set_die_type for current DIE as soon as
+       possible before fetching more types to complete the current type.
+     * Make the type as complete as possible before fetching more types.  */
 
 static struct type *
 set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
@@ -12132,6 +12213,10 @@
   ofs.type = type;
   slot = (struct dwarf2_offset_and_type **)
     htab_find_slot_with_hash (cu->type_hash, &ofs, ofs.offset, INSERT);
+  if (*slot)
+    complaint (&symfile_complaints,
+	       _("A problem internal to GDB: DIE 0x%x has type already set"),
+	       die->offset);
   *slot = obstack_alloc (&cu->objfile->objfile_obstack, sizeof (**slot));
   **slot = ofs;
   return type;