Re: [RFA] Add support for 64-bit MIPS GNU/Linux targets

[Daniel Jacobowitz <drow@mvista.com>]

On Mon, Dec 23, 2002 at 03:50:22PM -0700, Kevin Buettner wrote:
> The patch below adds GNU/Linux specific support for the n32 and n64
> MIPS ABIs.

Great!

> I've been able to do both core file debugging and remote debugging
> using this patch when GDB is built as a cross debugger to a
> mips64-linux target.  Unfortunately, I am not presently able to
> (completely) build a native mips64-linux debugger due to issues in
> other parts of the toolchain.

Right, I've heard about the problems.  I'm breathlessly awaiting a
solution :)

> Okay to commit?

I have a couple of comments.

> @@ -95,15 +109,15 @@ supply_gregset (elf_gregset_t *gregsetp)
>    memset (zerobuf, 0, MAX_REGISTER_RAW_SIZE);
>  
>    for (regi = EF_REG0; regi <= EF_REG31; regi++)
> -    supply_register ((regi - EF_REG0), (char *)(regp + regi));
> +    supply_32bit_reg ((regi - EF_REG0), (char *)(regp + regi));
>  
> -  supply_register (LO_REGNUM, (char *)(regp + EF_LO));
> -  supply_register (HI_REGNUM, (char *)(regp + EF_HI));
> +  supply_32bit_reg (LO_REGNUM, (char *)(regp + EF_LO));
> +  supply_32bit_reg (HI_REGNUM, (char *)(regp + EF_HI));
>  
> -  supply_register (PC_REGNUM, (char *)(regp + EF_CP0_EPC));
> -  supply_register (BADVADDR_REGNUM, (char *)(regp + EF_CP0_BADVADDR));
> -  supply_register (PS_REGNUM, (char *)(regp + EF_CP0_STATUS));
> -  supply_register (CAUSE_REGNUM, (char *)(regp + EF_CP0_CAUSE));
> +  supply_32bit_reg (PC_REGNUM, (char *)(regp + EF_CP0_EPC));
> +  supply_32bit_reg (BADVADDR_REGNUM, (char *)(regp + EF_CP0_BADVADDR));
> +  supply_32bit_reg (PS_REGNUM, (char *)(regp + EF_CP0_STATUS));
> +  supply_32bit_reg (CAUSE_REGNUM, (char *)(regp + EF_CP0_CAUSE));
>  
>    /* Fill inaccessible registers with zero.  */
>    supply_register (UNUSED_REGNUM, zerobuf);

Correct me if I'm wrong, but native thread debugging isn't going to
work if you do this.  proc-service.c calls supply_gregset directly and
everything blows up.

Is it time to multiarch supply_gregset?

> @@ -137,9 +151,8 @@ fill_gregset (elf_gregset_t *gregsetp, i
>  
>    if (regno < 32)
>      {
> -      src = &deprecated_registers[REGISTER_BYTE (regno)];
>        dst = regp + regno + EF_REG0;
> -      memcpy (dst, src, sizeof (elf_greg_t));
> +      regcache_collect (regno, dst);
>        return;
>      }
>  

Thanks, should have done this long ago.

> +	set_mips_linux_register_addr (gdbarch, mips_linux_register_addr);

>  void
>  _initialize_mips_linux_tdep (void)
>  {
> +  register_addr_data =
> +    register_gdbarch_data (init_register_addr_data, 0);
> +
>    gdbarch_register_osabi (bfd_arch_mips, 0, GDB_OSABI_LINUX,
>  			  mips_linux_init_abi);
>    add_core_fns (&regset_core_fns);

Blech.  So, the way _I_ would have done this would have been to put
this in the tdep structure.  In fact I have several patches which add
similar methods to the tdep structure, for signal handling.  Of course,
this is not compatible with the way Andrew asked to leave the tdep
struct in mips-tdep.c.  This is OK for now, but hopefully we can get
rid of it eventually.  We could multi-arch register_addr (is that
appropriate?  It's a native-only function, isn't it?) to do that.

> Index: config/mips/linux64.mt
> ===================================================================
> RCS file: config/mips/linux64.mt
> diff -N config/mips/linux64.mt
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ config/mips/linux64.mt	23 Dec 2002 22:35:29 -0000
> @@ -0,0 +1,9 @@
> +# Target: Linux/MIPS w/ support for 64-bit ABIs
> +TDEPFILES= mips-tdep.o mips-linux-tdep.o corelow.o \
> +	solib.o solib-svr4.o
> +TM_FILE= tm-linux64.h
> +
> +GDBSERVER_DEPFILES = linux-low.o linux-mips-low.o reg-mips.o

GDBSERVER_DEPFILES isn't actually used any more.  I'll go through and
remove it from everywhere sometime soon; please don't introduce it.

> +
> +SIM_OBS = remote-sim.o
> +SIM = ../sim/mips/libsim.a
> Index: config/mips/tm-linux64.h
> ===================================================================
> RCS file: config/mips/tm-linux64.h
> diff -N config/mips/tm-linux64.h
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ config/mips/tm-linux64.h	23 Dec 2002 22:35:29 -0000
> @@ -0,0 +1,53 @@
> +/* Target-dependent definitions for 64-bit GNU/Linux MIPS.
> +
> +   Copyright 2001, 2002 Free Software Foundation, Inc.

Should only say 2002.  Also, this file could be replaced by:
#include "mips/tm-mips64.h"
#include "mips/tm-linux.h"

because of the way the include guards on tm-mips.h work.  I think.  If
that works, please do it that way.  Hopefully I can kill this header
eventually; need to multi-arch solib handling, among other things.

So:
  supply_gregset?
  GDBSERVER_DEPFILES nit
  tm-linux64 nit

Otherwise, this is OK.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer