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: [RFC] Tighten and tweak ptrace argument checks


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.
  - use PTRACE_PEEKDATA to read n64 data spaces, much as Richard is
    doing now for ppc64.  This is important on multiple-architecture
    installations.

_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.

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.

> > 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_*".

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.

-- 
Daniel Jacobowitz
CodeSourcery, LLC


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