This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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
http://sergiodj.net/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]