Re: [RFC] Use target vector inheritance for GNU/Linux

Andrew Cagney
2004-12-13T20:44:31Z

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.

> +#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?

> +/* 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.

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

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

with the i386LinuxInferior.resume method overriding LinuxInferior.resume
(which overrid PtraceInferior.resume); and not:

LinuxInferior IS-A i386LinuxInferior IS-A PtraceInferior

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.

Andrew