From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16792 invoked by alias); 13 Dec 2004 21:34:49 -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 16707 invoked from network); 13 Dec 2004 21:34:39 -0000 Received: from unknown (HELO sibelius.xs4all.nl) (82.92.89.47) by sourceware.org with SMTP; 13 Dec 2004 21:34:39 -0000 Received: from elgar.sibelius.xs4all.nl (elgar.sibelius.xs4all.nl [192.168.0.2]) by sibelius.xs4all.nl (8.13.0/8.13.0) with ESMTP id iBDLYKI6012413; Mon, 13 Dec 2004 22:34:20 +0100 (CET) Received: from elgar.sibelius.xs4all.nl (localhost [127.0.0.1]) by elgar.sibelius.xs4all.nl (8.12.6p3/8.12.6) with ESMTP id iBDLYKmS002583; Mon, 13 Dec 2004 22:34:20 +0100 (CET) (envelope-from kettenis@elgar.sibelius.xs4all.nl) Received: (from kettenis@localhost) by elgar.sibelius.xs4all.nl (8.12.6p3/8.12.6/Submit) id iBDLYFHm002578; Mon, 13 Dec 2004 22:34:15 +0100 (CET) Date: Mon, 13 Dec 2004 22:29:00 -0000 Message-Id: <200412132134.iBDLYFHm002578@elgar.sibelius.xs4all.nl> From: Mark Kettenis To: cagney@gnu.org CC: drow@false.org, gdb-patches@sources.redhat.com In-reply-to: <41BDFE94.9060401@gnu.org> (message from Andrew Cagney on Mon, 13 Dec 2004 15:41:56 -0500) Subject: Re: [RFC] Use target vector inheritance for GNU/Linux References: <20041205184549.GA19814@nevyn.them.org> <41BC8521.9060107@gnu.org> <200412122107.iBCL7KEi011314@elgar.sibelius.xs4all.nl> <41BDFE94.9060401@gnu.org> X-SW-Source: 2004-12/txt/msg00367.txt.bz2 Date: Mon, 13 Dec 2004 15:41:56 -0500 From: Andrew Cagney Mark Kettenis wrote: > Can you rename saved_xfer_partial to super_xfer_partial_hack and add a > FIXME. It should be calling super.xfer_partial but that's not available :-( > > Can you explain what you're hinting at here Andrew? What makes this > specific saved_xfer_partial so different from the other saved_xxx > instances that the patch introduces? Nothing? Changing those to be consistent with this would be a logical next step. I suspect I don't see the big picture like you do, and I don't see why you'd want a super.xfer_partial_hack. This construction isn't a hack at all; it explicitly shows how the method is inherited. However, super_xfer_partial is perhaps a better name than saved_xfer_partial. > +#ifndef FETCH_INFERIOR_REGISTERS > + > +/* Fetch register REGNUM from the inferior. */ > + > +static void > +fetch_register (int regnum) > +{ > > Why is this wrapped in in an #ifdef? > > Some of the Linux target still need the crufty old code to read > registers using PTRACE_PEEKUSR. The new inf-ptrace.c doesn't provide > that functionality, so I guess Daniel inlined that bit of code here. > This is related to the FIXME below, and of course only temporary. So a fixme or other comment needs to be added at the point of this macro? Wouldn't hurt, although that FIXME below is already there. > +/* Create a generic GNU/Linux target vector. If T is non-NULL, base > + the new target vector on it. */ > + > +struct target_ops * > +linux_target (struct target_ops *t) > > Can this be renamed to inf_linux_target (to be consistent with the other > inf_*_target() methods? > > Apparently I don't agre with this since I already introduced > i386bsd_target and sparc_target; linux_target is consistent with that. You also added inf_ttrace_target. Yup, since that one lives in inf-ttrace.c. The inf_ttrace_ prefix is consistent with the naming of the other functions in the file, just as i386bsd_target and sparc_target are consistent with the files they're in. > No it isn't. At a very low level, all Linux ports are slightly > different. Most ports will need to adjust the generic ptrace target > before it can be inherited by the generic Linux target. In fact I > think that when the FETCH_INFERIOR_REGISTERS issue above is sorted > out, you'll see that *all* Linux ports will need to do this trick of > adjusting the ptrace target before passing it to linux_target(). > > (And yes, I'm fairly certain the method is still needed. While the > problem may have been fixed in recent kernels, there are many older > Linux kernels out there.) You're on the right track, however the inheritance structure that the C code is trying to mimic is: i386LinuxInferior IS-A LinuxInferior IS-A PtraceInferior Ah but that's not how the Linux-interfaces work (unless we drop support for multi-threading). LinuxInferior needs platform-dependent functionality that PtraceInferior doesn't (and shouldn't) provide. with the i386LinuxInferior.resume method overriding LinuxInferior.resume (which overrid PtraceInferior.resume); and not: But having i386LinuxInferior override LinuxInferior.resume really sucks because then it too has to do this insane dance of fiddling with LWPs because of the braindamaged Linux threading model. LinuxInferior IS-A i386LinuxInferior IS-A PtraceInferior This reflects reality better, although it might even be: i386LinuxInferior IS-A LinuxInferior IS-A i386LinuxPtraceInferior IS-A PtraceInferior But that breaks some OO rules of course. That the current inferior code doesn't facilitate this is a recognized problem, but not one that we should get hung-up over. Hence my suggestion of deprecated_set_super_linux_resume as a workaround until that is fixed. It's not a problem with the current inferior code. There may be some issues with the inferior code (like the ptrace_ops_hack), but this is a fundamental problem with the Linux threads debugging interface. Perhaps there simply shouldn't be a LinuxInferior: i386LinuxInferior IS-A PtraceInferior where we'de have some Linux-specific helper functions that can be used to implement i386LinuxInferior. Mark