Re: [rfa] MIPS/Linux, take 3

[Andrew Cagney <ac131313 at cygnus dot com>]

Daniel, see attached.  One big glitch, copyright, sigh.

	Andrew
> 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*.

> 	* configure.tgt: Likewise.
> 	* NEWS: Mention mips*-*-linux port.


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


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

> +    supply_register ((regi - EF_REG0), (char *)(regp + regi));

Just FYI, you don't need the cast and in general people are removing
them.

> +#define fill(regp, regi, offset)			\
> +	  memcpy((char *)((regp) + (offset)),		\
> +		 &registers[REGISTER_BYTE (regi)],	\
> +		 sizeof (elf_greg_t))

Hmm, this technique has become the norm in this code :-/

> +	  from = (char *) &registers[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?).

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

> +void
> +_initialize_core_regset ()
> +{
> +  add_core_fns (&regset_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.



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


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

> +#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 :-)

> +#define HAVE_TERMIOS

Is this needed?


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


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


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

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