This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] Tighten and tweak ptrace argument checks
> Date: Wed, 14 Sep 2005 10:43:03 -0400
> From: Daniel Jacobowitz <drow@false.org>
>
> 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.