Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [patch] Fix duplicate types for single DIE
@ 2010-05-13 13:18 Jan Kratochvil
  2010-05-20 23:35 ` Joel Brobecker
  2010-06-04 22:47 ` Tom Tromey
  0 siblings, 2 replies; 6+ messages in thread
From: Jan Kratochvil @ 2010-05-13 13:18 UTC (permalink / raw)
  To: gdb-patches; +Cc: Keith Seitz

Hi,

this patch can be considered a cleanup, two set_die_type for single DIE should
not hurt.  It just started to break new assertion in the pending patch:
	Re: [RFA] Delayed physname computation
	http://sourceware.org/ml/gdb-patches/2010-05/msg00248.html
# If you put a printf in set_die_type, you'll see that this function is called
# twice (with different type structs) for the same DIE.

Keith has provided "obstack-leak.patch" there as a prerequisite for the
physname patch there.  Attaching here a more complete solution of the problem.

On 250MB webkit debuginfo it reduced memory consumption by 3MB from 4616MB
(0.06%) (which is less than I hoped for; checked with Keith's physname patch).
I have not found measureable any performance difference.

No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu, on FSF GDB HEAD
and on FSF GDB HEAD with the Keith's physname patch referenced above.

Tom Tromey may not agree with new internal_error as expressed in
	Re: [patch] Fix dwarf2read to crash again
	http://sourceware.org/ml/gdb-patches/2010-04/msg00812.html
I am not sure what are his specific plans; if some TRY_CATCH should be
involved the new internal_error call should OK there.

With the new internal_error call I have verified at least the fixes of
read_tag_pointer_type and read_tag_const_type fixes (and not just these two)
to be required.  Other fixes are just written artificially to cover all the
cases of any indirect call of tag_type_to_type, that is die_type,
die_descriptive_type, set_descriptive_type and die_containing_type.
(I believe at least those *_descriptive_type are in fact never needed as their
DWARF usage should never lead to DWARF references loops.)


Thanks,
Jan


2010-05-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* 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.
	(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): Call internal_error if DIE has some type already set.

--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -5109,13 +5109,14 @@ read_structure_type (struct die_info *die, struct dwarf2_cu *cu)
        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;
@@ -5403,7 +5404,7 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
 {
   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;
@@ -5413,6 +5414,11 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
 
   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)
@@ -5479,12 +5485,15 @@ read_array_type (struct die_info *die, struct dwarf2_cu *cu)
   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
@@ -5527,7 +5536,16 @@ read_array_order (struct die_info *die, struct dwarf2_cu *cu)
 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 type *domain_type, *set_type;
+
+  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);
 
   return set_die_type (die, set_type, cu);
 }
@@ -5725,8 +5743,16 @@ read_tag_pointer_type (struct die_info *die, struct dwarf2_cu *cu)
   struct attribute *attr_byte_size;
   struct attribute *attr_address_class;
   int byte_size, addr_class;
+  struct type *target_type;
 
-  type = lookup_pointer_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_pointer_type (target_type);
 
   attr_byte_size = dwarf2_attr (die, DW_AT_byte_size, cu);
   if (attr_byte_size)
@@ -5781,6 +5807,11 @@ read_tag_ptr_to_member_type (struct die_info *die, struct dwarf2_cu *cu)
   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
@@ -5796,10 +5827,17 @@ static struct type *
 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)
     {
@@ -5818,6 +5856,12 @@ read_tag_const_type (struct die_info *die, struct dwarf2_cu *cu)
   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);
 }
@@ -5828,6 +5872,12 @@ read_tag_volatile_type (struct die_info *die, struct dwarf2_cu *cu)
   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);
 }
@@ -5892,6 +5942,12 @@ read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
   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.  */
@@ -6103,6 +6159,12 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
   LONGEST negative_mask;
   
   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 (TYPE_CODE (base_type) == TYPE_CODE_VOID)
     {
       complaint (&symfile_complaints,
@@ -6170,9 +6232,12 @@ read_subrange_type (struct die_info *die, struct dwarf2_cu *cu)
   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 *
@@ -12007,6 +12072,10 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   ofs.type = type;
   slot = (struct dwarf2_offset_and_type **)
     htab_find_slot_with_hash (cu->type_hash, &ofs, ofs.offset, INSERT);
+  if (*slot)
+    internal_error (__FILE__, __LINE__,
+		    _("set_die_type: DIE 0x%x has type already set"),
+		    die->offset);
   *slot = obstack_alloc (&cu->objfile->objfile_obstack, sizeof (**slot));
   **slot = ofs;
   return type;


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-06-05 14:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-13 13:18 [patch] Fix duplicate types for single DIE Jan Kratochvil
2010-05-20 23:35 ` Joel Brobecker
2010-05-21 12:18   ` Jan Kratochvil
2010-05-21 15:09     ` Joel Brobecker
2010-06-04 22:47 ` Tom Tromey
2010-06-05 14:55   ` Jan Kratochvil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox