Re: [RFA, doc RFA] work around gold/15646

[Doug Evans <dje at google dot com>]

Tom Tromey writes:
 > >>>>> "Doug" == Doug Evans <dje@google.com> writes:
 > 
 > Doug> This patch works around gold/15646.
 > 
 > Doug> Basically, gold can add multiple copies of global syms to the index
 > Doug> whereas gdb only adds one.  The workaround is easy enough,
 > Doug> just ignore global instances of a sym after the first one.
 > 
 > It seems reasonable to me.
 > 
 > Doug> +Here is a brief history of @code{.gdb_index} versions.
 > Doug> +
 > Doug> +@itemize @bullet
 > Doug> +@item
 > Doug> +Versions 1, 2 and 3 are obsolete.
 > Doug> +@item
 > Doug> +Version 4 is the oldest supported version.
 > Doug> +@item
 > Doug> +Version 5 introduces a different hashing function to support
 > Doug> +case-insensitive systems.
 > 
 > I think this should say "case-insensitive languages" or something like
 > that.  I think "systems" is too easily confused with filesystem issues;
 > but really this was about handling Fortran.

How about this?

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

	Work around gold/15646.
	* dwarf2read.c (read_index_from_section): Update comment.
	(struct dw2_symtab_iterator): New member global_seen.
	(dw2_symtab_iter_init): Initialize it.
	(dw2_symtab_iter_next): Skip duplicate global symbols.
	(dw2_expand_symtabs_matching): Ditto.

	doc/
	* gdb.texinfo (Index Section Format): Move version descriptions
	to separate section.  Give symbol attribute description a subheading.
	Document Gold issues.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.820
diff -u -p -r1.820 dwarf2read.c
--- dwarf2read.c	1 Aug 2013 23:35:04 -0000	1.820
+++ dwarf2read.c	2 Aug 2013 00:11:28 -0000
@@ -2751,9 +2751,12 @@ 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.  */
+     of the TU (for symbols coming from TUs),
+     http://sourceware.org/bugzilla/show_bug.cgi?id=15021.
+     Plus gold-generated indices can have duplicate entries for global symbols,
+     http://sourceware.org/bugzilla/show_bug.cgi?id=15646.
+     These are just performance bugs, and we can't distinguish gdb-generated
+     indices from gold-generated ones, so issue no warning here.  */
 
   /* Indexes with higher version than the one supported by GDB may be no
      longer backward compatible.  */
@@ -3165,6 +3168,11 @@ struct dw2_symtab_iterator
   int next;
   /* The number of elements in VEC, or zero if there is no match.  */
   int length;
+  /* Have we seen a global version of the symbol?
+     If so we can ignore all further global instances.
+     This is to work around gold/15646, inefficient gold-generated
+     indices.  */
+  int global_seen;
 };
 
 /* Initialize the index symtab iterator ITER.
@@ -3184,6 +3192,7 @@ dw2_symtab_iter_init (struct dw2_symtab_
   iter->block_index = block_index;
   iter->domain = domain;
   iter->next = 0;
+  iter->global_seen = 0;
 
   if (find_slot_in_mapped_hash (index, name, &iter->vec))
     iter->length = MAYBE_SWAP (*iter->vec);
@@ -3234,10 +3243,18 @@ dw2_symtab_iter_next (struct dw2_symtab_
       if (per_cu->v.quick->symtab)
 	continue;
 
-      if (attrs_valid
-	  && iter->want_specific_block
-	  && want_static != is_static)
-	continue;
+      /* Check static vs global.  */
+      if (attrs_valid)
+	{
+	  if (iter->want_specific_block
+	      && want_static != is_static)
+	    continue;
+	  /* Work around gold/15646.  */
+	  if (!is_static && iter->global_seen)
+	    continue;
+	  if (!is_static)
+	    iter->global_seen = 1;
+	}
 
       /* Only check the symbol's kind if it has one.  */
       if (attrs_valid)
