[rfa] Eliminate supply_gregset etc. from -tdep files

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[rfa] Eliminate supply_gregset etc. from -tdep files

Ulrich Weigand
Hello,

there are two -tdep files (m68k-tdep.c and mips-linux-tdep.c) that contain
implementations of the supply_greget family of functions.  This is incorrect;
those function should only ever be provided by native support files.

The definitions in m68k-tdep.c are protected by USE_PROC_FS, which is only
ever defined by three NM files -- none of which can be in scope when building
m68k-tdep.o, so this block can obviously be removed.   The definitions of the
regset functions in m68klinux-nat.c is protected by #ifndef USE_PROC_FS --
but at this point USE_PROC_FS cannot be defined, so the check can be removed.

The situation in mips-linux is a bit different.  The regset functions in
mips-linux-tdep.c are defined unconditionally, but those functions are
only ever required on native targets.  Thus they should really be moved
to mips-linux-nat.c.

Tested by building --target=mips-linux, --host=mips-linux, --target=m68k-linux
and --host=m68-linux debuggers.

OK to commit?

Bye,
Ulrich


ChangeLog:

        * m68klinux-nat.c: Remove #ifndef USE_PROC_FS check.
        * m68k-tdep.c: Remove code within #ifdef USE_PROC_FS.

        * mips-linux-nat.c: Include "gregset.h".
        (supply_gregset, fill_gregset, supply_fpregset, fill_fpregset): Move
        from mips-linux-tdep.c.  Change parameter type to gdb_gregset_t.
        * mips-linux-tdep.c (supply_gregset, fill_gregset, supply_fpregset,
        fill_fpregset): Move to mips-linux-nat.c.

        * Makefile.in (m68k-tdep.o, mips-linux-nat.o): Update dependencies.

diff -urNp gdb-orig/gdb/m68klinux-nat.c gdb-head/gdb/m68klinux-nat.c
--- gdb-orig/gdb/m68klinux-nat.c 2007-04-28 17:27:37.000000000 +0200
+++ gdb-head/gdb/m68klinux-nat.c 2007-04-29 16:37:03.063609450 +0200
@@ -51,6 +51,9 @@
 #include "floatformat.h"
 
 #include "target.h"
+
+/* Prototypes for supply_gregset etc. */
+#include "gregset.h"
 
 /* This table must line up with REGISTER_NAME in "m68k-tdep.c".  */
 static const int regmap[] =
@@ -238,20 +241,6 @@ old_store_inferior_registers (int regno)
    (elf_gregset_t *), unpack the register contents and supply
    them as gdb's idea of the current register values. */
 
