Patch for i386-linux-nat.c

[Mark Kettenis <kettenis at wins dot uva dot nl>]

Hi Jim,

Here is a patch for i386-linux-nat.c.  The main reason for the patch
is to change gregset_t and fpregset_t to elf_gregset_t and
elf_fpregset_t.  There were two different definitions of gregset_t and
fpregset_t on Linux, which were quite different from each other.  The
next glibc (version 2.1.3) will finaly put an end to this braindamage,
but this will break GDB.

While hacking on the file I noticed that there were quite a number of
inconsistencies in the comments and names of variables.  I've gone
over all the comments and rewritten most of them to describe in a more
precise way what they do and to use more consistent terminology.  I
know gratitious reformatting is frowned upon, but I honestly think
that this patch makes the file a lot easier to read.

Mark

PS Before christmas I sent a message about a fix to supply_fpregset to
mask of the reserved bits of the FP control registers:

   http://sourceware.cygnus.com/ml/gdb-patches/1999-q4/msg00375.html

You promised to look at it, but probably forgot about it.  Of course
the patch included in there interferes with the one here.  I'll create
a new patch when after this patch has been applied.


2000-01-29  Mark Kettenis  <kettenis@gnu.org>

	* i386-linux-nat.c: Use elf_gregset_t and elf_fpregset_t instead
	of gregset_t and fpregset_t.  Those are the only names that are
	guaranteed to specify the right types for all supported Linux
	systems out there.
	Various doc fixes and gratitious local variable renames, all in an
	attempt to stress similarities between the code and unify the
	terminology used.
	(regmap): Remove trailing comma.
	(FPREG_ADDR): Renamed from FPREGSET_T_FPREG_ADDR.
	(convert_to_gregset): Make static.  Remove GDB_REGS argument.  It
	is unnecessary and wasn't used anyway.  All callers changed.
	(convert_to_fpregset, convert_to_xfpregset): Likewise.
	

Index: gdb/i386-linux-nat.c
===================================================================
RCS file: /cvs/gdb/gdb/gdb/i386-linux-nat.c,v
retrieving revision 1.1.1.8
diff -u -r1.1.1.8 i386-linux-nat.c
--- gdb/i386-linux-nat.c	2000/01/18 00:54:14	1.1.1.8
+++ gdb/i386-linux-nat.c	2000/01/29 22:34:40
@@ -20,7 +20,7 @@
 #include "inferior.h"
 #include "gdbcore.h"
 
-/* For i386_linux_skip_solib_resolver */
+/* For i386_linux_skip_solib_resolver.  */
 #include "symtab.h"
 #include "frame.h"
 #include "symfile.h"
@@ -34,27 +34,40 @@
 #include <sys/reg.h>
 #endif
 
-/*
- * Some systems (Linux) may have threads implemented as pseudo-processes, 
- * in which case we may be tracing more than one process at a time.
- * In that case, inferior_pid will contain the main process ID and the 
- * individual thread (process) id mashed together.  These macros are 
- * used to separate them out.  The definitions may be overridden in tm.h
- */
+/* On Linux, threads are implemented as pseudo-processes, in which
+   case we may be tracing more than one process at a time.  In that
+   case, inferior_pid will contain the main process ID and the
+   individual thread (process) ID mashed together.  These macros are
+   used to separate them out.  These definitions should be overridden
+   if thread support is included.  */
 
 #if !defined (PIDGET)	/* Default definition for PIDGET/TIDGET.  */
-#define PIDGET(PID)	PID
-#define TIDGET(PID)	0
+# define PIDGET(PID)	PID
+# define TIDGET(PID)	0
 #endif
 
-/* This is a duplicate of the table in i386-xdep.c. */
 
+/* The register sets used in Linux ELF core-dumps are identical to the
+   register sets in `struct user' that was used for in a.out
+   core-dumps, that 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 file layout.  */
 static int regmap[] = 
 {
   EAX, ECX, EDX, EBX,
   UESP, EBP, ESI, EDI,
   EIP, EFL, CS, SS,
-  DS, ES, FS, GS,
+  DS, ES, FS, GS
 };
 
 
