On Mon, Aug 29, 2016 at 09:58:13AM +0300, Konstantin Belousov wrote:
> On Sun, Aug 28, 2016 at 04:09:51PM -0700, John Baldwin wrote:
> > OTOH, given that we explicitly documented it as not being true, I suspect
> > any applications that are using ptrace() are going off the documentation, 
> > not
> > the implementation artifact.  Note that Linux's ptrace() documents the same
> > requirement as before this change (caller is required to clear errno), so I
> > doubt there is any actual software out there that expects the
> > FreeBSD-specific behavior.  Given that and the extra maintenance overhead of
> > having to dink with errno in assembly on X architectures, I'd rather we keep
> > the old language in the manpage and remove the 'errno' frobbing in the 
> > system
> > call wrappers.  To be honest, my first response to this commit was one of
> > surprise that we modify errno directly as that is inconsistent with other
> > system calls.  (I haven't looked to see if any other system call wrappers
> > modify errno for non-error cases.)
> 
> The problematic calls are PT_PEEK_I and PT_PEEK_D, as far as I understand.
> 
> I dug into the ptrace(2) consumers, I found a lot of things using
> it which I would not expect to use, besides usual suspects of gdb
> lldb libunwind reptyr etc.  Most surprising was that even high-profile
> consumers including gdb sometimes fail to check errno after PT_PEEK. On
> the other hand, I did not found a case in gdb where errno is checked
> after PT_PEEK but not zeroed before the syscall.
> 
> I almost agreed with you after the reading, but then I decided to look
> into glibc just in case.  What I found there is really fascinating.
> From glibc/sysdeps/unix/sysv/linux:
>   res = INLINE_SYSCALL (ptrace, 4, request, pid, addr, data);
>   if (res >= 0 && request > 0 && request < 4)
>     {
>       __set_errno (0);
>       return ret;
>     }
> #define PTRACE_PEEKTEXT                  1
> #define PTRACE_PEEKDATA                  2
> #define PTRACE_PEEKUSR                   3
> 
> In the end, I might consider changing the ptrace wrappers into
> consolidated C source, it would look like that
> 
> int
> ptrace(int request, pid_t pid, caddr_t addr, int data)
> {
> 
>       errno = 0;
>       return (__sys_ptrace(request, pid, addr, data));
> }

And Solaris libc, where ptrace() is the wrapper around procfs, starts its
implementation this way:
usr/src/lib/libc/i386/sys/ptrace.c
        /*
         * Process the request.
         */
        errno = 0;
        switch (request) {
        case 1:         /* PTRACE_PEEKTEXT */
        case 2:         /* PTRACE_PEEKDATA */

_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to