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

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

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:
> 
>> Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
>>
>>> diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
>>> index e2086952ce6..36e0f5bf16a 100644
>>> --- a/gdb/nat/linux-procfs.c
>>> +++ b/gdb/nat/linux-procfs.c
>>> @@ -165,6 +165,9 @@ linux_proc_pid_is_gone (pid_t pid)
>>>      }
>>>    else if (have_state == 0)
>>>      {
>>> +      /* errno is ESRCH "No such process": assume thread has disappeared.  */
>>> +      if (errno == ESRCH)
>>> +	return 1;
>>>        /* No "State:" line, assume thread is alive.  */
>>>        return 0;
>>>      }
>>
>> With this patch applied on top of my patch series fixing attach to
>> zombie threads¹, I don't see these XFAILs anymore on an aarch64-linux
>> machine where previously I saw them on every run of this testcase. Nice!
>>
>> I would even suggest removing the XFAIL from the testcase, if other
>> people can confirm similar results.
>>
>> In any case:
>>
>> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> 
> 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'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;


Bernd.

> --
> Thiago