-
-/* Note both m68k-tdep.c and m68klinux-nat.c contain definitions
-   for supply_gregset and supply_fpregset. The definitions
-   in m68k-tdep.c are valid if USE_PROC_FS is defined. Otherwise,
-   the definitions in m68klinux-nat.c will be used. This is a
-   bit of a hack. The supply_* routines do not belong in
-   *_tdep.c files. But, there are several lynx ports that currently
-   depend on these definitions. */
-
-#ifndef USE_PROC_FS
-
-/* Prototypes for supply_gregset etc. */
-#include "gregset.h"
-
 void
 supply_gregset (elf_gregset_t *gregsetp)
 {
@@ -414,8 +403,6 @@ static void fetch_fpregs (int tid) {}
 static void store_fpregs (int tid, int regno) {}
 
 #endif
-
-#endif
 
 /* Transferring arbitrary registers between GDB and inferior.  */
 
diff -urNp gdb-orig/gdb/m68k-tdep.c gdb-head/gdb/m68k-tdep.c
--- gdb-orig/gdb/m68k-tdep.c 2007-04-28 17:27:37.000000000 +0200
+++ gdb-head/gdb/m68k-tdep.c 2007-04-29 16:37:03.070608451 +0200
@@ -953,139 +953,6 @@ m68k_unwind_dummy_id (struct gdbarch *gd
   return frame_id_build (fp + 8, frame_pc_unwind (next_frame));
 }
 
-#ifdef USE_PROC_FS /* Target dependent support for /proc */
-
-#include <sys/procfs.h>
-
-/* Prototypes for supply_gregset etc. */
-#include "gregset.h"
-
-/*  The /proc interface divides the target machine's register set up into
-   two different sets, the general register set (gregset) and the floating
-   point register set (fpregset).  For each set, there is an ioctl to get
-   the current register set and another ioctl to set the current values.
-
-   The actual structure passed through the ioctl interface is, of course,
-   naturally machine dependent, and is different for each set of registers.
-   For the m68k for example, the general register set is typically defined
-   by:
-
-   typedef int gregset_t[18];
-
-   #define      R_D0    0
-   ...
-   #define      R_PS    17
-
-   and the floating point set by:
-
-   typedef      struct fpregset {
-   int  f_pcr;
-   int  f_psr;
-   int  f_fpiaddr;
-   int  f_fpregs[8][3];         (8 regs, 96 bits each)
-   } fpregset_t;
-
-   These routines provide the packing and unpacking of gregset_t and
-   fpregset_t formatted data.
-
- */
-
-/* Atari SVR4 has R_SR but not R_PS */
-
-#if !defined (R_PS) && defined (R_SR)
-#define R_PS R_SR
-#endif
-
-/*  Given a pointer to a general register set in /proc format (gregset_t *),
-   unpack the register contents and supply them as gdb's idea of the current
-   register values. */
-
-void
-supply_gregset (gregset_t *gregsetp)
-{
-  int regi;
-  greg_t *regp = (greg_t *) gregsetp;
-
-  for (regi = 0; regi < R_PC; regi++)
-    {
-      regcache_raw_supply (current_regcache, regi, (char *) (regp + regi));
-    }
-  regcache_raw_supply (current_regcache, PS_REGNUM, (char *) (regp + R_PS));
-  regcache_raw_supply (current_regcache, PC_REGNUM, (char *) (regp + R_PC));
-}
-
-void
-fill_gregset (gregset_t *gregsetp, int regno)
-{
-  int regi;
-  greg_t *regp = (greg_t *) gregsetp;
-
-  for (regi = 0; regi < R_PC; regi++)
-    {
-      if (regno == -1 || regno == regi)
- regcache_raw_collect (current_regcache, regi, regp + regi);
-    }
-  if (regno == -1 || regno == PS_REGNUM)
-    regcache_raw_collect (current_regcache, PS_REGNUM, regp + R_PS);
-  if (regno == -1 || regno == PC_REGNUM)
-    regcache_raw_collect (current_regcache, PC_REGNUM, regp + R_PC);
-}
-
-#if defined (FP0_REGNUM)
-
-/*  Given a pointer to a floating point register set in /proc format
-   (fpregset_t *), unpack the register contents and supply them as gdb's
-   idea of the current floating point register values. */
-
-void
-supply_fpregset (fpregset_t *fpregsetp)
-{
-  int regi;
-  char *from;
-
-  for (regi = FP0_REGNUM; regi < M68K_FPC_REGNUM; regi++)
-    {
-      from = (char *) &(fpregsetp->f_fpregs[regi - FP0_REGNUM][0]);
-      regcache_raw_supply (current_regcache, regi, from);
-    }
-  regcache_raw_supply (current_regcache, M68K_FPC_REGNUM,
-       (char *) &(fpregsetp->f_pcr));
-  regcache_raw_supply (current_regcache, M68K_FPS_REGNUM,
-       (char *) &(fpregsetp->f_psr));
-  regcache_raw_supply (current_regcache, M68K_FPI_REGNUM,
-       (char *) &(fpregsetp->f_fpiaddr));
-}
-
-/*  Given a pointer to a floating point register set in /proc format
-   (fpregset_t *), update the register specified by REGNO from gdb's idea
-   of the current floating point register set.  If REGNO is -1, update
-   them all. */
-
-void
-fill_fpregset (fpregset_t *fpregsetp, int regno)
-{
-  int regi;
-
-  for (regi = FP0_REGNUM; regi < M68K_FPC_REGNUM; regi++)
-    {
-      if (regno == -1 || regno == regi)
- regcache_raw_collect (current_regcache, regi,
-      &fpregsetp->f_fpregs[regi - FP0_REGNUM][0]);
-    }
-  if (regno == -1 || regno == M68K_FPC_REGNUM)
-    regcache_raw_collect (current_regcache, M68K_FPC_REGNUM,
-  &fpregsetp->f_pcr);
-  if (regno == -1 || regno == M68K_FPS_REGNUM)
-    regcache_raw_collect (current_regcache, M68K_FPS_REGNUM,
-  &fpregsetp->f_psr);
-  if (regno == -1 || regno == M68K_FPI_REGNUM)
-    regcache_raw_collect (current_regcache, M68K_FPI_REGNUM,
-  &fpregsetp->f_fpiaddr);
-}
-
-#endif /* defined (FP0_REGNUM) */
-
-#endif /* USE_PROC_FS */
 
 /* Figure out where the longjmp will land.  Slurp the args out of the stack.
    We expect the first arg to be a pointer to the jmp_buf structure from which
diff -urNp gdb-orig/gdb/Makefile.in gdb-head/gdb/Makefile.in
--- gdb-orig/gdb/Makefile.in 2007-04-29 16:37:09.274653887 +0200
+++ gdb-head/gdb/Makefile.in 2007-04-29 16:37:03.119601455 +0200
@@ -2291,7 +2291,7 @@ m68k-tdep.o: m68k-tdep.c $(defs_h) $(dwa
  $(frame_base_h) $(frame_unwind_h) $(gdbtypes_h) $(symtab_h) \
  $(gdbcore_h) $(value_h) $(gdb_string_h) $(gdb_assert_h) \
  $(inferior_h) $(regcache_h) $(arch_utils_h) $(osabi_h) $(dis_asm_h) \
- $(m68k_tdep_h) $(gregset_h)
+ $(m68k_tdep_h)
 m88kbsd-nat.o: m88kbsd-nat.c $(defs_h) $(inferior_h) $(regcache_h) \
  $(target_h) $(m88k_tdep_h) $(inf_ptrace_h)
 m88k-tdep.o: m88k-tdep.c $(defs_h) $(arch_utils_h) $(dis_asm_h) $(frame_h) \
@@ -2341,8 +2341,8 @@ mips64obsd-tdep.o: mips64obsd-tdep.c $(d
  $(gdb_string_h) $(mips_tdep_h) $(solib_svr4_h)
 mips-irix-tdep.o: mips-irix-tdep.c $(defs_h) $(osabi_h) $(elf_bfd_h)
 mips-linux-nat.o: mips-linux-nat.c $(defs_h) $(mips_tdep_h) $(target_h) \
- $(linux_nat_h) $(gdb_proc_service_h) $(mips_linux_tdep_h) \
- $(inferior_h)
+ $(linux_nat_h) $(gdb_proc_service_h) $(gregset_h) \
+ $(mips_linux_tdep_h) $(inferior_h)
 mips-linux-tdep.o: mips-linux-tdep.c $(defs_h) $(gdbcore_h) $(target_h) \
  $(solib_svr4_h) $(osabi_h) $(mips_tdep_h) $(gdb_string_h) \
  $(gdb_assert_h) $(frame_h) $(regcache_h) $(trad_frame_h) \
diff -urNp gdb-orig/gdb/mips-linux-nat.c gdb-head/gdb/mips-linux-nat.c
--- gdb-orig/gdb/mips-linux-nat.c 2007-04-29 16:37:09.315648034 +0200
+++ gdb-head/gdb/mips-linux-nat.c 2007-04-29 16:36:57.120527610 +0200
@@ -28,6 +28,7 @@
 #include "mips-linux-tdep.h"
 
 #include "gdb_proc_service.h"
+#include "gregset.h"
 
 #include <sys/ptrace.h>
 
@@ -172,6 +173,45 @@ ps_get_thread_area (const struct ps_proc
   return PS_OK;
 }
 
+/* Wrapper functions.  These are only used by libthread_db.  */
+
+void
+supply_gregset (gdb_gregset_t *gregsetp)
+{
+  if (mips_isa_regsize (current_gdbarch) == 4)
+    mips_supply_gregset ((void *) gregsetp);
+  else
+    mips64_supply_gregset ((void *) gregsetp);
+}
+
+void
+fill_gregset (gdb_gregset_t *gregsetp, int regno)
+{
+  if (mips_isa_regsize (current_gdbarch) == 4)
+    mips_fill_gregset ((void *) gregsetp, regno);
+  else
+    mips64_fill_gregset ((void *) gregsetp, regno);
+}
+
+void
+supply_fpregset (gdb_fpregset_t *fpregsetp)
+{
+  if (mips_isa_regsize (current_gdbarch) == 4)
+    mips_supply_fpregset ((void *) fpregsetp);
+  else
+    mips64_supply_fpregset ((void *) fpregsetp);
+}
+
+void
+fill_fpregset (gdb_fpregset_t *fpregsetp, int regno)
+{
+  if (mips_isa_regsize (current_gdbarch) == 4)
+    mips_fill_fpregset ((void *) fpregsetp, regno);
+  else
+    mips64_fill_fpregset ((void *) fpregsetp, regno);
+}
+
+
 /* Fetch REGNO (or all registers if REGNO == -1) from the target
    using PTRACE_GETREGS et al.  */
 
diff -urNp gdb-orig/gdb/mips-linux-tdep.c gdb-head/gdb/mips-linux-tdep.c
--- gdb-orig/gdb/mips-linux-tdep.c 2007-04-29 16:37:09.321647177 +0200
+++ gdb-head/gdb/mips-linux-tdep.c 2007-04-29 16:36:57.128526467 +0200
@@ -979,48 +979,6 @@ mips_linux_n32n64_sigframe_init (const s
      func));
 }
 
