[PATCH][gdb/symtab] Make gold index workaround more precise

[Tom de Vries <tdevries@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1385 bytes --]

[ was: Re: [PATCH][gdb/symtab] Handle .gdb_index in ada language mode ]
On 22-05-2020 22:31, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> Using this patch, I managed to get it working.
> 
> Wow, cool.
> 
> Tom> The patch does the following:
> Tom> - enable ada .gdb_index by removing the ada check in write_psymbols
> Tom> - copy some ada-specific code from debug_names::insert to
> Tom>   write_psymbols
> Tom> - disable a workaround for gold/15646 in dw2_expand_marked_cus
> 
> Tom> As for the disabled workaround, I ran into trouble in
> Tom> gdb.ada/access_tagged_param.exp, where setting a breakpoint on foo failed.
> 
> Tom> The index shows that there are two entries for foo, one variable, one
> Tom> function:
> Tom> ...
> Tom> [3733] foo:
> Tom>         3 [global, variable]
> Tom>         5 [global, function]
> Tom> ...
> Tom> The workaround skips the function, so disabling the workaround allows
> Tom> the test to pass (my guess atm is that the workaround is not precise
> Tom> enough).
> 
> Looking at the gold bug, I suspect what's needed is to rule out
> duplicates that have the same kind.
> 

I've:
- committed a target board gold-gdb-index
- committed a test-case for the workaround
  (gdb.base/gold-gdb-index.exp), and
- wrote a patch to make the duplicate check more precise (attached
  below).

Any comments?

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-symtab-Make-gold-index-workaround-more-precise.patch --]
[-- Type: text/x-patch, Size: 3674 bytes --]

[gdb/symtab] Make gold index workaround more precise

There's a PR binutils/15646 - "gold-generated .gdb_index has duplicated
symbols that gdb-generated index doesn't", that causes gold to generate
duplicate symbols in the index.

F.i., a namespace N1 declared in a header file can be listed for two CUs that
include the header file:
...
[759] N1:
        2 [global type]
        3 [global type]
...

This causes a gdb performance problem: f.i. when attempting to set a
breakpoint on a non-existing function N1::misspelled, the symtab for both CUs
will be expanded.

Gdb contains a workaround for this, added in commit 8943b87476 "Work around
gold/15646", that skips duplicate global symbols in the index.

However, the workaround does not check for the symbol kind ("type" in the
example above).

Make the workaround more precise by taking symbol kind into account in the
check for duplicates.

Tested on x86_64-linux, with native and target board gold-gdb-index.

gdb/ChangeLog:

2020-05-26  Tom de Vries  <tdevries@suse.de>

	* dwarf2/read.c (NR_GDB_INDEX_SYMBOL_KINDS): Define.
	(struct dw2_symtab_iterator): Make seen_before a bool array.
	(dw2_symtab_iter_init): Update seen_before initialization.
	(dw2_symtab_iter_next, dw2_expand_marked_cus): Check for symbol_kind
	in duplicate check.

---
 gdb/dwarf2/read.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index ec3844188e..4a93cad301 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3344,6 +3344,8 @@ dw2_map_symtabs_matching_filename
   return false;
 }
 
+#define NR_GDB_INDEX_SYMBOL_KINDS (GDB_INDEX_SYMBOL_KIND_MASK + 1)
+
 /* Struct used to manage iterating over all CUs looking for a symbol.  */
 
 struct dw2_symtab_iterator
@@ -3366,7 +3368,7 @@ struct dw2_symtab_iterator
      If so we can ignore all further global instances.
      This is to work around gold/15646, inefficient gold-generated
      indices.  */
-  int global_seen;
+  bool global_seen[NR_GDB_INDEX_SYMBOL_KINDS];
 };
 
 /* Initialize the index symtab iterator ITER.  */
@@ -3382,7 +3384,8 @@ dw2_symtab_iter_init (struct dw2_symtab_iterator *iter,
   iter->block_index = block_index;
   iter->domain = domain;
   iter->next = 0;
-  iter->global_seen = 0;
+  for (unsigned i = 0; i < NR_GDB_INDEX_SYMBOL_KINDS; ++i)
+    iter->global_seen[i] = false;
 
   mapped_index *index = dwarf2_per_objfile->index_table.get ();
 
@@ -3448,10 +3451,10 @@ dw2_symtab_iter_next (struct dw2_symtab_iterator *iter)
 	    }
 
 	  /* Work around gold/15646.  */
-	  if (!is_static && iter->global_seen)
+	  if (!is_static && iter->global_seen[symbol_kind])
 	    continue;
 	  if (!is_static)
-	    iter->global_seen = 1;
+	    iter->global_seen[symbol_kind] = true;
 	}
 
       /* Only check the symbol's kind if it has one.  */
@@ -4514,9 +4517,12 @@ dw2_expand_marked_cus
    search_domain kind)
 {
   offset_type *vec, vec_len, vec_idx;
-  bool global_seen = false;
+  bool global_seen[NR_GDB_INDEX_SYMBOL_KINDS];
   mapped_index &index = *dwarf2_per_objfile->index_table;
 
+  for (unsigned i = 0; i < NR_GDB_INDEX_SYMBOL_KINDS; ++i)
+    global_seen[i] = false;
+
   vec = (offset_type *) (index.constant_pool
 			 + MAYBE_SWAP (index.symbol_table[idx].vec));
   vec_len = MAYBE_SWAP (vec[0]);
@@ -4539,10 +4545,10 @@ dw2_expand_marked_cus
       /* Work around gold/15646.  */
       if (attrs_valid)
 	{
-	  if (!is_static && global_seen)
+	  if (!is_static && global_seen[symbol_kind])
 	    continue;
 	  if (!is_static)
-	    global_seen = true;
+	    global_seen[symbol_kind] = true;
 	}
 
       /* Only check the symbol's kind if it has one.  */