@@ -92,40 +105,31 @@
 
 
 
-/* Transfering the general registers between GDB, inferiors and core files.  */
+/* Transfering the general-purpose registers between GDB, inferiors
+   and core files.  */
 
-/* Given a pointer to a general register set in struct user format
-   (gregset_t *), unpack the register contents and supply them as
-   gdb's idea of the current register values. */
+/* Fill GDB's register file with the genereal-purpose register values
+   in *GREGSETP.  */
 void
-supply_gregset (gregsetp)
-     gregset_t *gregsetp;
+supply_gregset (elf_gregset_t *gregsetp)
 {
-  register int regi;
-  register greg_t *regp = (greg_t *) gregsetp;
+  elf_greg_t *regp = (elf_greg_t *) gregsetp;
+  int regi;
 
   for (regi = 0; regi < NUM_GREGS; regi++)
-    {
-      supply_register (regi, (char *) (regp + regmap[regi]));
-    }
+    supply_register (regi, (char *) (regp + regmap[regi]));
 }
 
 
-/* Fill in a gregset_t object with selected data from a gdb-format
-   register file.
-   - GREGSETP points to the gregset_t object to be filled.
-   - GDB_REGS points to the GDB-style register file providing the data.
-   - VALID is an array indicating which registers in GDB_REGS are
-     valid; the parts of *GREGSETP that would hold registers marked
-     invalid in GDB_REGS are left unchanged.  If VALID is zero, all
-     registers are assumed to be valid.  */
-void
-convert_to_gregset (gregset_t *gregsetp,
-		    char *gdb_regs,
-		    signed char *valid)
+/* Convert the valid general-purpose register values in GDB's register
+   file to `struct user' format and store them in *GREGSETP.  The
+   array VALID indicates which register values are valid.  If VALID is
+   NULL, all registers are assumed to be valid.  */
+static void
+convert_to_gregset (elf_gregset_t *gregsetp, signed char *valid)
 {
+  elf_greg_t *regp = (elf_greg_t *) gregsetp;
   int regi;
-  register greg_t *regp = (greg_t *) gregsetp;
 
   for (regi = 0; regi < NUM_GREGS; regi++)
     if (! valid || valid[regi])
@@ -133,64 +137,70 @@
 }
 
 
-/* Store GDB's value for REGNO in *GREGSETP.  If REGNO is -1, do all
-   of them.  */
+/* Fill register REGNO (if it is a general-purpose register) in
+   *GREGSETPS with the value in GDB's register file.  If REGNO is -1,
+   do this for all registers.  */
 void
-fill_gregset (gregset_t *gregsetp,
-	      int regno)
+fill_gregset (elf_gregset_t *gregsetp, int regno)
 {
   if (regno == -1)
-    convert_to_gregset (gregsetp, registers, 0);
-  else if (regno >= 0 && regno < NUM_GREGS)
     {
+      convert_to_gregset (gregsetp, NULL);
+      return;
+    }
+
+  if (GETREGS_SUPPLIES (regno))
+    {
       signed char valid[NUM_GREGS];
+
       memset (valid, 0, sizeof (valid));
       valid[regno] = 1;
-      convert_to_gregset (gregsetp, registers, valid);
+
+      convert_to_gregset (gregsetp, valid);
     }
 }
 
 
-/* Read the general registers from the process, and store them
-   in registers[].  */
+/* Fetch all general-purpose registers from process/thread TID and
+   store their values in GDB's register file.  */
 static void
 fetch_regs (int tid)
 {
-  int ret, regno;
-  gregset_t buf;
+  elf_gregset_t regs;
+  int ret;
 
-  ret = ptrace (PTRACE_GETREGS, tid, 0, (int) &buf);
+  ret = ptrace (PTRACE_GETREGS, tid, 0, (int) &regs);
   if (ret < 0)
     {
-      warning ("Couldn't get registers");
+      warning ("Couldn't get registers.");
       return;
     }
 
-  supply_gregset (&buf);
+  supply_gregset (&regs);
 }
 
 
