This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [rfa] MIPS/Linux, take 3
I've tried three times now to get a response from David about this,
without a word of response. What I will do at this stage is rewrite
the last remaining bits. The bulk of what you see here is not David's
code any longer; I only left the contributed notice out of courtesy,
since I was basing my work on his. I'll repost the patch with the very
few remaining pieces and the notices removed. That's legitimate, by my
understanding - right?
On Mon, Jul 09, 2001 at 03:04:27PM -0400, Andrew Cagney wrote:
> > 2001-07-06 Daniel Jacobowitz <drow@mvista.com>
> >
> > * mips-linux-tdep.c: New file.
> > * mips-linux-nat.c: New file.
> > * config/mips/linux.mh: New file.
> > * config/mips/linux.mt: New file.
> > * config/mips/xm-linux.h: New file.
> > * config/mips/nm-linux.h: New file.
> > * config/mips/tm-linux.h: New file.
> > * configure.host: Recognize mips*-*-linux.
>
> To be pedantic, mips*-*-linux*.
Sure. Just the changelog is wrong.
> > --- /dev/null Wed Dec 31 16:00:00 1969
> > +++ gdb/mips-linux-tdep.c Wed Jul 4 16:33:13 2001
> > @@ -0,0 +1,313 @@
> > +/* Target-dependent code for Linux/MIPS.
> > + Copyright 1996, 2001 Free Software Foundation, Inc.
> > +
> > + Originally contributed by David S. Miller (davem@caip.rutgers.edu) at
> > + Rutgers University CAIP Research Center.
>
> Um, there isn't anything showing that Rutgers University disclaimed
> work by David S. Miller back in '96. Rutgers Uni does have several
> other disclaimers on file. Daniel, could you please you dig a little
> bit more into the history of these files.
>
> Sigh.
See above.
> > +/* Copied from <asm/elf.h>. */
> > +#define ELF_NGREG 45
> > +#define ELF_NFPREG 33
>
> Strongly recommend using enum's for this. Check what Michael Snyder
> did to the SPARC. He says that he never looked back.
>
> > +/* 0 - 31 are integer registers, 32 - 63 are fp registers. */
> > +#define FPR_BASE 32
> > +#define PC 64
> > +#define CAUSE 65
> > +#define BADVADDR 66
> > +#define MMHI 67
> > +#define MMLO 68
> > +#define FPC_CSR 69
> > +#define FPC_EIR 70
>
> You may want to name space proof these with some sort of prefix? Don't
> know.
As the headers say, I just copied these from kernel headers; the
functions using them used to be in the -nat file and include the kernel
headers. I'll look into improving how this is handled later, but for
now I'd prefer to leave the names as they are.
> > + supply_register ((regi - EF_REG0), (char *)(regp + regi));
>
> Just FYI, you don't need the cast and in general people are removing
> them.
OK, I'll kill it. Thanks.
> > +#define fill(regp, regi, offset) \
> > + memcpy((char *)((regp) + (offset)), \
> > + ®isters[REGISTER_BYTE (regi)], \
> > + sizeof (elf_greg_t))
>
> Hmm, this technique has become the norm in this code :-/
Alas :) I'll clean up these functions as soon as I get a chance;
they're a little obtuse.
> > + from = (char *) ®isters[REGISTER_BYTE (regi + FP0_REGNUM)];
> > + to = (char *) (*fpregsetp + regi);
>
> This is pretty good code, at least someone can step through it and see
> what exactly it is doing. (Is the cast avoidable?).
Ditto.
> > +/* Map gdb internal register number to ptrace ``address''.
> > + These ``addresses'' are defined in <asm/ptrace.h>. */
> > +
> > +#define REGISTER_PTRACE_ADDR(regno) \
> > + (regno < 32 ? regno \
> > + : (regno >= FP0_REGNUM && regno < FP0_REGNUM + 32) ? \
> > + FPR_BASE + (regno - FP0_REGNUM) \
> > + : regno == PC_REGNUM ? PC \
> > + : regno == CAUSE_REGNUM ? CAUSE \
> > + : regno == BADVADDR_REGNUM ? BADVADDR \
> > + : regno == LO_REGNUM ? MMLO \
> > + : regno == HI_REGNUM ? MMHI \
> > + : regno == FCRCS_REGNUM ? FPC_CSR \
> > + : regno == FCRIR_REGNUM ? FPC_EIR \
> > + : 0)
>
> Could you please change this to a function.
Certainly.
> > +void
> > +_initialize_core_regset ()
> > +{
> > + add_core_fns (®set_core_fns);
> > +}
>
> Can you please change the function name to
> _initialize_mips_linux_tdep() to match the .c file and move it to the
> bottom.
Ditto.
> > Index: gdb/mips-linux-nat.c
> > ===================================================================
> > RCS file: N/A
> > diff -u /dev/null gdb/mips-linux-nat.c
> > --- /dev/null Wed Dec 31 16:00:00 1969
> > +++ gdb/mips-linux-nat.c Wed Jul 4 15:46:03 2001
>
> I would suggest deleting this file and then committing something based
> on ppc-linux-nat.c. The result of doing that is approved.
Given the contents of this file (byte for byte identical to the parts
of ppc-linux-nat that are relevant) that's easy enough.
> > Index: gdb/config/mips/xm-linux.h
> > ===================================================================
> > RCS file: N/A
> > diff -u /dev/null gdb/config/mips/xm-linux.h
> > --- /dev/null Wed Dec 31 16:00:00 1969
> > +++ gdb/config/mips/xm-linux.h Wed Jul 4 16:27:53 2001
>
> > + Originally contributed by David S. Miller (davem@caip.rutgers.edu) at
> > + Rutgers University CAIP Research Center.
>
> Again suggest deleting this file and implementing something based on
> config/powerpc/xm-linux.h. The result of doing that is approved.
As above, easy enough. Will do.
> > +#ifndef HOST_BYTE_ORDER
> > +#include <endian.h>
> > +#if __BYTE_ORDER == __BIG_ENDIAN
> > +#define HOST_BYTE_ORDER BIG_ENDIAN
> > +#else
> > +#define HOST_BYTE_ORDER LITTLE_ENDIAN
> > +#endif
> > +#endif
>
> I'm working on it :-)
If anything was ever screaming for autoconf... :)
> > +#define HAVE_TERMIOS
>
> Is this needed?
I'll double-check. It's in every other xm-linux.h.
> > Index: gdb/config/mips/tm-linux.h
> > ===================================================================
> > RCS file: N/A
> > diff -u /dev/null gdb/config/mips/tm-linux.h
> > --- /dev/null Wed Dec 31 16:00:00 1969
> > +++ gdb/config/mips/tm-linux.h Thu Jul 5 15:59:13 2001
> > @@ -0,0 +1,93 @@
>
> > + Originally contributed by David S. Miller (davem@caip.rutgers.edu) at
> > + Rutgers University CAIP Research Center.
>
> See above about copyright. Otherwize approved (but see below).
>
> > +#define GET_LONGJMP_TARGET(ADDR) get_longjmp_target(ADDR)
> > +extern int get_longjmp_target (CORE_ADDR *);
>
> If this is a mips specific function then this should be called
> mips_get_longjmp_target().
I'll correct that.
> > Index: gdb/config/mips/nm-linux.h
> > ===================================================================
> > RCS file: N/A
> > diff -u /dev/null gdb/config/mips/nm-linux.h
> > --- /dev/null Wed Dec 31 16:00:00 1969
> > +++ gdb/config/mips/nm-linux.h Fri Jul 6 14:02:28 2001
> > @@ -0,0 +1,83 @@
>
> > + Originally contributed by David S. Miller (davem@caip.rutgers.edu) at
> > + Rutgers University CAIP Research Center.
>
> Appart from the copyright problem, approved (but see below).
>
> > +#define CANNOT_FETCH_REGISTER(regno) \
> > + ((regno) >= FP_REGNUM \
> > + || (regno) == PS_REGNUM \
> > + || (regno) == ZERO_REGNUM)
> > +#define CANNOT_STORE_REGISTER(regno) \
> > + ((regno) >= FP_REGNUM \
> > + || (regno) == PS_REGNUM \
> > + || (regno) == ZERO_REGNUM \
> > + || (regno) == BADVADDR_REGNUM \
> > + || (regno) == CAUSE_REGNUM \
> > + || (regno) == FCRIR_REGNUM)
>
> Please change this to a function.
Will do.
> > Index: gdb/config/mips/linux.mh
> > Index: gdb/config/mips/linux.mt
>
> Both approved.
>
>
> > Index: gdb/NEWS
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/NEWS,v
> > retrieving revision 1.26
> > diff -u -r1.26 NEWS
> > --- gdb/NEWS 2001/06/28 19:04:10 1.26
> > +++ gdb/NEWS 2001/07/06 22:08:43
> > @@ -14,6 +14,7 @@
> >
> > Alpha FreeBSD alpha*-*-freebsd*
> > x86 FreeBSD 3.x and 4.x i[3456]86*-freebsd[34]*
> > +MIPS Linux mips{,el}-*-linux*
>
> Suggest using just ``mips*-*-linux''. To be pedantic,
> mipseb-unknown-linux-gnu is also accepted.
That makes sense. Sure.
> OK with me, news items don't need approval.
>
> > Index: gdb/configure.host
>
> OK with me, configury changes don't need approval.
>
> > Index: gdb/configure.tgt
>
> OK with me, configury changes don't need approval.
>
> Andrew
--
Daniel Jacobowitz Carnegie Mellon University
MontaVista Software Debian GNU/Linux Developer