Re: [rfa/ppc/branch too] Fix PowerPC/Linux cores

[Kevin Buettner <kevinb at cygnus dot com>]

On Jul 30,  3:54pm, Daniel Jacobowitz wrote:

> > > OK to commit, trunk and branch?
> > 
> > I think I'd like to explore some alternatives first.  The only
> > real difference that I see between the version of fetch_core_registers()
> > that you want to add to ppc-linux-nat.c and the one in core-regset.c is
> > that the former uses elf_{g,fp}regset_t whereas the latter uses
> > {g,fp}regset_t, right?
> > 
> > If that's the case, couldn't we simply make the core-regset.c version
> > use gdb_gregset_t and gdb_fpgregset_t?  That seems cleaner to me than
> > propagating nearly identical copies of fetch_core_registers() to the
> > various *-nat.c files.  (I see that mips-linux-tdep.c and
> > m68klinux-nat.c have taken the approach that you're suggesting;
> > i386-linux-nat.c has a good excuse for having its own version since
> > the format that it needs to support can't be handled by the generic
> > code.)
> 
> The danger in that is that the gdb_*regset_t types mean a lot of
> different things to a lot of different ports.  For all Linux targets,
> certainly, such a change would be appropriate.  I have no idea what
> other targets it would or wouldn't work for.

If you look in gregset.h, you'll see that gdb_gregset_t is typedef'd
to be gregset_t and gdb_fpregset_t is typedef'd to be fpregset_t
unless GDB_GREGSET_T / GDB_FPREGSET_T are defined in which case
these'll override the default values.

So all targets which *don't* define GDB_GREGSET_T / GDB_FPREGSET_T
won't be affected.  I.e, that means that we only need to worry about
those ports which *do* define GDB_*REGSET_T.  That means that
we have to consider Linux (all architectures), NetBSD/sparc,
Solaris/sparc and AIX/IA-64.

For Linux, using gdb_*regset_t in fetch_core_registers() will do the
right thing because we really want to use elf_*regset_t.  (Note,
however, that Linux/i386 isn't affected because it has its own
fetch_core_registers function.)

Neither NetBSD/sparc nor Solaris/sparc use core-regset.c, so we don't
have to worry about them.

Finally, AIX/IA-64 is a work in progress (and I'm working on it).
I'll make whatever adjustments are necessary...

> As I've said, half of the binutils changes for cross core debugging
> have been checked in, and I'm just cleaning up the other
> platform-specific bits now.  Once I've done that I'm going to tweak
> (replace?) core-regset.c to not use the sizeof() operator; that's the
> only thing it does at present which causes it to be native-dependent. 
> After that, the copies in mips-linux-tdep, m68klinux-nat, and maybe now
> ppc-linux-nat (which are all my fault to some degree) can disappear.

Great!

> My preference would be to use this patch for the moment, given the
> branch timeframe; while not ideal, it is certainly correct.

I have no objection to it going on the branch; but I'd prefer to not
see it go in on the trunk.  OTOH, I'd have no objection to a patch
which updates core-regset.c to use gdb_gregset_t / gdb_fpregset_t
being applied to either the 5.1 release branch or the trunk.  I'm
not quibbling about correctness, but I'd prefer to see it done the
"right" way on the trunk.  That could mean using gdb_*regset_t in
core-regset.c or it could mean using your upcoming cross platform
corefile support or perhaps some combination.

Before you commit anything along these lines (or along the lines of
the patch that you submitted), do check with Andrew first.  I'm not
the core-regset.c maintainer and Andrew may want to see all patches
that go in on the release branch go in on the trunk as well...

Kevin