-/* Set the inferior's general registers to the values in registers[]
-   --- but only those registers marked as valid.  */
+/* Store all valid general-purpose registers in GDB's register file
+   into the process/thread specified by TID.  */
 static void
 store_regs (int tid)
 {
-  int ret, regno;
-  gregset_t buf;
+  elf_gregset_t regs;
+  int ret;
 
-  ret = ptrace (PTRACE_GETREGS, tid, 0, (int) &buf);
+  ret = ptrace (PTRACE_GETREGS, tid, 0, (int) &regs);
   if (ret < 0)
     {
-      warning ("Couldn't get registers");
+      warning ("Couldn't get registers.");
       return;
     }
 
-  convert_to_gregset (&buf, registers, register_valid);
+  convert_to_gregset (&regs, register_valid);
 
-  ret = ptrace (PTRACE_SETREGS, tid, 0, (int)buf);
+  ret = ptrace (PTRACE_SETREGS, tid, 0, (int) &regs);
   if (ret < 0)
     {
-      warning ("Couldn't write registers");
+      warning ("Couldn't write registers.");
       return;
     }
 }
@@ -199,20 +209,19 @@
 
 /* Transfering floating-point registers between GDB, inferiors and cores.  */
 
-/* What is the address of st(N) within the fpregset_t structure F?  */
-#define FPREGSET_T_FPREG_ADDR(f, n) \
-  ((char *) &(f)->st_space + (n) * 10)
+/* What is the address of st(N) within the floating-point register set F?  */
+#define FPREG_ADDR(f, n) ((char *) &(f)->st_space + (n) * 10)
 
 /* Fill GDB's register file with the floating-point register values in
    *FPREGSETP.  */
 void 
-supply_fpregset (fpregset_t *fpregsetp)
+supply_fpregset (elf_fpregset_t *fpregsetp)
 {
-  int i;
+  int reg;
 
   /* Supply the floating-point registers.  */
-  for (i = 0; i < 8; i++)
-    supply_register (FP0_REGNUM + i, FPREGSET_T_FPREG_ADDR (fpregsetp, i));
+  for (reg = 0; reg < 8; reg++)
+    supply_register (FP0_REGNUM + reg, FPREG_ADDR (fpregsetp, reg));
 
   supply_register (FCTRL_REGNUM, (char *) &fpregsetp->cwd);
   supply_register (FSTAT_REGNUM, (char *) &fpregsetp->swd);
@@ -234,27 +243,21 @@
 }
 
 
-/* Fill in an fpregset_t structure with selected data from a
-   gdb-format register file.
-   - FPREGSETP points to the structure to be filled. 
-   - GDB_REGS points to the GDB-style register file providing the data.
-   - VALID is an array indicating which registers in GDB_REGS are
-     valid; the parts of *FPREGSETP that would hold registers marked
-     invalid in GDB_REGS are left unchanged.  If VALID is zero, all
-     registers are assumed to be valid.  */
-void
-convert_to_fpregset (fpregset_t *fpregsetp,
-		     char *gdb_regs,
-		     signed char *valid)
+/* Convert the valid floating-point register values in GDB's register
+   file to `struct user' format and store them in *FPREGSETP.  The
+   array VALID indicates which register values are valid.  If VALID is
+   NULL, all registers are assumed to be valid.  */
+static void
+convert_to_fpregset (elf_fpregset_t *fpregsetp, signed char *valid)
 {
-  int i;
+  int reg;
 
   /* Fill in the floating-point registers.  */
-  for (i = 0; i < 8; i++)
-    if (!valid || valid[i])
-      memcpy (FPREGSET_T_FPREG_ADDR (fpregsetp, i),
-	      &registers[REGISTER_BYTE (FP0_REGNUM + i)],
-	      REGISTER_RAW_SIZE(FP0_REGNUM + i));
+  for (reg = 0; reg < 8; reg++)
+    if (!valid || valid[reg])
+      memcpy (FPREG_ADDR (fpregsetp, reg),
+	      &registers[REGISTER_BYTE (FP0_REGNUM + reg)],
+	      REGISTER_RAW_SIZE(FP0_REGNUM + reg));
 
 #define fill(MEMBER, REGNO)						\
   if (! valid || valid[(REGNO)])					\
@@ -283,68 +286,78 @@
 }
 
 
