[RFA] symtab/11942: rewrite dwarf2read.c type_hash usage

[dje at google dot com (Doug Evans)]

Hi.

This patch does two things.
First, it fixes PR 11942.  Second it does so in a way that improves
the type_hash usage in dwarf2read.c.

Since this is a significant change to the use of the type hashing
in dwarf2read.c, I'd like another set of eyes to review it.  Thanks.

The basic idea here is that instead of having one type_hash per CU,
we have one type_hash for dies from .debug_info and have another type_hash
for dies from .debug_types.  Types are hashed based on the associated
die's offset and are not freed when a CUs dwarf2_cu structure is freed,
thus I can't see a need for one type_hash per CU.
Plus, when looking up a type for a given die, gdb first reads in the
die at the given offset and then looks up the type for the die at that
offset.  Since we hash types based on die offsets, I can't see a reason
why we don't just lookup the type for the die at the given offset first,
and only if that fails read in the die (this is useful, e.g., when
dies have been flushed because the CU has reached max-cache-age).

Regression tested on amd64-linux (with a gcc that understands -gdwarf-4).

Ok to check in?

2010-08-23  Doug Evans  <dje@google.com>

	PR symtab/11942
	* dwarf2read.c (dwarf2_per_objfile): New members debug_info_type_hash,
	debug_types_type_hash.
	(dwarf2_cu, dwarf2_per_cu_data): Delete member type_hash.
	All uses updated.
	(lookup_die_type): Renamed from tag_type_to_tag.  First look in
	appropriate type_hash table.  All callers updated.
	(allocate_signatured_type_table): Renamed from
	allocate_signatured_type_hash_table.  All callers updated.
	(create_signatured_type_table_from_index): Renamed from
	create_signatured_type_hash_from_index.  All callers updated.
	(read_die_type): Add comment.
	(follow_die_ref_or_sig): Tweak comment.
	(set_die_type): Rewrite to use appropriate choice of
	debug_info_type_hash or debug_types_type_hash.
	(get_die_type_at_offset): New function.
	(get_die_type): Call it.

	testsuite/
	PR symtab/11942
	* gdb.dwarf2/dw4-sig-types.cc: New file.
	* gdb.dwarf2/dw4-sig-types.h: New file.
	* gdb.dwarf2/dw4-sig-types-b.cc: New file.
	* gdb.dwarf2/dw4-sig-types.exp: New file.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.441
diff -u -p -r1.441 dwarf2read.c
--- dwarf2read.c	23 Aug 2010 21:49:26 -0000	1.441
+++ dwarf2read.c	23 Aug 2010 23:36:55 -0000
@@ -213,6 +213,16 @@ struct dwarf2_per_objfile
   /* Set during partial symbol reading, to prevent queueing of full
      symbols.  */
   int reading_partial_symbols;
+
+  /* Table mapping type .debug_info DIE offsets to types.
+     This is NULL if not allocated yet.
+     It (currently) makes sense to allocate debug_types_type_hash lazily.
+     To keep things simple we allocate both lazily.  */
+  htab_t debug_info_type_hash;
+
+  /* Table mapping type .debug_types DIE offsets to types.
+     This is NULL if not allocated yet.  */
+  htab_t debug_types_type_hash;
 };
 
 static struct dwarf2_per_objfile *dwarf2_per_objfile;
@@ -345,11 +355,6 @@ struct dwarf2_cu
   /* Backchain to our per_cu entry if the tree has been built.  */
   struct dwarf2_per_cu_data *per_cu;
 
-  /* Pointer to the die -> type map.  Although it is stored
-     permanently in per_cu, we copy it here to avoid double
-     indirection.  */
-  htab_t type_hash;
-
   /* How many compilation units ago was this CU last referenced?  */
   int last_used;
 
@@ -445,12 +450,6 @@ struct dwarf2_per_cu_data
      of the CU cache it gets reset to NULL again.  */
   struct dwarf2_cu *cu;
 
-  /* If full symbols for this CU have been read in, then this field
-     holds a map of DIE offsets to types.  It isn't always possible
-     to reconstruct this information later, so we have to preserve
-     it.  */
-  htab_t type_hash;
-
   /* The corresponding objfile.  */
   struct objfile *objfile;
 