-/* Wrapper functions.  These are only used by libthread_db.  */
-
-void
-supply_gregset (mips_elf_gregset_t *gregsetp)
-{
-  if (mips_isa_regsize (current_gdbarch) == 4)
-    mips_supply_gregset (gregsetp);
-  else
-    mips64_supply_gregset ((void *) gregsetp);
-}
-
-void
-fill_gregset (mips_elf_gregset_t *gregsetp, int regno)
-{
-  if (mips_isa_regsize (current_gdbarch) == 4)
-    mips_fill_gregset (gregsetp, regno);
-  else
-    mips64_fill_gregset ((void *) gregsetp, regno);
-}
-
-/* Likewise, unpack an elf_fpregset_t.  */
-
-void
-supply_fpregset (mips_elf_fpregset_t *fpregsetp)
-{
-  if (mips_isa_regsize (current_gdbarch) == 4)
-    mips_supply_fpregset (fpregsetp);
-  else
-    mips64_supply_fpregset ((void *) fpregsetp);
-}
-
-/* Likewise, pack one or all floating point registers into an
-   elf_fpregset_t.  */
-
-void
-fill_fpregset (mips_elf_fpregset_t *fpregsetp, int regno)
-{
-  if (mips_isa_regsize (current_gdbarch) == 4)
-    mips_fill_fpregset (fpregsetp, regno);
-  else
-    mips64_fill_fpregset ((void *) fpregsetp, regno);
-}
 
 /* Initialize one of the GNU/Linux OS ABIs.  */
 
