Re: [PATCH] PR gdb/16188: Verify PTRACE_TRACEME succeeded

Sergio Durigan Junior
2017-02-20T12:51:47Z

On Monday, February 20 2017, Pedro Alves wrote:

> Hi Sergio,
>
> This LGTM, save for the errno handling in Darwin bits:
>
> On 02/18/2017 05:09 AM, Sergio Durigan Junior wrote:
>> diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
>> index 8c5e8a0..e02e51d 100644
>> --- a/gdb/darwin-nat.c
>> +++ b/gdb/darwin-nat.c
>> @@ -254,7 +254,6 @@ darwin_ptrace (const char *name,
>> {
>> int ret;
>>
>> - errno = 0;
>> ret = ptrace (request, pid, arg3, arg4);
>> if (ret == -1 && errno == 0)
>> ret = 0;
>
> Removing "errno = 0" here is incorrect. There are ptrace calls where a -1
> return is not an error, thus that check for "errno==0" after the
> ptrace call. Since system calls are not required to clear errno on
> success, that errno=0 is required.
>
> This is Darwin, but the Linux man pages, in "man ptrace" say:
>
> On error, all requests return -1, and errno is set appropriately. Since the
> value returned by a successful PTRACE_PEEK* request may be -1, the caller
> must clear errno before the call, and then check it afterward to determine whether
> or not an error occurred.
>
> And actually, the comment just above darwin_ptrace talks
> about clearning errno. So it's really incorrect.

Oh, I'm really sorry, this was actually a mistake on the patch. I meant
to delete the 'errno = 0;' on darwin_ptrace_me, not on darwin_ptrace.
Of course, I understand that errno must be cleared before the ptrace
call and I had read the exact same paragraph on the manpage. Anyway,
sorry for wasting your time on this.

>> @@ -1728,23 +1727,30 @@ darwin_ptrace_me (void)
>> int res;
>> char c;
>>
>> + errno = 0;
>
> OTOH, I don't see the need to clear it here. Below,
> errno will only be used when a syscall fails, and in
> failure case, the syscall must set errno.

Yeah.

I fixed the mistake and pushed the patch. Thanks.

0db8980cc0ee05727c11f8b7c6674137a4d5de4e

--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible