Fw: Unreviewed patch (m32r-linux support)

Kei Sakamoto sakamoto.kei@renesas.com
Wed Sep 15 08:01:00 GMT 2004


Mark,

Would you review the attached patch?
I'm afraid that you are still busy because of moving.
But it seems you are the best person to review it
because I revised it based on your advise.
If you are too busy, please let me know.
Thank you.

Kei Sakamoto


> Hello,
>
> Would anyone take a look at this patch?
> Thank you in advance.
>
> Kei Sakamoto
>
> From: "Kei Sakamoto" <sakamoto.kei@renesas.com>
> To: <gdb-patches@sources.redhat.com>
> Sent: Friday, August 20, 2004 3:14 PM
> Subject: Re: [RFA/m32r] m32r-linux support
>
>
> > Hello,
> >
> > I revised the patch of m32r-linux support.
> >
> > Thanks to Mark Kettenis' advice, I removed many unnecessary codes
> > from it and simplified it. But it still contains some stuff
> > to support older versions of Linux because Linux/M32R does not
> > support PTRACE_GETREGS.
> >
> > I also removed the gdbserver support from it. I'll post it
> > as another patch after this patch is accepted.
> >
> > OK to commit?
> >
> > 2004-08-20    Kei Sakamoto  <sakamoto.kei@renesas.com>
> >
> >     Add m32r-linux support.
> >     * configure.tgt: Add m32r*-*-linux*.
> >     * Makefile.in (ALLDEPFILES): Add m32r-tdep.c, m32r-linux-nat.c and
> >     m32r-linux-tdep.c.
> >     (m32r-linux-nat.o, m32r-linux-tdep.o): New dependencies.
> >     * m32r-linux-nat.c, m32r-linux-tdep.c, config/m32r/linux.mh,
> >     config/m32r/linux.mt, config/m32r/m32r.mh, config/m32r/nm-linux.h,
> >     config/m32r/tm-linux.h: New files.
> >
> > Kei Sakamoto said:
> >
> > > Hello, Mark,
> > >
> > > Thank you for your reply. It is very useful.
> > >
> > > There is the summer holidays in Japan next week. So I'll modify my
> > > patch and post it after that.
> > >
> > > Kei Sakamoto
> > >
> > > Mark Kettenis said:
> > > >    From: "Kei Sakamoto" <sakamoto.kei@renesas.com>
> > > >    Date: Thu, 5 Aug 2004 14:11:07 +0900
> > > >
> > > >    Hello,
> > > >
> > > > Hi Kei,
> > > >
> > > > First can you seperate out the gdbserver support, and submit that as a
> > > > seperate patch?  Daniel will have to review that part of the code.
> > > > Making it seperate will make it more likely that the code can be
> > > > checked in soon.
> > > >
> > > >    The attached patch adds m32r-linux support.
> > > >    The M32R architecture is not added to the official Linux kernel yet.
> > > >    But Linux on M32R is already available.
> > > >    http://www.linux-m32r.org/
> > > >
> > > > So Linux/M32R is a brand new port?  Great, but in that case I have
> > > > some comments.  It looks like you based your GDB port on the
> > > > Linux/i386 stuff.  That's great, but you must realize that part of the
> > > > i386 code that you copied and modified is only there to support older
> > > > versions of Linux.  I hope it isn't necessary for Linux/M32R and would
> > > > like you to remove it before this gets committed.  I've got a few
> > > > other comments too.  Here we go...
> > > >
> > > >    diff -u -r1.144 configure.tgt=0A=
> > > >    --- configure.tgt 26 Jun 2004 10:06:34 -0000 1.144=0A=
> > > >    +++ configure.tgt 5 Aug 2004 04:48:33 -0000=0A=
> > > >    @@ -102,7 +103,10 @@
> > > >    ;;
> > > >     ia64*-*-*) gdb_target=ia64 ;;
> > > >
> > > >    -m32r-*-*) gdb_target=m32r ;;
> > > >    +m32r*-*-elf*) gdb_target=m32r ;;
> > > >    +m32r*-*-linux*) gdb_target=linux
> > > >    + build_gdbserver=3Dyes
> > > >    + ;;
> > > >
> > > >     m68hc11*-*-*|m6811*-*-*) gdb_target=m68hc11 ;;
> > > >
> > > > It might be a good idea to keep m32r-*-* as the generic M32R target.
> > > > Just put the m32r-*-* entry in front of it.
> > > >
> > > >    diff -N m32r-linux-nat.c
> > > >    --- /dev/null 1 Jan 1970 00:00:00 -0000
> > > >    +++ m32r-linux-nat.c 5 Aug 2004 04:48:33 -0000
> > > >    +/* The register sets used in GNU/Linux ELF core-dumps are identical
to
> > > >    +   the register sets in `struct user' that is used for a.out
> > > >    +   core-dumps, and is also used by `ptrace'.  The corresponding
types
> > > >    +   are `elf_gregset_t' for the general-purpose registers (with
> > > >    +   `elf_greg_t' the type of a single GP register) and
`elf_fpregset_t'
> > > >    +   for the floating-point registers.
> > > >    +
> > > >    +   Those types used to be available under the names `gregset_t' and
> > > >    +   `fpregset_t' too, and this file used those names in the past.
But
> > > >    +   those names are now used for the register sets used in the
> > > >    +   `mcontext_t' type, and have a different size and layout.  */
> > > >    +
> > > >    +/* Mapping between the general-purpose registers in `struct user'
> > > >    +   format and GDB's register array layout.  */
> > > >    +static int regmap[] = {
> > > >    +  4, 5, 6, 7, 0, 1, 2, 8,
> > > >    +  9, 10, 11, 12, 13, 24, 25, 23,
> > > >    +  19, 31, 26, 23, 20, 30, 16, 15,
> > > >    +  32
> > > >    +};
> > > >
> > > > Does Linux/M32R support a.out?  That would really surprise me.  So I
> > > > don't think you need this stuff.
> > > >
> > > >    +#define M32R_NUM_GREGS 25=0A=
> > > >
> > > > I think that one really belongs in m32r-tdep.h.
> > > >
> > > >    +/* Doee apply to the corresponding SET requests as well.  *
> > > >    +#define GETREGS_SUPPLIES(regno) (0 <=3D (regno) && (regno) <=
> > > M32R_NUM_GREGS)
> > > >    +#define GETFPREGS_SUPPLIES(regno) 0
> > > >
> > > > Does the M32R support floating point registers at all?  If not, you
> > > > could rip out all the FPREGS stuff.
> > > >
> > > >    +/* Does the current host support the GETREGS request?  */
> > > >    +int have_ptrace_getregs =
> > > >    +#ifdef HAVE_PTRACE_GETREGS
> > > >    +  1
> > > >    +#else
> > > >    +  0
> > > >    +#endif
> > > >    +  ;
> > > >
> > > > I really hope that all released M32R kernels support the
> > > > PTRACE_GETREGS request.  In that case this should not be necessary.
> > > >
> > > >
> > > >    +/* Support for the user struct.  */
> > > >    +
> > > >    +/* Return the address of register REGNUM.  BLOCKEND is the value of
> > > >    +   u.u_ar0, which should point to the registers.  */
> > > >    +
> > > >    +CORE_ADDR
> > > >    +register_u_addr (CORE_ADDR blockend, int regnum)
> > > >    +{
> > > >    +  return (blockend + 4 * regmap[regnum]);
> > > >    +}
> > > >    +
> > > >    +/* Return the size of the user struct.  */
> > > >    +
> > > >    +int
> > > >    +kernel_u_size (void)
> > > >    +{
> > > >    +  return (sizeof (struct user));
> > > >    +}
> > > >
> > > > The `user struct' is a wart from ancient UNIX.  There really is no
> > > > point in supporting it on modern versions of Linux.  If Linux/M32R is
> > > > ELF-only and fully supports PTRACE_GETREGS, this can go too.
> > > >
> > > >    +#ifdef HAVE_PTRACE_GETREGS
> > > >    +
> > > >    +/* Fetch all general-purpose registers from process/thread TID and
> > > >    +   store their values in GDB's register array.  */
> > > >    +
> > > >    +static void
> > > >    +fetch_regs (int tid)
> > > >    +{
> > > >    +  elf_gregset_t regs;
> > > >    +
> > > >
> > > > This code in #ifdef HAVE_PTRACE_GETREGS can be simplified considerably
> > > > if you assume that PTRACE_GETREGS works.  Move the code to
> > > > fetch_inferior_registers() and store_inferior_registers() below.
> > > >
> > > >    +/* Transferring arbitrary registers between GDB and inferior.  */
> > > >    +
> > > >    +/* Check if register REGNO in the child process is accessible.
> > > >    +   If we are accessing registers directly via the U area, only the
> > > >    +   general-purpose registers are available.
> > > >    +   All registers should be accessible if we have GETREGS support.
*/
> > > >    +
> > > >    +int
> > > >    +cannot_fetch_register (int regno)
> > > >    +{
> > > >    +  gdb_assert (regno >= 0 && regno < NUM_REGS);
> > > >    +  return (!have_ptrace_getregs && regmap[regno] == -1);
> > > >    +}
> > > >    +
> > > >    +int
> > > >    +cannot_store_register (int regno)
> > > >    +{
> > > >    +  gdb_assert (regno >= 0 && regno < NUM_REGS);
> > > >    +  return (!have_ptrace_getregs && regmap[regno] == -1);
> > > >    +}
> > > >
> > > > These functions should no longer be necessary.
> > > >
> > > >    +/* Accessing registers through the U area, one at a time.  */
> > > >    +
> > > >    +/* Fetch one register.  */
> > > >    +
> > > >    +static void
> > > >    +fetch_register (int regno)
> > > >    +{
> > > >    +  int tid;
> > > >    +  int val;
> > > >    +
> > > >    +  gdb_assert (!have_ptrace_getregs);
> > > >    +  if (cannot_fetch_register (regno))
> > > >    +    {
> > > >    +      regcache_raw_supply (current_regcache, regno, NULL);
> > > >    +      return;
> > > >    +    }=
> > > >
> > > > This can go too; no U area anymore :-).
> > > >
> > > >    diff -N config/m32r/linux.mh
> > > >    --- /dev/null 1 Jan 1970 00:00:00 -0000
> > > >    +++ config/m32r/linux.mh 5 Aug 2004 04:48:34 -0000
> > > >    @@ -0,0 +1,9 @@
> > > >    +# Host: M32R based machine running GNU/Linux
> > > >    +
> > > >    +NAT_FILE= nm-linux.h
> > > >    +NATDEPFILES= infptrace.o inftarg.o fork-child.o corelow.o \
> > > >    + core-aout.o m32r-linux-nat.o linux-proc.o gcore.o \
> > > >    + proc-service.o thread-db.o lin-lwp.o linux-nat.o
> > > >    +
> > > >    +LOADLIBES=3D -ldl -rdynamic
> > > >
> > > > core-aout.o shouldn't be necessary.
> > > >
> > > >    diff -N config/m32r/m32r.mh
> > > >
> > > > Please remove this file.
> > > >
> > > >    diff -u -r1.7 m32r.mt
> > > >    --- config/m32r/m32r.mt 16 Oct 2003 02:36:39 -0000 1.7
> > > >    +++ config/m32r/m32r.mt 5 Aug 2004 04:48:34 -0000
> > > >    @@ -1,4 +1,5 @@
> > > >     # Target: Renesas m32r processor
> > > >     TDEPFILES= m32r-tdep.o monitor.o m32r-rom.o dsrec.o
remote-m32r-sdi.o
> > > >    +TM_FILE= tm-m32r.h
> > > >     SIM_OBS = remote-sim.o
> > > >     SIM = ../sim/m32r/libsim.a
> > > >
> > > > This shouldn't be necessary.  See below.
> > > >
> > > >    diff -N config/m32r/nm-linux.h
> > > >    --- /dev/null 1 Jan 1970 00:00:00 -0000
> > > >    +++ config/m32r/nm-linux.h 5 Aug 2004 04:48:34 -0000
> > > >
> > > >    [snip]
> > > >
> > > >    +/* Support for the user area.  */
> > > >    +
> > > >    +/* Return the size of the user struct.  */
> > > >    +extern int kernel_u_size (void);
> > > >    +#define KERNEL_U_SIZE kernel_u_size()
> > > >    +
> > > >    +/* This is the amount to subtract from u.u_ar0
> > > >    +   to get the offset in the core file of the register values.  */
> > > >    +#define KERNEL_U_ADDR 0x0
> > > >    +
> > > >    +/* Offset of the registers within the user area.  */
> > > >    +#define U_REGS_OFFSET 0
> > > >    +
> > > >    +extern CORE_ADDR register_u_addr (CORE_ADDR blockend, int regnum);
> > > >    +#define REGISTER_U_ADDR(addr, blockend, regnum) \
> > > >    +  (addr) = register_u_addr (blockend, regnum)
> > > >
> > > > This user area stuff can go too.
> > > >
> > > >    diff -N config/m32r/tm-m32r.h
> > > >
> > > > This file contains nothing useful.  Please remove and adjust tm-linux.h.
> > > >
> > > > I hope you can make the changes.  The simplifications will make
> > > > maintaining the code much easier.
> >
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: m32r-linux.patch
Type: application/octet-stream
Size: 25600 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20040915/4dfbfec3/attachment.obj>


More information about the Gdb-patches mailing list