@@ -1039,7 +1038,8 @@ static void set_descriptive_type (struct
 static struct type *die_containing_type (struct die_info *,
 					 struct dwarf2_cu *);
 
-static struct type *tag_type_to_type (struct die_info *, struct dwarf2_cu *);
+static struct type *lookup_die_type (struct die_info *, struct attribute *,
+				     struct dwarf2_cu *);
 
 static struct type *read_type_die (struct die_info *, struct dwarf2_cu *);
 
@@ -1265,6 +1265,8 @@ static void dwarf2_mark (struct dwarf2_c
 
 static void dwarf2_clear_marks (struct dwarf2_per_cu_data *);
 
+static struct type *get_die_type_at_offset (unsigned int, struct dwarf2_cu *cu);
+
 static struct type *get_die_type (struct die_info *die, struct dwarf2_cu *cu);
 
 static void dwarf2_release_queue (void *dummy);
@@ -1290,7 +1292,7 @@ static gdb_byte *partial_read_comp_unit_
 static void init_cu_die_reader (struct die_reader_specs *reader,
 				struct dwarf2_cu *cu);
 
-static htab_t allocate_signatured_type_hash_table (struct objfile *objfile);
+static htab_t allocate_signatured_type_table (struct objfile *objfile);
 
 #if WORDS_BIGENDIAN
 
@@ -1744,13 +1746,14 @@ create_cus_from_index (struct objfile *o
 }
 
 /* Create the signatured type hash table from the index.  */
+
 static int
-create_signatured_type_hash_from_index (struct objfile *objfile,
-					const gdb_byte *bytes,
-					offset_type elements)
+create_signatured_type_table_from_index (struct objfile *objfile,
+					 const gdb_byte *bytes,
+					 offset_type elements)
 {
   offset_type i;
-  htab_t type_hash;
+  htab_t sig_types_hash;
 
   dwarf2_per_objfile->n_type_comp_units = elements / 3;
   dwarf2_per_objfile->type_comp_units
@@ -1758,7 +1761,7 @@ create_signatured_type_hash_from_index (
 		     dwarf2_per_objfile->n_type_comp_units
 		     * sizeof (struct dwarf2_per_cu_data *));
 
-  type_hash = allocate_signatured_type_hash_table (objfile);
+  sig_types_hash = allocate_signatured_type_table (objfile);
 
   for (i = 0; i < elements; i += 3)
     {
@@ -1784,13 +1787,13 @@ create_signatured_type_hash_from_index (
 	= OBSTACK_ZALLOC (&objfile->objfile_obstack,
 			  struct dwarf2_per_cu_quick_data);
 
-      slot = htab_find_slot (type_hash, type_sig, INSERT);
+      slot = htab_find_slot (sig_types_hash, type_sig, INSERT);
       *slot = type_sig;
 
       dwarf2_per_objfile->type_comp_units[i / 3] = &type_sig->per_cu;
     }
 
-  dwarf2_per_objfile->signatured_types = type_hash;
+  dwarf2_per_objfile->signatured_types = sig_types_hash;
 
   return 1;
 }
@@ -1956,8 +1959,8 @@ dwarf2_read_index (struct objfile *objfi
 
   if (version == 2
       && types_list_elements
-      && !create_signatured_type_hash_from_index (objfile, types_list,
-						  types_list_elements))
+      && !create_signatured_type_table_from_index (objfile, types_list,
+						   types_list_elements))
     return 0;
 
   create_addrmap_from_index (objfile, map);
@@ -2783,7 +2786,7 @@ eq_type_signature (const void *item_lhs,
 /* Allocate a hash table for signatured types.  */
 
 static htab_t
-allocate_signatured_type_hash_table (struct objfile *objfile)
+allocate_signatured_type_table (struct objfile *objfile)
 {
   return htab_create_alloc_ex (41,
 			       hash_type_signature,
@@ -2828,7 +2831,7 @@ create_debug_types_hash_table (struct ob
       return 0;
     }
 
-  types_htab = allocate_signatured_type_hash_table (objfile);
+  types_htab = allocate_signatured_type_table (objfile);
 
   if (dwarf2_die_debug)
     fprintf_unfiltered (gdb_stdlog, "Signatured types:\n");
@@ -3310,7 +3313,6 @@ load_partial_comp_unit (struct dwarf2_pe
       /* Link this compilation unit into the compilation unit tree.  */
       this_cu->cu = cu;
       cu->per_cu = this_cu;
-      cu->type_hash = this_cu->type_hash;
 
       /* Link this CU into read_in_chain.  */
       this_cu->cu->read_in_chain = dwarf2_per_objfile->read_in_chain;
@@ -4332,7 +4334,6 @@ load_full_comp_unit (struct dwarf2_per_c
       /* Link this compilation unit into the compilation unit tree.  */
       per_cu->cu = cu;
       cu->per_cu = per_cu;
-      cu->type_hash = per_cu->type_hash;
 
       /* Link this CU into read_in_chain.  */
       per_cu->cu->read_in_chain = dwarf2_per_objfile->read_in_chain;
@@ -10905,14 +10909,12 @@ dwarf2_const_value (struct attribute *at
     }
 }
 
-
 /* Return the type of the die in question using its DW_AT_type attribute.  */
 
 static struct type *
 die_type (struct die_info *die, struct dwarf2_cu *cu)
 {
   struct attribute *type_attr;
-  struct die_info *type_die;
 
   type_attr = dwarf2_attr (die, DW_AT_type, cu);
   if (!type_attr)
@@ -10921,9 +10923,7 @@ die_type (struct die_info *die, struct d
       return objfile_type (cu->objfile)->builtin_void;
     }
 
-  type_die = follow_die_ref_or_sig (die, type_attr, &cu);
-
-  return tag_type_to_type (type_die, cu);
+  return lookup_die_type (die, type_attr, cu);
 }
 
 /* True iff CU's producer generates GNAT Ada auxiliary information
@@ -10944,7 +10944,6 @@ need_gnat_info (struct dwarf2_cu *cu)
   return 0;
 }
 
-
 /* Return the auxiliary type of the die in question using its
    DW_AT_GNAT_descriptive_type attribute.  Returns NULL if the
    attribute is not present.  */
@@ -10953,14 +10952,12 @@ static struct type *
 die_descriptive_type (struct die_info *die, struct dwarf2_cu *cu)
 {
   struct attribute *type_attr;
-  struct die_info *type_die;
 
   type_attr = dwarf2_attr (die, DW_AT_GNAT_descriptive_type, cu);
   if (!type_attr)
     return NULL;
 
-  type_die = follow_die_ref (die, type_attr, &cu);
-  return tag_type_to_type (type_die, cu);
+  return lookup_die_type (die, type_attr, cu);
 }
 
 /* If DIE has a descriptive_type attribute, then set the TYPE's
@@ -10986,24 +10983,80 @@ static struct type *
 die_containing_type (struct die_info *die, struct dwarf2_cu *cu)
 {
   struct attribute *type_attr;
-  struct die_info *type_die;
 
   type_attr = dwarf2_attr (die, DW_AT_containing_type, cu);
   if (!type_attr)
     error (_("Dwarf Error: Problem turning containing type into gdb type "
 	     "[in module %s]"), cu->objfile->name);
 
-  type_die = follow_die_ref_or_sig (die, type_attr, &cu);
-  return tag_type_to_type (type_die, cu);
+  return lookup_die_type (die, type_attr, cu);
 }
 
+/* Look up the type of DIE in CU using its type attribute ATTR.
+   If there is no type substitute an error marker.  */
+
 static struct type *
-tag_type_to_type (struct die_info *die, struct dwarf2_cu *cu)
+lookup_die_type (struct die_info *die, struct attribute *attr,
+		 struct dwarf2_cu *cu)
 {
   struct type *this_type;
 
-  this_type = read_type_die (die, cu);
-  if (!this_type)
+  /* First see if we have it cached.  */
+
+  if (is_ref_attr (attr))
+    {
+      unsigned int offset = dwarf2_get_ref_die_offset (attr);
+
+      this_type = get_die_type_at_offset (offset, cu);
+    }
+  else if (attr->form == DW_FORM_sig8)
+    {
+      struct signatured_type *sig_type = DW_SIGNATURED_TYPE (attr);
+      struct dwarf2_cu *sig_cu;
+      unsigned int offset;
+
+      /* sig_type will be NULL if the signatured type is missing from
+	 the debug info.  */
+      if (sig_type == NULL)
+	error (_("Dwarf Error: Cannot find signatured DIE referenced from DIE "
+		 "at 0x%x [in module %s]"),
+	       die->offset, cu->objfile->name);
+
+      /* Has the CU been read in?  */
+      sig_cu = sig_type->per_cu.cu;
+      if (sig_cu != NULL)
+	{
+	  gdb_assert (sig_type->per_cu.from_debug_types);
+	  offset = sig_cu->header.offset + sig_type->type_offset;
+	  this_type = get_die_type_at_offset (offset, sig_cu);
+	}
+      else
+	{
+	  /* Nope, we need to read it in.  */
+	  this_type = NULL;
+	}
+    }
+  else
+    {
+      dump_die_for_error (die);
+      error (_("Dwarf Error: Bad type attribute %s [in module %s]"),
+	     dwarf_attr_name (attr->name), cu->objfile->name);
+    }
+
+  /* If not cached we need to read it in.  */
+
+  if (this_type == NULL)
+    {
+      struct die_info *type_die;
+      struct dwarf2_cu *type_cu = cu;
+
+      type_die = follow_die_ref_or_sig (die, attr, &type_cu);
+      this_type = read_type_die (type_die, type_cu);
+    }
+
+  /* If we still don't have a type use an error marker.  */
+
+  if (this_type == NULL)
     {
       char *message, *saved;
 
@@ -11018,9 +11071,19 @@ tag_type_to_type (struct die_info *die, 
 
       this_type = init_type (TYPE_CODE_ERROR, 0, 0, saved, cu->objfile);
     }
+
   return this_type;
 }
 
+/* Return the type for DIE in CU.
+   Returns NULL for invalid types.
+
+   This first does a lookup in the appropriate type_hash table,
+   and only reads the die in if necessary.
+   If the die is read in, an entry is added to its type_hash table.
+
+   NOTE: This can be called when reading in partial or full symbols.  */
+
 static struct type *
 read_type_die (struct die_info *die, struct dwarf2_cu *cu)
 {
@@ -12656,8 +12719,9 @@ follow_die_ref_or_sig (struct die_info *
 }
 
 /* Follow reference OFFSET.
-   On entry *REF_CU is the CU of source DIE referencing OFFSET.
-   On exit *REF_CU is the CU of the result.  */
+   On entry *REF_CU is the CU of the source die referencing OFFSET.
+   On exit *REF_CU is the CU of the result.
+   Returns NULL if OFFSET is invalid.  */
 
 static struct die_info *
 follow_die_offset (unsigned int offset, struct dwarf2_cu **ref_cu)
@@ -14195,6 +14259,8 @@ static struct type *
 set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 {
   struct dwarf2_offset_and_type **slot, ofs;
+  struct objfile *objfile = cu->objfile;
+  htab_t *type_hash_ptr;
 
   /* For Ada types, make sure that the gnat-specific data is always
      initialized (if not already set).  There are a few types where
@@ -14209,46 +14275,53 @@ set_die_type (struct die_info *die, stru
       && !HAVE_GNAT_AUX_INFO (type))
     INIT_GNAT_SPECIFIC (type);
 
-  if (cu->type_hash == NULL)
+  if (cu->per_cu->from_debug_types)
+    type_hash_ptr = &dwarf2_per_objfile->debug_types_type_hash;
+  else
+    type_hash_ptr = &dwarf2_per_objfile->debug_info_type_hash;
+
+  if (*type_hash_ptr == NULL)
     {
-      gdb_assert (cu->per_cu != NULL);
-      cu->per_cu->type_hash
-	= htab_create_alloc_ex (cu->header.length / 24,
+      *type_hash_ptr
+	= htab_create_alloc_ex (127,
 				offset_and_type_hash,
 				offset_and_type_eq,
 				NULL,
-				&cu->objfile->objfile_obstack,
+				&objfile->objfile_obstack,
 				hashtab_obstack_allocate,
 				dummy_obstack_deallocate);
-      cu->type_hash = cu->per_cu->type_hash;
     }
 
   ofs.offset = die->offset;
   ofs.type = type;
   slot = (struct dwarf2_offset_and_type **)
-    htab_find_slot_with_hash (cu->type_hash, &ofs, ofs.offset, INSERT);
+    htab_find_slot_with_hash (*type_hash_ptr, &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 = obstack_alloc (&objfile->objfile_obstack, sizeof (**slot));
   **slot = ofs;
   return type;
 }
 
-/* Find the type for DIE in CU's type_hash, or return NULL if DIE does
-   not have a saved type.  */
+/* Look up the type for the die at DIE_OFFSET in the appropriate type_hash
+   table, or return NULL if the die does not have a saved type.  */
 
 static struct type *
-get_die_type (struct die_info *die, struct dwarf2_cu *cu)
+get_die_type_at_offset (unsigned int offset, struct dwarf2_cu *cu)
 {
   struct dwarf2_offset_and_type *slot, ofs;
-  htab_t type_hash = cu->type_hash;
+  htab_t type_hash;
 
+  if (cu->per_cu->from_debug_types)
+    type_hash = dwarf2_per_objfile->debug_types_type_hash;
+  else
+    type_hash = dwarf2_per_objfile->debug_info_type_hash;
   if (type_hash == NULL)
     return NULL;
 
-  ofs.offset = die->offset;
+  ofs.offset = offset;
   slot = htab_find_with_hash (type_hash, &ofs, ofs.offset);
   if (slot)
     return slot->type;
@@ -14256,6 +14329,15 @@ get_die_type (struct die_info *die, stru
     return NULL;
 }
 
+/* Look up the type for DIE in the appropriate type_hash table,
+   or return NULL if DIE does not have a saved type.  */
+
+static struct type *
+get_die_type (struct die_info *die, struct dwarf2_cu *cu)
+{
+  return get_die_type_at_offset (die->offset, cu);
+}
+
 /* Add a dependence relationship from CU to REF_PER_CU.  */
 
 static void
Index: testsuite/gdb.dwarf2/dw4-sig-types-b.cc
===================================================================
RCS file: testsuite/gdb.dwarf2/dw4-sig-types-b.cc
diff -N testsuite/gdb.dwarf2/dw4-sig-types-b.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.dwarf2/dw4-sig-types-b.cc	23 Aug 2010 23:36:55 -0000
@@ -0,0 +1,16 @@
+
+#include "dw4-sig-types.h"
+
+extern myns::bar_type myset;
+
+static myns::bar_type *
+bar ()
+{
+  return &myset;
+}
+
+void
+foo ()
+{
+  bar ();
+}
Index: testsuite/gdb.dwarf2/dw4-sig-types.cc
===================================================================
RCS file: testsuite/gdb.dwarf2/dw4-sig-types.cc
diff -N testsuite/gdb.dwarf2/dw4-sig-types.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.dwarf2/dw4-sig-types.cc	23 Aug 2010 23:36:55 -0000
@@ -0,0 +1,12 @@
+
+#include "dw4-sig-types.h"
+
+myns::bar_type myset;
+
+int
+main ()
+{
+  foo ();
+
+  return 0;
+}
Index: testsuite/gdb.dwarf2/dw4-sig-types.exp
===================================================================
RCS file: testsuite/gdb.dwarf2/dw4-sig-types.exp
diff -N testsuite/gdb.dwarf2/dw4-sig-types.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.dwarf2/dw4-sig-types.exp	23 Aug 2010 23:36:55 -0000
@@ -0,0 +1,42 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test dwarf4 signatured types.
+
+set test "dw4-sig-types"
+
+# This test is intended for targets which support DWARF-4.
+# Since we pass an explicit -gdwarf-4 to the compiler, we let
+# that be the test of whether the target supports it.
+
+if { [prepare_for_testing "${test}.exp" "${test}" \
+	{dw4-sig-types.cc dw4-sig-types-b.cc} \
+	{debug c++ additional_flags=-gdwarf-4}] } {
+    return -1
+}
+
+# Stress test gdb's handling of cached comp units, disable the cache.
+gdb_test_no_output "maint set dwarf2 max-cache-age 0"
+
+if ![runto_main] {
+    return -1
+}
+
+# Bring symtab for myset into gdb.
+gdb_test "p myset" ".*"
+
+# This is enough to trigger the problem in PR 11942.
+gdb_breakpoint "foo"
+gdb_continue "foo"
Index: testsuite/gdb.dwarf2/dw4-sig-types.h
===================================================================
RCS file: testsuite/gdb.dwarf2/dw4-sig-types.h
diff -N testsuite/gdb.dwarf2/dw4-sig-types.h
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.dwarf2/dw4-sig-types.h	23 Aug 2010 23:36:55 -0000
@@ -0,0 +1,15 @@
+
+#include <set>
+using std::set;
+
+namespace myns
+{
+
+struct bar_type
+{
+  set<int> foo;
+};
+
+} // myns
+
+extern void foo ();