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 9/14/26 9:23 AM, Andrew Burgess wrote:
>>>> 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.
>
> That's why I was wondering, but really I am not opposed to a flag, I
> just wanted to know the rationale.  We have 1 bit free in
> cooked_index_flag, so it wouldn't take up any more space.  It's just
> that having a flag that says "this entry has no name and is therefore
> invalid" seems redundant with the name being nullptr.
>
> Instead of leaving them nullptr, another option would be delete those
> cooked_index_entries from the vectors, if we never plan to do anything
> with them.

This is what I'm doing in v4.  The entries all live on the obstack, so I
can just remove them from the vector without concern.  But ....

>            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.

This is the problem I'm currently trying to solve.

I also reached the conclusion that deleting the child entries would be
too expensive, so my second plan was to just delete the parent pointer
from child entries if the parent is nameless.  This would leave the
child entries in a weird state, e.g. 'the_type::method' would appear in
the index as just 'method', but I think this would be fine.  This isn't
"normal" behaviour, and only triggers in the case where the parent's
name cannot be found.

The problem with this approach is that the parent might be from another
shard, potentially resolved due to the IS_PARENT_DEFERRED flag from the
parent map.  The race is on the read of the parent's name field, the
parent might appear nameless, but it might in fact be the case that the
name hasn't been assigned yet.

So the current idea I'm considering is leaving "nameless" entries
around, but giving them a non-empty name, something like
"__signature_0x..._not_found__".  This name would then show up in the
index, and a user could, in theory, say:

  (gdb) print __signature_0x..._not_found__::method

which seems weird, but remember, this really is an edge case, for when a
referenced signature isn't found.

The other possibility is that, because this is an error case, we could
have a serial action that cleans up the mess, deleting child entries
with nameless parents.  This would be done in
cooked_index::set_contents, as part of this code:

  gdb::task_group finalizers ([this] ()
  {
    // TODO: Fix up the state here.
    m_state->set (cooked_state::FINALIZED);
    m_state->write_to_cache (index_for_writing ());
    m_state->set (cooked_state::CACHE_DONE);
  });

The fix up would be cheap if there was nothing to do, which would be the
normal case, but in the error case, we'd search through and delete any
child with a nameless parent, and their children, and their children,
etc.  Or maybe just clear the parent pointer at this point?  I'm not
really sure yet.

Anyway, if you have any thoughts, I'd love to hear them.

Thanks,
Andrew