--
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  [hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [rfa] Eliminate supply_gregset etc. from -tdep files

Daniel Jacobowitz-2
On Sun, Apr 29, 2007 at 06:18:24PM +0200, Ulrich Weigand wrote:
> Hello,
>
> there are two -tdep files (m68k-tdep.c and mips-linux-tdep.c) that contain
> implementations of the supply_greget family of functions.  This is incorrect;
> those function should only ever be provided by native support files.

"Incorrect" may be a bit strong.  core-regset.c needs them to link; at
one point, that was the right way to implement core file support.  I
don't remember how the history worked out for mips, since it doesn't
look like the version in the FSF repository ever used core-regset.o,
but I think that was the original motivation.

Anyway, now there are no targets that use core-regset.o in .mt files,
and no good reason to do so.  So now it's incorrect :-)  This patch is
OK.

--
Daniel Jacobowitz
CodeSourcery
Reply | Threaded
Open this post in threaded view
|

Re: [rfa] Eliminate supply_gregset etc. from -tdep files

Ulrich Weigand
Daniel Jacobowitz wrote:

> On Sun, Apr 29, 2007 at 06:18:24PM +0200, Ulrich Weigand wrote:
> > there are two -tdep files (m68k-tdep.c and mips-linux-tdep.c) that contain
> > implementations of the supply_greget family of functions.  This is incorrect;
> > those function should only ever be provided by native support files.
>
> "Incorrect" may be a bit strong.  core-regset.c needs them to link; at
> one point, that was the right way to implement core file support.  I
> don't remember how the history worked out for mips, since it doesn't
> look like the version in the FSF repository ever used core-regset.o,
> but I think that was the original motivation.

Right, but I thought core-regset.o was also supposed to used only as a
native support file, never in an .mt file.  Was this ever done in the
past?

Anyway, the reason why I'd like to get of the use in -tdep files is that
it would prevent building GDB with multiple architecture targets built-in
at the same time, if multiple tdep files define different copies of
routines with the same name ...

(B.t.w. it doesn't look too hard to get completely rid of core-regset.o
at this time, but it would need someone to test on i386-gnu, alpha-freebsd,
sparc-solaris ...   I can take care of ia64-linux.)

> Anyway, now there are no targets that use core-regset.o in .mt files,
> and no good reason to do so.  So now it's incorrect :-)  This patch is
> OK.

Thanks, committed.

Bye,
Ulrich

--
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  [hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [rfa] Eliminate supply_gregset etc. from -tdep files

Daniel Jacobowitz-2
On Sun, Apr 29, 2007 at 09:52:18PM +0200, Ulrich Weigand wrote:
> Right, but I thought core-regset.o was also supposed to used only as a
> native support file, never in an .mt file.  Was this ever done in the
> past?

You're probably right that it wasn't supposed to, but anyway I'm
pretty sure I did it.  That may only have been local patches, though.

--
Daniel Jacobowitz
CodeSourcery