From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6849 invoked by alias); 18 Sep 2005 10:36:35 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 6577 invoked by uid 22791); 18 Sep 2005 10:36:21 -0000 Received: from sibelius.xs4all.nl (HELO sibelius.xs4all.nl) (82.92.89.47) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Sun, 18 Sep 2005 10:36:21 +0000 Received: from elgar.sibelius.xs4all.nl (root@elgar.sibelius.xs4all.nl [192.168.0.2]) by sibelius.xs4all.nl (8.13.4/8.13.4) with ESMTP id j8IAZjUI008889; Sun, 18 Sep 2005 12:35:45 +0200 (CEST) Received: from elgar.sibelius.xs4all.nl (kettenis@localhost.sibelius.xs4all.nl [127.0.0.1]) by elgar.sibelius.xs4all.nl (8.13.4/8.13.3) with ESMTP id j8IAZjwI012915; Sun, 18 Sep 2005 12:35:45 +0200 (CEST) Received: (from kettenis@localhost) by elgar.sibelius.xs4all.nl (8.13.4/8.13.4/Submit) id j8IAZjPV008619; Sun, 18 Sep 2005 12:35:45 +0200 (CEST) Date: Sun, 18 Sep 2005 10:36:00 -0000 Message-Id: <200509181035.j8IAZjPV008619@elgar.sibelius.xs4all.nl> From: Mark Kettenis To: drow@false.org CC: gdb-patches@sourceware.org In-reply-to: <20050914144303.GA20354@nevyn.them.org> (message from Daniel Jacobowitz on Wed, 14 Sep 2005 10:43:03 -0400) Subject: Re: [RFC] Tighten and tweak ptrace argument checks References: <20050909212825.GA9449@nevyn.them.org> <200509122147.j8CLluXQ011168@elgar.sibelius.xs4all.nl> <20050914144303.GA20354@nevyn.them.org> X-SW-Source: 2005-09/txt/msg00160.txt.bz2 > Date: Wed, 14 Sep 2005 10:43:03 -0400 > From: Daniel Jacobowitz > > On Mon, Sep 12, 2005 at 11:47:56PM +0200, Mark Kettenis wrote: > > > On i386-pc-linux-gnu the defaults work so this is merely an annoyance. > > > When the real type is long long instead of long, well, it wasn't > > > pretty... > > > > But 'long long' as a return type or argument type really doesn't make > > sense for ptrace(2). But then of course MIPS doesn't make any sense > > either, so that's perfectly fine ;-). > > > > > So here's a proposed patch. It handles the GNU/Linux case. It handles > > > long long. It also is violently fatal if it fails, instead of making > > > up something sensible - this is because I wasted several days trying to > > > figure out what was wrong with GDB when it was casting all the ptrace > > > arguments to the wrong type. I'm sure it'll break the build in a lot > > > of places but I think it's worth fixing if you want to use autoconf for > > > this at all! > > > > Sorry 'bout that, but you've created a ptrace variant that's > > incompatible with everything else on the planet. All other Linux > > ports use long. I presume you invented the 'long long' to be able to > > return 64-bit register values with PTRACE_PEEKUSER. That interface > > should really die, and be replaced with PTRACE_GETREGS and friends. > > I "invented" the long long in order to: > - be compatible with the n64 ptrace interface; the design of n32 > Linux is to be compatible with o32 wherever possible, be compatible > with n64 where it isn't possible, and discourage n32-specific ABIs. But that means you now have an n32-specific API. I'd say that's much worse than an n32-specific ABI. As a programmer I don't really care about the ABI, as long as it is stable. > - use PTRACE_PEEKDATA to read n64 data spaces, much as Richard is > doing now for ppc64. This is important on multiple-architecture > installations. But this would work (almost) just as well if ptrace(2) would return a `long' as you indicate below. > _Constructive_ criticism is welcome. I did this work a long time ago, > but never got it merged to either gdb or glibc (just the kernel bits). > So if you have an alternate suggestion, I don't feel too bad about > making an incompatible change to the kernel ABI here. Ralf probably > won't object either. I think it would be better to change things such that there is a consistent API across all the MIPS ABI's. > > PTRACE_PEEKUSER is not a big deal for this interface; you can return > 32-bit chunks of the 64-bit registers. It just requires an > n32-specific ABI for ptrace. The way I view ptrace, this would be the right API. Think about the PTRACE_PEEKXXXX (or PT_READ_X) requests as "return the memory at this particular address in area XXXX as a word-sized chunk". Then realize that USER is nothing but a funky area that stores some interesting information about the process, such as the registers. On traditional UNIX systems, the user area did contain quite a bit of interesting stuff, but on Linux I think the registers are the only stuff left there. In fact the user area doesn't really exists anaymore and is synthesised by ptrace, but that doesn't really change the way the interface should be viewed. Please take a look at inf-ptrace.c:inf_ptrace_fetch_register(); the code to read multiple-word sized registers is already there! The actual code could still be almost identical to the n64 ptrace; just return things in chucks based on the ABI's (n32 or n64) wordsize. Now this of course sucks performance-wise, but than these interfaces already suck performance-wise. The BSD's have a nice PT_IO request, that lets you transfer chunks of memory with a single request: http://www.openbsd.org/cgi-bin/man.cgi?query=ptrace it'd be nice if Linux has a similar thing. > > > > Any comments on the patch? > > > > Sorry, yes, I'm almost certain this will break on OSF/1 or AIX or > > HP-UX. It'll probably break even for Linux if you're using an older > > compiler. > > Hmm, you're right - I got the logic backwards. > > > > I also noticed considerable PTRACE_ARG3_TYPE vs PTRACE_TYPE_ARG3 > > > confusion in the sources. I think that the last time I looked at this, > > > I convinced myself that it was possible to get them out of sync, and > > > the results would be pretty gruesome. I didn't touch that for now. > > > > This is because there are several ports that are basically > > unmaintained; nobody feels responsible enough to keep them up to date. > > I think that you bear some responsibility here, for introducing one > without getting rid of the other. It's impossible to keep them > straight and there's no obvious difference between them. This sort of > unfinished transition is exactly what I remember arguing against when > we last discussed "deprecated_*". Yup, I'm getting really frustrated with this. We have a lot of ports where no-one cares enough about the port to periodically go over the code and replace the deprecated_ stuff. I suspect the fact that with the commercialization of Linux a lot of people are no longer allowed to do "turd-shining" and their stupid short-term shareholder value focussed bosses don't realize that a certain amount of turd-shining is essential to keep the code they depend on maintainable on a longer time-scale. It's exactly this frustration that made me send my initial, rather unconstructive reply; sorry 'bout that. However, I really do think that an attempt to unify the ptrace API on the different Linux platforms will actually help to remidy the situation. It'll allow us to share more code between them and therefore less likely that they'll get out of sync. > There's all of five definitions under config/, the default definition > in inferior.h, and a couple of uses. I don't see any reason why any of > the existing definitions are necessary, so why not just remove them and > sed the uses? If there's some particular platform from the bunch that > you think would be non-obvious, you could always ask specifically for > someone to try it. I can't help with (and don't care about) lynx, but > I have access to all the others. > > The above is for any value of "you". I don't think you, Mark, have an > obligation to fix it at this date. I'll be happy to simply weed out the obsolete PTRACE_XXX defs from config/ for the ports that I can't test. Of course that means risking potentially breaking those ports. Thus far we have been rather conservative about doing such things. We (the core GDB maintainers) should probably become a bit more active in prodding people to clean up the ports they're looking after. We probably could hack up a patch that we think should work, and ask people to test it for us. So, yes, I think it'd be good if you could still change things such that the n32 ptrace API would be identical to the o32 and n64 APIs. If not, I think the patch below is a low-risk way to check for the right prototype on n32 MIPS Linux. Cheers, Mark Index: configure.ac =================================================================== RCS file: /cvs/src/src/gdb/configure.ac,v retrieving revision 1.24 diff -u -p -r1.24 configure.ac --- configure.ac 25 Jul 2005 15:08:40 -0000 1.24 +++ configure.ac 18 Sep 2005 10:30:23 -0000 @@ -489,9 +489,14 @@ AC_CHECK_DECLS(ptrace, [], [ # Check return type. AC_CACHE_CHECK([return type of ptrace], gdb_cv_func_ptrace_ret, AC_TRY_COMPILE($gdb_ptrace_headers, + [extern long long ptrace(enum __ptrace_request, ...);], + [gdb_cv_func_ptrace_ret='long long'; + gdb_cv_func_ptrace_args=['enum __ptrace_request,...,long long,...']]) + AC_TRY_COMPILE($gdb_ptrace_headers, [extern int ptrace ();], - gdb_cv_func_ptrace_ret='int', - gdb_cv_func_ptrace_ret='long')) + gdb_cv_func_ptrace_ret='int') + # Provide a safe default value. + : ${gdb_cv_func_ptrace_ret='long'}) AC_DEFINE_UNQUOTED(PTRACE_TYPE_RET, $gdb_cv_func_ptrace_ret, [Define as the return type of ptrace.]) # Check argument types.