[0/2] Fix .gdb_index with Ada

  This series improves .gdb_index support for Ada, fixing a regression
introduced by the DWARF reader rewrite.

I regression tested this using the cc-with-gdb-index target board on
x86-64 Fedora 34.  There were no regressions, only improvements.

Tom
  

On 9/22/22 22:20, Tom Tromey via Gdb-patches wrote:
> This series improves .gdb_index support for Ada, fixing a regression
> introduced by the DWARF reader rewrite.
> 
> I regression tested this using the cc-with-gdb-index target board on
> x86-64 Fedora 34.  There were no regressions, only improvements.
> 

Hi,

I've tested this series with target board cc-with-gdb-index.

The only FAILs I found were:
...
$ grep ^FAIL: gdb.sum
FAIL: gdb.base/c-linkage-name.exp: print symada__cS before partial 
symtab expansion
FAIL: gdb.dwarf2/dw2-zero-range.exp: ranges_sect=ranges: Zero address 
complaint - relocated - psymtab
FAIL: gdb.dwarf2/dw2-zero-range.exp: ranges_sect=ranges: Zero address 
complaint - unrelocated - psymtab
FAIL: gdb.dwarf2/dw2-zero-range.exp: ranges_sect=rnglists: Zero address 
complaint - relocated - psymtab
FAIL: gdb.dwarf2/dw2-zero-range.exp: ranges_sect=rnglists: Zero address 
complaint - unrelocated - psymtab
FAIL: gdb.python/py-symbol.exp: print (len (gdb.lookup_static_symbols 
('rr')))
...

The FAILs from gdb.python/py-symbol.exp and 
gdb.dwarf2/dw2-zero-range.exp are know, they also fail with target board 
cc-with-debug-names.

The gdb.base/c-linkage-name.exp FAIL does look relevant to this series, 
and probably was regressed by the same offending commit.

Doing:
...
diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index ae05946e790..0268371ec2e 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -1167,7 +1167,6 @@ write_cooked_index (cooked_index_vector *table,
              minimal symbols anyway, so including it in the index is
              usually redundant -- and the cases where it would not be
              redundant are rare and not worth supporting.  */
-         continue;
         }

        gdb_index_symbol_kind kind;
...
fixes the FAIL, so is this one of the "rare and not worth supporting" 
cases you're referring to?

Thanks,
- Tom
  

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> The gdb.base/c-linkage-name.exp FAIL does look relevant to this
Tom> series, and probably was regressed by the same offending commit.

I see this was failing before my series -- but it works in gdb 12.
So, it's one of the regressions the series was intended to fix :(

Tom> Doing:
Tom> ...
Tom> diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
Tom> index ae05946e790..0268371ec2e 100644
Tom> --- a/gdb/dwarf2/index-write.c
Tom> +++ b/gdb/dwarf2/index-write.c
Tom> @@ -1167,7 +1167,6 @@ write_cooked_index (cooked_index_vector *table,
Tom>              minimal symbols anyway, so including it in the index is
Tom>              usually redundant -- and the cases where it would not be
Tom>              redundant are rare and not worth supporting.  */
Tom> -         continue;
Tom>         }

Tom>        gdb_index_symbol_kind kind;
Tom> ...
Tom> fixes the FAIL, so is this one of the "rare and not worth supporting"
Tom> cases you're referring to?

I thought that was necessary to avoid redundancy in the index, but I see
now it isn't, or at least not in that way.  I'm looking again at why the
new indices are larger in general.

Tom
  

Tom> [ ... patch ... ]
Tom> fixes the FAIL, so is this one of the "rare and not worth supporting"
Tom> cases you're referring to?

> I thought that was necessary to avoid redundancy in the index, but I see
> now it isn't, or at least not in that way.  I'm looking again at why the
> new indices are larger in general.

I looked into this more.

Older versions of gdb don't add C++ symbols to the index, so when I
diff'd the indexes I saw a lot of "_Z" additions.  Locally I've changed
this code to skip linkage names for C++ only.

I compared the symbols from old and new indexes.  In every case (except
the one below) I checked, the new gdb seemed more correct.  In
particular it added inlined functions to the index, and it used the
correct name for "enum class" enumerator constants.

I did find out that the new index included entries for the linkage names
of classes.  This isn't generally useful, and they have weird names like
"6mumble", so I also have a patch to drop these entries from the cooked
index entirely.

Tom
  

On 10/13/22 22:40, Tom Tromey wrote:
> Tom> [ ... patch ... ]
> Tom> fixes the FAIL, so is this one of the "rare and not worth supporting"
> Tom> cases you're referring to?
> 
>> I thought that was necessary to avoid redundancy in the index, but I see
>> now it isn't, or at least not in that way.  I'm looking again at why the
>> new indices are larger in general.
> 
> I looked into this more.
> 
> Older versions of gdb don't add C++ symbols to the index, so when I
> diff'd the indexes I saw a lot of "_Z" additions.  Locally I've changed
> this code to skip linkage names for C++ only.
> 

Ah, does that then fix the c-linkage-name.exp regression?

Thanks,
- Tom


> I compared the symbols from old and new indexes.  In every case (except
> the one below) I checked, the new gdb seemed more correct.  In
> particular it added inlined functions to the index, and it used the
> correct name for "enum class" enumerator constants.
> 
> I did find out that the new index included entries for the linkage names
> of classes.  This isn't generally useful, and they have weird names like
> "6mumble", so I also have a patch to drop these entries from the cooked
> index entirely.
> 
> Tom
  

>> I looked into this more.
>> Older versions of gdb don't add C++ symbols to the index, so when I
>> diff'd the indexes I saw a lot of "_Z" additions.  Locally I've changed
>> this code to skip linkage names for C++ only.
>> 

Tom> Ah, does that then fix the c-linkage-name.exp regression?

Yeah.  I'm testing the updated series now, I hope to send it today.

Tom