From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4654 invoked by alias); 12 Dec 2004 21:07:39 -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 4516 invoked from network); 12 Dec 2004 21:07:34 -0000 Received: from unknown (HELO sibelius.xs4all.nl) (82.92.89.47) by sourceware.org with SMTP; 12 Dec 2004 21:07:34 -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 iBCL7PuN021715; Sun, 12 Dec 2004 22:07:25 +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 iBCL7Oc4011319; Sun, 12 Dec 2004 22:07:24 +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 iBCL7KEi011314; Sun, 12 Dec 2004 22:07:20 +0100 (CET) Date: Sun, 12 Dec 2004 21:45:00 -0000 Message-Id: <200412122107.iBCL7KEi011314@elgar.sibelius.xs4all.nl> From: Mark Kettenis To: cagney@gnu.org CC: drow@false.org, gdb-patches@sources.redhat.com In-reply-to: <41BC8521.9060107@gnu.org> (message from Andrew Cagney on Sun, 12 Dec 2004 12:51:29 -0500) Subject: Re: [RFC] Use target vector inheritance for GNU/Linux References: <20041205184549.GA19814@nevyn.them.org> <41BC8521.9060107@gnu.org> X-SW-Source: 2004-12/txt/msg00347.txt.bz2 Date: Sun, 12 Dec 2004 12:51:29 -0500 From: Andrew Cagney > -LONGEST > -ia64_linux_xfer_unwind_table (struct target_ops *ops, > - enum target_object object, > - const char *annex, > - void *readbuf, const void *writebuf, > - ULONGEST offset, LONGEST len) > +LONGEST (*saved_xfer_partial) (struct target_ops *, enum target_object, > + const char *, void *, const void *, > + ULONGEST, LONGEST); > + > +static LONGEST > +ia64_linux_xfer_partial (struct target_ops *ops, > + enum target_object object, > + const char *annex, > + void *readbuf, const void *writebuf, > + ULONGEST offset, LONGEST len) > { > - return syscall (__NR_getunwind, readbuf, len); > + if (object == TARGET_OBJECT_UNWIND_TABLE && writebuf == NULL && offset == 0) > + return syscall (__NR_getunwind, readbuf, len); > + > + return saved_xfer_partial (ops, object, annex, readbuf, writebuf, > + offset, len); > +} 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? +#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. I'm working on something that will help resolving this problem, by re-adding PT_READ_U support back into inf-ptrace.c in a clean fashion. In the meantime, this hack allows us to go forwards. +/* 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. > A new function, linux_target, is added to linux-nat.c. Then any GNU/Linux > target can call it, and pass the result to add_target - after specializing > whatever methods it needs to. Sometimes it's necessary to specialize a > method between inf_ptrace_target and linux_target, so it accepts an optional > argument. This wouldn't be necessary if all target methods took a > target_ops parameter, so they could call the overridden method. As in this? > +void > +_initialize_i386_linux_nat (void) > +{ > + struct target_ops *t = inf_ptrace_target (); > + > + /* Override the default ptrace resume method. */ > + t->to_resume = i386_linux_resume; > + > + /* Fill in the generic GNU/Linux methods. */ > + t = linux_target (t); which is violating the inheritance structure. Can you instead add a one-of method (deprecated_set_super_linux_resume?) to handle this case? We can then see about fixing the problem (I'm left wondering if that method is still needed). 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.) > + /* FIXME drow/2004-12-04: For now, these functions must keep the standard > + names because only some GNU/Linux targets use inf-ptrace.o. When all > + have been converted, these functions can be renamed and made static. */ Is this still true? I thought the patch was so large because it touched all the linux targets. Yes it is true. Daniels patch touches most (if not all) Linux targets, but doesn't resolve all the issues. Some more work is necessary. However, most of that work is non-mechanical. Given the fact that many Linux targets are not actively maintained making the acceptance of this patch depend on this patch depend on that work would probably mean it will be dropped. Mark