-/* Given a pointer to a floating point register set in (fpregset_t *)
-   format, update all of the registers from gdb's idea of the current
-   floating point register set.  */
-
+/* Fill register REGNO (if it is a floating-point register) in
+   *FPREGSETP with the value in GDB's register file.  If REGNO is -1,
+   do this for all registers.  */
 void
-fill_fpregset (fpregset_t *fpregsetp,
-	       int regno)
+fill_fpregset (elf_fpregset_t *fpregsetp, int regno)
 {
-  convert_to_fpregset (fpregsetp, registers, 0);
+  if (regno == -1)
+    {
+      convert_to_fpregset (fpregsetp, NULL);
+      return;
+    }
+
+  if (GETFPREGS_SUPPLIES(regno))
+    {
+      signed char valid[MAX_NUM_REGS];
+      
+      memset (valid, 0, sizeof (valid));
+      valid[regno] = 1;
+	      
+      convert_to_fpregset (fpregsetp, valid);
+    }
 }
 
 
-/* Get the whole floating point state of the process and store the
-   floating point stack into registers[].  */
+/* Fetch all floating-point registers from process/thread TID and store
+   thier values in GDB's register file.  */
 static void
 fetch_fpregs (int tid)
 {
-  int ret, regno;
-  fpregset_t buf;
+  elf_fpregset_t fpregs;
+  int ret;
 
-  ret = ptrace (PTRACE_GETFPREGS, tid, 0, (int) &buf);
+  ret = ptrace (PTRACE_GETFPREGS, tid, 0, (int) &fpregs);
   if (ret < 0)
     {
-      warning ("Couldn't get floating point status");
+      warning ("Couldn't get floating point status.");
       return;
     }
 
-  /* ptrace fills an fpregset_t, so we can use the same function we do
-     for core files.  */
-  supply_fpregset (&buf);
+  supply_fpregset (&fpregs);
 }
 
 
-/* Set the inferior's floating-point registers to the values in
-   registers[] --- but only those registers marked valid.  */
+/* Store all valid floating-point registers in GDB's register file
+   into the process/thread specified by TID.  */
 static void
 store_fpregs (int tid)
 {
+  elf_fpregset_t fpregs;
   int ret;
-  fpregset_t buf;
 
-  ret = ptrace (PTRACE_GETFPREGS, tid, 0, (int) &buf);
+  ret = ptrace (PTRACE_GETFPREGS, tid, 0, (int) &fpregs);
   if (ret < 0)
     {
-      warning ("Couldn't get floating point status");
+      warning ("Couldn't get floating point status.");
       return;
     }
 
-  convert_to_fpregset (&buf, registers, register_valid);
+  convert_to_fpregset (&fpregs, register_valid);
 
-  ret = ptrace (PTRACE_SETFPREGS, tid, 0, (int) &buf);
+  ret = ptrace (PTRACE_SETFPREGS, tid, 0, (int) &fpregs);
   if (ret < 0)
     {
-      warning ("Couldn't write floating point status");
+      warning ("Couldn't write floating point status.");
       return;
     }
 }
 
+
 
 /* Transfering floating-point and SSE registers to and from GDB.  */
 
