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

Mark Kettenis
2004-12-13T21:34:49Z

Date: Mon, 13 Dec 2004 15:41:56 -0500
From: Andrew Cagney <cagney@gnu.org>

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