Re: [PATCH] Fix sporadic XFAILs in, gdb.threads/attach-many-short-lived-threads.exp

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Bernd Edlinger <bernd.edlinger@hotmail.de> writes:

> On 4/4/24 18:25, Thiago Jung Bauermann wrote:
>>
>> Hello again,
>>
>> Sorry, one more comment that occurred to me today.
>>
>> Thiago Jung Bauermann <thiago.bauermann@linaro.org> writes:
>>
>> Actually, thinking more about it I think it would be better if
>> linux_proc_pid_get_state checked for errno == ESRCH itself and returned
>> -1 in that case (and also warn if that parameter is true), instead of
>> making its caller do that check.
>>
>> The function documentation says:
>>
>> /* Fill in STATE, a buffer with BUFFER_SIZE bytes with the 'State'
>>    line of /proc/PID/status.  Returns -1 on failure to open the /proc
>>    file, 1 if the line is found, and 0 if not found.  If WARN, warn on
>>    failure to open the /proc file.  */
>>
>> I think that getting the ESRCH error while reading is semantically
>> equivalent to failing to open the /proc file. Returning 0 when the line
>> wasn't found because of the ESRCH error adheres to the letter of that
>> comment but not to its spirit. :-)
>>
>
> The patch only works because linux_proc_pid_is_gone is just called
> on two places, where errno == EPERM:
>
> gdb/linux-nat.c:	      || (err == EPERM && linux_proc_pid_is_gone (lwpid)))
> gdbserver/linux-low.cc:	  || (err == EPERM && linux_proc_pid_is_gone (lwpid)))
>
> so when errno is changed from EPERM->ESRCH and there was no Status line
> found, then it is pretty clear what happened.
>
> But linux_proc_pid_get_state is called from other places where errno may
> be by chance already ESRCH.

I don't understand why the previous errno value is relevant. The code
needs to make sure that the errno it is checking comes from the fgets
call.

Which actually shows a problem with this patch that I didn't notice
before: it either needs to set errno = 0 before calling fgets, or call
ferror after the fgets call and before checking errno.

> I've spent some time reading the kernel sources, where the ESRCH return code
> was first introduced with v2.6.18, but unless the ESRCH is returned, all
> linux versions seem to always return the "Name:" followed by "State:"
> etc.
>
> So actually I don't see under which condition a thread can be alive
> when no "State:" line is found, as the comment here is probably
> just wrong:
>
>       /* No "State:" line, assume thread is alive.  */
>       return 0;

Good point. At least since Linux 2.6.12-rc2 (the first commit in git
history) there's always a "State:"" line. I also can't think of a
scenario where the assumption in the comment is valid.

--
Thiago