Re: [PATCHv3] gdb: resolve class name via DW_AT_signature in cooked index

[Andrew Burgess <aburgess@redhat.com>]

Simon Marchi <simark@simark.ca> writes:

> On 2026-09-11 15:18, Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
>> 
>>>> (a) If a DIE has no name, but does have a signature, then give the
>>>> DIE a fake name (the empty string), and create an index entry
>>>> for the DIE.  Also keep a record that the cooked_index_entry
>>>> for this DIE has a deferred name.
>> 
>> Simon> Just wondering, if we end up not patching the entry for some reason,
>> Simon> will an entry with an empty name cause problems / match things it's not
>> Simon> supposed to match?  Like will the child of that nameless entry be
>> Simon> considered to be part of the top-level namespace or something like that?
>> 
>> Simon> If it happens that we have an entry with an unresolved name, perhaps we
>> Simon> should consider this entry invalid and just skip anything that would
>> Simon> require it.
>> 
>> Simon>    So yeah, I wonder if it wouldn't be better to leave the name as
>> Simon>    nullptr, that would force us to add some nullptr checks to realize
>> Simon>    that the entry doesn't have a valid name, and we would skip it.
>> 
>> I would much prefer a new cooked_index_flag_enum value over allowing
>> NULL pointers.
>
> Why?  Just wondering.

Also, in this case, the point is that we end up creating the
cooked_index_entry before we know the name, so what value should the
name pointer hold?

My V1 patch tried to find the name before the entry was created, but Tom
correctly pointed out that this was not thread safe, and would fail to
find the name in some cases.

My V2 used the empty string in order to avoid NULL pointers, but empty
name strings cannot usually (outside of this patch) be created, and as
Simon pointed out, if these empty strings "escape" into the rest of GDB
then problems arise.

So V3 switched to NULL pointers as something that is fairly obviously an
"unset" string.

I haven't looked into it, but I'm sure I could add an enum flag, but
this would still leave the question of what value to give NAME until
it's actually filled in.

>
>> Simon>  - the complaint runs on a thread pool worker, but no
>> Simon>    complaint_interceptor is installed in those threads, so
>> Simon>    complaint_internal writes straight to gdb_stderr from a worker
>> Simon>    thread, outside the collect-and-re-emit-on-the-main-thread machinery.
>> Simon>    Besides the raw thread-safety issue, the message can land at an
>> Simon>    arbitrary point in the main thread's output.  Using the
>> Simon>    complaint interceptor would fix both.
>> 
>> Complaints are worthless IMO.
>> 
>> If this is user-actionable or interesting in any way, it's better to
>> warn.  If it isn't user-actionable, then it can just be ignored.
>
> I don't recall, are we allowed to use debug_printf functions in
> non-main-threads?  I'd like if that kind of anomaly left a trace
> somewhere, that you can look at without having to debug gdb itself.  Of
> course the debug output will probably not look pretty if multiple
> threads spew some simultaneously, but you can always disable background
> threads just for this.

I wouldn't want to turn these into debug prints.  I understand Tom's
point though, the complaints are off by default, and what are users
actually going to do even if they are turned on?  Usually the problem is
one of the compiler's making, and the user is likely stuck.

Still, it might be nice if we did a better job of altering the user in
some way that we found issues with the DWARF, and some things might not
work as they expect.  I've long wondered if the problem is that the
choice right now is "print all complaints" or "print no complaints", but
that's not always that useful.  Users likely care even less, or have
even less power to change things, if the complaints are from some system
library.

What if, instead of printing the complaints (or not printing them) we
instead stored the complaints somewhere, and associated the complaint
with the objfile for which the debug information was loaded.  Then,
before printing the CLI prompt, we could inform the user:

  123 debug information complaints seen.  Use 'info complaints' for more details.
  (gdb)

then 'info complaints' would like each objfile and the number of
complaints seen, and 'info complaints <objfile name>' would actually
list the complaints for the given objfile.

Having the output come from a command would also allow us produce more
verbose output, instead of the current 1 or 2 sentence style, we could
fully explain what the issue is, and what this might mean.

Of course the 'xxx debug information complaints seen' would actually be
'complaints seen since the last time GDB informed the user', so the user
wouldn't be constantly spammed with that message.

>
>> Simon>  - The comment on cooked_index_entry::name says that it always points
>> Simon>    into mapped DWARF sections, which is not true anymore.  The comment
>> Simon>    could talk about the "" case (or nullptr if we decided to go that
>> Simon>    route).
>> 
>> I think it's actually wrong already since cooked_index_shard::finalize
>> can synthesize names.  It's really the lifetime of the pointer that is
>> important, not the storage location; and the important invariant is that
>> there's never a case where the string is freed but the entry is live.
>
> Makes sense yeah.

I need to update the comment, so I'll try to write something that covers
this case too.

Thanks,
Andrew