[PATCH] Do not put linkage names into .gdb_index

[PATCH] Do not put linkage names into .gdb_index

[Tom Tromey]

This changes the .gdb_index writer to skip linkage names.  This was
always done historically (though somewhat implicitly).
---
 gdb/dwarf2/index-write.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index b7a2e214f6b..b7bc1be3fee 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -1093,6 +1093,10 @@ write_cooked_index (cooked_index_vector *table,
 {
   for (const cooked_index_entry *entry : table->all_entries ())
     {
+      /* GDB never put linkage names into .gdb_index.  */
+      if ((entry->flags & IS_LINKAGE) != 0)
+	continue;
+
       const auto it = cu_index_htab.find (entry->per_cu);
       gdb_assert (it != cu_index_htab.cend ());
 
-- 
2.34.1

Re: [PATCH] Do not put linkage names into .gdb_index

[Pedro Alves]

On 2022-04-22 18:48, Tom Tromey via Gdb-patches wrote:
> This changes the .gdb_index writer to skip linkage names.  This was
> always done historically (though somewhat implicitly).

I assumed this had been done on purpose, I was meaning to ask you what was the main benefit.

If there's no benefit, then let's get rid of them.  OK.

Re: [PATCH] Do not put linkage names into .gdb_index

[Tom Tromey]

>> This changes the .gdb_index writer to skip linkage names.  This was
>> always done historically (though somewhat implicitly).

Pedro> I assumed this had been done on purpose, I was meaning to ask you what was the main benefit.

Pedro> If there's no benefit, then let's get rid of them.  OK.

The benefit is something that Simon pointed out on irc: if the DWARF has
a linkage name, but the actual linker symbols do not have this name (so
gdb does not make a minimal symbol with the name), then "break _Zmumble"
will not work.

I tend to think this isn't important, since normally I suppose there
wouldn't be a reason to emit a DW_AT_linkage_name without actually
emitting the corresponding linker symbol.

Tom

Re: [PATCH] Do not put linkage names into .gdb_index

[Pedro Alves]

On 2022-04-22 20:53, Tom Tromey wrote:
>>> This changes the .gdb_index writer to skip linkage names.  This was
>>> always done historically (though somewhat implicitly).
> 
> Pedro> I assumed this had been done on purpose, I was meaning to ask you what was the main benefit.
> 
> Pedro> If there's no benefit, then let's get rid of them.  OK.
> 
> The benefit is something that Simon pointed out on irc: if the DWARF has
> a linkage name, but the actual linker symbols do not have this name (so
> gdb does not make a minimal symbol with the name), then "break _Zmumble"
> will not work.
> 
> I tend to think this isn't important, since normally I suppose there
> wouldn't be a reason to emit a DW_AT_linkage_name without actually
> emitting the corresponding linker symbol.

I agree.  And binaries with stripped ELF symbols but present DWARF just seems a weird combo that
probably never happens in practice.

Re: [PATCH] Do not put linkage names into .gdb_index

[Joel Brobecker]

> The benefit is something that Simon pointed out on irc: if the DWARF has
> a linkage name, but the actual linker symbols do not have this name (so
> gdb does not make a minimal symbol with the name), then "break _Zmumble"
> will not work.
> 
> I tend to think this isn't important, since normally I suppose there
> wouldn't be a reason to emit a DW_AT_linkage_name without actually
> emitting the corresponding linker symbol.

Can we put that information as part of the comment directly in the
code? That way, someone looking for the "why" won't have to track
the commit all the way back to this email to get that piece of info.

-- 
Joel

Re: [PATCH] Do not put linkage names into .gdb_index

[Tom Tromey]

Joel> Can we put that information as part of the comment directly in the
Joel> code? That way, someone looking for the "why" won't have to track
Joel> the commit all the way back to this email to get that piece of info.

Sure, I'll do that.

Tom

RE: [PATCH] Do not put linkage names into .gdb_index

[George, Jini Susan]

[Public]

If the patch from Alok (cc-ed) to include function symbols with no names (but with only linkage names present), like symbols for artificial functions generated by clang for OpenMP constructs, gets accepted, then this patch would  prevent those symbols from  getting included in the .gdb_index.

The link to the patch review thread here: https://sourceware.org/pipermail/gdb-patches/2022-April/187707.html

Thanks,
Jini.

>>-----Original Message-----
>>From: Gdb-patches <gdb-patches-
>>bounces+jigeorge=amd.com@sourceware.org> On Behalf Of Tom Tromey via
>>Gdb-patches
>>Sent: Friday, April 22, 2022 11:19 PM
>>To: gdb-patches@sourceware.org
>>Cc: Tom Tromey <tromey@adacore.com>
>>Subject: [PATCH] Do not put linkage names into .gdb_index
>>
>>[CAUTION: External Email]
>>
>>This changes the .gdb_index writer to skip linkage names.  This was always done
>>historically (though somewhat implicitly).
>>---
>> gdb/dwarf2/index-write.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>>diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c index
>>b7a2e214f6b..b7bc1be3fee 100644
>>--- a/gdb/dwarf2/index-write.c
>>+++ b/gdb/dwarf2/index-write.c
>>@@ -1093,6 +1093,10 @@ write_cooked_index (cooked_index_vector *table,
>>{
>>   for (const cooked_index_entry *entry : table->all_entries ())
>>     {
>>+      /* GDB never put linkage names into .gdb_index.  */
>>+      if ((entry->flags & IS_LINKAGE) != 0)
>>+       continue;
>>+
>>       const auto it = cu_index_htab.find (entry->per_cu);
>>       gdb_assert (it != cu_index_htab.cend ());
>>
>>--
>>2.34.1

Re: [PATCH] Do not put linkage names into .gdb_index

[Tom Tromey]

>>>>> George, Jini Susan <JiniSusan.George@amd.com> writes:

> If the patch from Alok (cc-ed) to include function symbols with no
> names (but with only linkage names present), like symbols for
> artificial functions generated by clang for OpenMP constructs, gets
> accepted, then this patch would prevent those symbols from getting
> included in the .gdb_index.

It would just need some additional change, I think.
Perhaps the "linkage" flag could be removed from symbols that don't have
an ordinary name.  Or perhaps a new flag could be added.

Tom

RE: [PATCH] Do not put linkage names into .gdb_index

[George, Jini Susan]

[AMD Official Use Only - General]

Thanks, makes sense.

-Jini.

>>-----Original Message-----
>>From: Tom Tromey <tromey@adacore.com>
>>Sent: Monday, April 25, 2022 6:38 PM
>>To: George, Jini Susan <JiniSusan.George@amd.com>
>>Cc: Tom Tromey <tromey@adacore.com>; gdb-patches@sourceware.org;
>>Sharma, Alok Kumar <AlokKumar.Sharma@amd.com>
>>Subject: Re: [PATCH] Do not put linkage names into .gdb_index
>>
>>[CAUTION: External Email]
>>
>>>>>>> George, Jini Susan <JiniSusan.George@amd.com> writes:
>>
>>> If the patch from Alok (cc-ed) to include function symbols with no
>>> names (but with only linkage names present), like symbols for
>>> artificial functions generated by clang for OpenMP constructs, gets
>>> accepted, then this patch would prevent those symbols from getting
>>> included in the .gdb_index.
>>
>>It would just need some additional change, I think.
>>Perhaps the "linkage" flag could be removed from symbols that don't have an
>>ordinary name.  Or perhaps a new flag could be added.
>>
>>Tom