Date: Mon, 29 Aug 2016 09:51:50 -0700 From: John Baldwin <jhb@freebsd.org> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Andrey Chernov <ache@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r304928 - in head/lib/libc: amd64/sys i386/sys sys Message-ID: <18206408.GMLM77D01s@ralph.baldwin.cx> In-Reply-To: <20160829065813.GP83214@kib.kiev.ua> References: <201608272303.u7RN3N0D078505@repo.freebsd.org> <1595604.93PBdSz0kX@ralph.baldwin.cx> <20160829065813.GP83214@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, August 29, 2016 09:58:13 AM 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.
So the consumers are generally doing the right thing.
> 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));
> }
Certainly I think having a C wrapper like this makes more sense than
doing it all in assembly N times. I would probably prefer to keep the
manpage language the way it is though.
If we are really worried about compat, we could bump the ptrace symver
and use the function above as the FBSD_1.0 version perhaps.
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?18206408.GMLM77D01s>