-
 /* PTRACE_GETXFPREGS is a Cygnus invention, since we wrote our own
    Linux kernel patch for SSE support.  That patch may or may not
    actually make it into the official distribution.  If you find that
@@ -353,6 +366,9 @@
    you can delete this code.  */
 
 #ifdef HAVE_PTRACE_GETXFPREGS
+
+/* Fill GDB's register file with the floating-point and SSE register
+   values in *XFPREGS.  */
 static void
 supply_xfpregset (struct user_xfpregs_struct *xfpregs)
 {
@@ -389,9 +405,12 @@
 }
 
 
+/* Convert the valid floating-point and SSE registers in GDB's
+   register file to `struct user' format and store them in *XFPREGS.
+   The array VALID indicates which registers are valid.  If VALID is
+   NULL, all registers are assumed to be valid.  */
 static void
 convert_to_xfpregset (struct user_xfpregs_struct *xfpregs,
-		      char *gdb_regs,
 		      signed char *valid)
 {
   int reg;
@@ -437,13 +456,14 @@
 }
 
 
-/* Make a PTRACE_GETXFPREGS request, and supply all the register
-   values that yields to GDB.  */
+/* Fetch all registers covered by the PTRACE_SETXFPREGS request from
+   process/thread TID and store their values in GDB's register file.
+   Return non-zero if successful, zero otherwise.  */
 static int
 fetch_xfpregs (int tid)
 {
-  int ret;
   struct user_xfpregs_struct xfpregs;
+  int ret;
 
   if (! have_ptrace_getxfpregs) 
     return 0;
@@ -457,7 +477,7 @@
 	  return 0;
 	}
 
-      warning ("couldn't read floating-point and SSE registers.");
+      warning ("Couldn't read floating-point and SSE registers.");
       return 0;
     }
 
@@ -466,13 +486,14 @@
 }
 
 
-/* Send all the valid register values in GDB's register file covered
-   by the PTRACE_SETXFPREGS request to the inferior.  */
+/* Store all valid registers in GDB's register file covered by the
+   PTRACE_SETXFPREGS request into the process/thread specified by TID.
+   Return non-zero if successful, zero otherwise.  */
 static int
 store_xfpregs (int tid)
 {
-  int ret;
   struct user_xfpregs_struct xfpregs;
+  int ret;
 
   if (! have_ptrace_getxfpregs)
     return 0;
@@ -486,11 +507,11 @@
 	  return 0;
 	}
 
-      warning ("couldn't read floating-point and SSE registers.");
+      warning ("Couldn't read floating-point and SSE registers.");
       return 0;
     }
 