@@ -3626,6 +3643,7 @@ dw2_expand_symtabs_matching
       offset_type idx = 2 * iter;
       const char *name;
       offset_type *vec, vec_len, vec_idx;
+      int global_seen = 0;
 
       if (index->symbol_table[idx] == 0 && index->symbol_table[idx + 1] == 0)
 	continue;
@@ -3644,6 +3662,8 @@ dw2_expand_symtabs_matching
 	{
 	  struct dwarf2_per_cu_data *per_cu;
 	  offset_type cu_index_and_attrs = MAYBE_SWAP (vec[vec_idx + 1]);
+	  /* This value is only valid for index versions >= 7.  */
+	  int is_static = GDB_INDEX_SYMBOL_STATIC_VALUE (cu_index_and_attrs);
 	  gdb_index_symbol_kind symbol_kind =
 	    GDB_INDEX_SYMBOL_KIND_VALUE (cu_index_and_attrs);
 	  int cu_index = GDB_INDEX_CU_VALUE (cu_index_and_attrs);
@@ -3655,6 +3675,15 @@ dw2_expand_symtabs_matching
 	    (index->version >= 7
 	     && symbol_kind != GDB_INDEX_SYMBOL_KIND_NONE);
 
+	  /* Work around gold/15646.  */
+	  if (attrs_valid)
+	    {
+	      if (!is_static && global_seen)
+		continue;
+	      if (!is_static)
+		global_seen = 1;
+	    }
+
 	  /* Only check the symbol's kind if it has one.  */
 	  if (attrs_valid)
 	    {
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.1102
diff -u -p -r1.1102 gdb.texinfo
--- doc/gdb.texinfo	25 Jul 2013 10:16:07 -0000	1.1102
+++ doc/gdb.texinfo	2 Aug 2013 00:11:28 -0000
@@ -42786,16 +42786,9 @@ unless otherwise noted:
 @enumerate
 @item
 The version number, currently 8.  Versions 1, 2 and 3 are obsolete.
-Version 4 uses a different hashing function from versions 5 and 6.
-Version 6 includes symbols for inlined functions, whereas versions 4
-and 5 do not.  Version 7 adds attributes to the CU indices in the
-symbol table.  Version 8 specifies that symbols from DWARF type units
-(@samp{DW_TAG_type_unit}) refer to the type unit's symbol table and not the
-compilation unit (@samp{DW_TAG_comp_unit}) using the type.
-
 @value{GDBN} will only read version 4, 5, or 6 indices
 by specifying @code{set use-deprecated-index-sections on}.
-GDB has a workaround for potentially broken version 7 indices so it is
+GDB has workarounds for potentially broken version 7 indices so it is
 currently not flagged as deprecated.
 
 @item
@@ -42903,6 +42896,8 @@ See below for the format of each CU inde
 A string in the constant pool is zero-terminated.
 @end enumerate
 
+@subheading Symbol Attributes
+
 Attributes were added to CU index values in @code{.gdb_index} version 7.
 If a symbol has multiple uses within a CU then there is one
 CU index+attributes value for each use.
@@ -42992,6 +42987,37 @@ switch (die->tag)
   @}
 @end smallexample
 
+@subheading Version History
+
+Here is a brief history of @code{.gdb_index} versions.
+
+@itemize @bullet
+@item
+Versions 1, 2 and 3 are obsolete.
+@item
+Version 4 is the oldest supported version.
+@item
+Version 5 introduces a different hashing function to support
+case-insensitive languages (e.g., Fortran).
+@item
+Version 6 adds symbols for inlined functions.
+@item
+Version 7 adds symbol attributes.
+@item
+Version 8 specifies that the performance issues in
+Gold generated version 7 indices are fixed.
+@end itemize
+
+Gold generated version 7 indices have the following problems:
+
+@itemize @bullet
+@item
+Symbols from DWARF type units refer to a compilation unit that uses
+the type, instead of the type unit itself.
+@item
+Global symbols appear multiple times, whereas only one is needed.
+@end itemize
+
 @node Man Pages
 @appendix Manual pages
 @cindex Man pages