Re: [PATCH v3 6/8] gdb/remote: Parse tdesc field in stop reply and threads list XML

Luis Machado via Gdb-patches
2023-02-03T11:28:36Z

On 2/1/23 20:16, Simon Marchi via Gdb-patches wrote:
>> IIUC, the tdescs would be deleted during the
>> pop_all_targets_at_and_above, when the refcount of the remote_target
>> gets to 0 and it gets deleted. And the threads would be removed in
>> generic_mourn_inferior just after.
>>
>> An idea could be to call generic_mourn_inferior before
>> remote_unpush_target (no idea if it works). Another one would be to
>> get a temporary reference to the remote_target object in
>> remote_unpush_target, just so that it outlives the threads.
>> Or maybe we should say that it's a process target's responsibility to
>> delete any thread it "owns" before getting deleted itself.
>
> Another question related to this popped while reading the following
> patch. When creating a gdbarch from a tdesc, the gdbarch keeps a
> pointer to that tdesc (accessible through gdbarch_target_desc). And
> AFAIK, we never delete gdbarches. So I suppose the gdbarch will refer a
> stale target desc. At first I thought it wouldn't be a problem in
> practice, because while that gdbarch object still exists, nothing
> references it (it is effectively leaked). But then I remember that we
> cache gdbarches to avoid creating arches with duplicate features. So
> later (let's say if you connect again to a remote), we might want to
> create a gdbarch with the same features as before, and we'll dig up the
> old gdbarch, that points to the now deleted tdesc.

The target descriptions for aarch64 are all cached using a map in gdb/aarch64-tdep.c:

/* All possible aarch64 target descriptors. */
static std::unordered_map <aarch64_features, target_desc *> tdesc_aarch64_map;

I don't think we should try to delete those, and they should live throughout the life of gdb (unless things get large, then we might consider cleanups).

>
> Perhaps it's possible to generate a crash with the current
> implementation by connecting, disconnecting, connecting again, and then
> doing something that uses the thread-specific gdbarch.
>
> Simon
>