Re: [PATCH v3 2/8] gdbserver: Add PID parameter to linux_get_auxv and linux_get_hwcap

Thiago Jung Bauermann via Gdb-patches
2023-02-07T21:47:44Z

Pedro Alves <pedro@palves.net> writes:

> On 2023-02-06 8:16 p.m., Simon Marchi wrote:
>> On 2/6/23 14:54, Pedro Alves wrote:
>>> AFAICT by playing with debugging the gdb.threads/leader-exit.exp program,
>>> you can't read /proc/PID/auxv if the leader thread has exited (is zombie).
>>>
>>> Maybe that ends up not mattering in practice, not sure, but it is a
>>> behavior change.
>>
>> Even if we can't read /proc/PID/auxv if the leader thread has exited,
>> auxv is still a process-specific resource, not a thread-specific one,
>> right?
>
> Right.
>
>> So, I suppose that the target interface could still accept a
>> pid, but linux_process_target could pick an arbitrary non-exited thread
>> from that pid to read auxv from?
>
> Right, I think we can do that.
>
>> Although I don't know what would
>> happen if that chose thread has just exited and we haven't consumed the
>> event yet.
>
> Not sure either. And I don't know what happens if the thread hasn't
> exited when when we open /proc/TID/auxv and then the thread exits
> before we read from the file. These cases should be confirmed. We may
> need to keep looking for a thread until we manage to open & read the
> file or run out of threads.

Ok, I will look into that. A simpler approach though would be to fetch
auxv at process starts and cache it for the duration of the debugging
session (since auxv doesn't change at runtime, AFAIK). Would that be a
good solution?

> BTW, I just noticed this "current_thread == NULL" check here:
>
> /* Handle qXfer:auxv:read. */
>
> static int
> handle_qxfer_auxv (const char *annex,
> gdb_byte *readbuf, const gdb_byte *writebuf,
> ULONGEST offset, LONGEST len)
> {
> if (!the_target->supports_read_auxv () || writebuf != NULL)
> return -2;
>
> if (annex[0] != '\0' || current_thread == NULL)
> return -1;
>
>
> Since we're removing the thread dependency, I think that check should
> be replaced by current_process() == NULL instead.

Ok, I'll submit a patch with this change.

--
Thiago