-  convert_to_xfpregset (&xfpregs, registers, register_valid);
+  convert_to_xfpregset (&xfpregs, register_valid);
 
   if (ptrace (PTRACE_SETXFPREGS, tid, 0, &xfpregs) < 0)
     {
@@ -530,26 +551,26 @@
 
 #endif
 
+
 
 /* Transferring arbitrary registers between GDB and inferior.  */
-
-/* Fetch registers from the child process.
-   Fetch all if regno == -1, otherwise fetch all ordinary
-   registers or all floating point registers depending
-   upon the value of regno. */
 
+/* Fetch register REGNO from the child process.  If REGNO is -1, do
+   this for all registers (including the floating point and SSE
+   registers).  */
 void
 fetch_inferior_registers (int regno)
 {
-  /* linux lwp id's are process id's */
   int tid;
 
+  /* Linux LWP ID's are process ID's.  */
   if ((tid = TIDGET (inferior_pid)) == 0)
-    tid = inferior_pid;		/* not a threaded program */
+    tid = inferior_pid;		/* Not a threaded program.  */
 
-  /* Use the xfpregs requests whenever possible, since they transfer
-     more registers in one system call, and we'll cache the results.
-     But remember that fetch_xfpregs can fail, and return zero.  */
+  /* Use the PTRACE_GETXFPREGS request whenever possible, since it
+     transfers more registers in one system call, and we'll cache the
+     results.  But remember that fetch_xfpregs can fail, and return
+     zero.  */
   if (regno == -1)
     {
       fetch_regs (tid);
@@ -586,24 +607,20 @@
 }
 
 
-/* Store our register values back into the inferior.
-   If REGNO is -1, do this for all registers.
-   Otherwise, REGNO specifies which register, which
-   then determines whether we store all ordinary
-   registers or all of the floating point registers. */
-
+/* Store register REGNO back into the child process.  If REGNO is -1,
+   do this for all registers (including the floating point and SSE
+   registers).  */
 void
-store_inferior_registers (regno)
-     int regno;
+store_inferior_registers (int regno)
 {
-  /* linux lwp id's are process id's */
   int tid;
 
+  /* Linux LWP ID's are process ID's.  */
   if ((tid = TIDGET (inferior_pid)) == 0)
-    tid = inferior_pid;		/* not a threaded program */
+    tid = inferior_pid;		/* Not a threaded program.  */
 
-  /* Use the xfpregs requests whenever possible, since they transfer
-     more registers in one system call.  But remember that
+  /* Use the PTRACE_SETXFPREGS requests whenever possibl, since it
+     transfers more registers in one system call.  But remember that
      store_xfpregs can fail, and return zero.  */
   if (regno == -1)
     {
@@ -626,7 +643,8 @@
 	return;
 
       /* Either our processor or our kernel doesn't support the SSE
-	 registers, so just write the FP registers in the traditional way.  */
+	 registers, so just write the FP registers in the traditional
+	 way.  */
       store_fpregs (tid);
       return;
     }
@@ -651,26 +669,25 @@
    register contents.  CORE_REG_SIZE is its size.
 
    WHICH says which register set corelow suspects this is:
-   0 --- the general register set, in gregset format
-   2 --- the floating-point register set, in fpregset format
-   3 --- the extended floating-point register set, in struct
-         user_xfpregs_struct format
+     0 --- the general-purpose register set, in elf_gregset_t format
+     2 --- the floating-point register set, in elf_fpregset_t format
+     3 --- the extended floating-point register set, in struct
+           user_xfpregs_struct format
 
-   DUMMY isn't used on Linux.  */
+   REG_ADDR isn't used on Linux.  */
 static void
 i386_linux_fetch_core_registers (char *core_reg_sect,
 				 unsigned core_reg_size,
-				 int which,
-				 CORE_ADDR dummy)
+				 int which, CORE_ADDR reg_addr)
 {
-  gregset_t gregset;
-  fpregset_t fpregset;
+  elf_gregset_t gregset;
+  elf_fpregset_t fpregset;
 
   switch (which)
     {
     case 0:
       if (core_reg_size != sizeof (gregset))
-	warning ("wrong size gregset struct in core file");
+	warning ("Wrong size gregset in core file.");
       else
 	{
 	  memcpy (&gregset, core_reg_sect, sizeof (gregset));
@@ -680,7 +697,7 @@
 
     case 2:
       if (core_reg_size != sizeof (fpregset))
-	warning ("wrong size fpregset struct in core file");
+	warning ("Wrong size fpregset in core file.");
       else
 	{
 	  memcpy (&fpregset, core_reg_sect, sizeof (fpregset));
@@ -691,9 +708,10 @@
 #ifdef HAVE_PTRACE_GETXFPREGS
       {
 	struct user_xfpregs_struct xfpregset;
+
       case 3:
-	if (core_reg_size != sizeof (struct user_xfpregs_struct))
-	  warning ("wrong size user_xfpregs_struct in core file");
+	if (core_reg_size != sizeof (xfpregset))
+	  warning ("Wrong size user_xfpregs_struct in core file.");
 	else
 	  {
 	    memcpy (&xfpregset, core_reg_sect, sizeof (xfpregset));
@@ -720,6 +738,7 @@
   i386_linux_fetch_core_registers,	/* core_read_registers */
   NULL					/* next */
 };
+
 
 
 /* Calling functions in shared libraries.  */