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

[Simon Marchi <simark@simark.ca>]

On 2026-09-15 11:01, Tom Tromey wrote:
>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
> 
>>> We would have to delete the name-less entries, and any child
>>> entry that refers to them, not sure how to do that efficiently though.
> 
> Andrew> This is the problem I'm currently trying to solve.
> 
> Andrew> The problem with this approach is that the parent might be from another
> Andrew> shard, potentially resolved due to the IS_PARENT_DEFERRED flag from the
> Andrew> parent map.  The race is on the read of the parent's name field, the
> Andrew> parent might appear nameless, but it might in fact be the case that the
> Andrew> name hasn't been assigned yet.
> 
> Andrew> The other possibility is that, because this is an error case, we could
> Andrew> have a serial action that cleans up the mess, deleting child entries
> Andrew> with nameless parents.  This would be done in
> Andrew> cooked_index::set_contents, as part of this code:
> 
> Right now your patch is fixing up these entries in parallel, in each
> shard.  But if this is uncommon enough, it could be done in
> cooked_index::set_contents instead, say when setting up the finalizer
> tasks:
> 
>   for (auto &shard : m_shards)
>     {
>       auto this_shard = shard.get ();
>       const parent_map_map *parent_maps = m_state->get_parent_map_map ();
> ... signature->entry lookup here
> 
> Then entries could be filtered out in cooked_index_shard::finalize if
> they have a "bad" parent somewhere in their "parent" chain.

Having a bad parent is rare, that would only happen when some
(nameless) type has a DW_AT_signature and the matching type unit can't
be found.  That would be a buggy producer that "forgets" to add the
necessary type units.

But the basic case where the name fixup is needed isn't an error case,
it's just what happens when using clang with -fdebug-types-section,
since structures/classes are nameless:

0x0000099e:   DW_TAG_structure_type
                DW_AT_declaration [DW_FORM_flag_present]        (true)
                DW_AT_signature [DW_FORM_ref_sig8]      (0xdf1329ababfff8ca)

So I suppose you'll get one fixup to do per structure/class type.

Doing the fixups in parallel is easy, since it's just looking up the
signature -> maps, and the "finalize" step already exists, so why not.

> 
> I'm not sure if this would work or not.  TBH I find all this stuff in
> DWARF pretty maddening and also difficult to reason about.  Like, even
> constructing the case you are talking about seems very tricky, seeing
> that it has to involve type signatures and somehow also cross-CU parent
> references.
> 
> A different option might be to ignore such entries at lookup time.  That
> is, let the child entries stay in the vector and just skip them in the
> relevant lookup loops.  My intuition generally is that DWARF reading is
> slow and user-visible, as is CU expansion -- but the lookups themselves
> are not.

> 
> This would probably just mean touching the index writers and
> cooked_index_functions::search.  Perhaps the bad entries themselves (an
> entry with a signature that couldn't be found) could simply not appear
> in the shard vector, to avoid problems with their anonymity.
> 
> In cooked_index_functions::search you could just stick a check here:
> 
> 	  if (!entry->matches (search_flags)
> 	      || !entry->matches (domain))
> 	    continue;
> 
> Like "entry->valid () || ..."

This was our earlier suggestion.  Leave the name nullptr (or set a flag,
whatever), marking them as invalid.  When you need to compute the full
name of an entry, you walk up the parent chain.  If one of those parents
is invalid, bail out.  You can't correctly an entry's full name if the
name of a parent is missing, as simple as that.  And then you have to
handle invalid entries in a few other locations (like
cooked_index_functions::search and the index writers, as you said), but
it shouldn't be too bad.  That sounds simpler than trying to remove the
entries.

Simon