Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] Work around binutils/15021
@ 2013-01-16 23:21 Doug Evans
  2013-01-17 17:53 ` Tom Tromey
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Doug Evans @ 2013-01-16 23:21 UTC (permalink / raw)
  To: gdb-patches, tromey; +Cc: ccoutant, saugustine

Hi.
I'd like to get this patch checked in, but I think it needs
another pair of eyes.

The core of the patch is really simple:

Split this union out:

  union
  {
    /* The CUs we import using DW_TAG_imported_unit.  This is filled in
       while reading psymtabs, used to compute the psymtab dependencies,
       and then cleared.  Then it is filled in again while reading full
       symbols, and only deleted when the objfile is destroyed.  */
    VEC (dwarf2_per_cu_ptr) *imported_symtabs;

    /* Type units are grouped by their DW_AT_stmt_list entry so that they
       can share them.  If this is a TU, this points to the containing
       symtab.  */
    struct type_unit_group *type_unit_group;
  } s;

And then add this bit of code:

@@ -17584,6 +17600,16 @@ follow_die_sig (struct die_info *src_die
 			     temp_die.offset.sect_off);
   if (die)
     {
+      /* For .gdb_index version 7 keep track of included TUs.
+	 http://sourceware.org/bugzilla/show_bug.cgi?id=15021.  */
+      if (dwarf2_per_objfile->index_table != NULL
+	  && dwarf2_per_objfile->index_table->version <= 7)
+	{
+	  VEC_safe_push (dwarf2_per_cu_ptr,
+			 (*ref_cu)->per_cu->imported_symtabs,
+			 sig_cu->per_cu);
+	}
+
       *ref_cu = sig_cu;
       return die;
     }

The bulk of the patch is just handling splitting up the union.

Regression tested with gdb-generated .gdb_index version 7 (by hacking the
gdb-under-test to generate version 7), version 8, with dwz, and with gold.

2013-01-16  Doug Evans  <dje@google.com>

	Work around binutils/15021
	* dwarf2read.c (dwarf2_per_cu_data): Split imported_symtabs and
	type_unit_group out of union s.  All uses updated.
	(read_index_from_section): Watch for index version 8.
	(follow_die_sig): If using .gdb_index version <= 7, record the TU as
	an imported symtab.
	(write_psymtabs_to_index): Increment version number to 8.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.728
diff -u -p -r1.728 dwarf2read.c
--- dwarf2read.c	14 Jan 2013 10:39:33 -0000	1.728
+++ dwarf2read.c	16 Jan 2013 22:00:11 -0000
@@ -566,19 +566,29 @@ struct dwarf2_per_cu_data
     struct dwarf2_per_cu_quick_data *quick;
   } v;
 
-  union
-  {
-    /* The CUs we import using DW_TAG_imported_unit.  This is filled in
-       while reading psymtabs, used to compute the psymtab dependencies,
-       and then cleared.  Then it is filled in again while reading full
-       symbols, and only deleted when the objfile is destroyed.  */
-    VEC (dwarf2_per_cu_ptr) *imported_symtabs;
-
-    /* Type units are grouped by their DW_AT_stmt_list entry so that they
-       can share them.  If this is a TU, this points to the containing
-       symtab.  */
-    struct type_unit_group *type_unit_group;
-  } s;
+  /* The CUs we import using DW_TAG_imported_unit.  This is filled in
+     while reading psymtabs, used to compute the psymtab dependencies,
+     and then cleared.  Then it is filled in again while reading full
+     symbols, and only deleted when the objfile is destroyed.
+
+     This is also used to work around a difference between the way gold
+     generates .gdb_index version <=7 and the way gdb does.  Arguably this
+     is a gold bug.  For symbols coming from TUs, gold records in the index
+     the CU that includes the TU instead of the TU itself.  This breaks
+     dw2_lookup_symbol: It assumes that if the index says symbol X lives
+     in CU/TU Y, then one need only expand Y and a subsequent lookup in Y
+     will find X.  Alas TUs live in their own symtab, so after expanding CU Y
+     we need to look in TU Z to find X.  Fortunately, this is akin to
+     DW_TAG_imported_unit, so we just use the same mechanism: For
+     .gdb_index version <=7 this also records the TUs that the CU referred
+     to.  Concurrently with this change gdb was modified to emit version 8
+     indices so we only pay a price for gold generated indices.  */
+  VEC (dwarf2_per_cu_ptr) *imported_symtabs;
+
+  /* Type units are grouped by their DW_AT_stmt_list entry so that they
+     can share them.  If this is a TU, this points to the containing
+     symtab.  */
+  struct type_unit_group *type_unit_group;
 };
 
 /* Entry in the signatured_types hash table.  */
@@ -2690,9 +2700,14 @@ to use the section anyway."),
 	}
       return 0;
     }
+  /* Version 7 indices generated by gold refer to the CU for a symbol instead
+     of the TU (for symbols coming from TUs).  It's just a performance bug, and
+     we can't distinguish gdb-generated indices from gold-generated ones, so
+     nothing to do here.  */
+
   /* Indexes with higher version than the one supported by GDB may be no
      longer backward compatible.  */
-  if (version > 7)
+  if (version > 8)
     return 0;
 
   map->version = version;
@@ -2963,7 +2978,7 @@ dw2_get_file_names (struct objfile *objf
      DWO file.  */
   if (this_cu->is_debug_types)
     {
-      struct type_unit_group *tu_group = this_cu->s.type_unit_group;
+      struct type_unit_group *tu_group = this_cu->type_unit_group;
 
       init_cutu_and_read_dies (tu_group->t.first_tu, NULL, 0, 0,
 			       dw2_get_file_names_reader, tu_group);
@@ -4981,10 +4996,10 @@ process_psymtab_comp_unit_reader (const 
     (objfile->static_psymbols.list + pst->statics_offset);
   sort_pst_symbols (objfile, pst);
 
-  if (!VEC_empty (dwarf2_per_cu_ptr, cu->per_cu->s.imported_symtabs))
+  if (!VEC_empty (dwarf2_per_cu_ptr, cu->per_cu->imported_symtabs))
     {
       int i;
-      int len = VEC_length (dwarf2_per_cu_ptr, cu->per_cu->s.imported_symtabs);
+      int len = VEC_length (dwarf2_per_cu_ptr, cu->per_cu->imported_symtabs);
       struct dwarf2_per_cu_data *iter;
 
       /* Fill in 'dependencies' here; we fill in 'users' in a
@@ -4993,12 +5008,12 @@ process_psymtab_comp_unit_reader (const 
       pst->dependencies = obstack_alloc (&objfile->objfile_obstack,
 					 len * sizeof (struct symtab *));
       for (i = 0;
-	   VEC_iterate (dwarf2_per_cu_ptr, cu->per_cu->s.imported_symtabs,
+	   VEC_iterate (dwarf2_per_cu_ptr, cu->per_cu->imported_symtabs,
 			i, iter);
 	   ++i)
 	pst->dependencies[i] = iter->v.psymtab;
 
-      VEC_free (dwarf2_per_cu_ptr, cu->per_cu->s.imported_symtabs);
+      VEC_free (dwarf2_per_cu_ptr, cu->per_cu->imported_symtabs);
     }
 
   /* Get the list of files included in the current compilation unit,
@@ -5096,7 +5111,7 @@ create_type_unit_group (struct dwarf2_cu
   per_cu = &tu_group->per_cu;
   per_cu->objfile = objfile;
   per_cu->is_debug_types = 1;
-  per_cu->s.type_unit_group = tu_group;
+  per_cu->type_unit_group = tu_group;
 
   if (dwarf2_per_objfile->using_index)
     {
@@ -5433,7 +5448,7 @@ build_type_psymtab_dependencies (void **
        ++i)
     {
       pst->dependencies[i] = iter->v.psymtab;
-      iter->s.type_unit_group = tu_group;
+      iter->type_unit_group = tu_group;
     }
 
   VEC_free (dwarf2_per_cu_ptr, tu_group->t.tus);
@@ -5754,7 +5769,7 @@ scan_partial_symbols (struct partial_die
 		  process_psymtab_comp_unit (per_cu, 1);
 
 		VEC_safe_push (dwarf2_per_cu_ptr,
-			       cu->per_cu->s.imported_symtabs, per_cu);
+			       cu->per_cu->imported_symtabs, per_cu);
 	      }
 	      break;
 	    default:
@@ -6886,7 +6901,7 @@ recursively_compute_inclusions (VEC (dwa
     VEC_safe_push (dwarf2_per_cu_ptr, *result, per_cu);
 
   for (ix = 0;
-       VEC_iterate (dwarf2_per_cu_ptr, per_cu->s.imported_symtabs, ix, iter);
+       VEC_iterate (dwarf2_per_cu_ptr, per_cu->imported_symtabs, ix, iter);
        ++ix)
     recursively_compute_inclusions (result, all_children, iter);
 }
@@ -6899,7 +6914,7 @@ compute_symtab_includes (struct dwarf2_p
 {
   gdb_assert (! per_cu->is_debug_types);
 
-  if (!VEC_empty (dwarf2_per_cu_ptr, per_cu->s.imported_symtabs))
+  if (!VEC_empty (dwarf2_per_cu_ptr, per_cu->imported_symtabs))
     {
       int ix, len;
       struct dwarf2_per_cu_data *iter;
@@ -6915,13 +6930,14 @@ compute_symtab_includes (struct dwarf2_p
 					NULL, xcalloc, xfree);
 
       for (ix = 0;
-	   VEC_iterate (dwarf2_per_cu_ptr, per_cu->s.imported_symtabs,
+	   VEC_iterate (dwarf2_per_cu_ptr, per_cu->imported_symtabs,
 			ix, iter);
 	   ++ix)
 	recursively_compute_inclusions (&result_children, all_children, iter);
 
-      /* Now we have a transitive closure of all the included CUs, so
-	 we can convert it to a list of symtabs.  */
+      /* Now we have a transitive closure of all the included CUs, and
+	 for .gdb_index version 7 the included TUs, so we can convert it
+	 to a list of symtabs.  */
       len = VEC_length (dwarf2_per_cu_ptr, result_children);
       symtab->includes
 	= obstack_alloc (&dwarf2_per_objfile->objfile->objfile_obstack,
@@ -7004,7 +7020,7 @@ process_full_comp_unit (struct dwarf2_pe
 
   static_block
     = end_symtab_get_static_block (highpc + baseaddr, objfile, 0,
-				   per_cu->s.imported_symtabs != NULL);
+				   per_cu->imported_symtabs != NULL);
 
   /* If the comp unit has DW_AT_ranges, it may have discontiguous ranges.
      Also, DW_AT_ranges may record ranges not belonging to any child DIEs
@@ -7100,10 +7116,10 @@ process_full_type_unit (struct dwarf2_pe
      If this is the first TU to use this symtab, complete the construction
      of it with end_expandable_symtab.  Otherwise, complete the addition of
      this TU's symbols to the existing symtab.  */
-  if (per_cu->s.type_unit_group->primary_symtab == NULL)
+  if (per_cu->type_unit_group->primary_symtab == NULL)
     {
       symtab = end_expandable_symtab (0, objfile, SECT_OFF_TEXT (objfile));
-      per_cu->s.type_unit_group->primary_symtab = symtab;
+      per_cu->type_unit_group->primary_symtab = symtab;
 
       if (symtab != NULL)
 	{
@@ -7118,8 +7134,8 @@ process_full_type_unit (struct dwarf2_pe
   else
     {
       augment_type_symtab (objfile,
-			   per_cu->s.type_unit_group->primary_symtab);
-      symtab = per_cu->s.type_unit_group->primary_symtab;
+			   per_cu->type_unit_group->primary_symtab);
+      symtab = per_cu->type_unit_group->primary_symtab;
     }
 
   if (dwarf2_per_objfile->using_index)
@@ -7165,7 +7181,7 @@ process_imported_unit_die (struct die_in
       if (maybe_queue_comp_unit (cu, per_cu, cu->language))
 	load_full_comp_unit (per_cu, cu->language);
 
-      VEC_safe_push (dwarf2_per_cu_ptr, cu->per_cu->s.imported_symtabs,
+      VEC_safe_push (dwarf2_per_cu_ptr, cu->per_cu->imported_symtabs,
 		     per_cu);
     }
 }
@@ -8054,9 +8070,9 @@ setup_type_unit_groups (struct die_info 
 
   /* If we're using .gdb_index (includes -readnow) then
      per_cu->s.type_unit_group may not have been set up yet.  */
-  if (per_cu->s.type_unit_group == NULL)
-    per_cu->s.type_unit_group = get_type_unit_group (cu, attr);
-  tu_group = per_cu->s.type_unit_group;
+  if (per_cu->type_unit_group == NULL)
+    per_cu->type_unit_group = get_type_unit_group (cu, attr);
+  tu_group = per_cu->type_unit_group;
 
   /* If we've already processed this stmt_list there's no real need to
      do it again, we could fake it and just recreate the part we need
@@ -17584,6 +17600,16 @@ follow_die_sig (struct die_info *src_die
 			     temp_die.offset.sect_off);
   if (die)
     {
+      /* For .gdb_index version 7 keep track of included TUs.
+	 http://sourceware.org/bugzilla/show_bug.cgi?id=xyz.  */
+      if (dwarf2_per_objfile->index_table != NULL
+	  && dwarf2_per_objfile->index_table->version <= 7)
+	{
+	  VEC_safe_push (dwarf2_per_cu_ptr,
+			 (*ref_cu)->per_cu->imported_symtabs,
+			 sig_cu->per_cu);
+	}
+
       *ref_cu = sig_cu;
       return die;
     }
@@ -19634,7 +19660,7 @@ dwarf2_per_objfile_free (struct objfile 
 
   for (ix = 0; ix < dwarf2_per_objfile->n_comp_units; ++ix)
     VEC_free (dwarf2_per_cu_ptr,
-	      dwarf2_per_objfile->all_comp_units[ix]->s.imported_symtabs);
+	      dwarf2_per_objfile->all_comp_units[ix]->imported_symtabs);
 
   VEC_free (dwarf2_section_info_def, data->types);
 
@@ -20456,7 +20482,7 @@ write_psymtabs_to_index (struct objfile 
   total_len = size_of_contents;
 
   /* The version number.  */
-  val = MAYBE_SWAP (7);
+  val = MAYBE_SWAP (8);
   obstack_grow (&contents, &val, sizeof (val));
 
   /* The offset of the CU list from the start of the file.  */


^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-06-12 18:29 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-16 23:21 [RFA] Work around binutils/15021 Doug Evans
2013-01-17 17:53 ` Tom Tromey
2013-01-17 18:42   ` Doug Evans
2013-01-17 21:04   ` [RFA, doc RFA] " Doug Evans
2013-01-18  7:17     ` Eli Zaretskii
2013-01-18 14:32     ` Tom Tromey
2013-01-24 14:59 ` Regressions with dwz [Re: [RFA] Work around binutils/15021] Jan Kratochvil
     [not found]   ` <CADPb22SoND17XeXBL2N+-DXf+yKuWe4PJEXR86qT06-q_LA+7g@mail.gmail.com>
2013-01-24 17:16     ` Jan Kratochvil
2013-01-24 19:08       ` Jan Kratochvil
2013-01-30 20:41         ` Tom Tromey
2013-06-09 17:09 ` [RFA] Work around binutils/15021 Jan Kratochvil
2013-06-12  9:49   ` Doug Evans
2013-06-12 18:29     ` Jan Kratochvil
2013-06-12 19:50       ` Cary Coutant

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