ppc32 debugging ppc64, part 1

ppc32 debugging ppc64, part 1

[Richard Henderson]

This is just enough modifications to not immediately fail 
debugging a 64-bit process from a 32-bit debugger.

We don't get very far in the actual debugging:

(gdb) b main
warning: Breakpoint address adjusted from 0x10093aa0 to 0x10000370.
Breakpoint 1 at 0x10000370
(gdb) run
Starting program: /home/rth/work/gcc/bld-binu64/gdb/z
warning: Breakpoint address adjusted from 0x10093a40 to 0x100001c0.
warning: Breakpoint 1 address previously adjusted from 0x10093aa0 to 0x10000370.
Breakpoint 1, 0x0000000010000370 in ?? ()
(gdb) ppc
0x10000370 <__libc_tsd_CTYPE_B+268436264>:      std     r31,-8(r1)

But this is better than the original:

Starting program: /home/rth/work/gcc/bld-binu64/gdb/z
warning: Breakpoint address adjusted from 0x10093a40 to 0x100001c0.
Failed to read a valid object file image from memory.

Program received signal SIGTRAP, Trace/breakpoint trap.
0x100001c00002d932 in ?? ()


Comments?


r~


	* ppc-linux-nat.c (PTRACE_XFER_TYPE): Set to long.
	(ppc_register_u_addr): Compute wordsize correctly for 64-bit target.
	(ptrace_read_u, ptrace_write_u): New.
	(fetch_register): Simplify.  Use PPC_PTRACE_PEEKUSR_3264 when needed.
	(store_register): Similarly with PPC_PTRACE_POKEUSR_3264.
	(default_xfer_partial, ppc32_linux_xfer_partial): New.
	(_initialize_ppc_linux_nat): Use them.

Index: ppc-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-nat.c,v
retrieving revision 1.55
diff -u -p -c -r1.55 ppc-linux-nat.c
*** ppc-linux-nat.c	10 Sep 2005 18:11:04 -0000	1.55
--- ppc-linux-nat.c	12 Sep 2005 12:37:40 -0000
***************
*** 39,44 ****
--- 39,45 ----
  #include <fcntl.h>
  #include <sys/procfs.h>
  #include <sys/ptrace.h>
+ #include <sys/syscall.h>
  
  /* Prototypes for supply_gregset etc. */
  #include "gregset.h"
***************
*** 51,59 ****
  #define PT_WRITE_U PTRACE_POKEUSR
  #endif
  
- /* Default the type of the ptrace transfer to int.  */
  #ifndef PTRACE_XFER_TYPE
! #define PTRACE_XFER_TYPE int
  #endif
  
  /* Glibc's headers don't define PTRACE_GETVRREGS so we cannot use a
--- 52,59 ----
  #define PT_WRITE_U PTRACE_POKEUSR
  #endif
  
  #ifndef PTRACE_XFER_TYPE
! #define PTRACE_XFER_TYPE long
  #endif
  
  /* Glibc's headers don't define PTRACE_GETVRREGS so we cannot use a
*************** ppc_register_u_addr (int regno)
*** 180,186 ****
    struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
    /* NOTE: cagney/2003-11-25: This is the word size used by the ptrace
       interface, and not the wordsize of the program's ABI.  */
!   int wordsize = sizeof (PTRACE_XFER_TYPE);
  
    /* General purpose registers occupy 1 slot each in the buffer */
    if (regno >= tdep->ppc_gp0_regnum 
--- 180,194 ----
    struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
    /* NOTE: cagney/2003-11-25: This is the word size used by the ptrace
       interface, and not the wordsize of the program's ABI.  */
!   /* NOTE: rth/2005-09-12: Which is complicated by the fact that there
!      are in fact two ptrace interfaces: the normal one, and a different
!      one for 32-bit gdb debugging 64-bit target.  */
!   int wordsize;
! 
!   if (sizeof (PTRACE_XFER_TYPE) == 8)
!     wordsize = sizeof (PTRACE_XFER_TYPE);
!   else
!     wordsize = tdep->wordsize;
  
    /* General purpose registers occupy 1 slot each in the buffer */
    if (regno >= tdep->ppc_gp0_regnum 
*************** fetch_spe_register (int tid, int regno)
*** 337,350 ****
  }
  
  static void
  fetch_register (int tid, int regno)
  {
    struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
    /* This isn't really an address.  But ptrace thinks of it as one.  */
    CORE_ADDR regaddr = ppc_register_u_addr (regno);
!   int bytes_transferred;
!   unsigned int offset;         /* Offset of registers within the u area. */
    char buf[MAX_REGISTER_SIZE];
  
    if (altivec_register_p (regno))
      {
--- 345,364 ----
  }
  
  static void
+ ptrace_read_u (int cmd, int tid, CORE_ADDR regaddr, char *buf)
+ {
+   syscall (SYS_ptrace, cmd, tid, (PTRACE_ARG3_TYPE)(size_t)regaddr, buf);
+ }
+ 
+ static void
  fetch_register (int tid, int regno)
  {
    struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
    /* This isn't really an address.  But ptrace thinks of it as one.  */
    CORE_ADDR regaddr = ppc_register_u_addr (regno);
!   int regsize = register_size (current_gdbarch, regno);
    char buf[MAX_REGISTER_SIZE];
+   int bytes_transferred;
  
    if (altivec_register_p (regno))
      {
*************** fetch_register (int tid, int regno)
*** 369,397 ****
  
    if (regaddr == -1)
      {
!       memset (buf, '\0', register_size (current_gdbarch, regno));   /* Supply zeroes */
        regcache_raw_supply (current_regcache, regno, buf);
        return;
      }
  
!   /* Read the raw register using PTRACE_XFER_TYPE sized chunks.  On a
!      32-bit platform, 64-bit floating-point registers will require two
!      transfers.  */
!   for (bytes_transferred = 0;
!        bytes_transferred < register_size (current_gdbarch, regno);
!        bytes_transferred += sizeof (PTRACE_XFER_TYPE))
      {
!       errno = 0;
!       *(PTRACE_XFER_TYPE *) & buf[bytes_transferred]
!         = ptrace (PT_READ_U, tid, (PTRACE_ARG3_TYPE) regaddr, 0);
!       regaddr += sizeof (PTRACE_XFER_TYPE);
!       if (errno != 0)
! 	{
!           char message[128];
! 	  sprintf (message, "reading register %s (#%d)", 
! 		   REGISTER_NAME (regno), regno);
! 	  perror_with_name (message);
! 	}
      }
  
    /* Now supply the register.  Keep in mind that the regcache's idea
--- 383,424 ----
  
    if (regaddr == -1)
      {
!       /* Supply zeroes */
!       memset (buf, '\0', register_size (current_gdbarch, regno));
        regcache_raw_supply (current_regcache, regno, buf);
        return;
      }
  
!   /* We need to distinguish between 32/64-bit gdb and 32/64-bit target.  */
! 
!   errno = 0;
!   memset (buf, '\0', register_size (current_gdbarch, regno));
!   if (regsize <= sizeof (PTRACE_XFER_TYPE))
      {
!       ptrace_read_u (PT_READ_U, tid, regaddr, buf);
!       bytes_transferred = sizeof (PTRACE_XFER_TYPE);
!     }
!   else
!     {
!       int cmd;
! 
!       gdb_assert (sizeof (PTRACE_XFER_TYPE) == 4 && regsize == 8);
!       bytes_transferred = 2 * sizeof (PTRACE_XFER_TYPE);
! 
!       if (tdep->wordsize == 8)
! 	cmd = PPC_PTRACE_PEEKUSR_3264;
!       else
! 	cmd = PT_READ_U;
! 
!       ptrace_read_u (cmd, tid, regaddr, buf);
!       ptrace_read_u (cmd, tid, regaddr + 4, buf + 4);
!     }
!   if (errno != 0)
!     {
!       char message[128];
!       sprintf (message, "reading register %s (#%d)", 
! 	       REGISTER_NAME (regno), regno);
!       perror_with_name (message);
      }
  
    /* Now supply the register.  Keep in mind that the regcache's idea
*************** fetch_register (int tid, int regno)
*** 407,414 ****
      {
        /* Big-endian values are found at the right end of the bytes
           transferred.  */
!       size_t padding = (bytes_transferred
!                         - register_size (current_gdbarch, regno));
        regcache_raw_supply (current_regcache, regno, buf + padding);
      }
    else 
--- 434,440 ----
      {
        /* Big-endian values are found at the right end of the bytes
           transferred.  */
!       size_t padding = bytes_transferred - regsize;
        regcache_raw_supply (current_regcache, regno, buf + padding);
      }
    else 
*************** fetch_ppc_registers (int tid)
*** 466,476 ****
--- 492,504 ----
    int i;
    struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
  
+   /* ??? Use PPC_PTRACE_GETREGS / PPC_PTRACE_GETFPREGS.  */
    for (i = 0; i < ppc_num_gprs; i++)
      fetch_register (tid, tdep->ppc_gp0_regnum + i);
    if (tdep->ppc_fp0_regnum >= 0)
      for (i = 0; i < ppc_num_fprs; i++)
        fetch_register (tid, tdep->ppc_fp0_regnum + i);
+ 
    fetch_register (tid, PC_REGNUM);
    if (tdep->ppc_ps_regnum != -1)
      fetch_register (tid, tdep->ppc_ps_regnum);
*************** store_spe_register (int tid, int regno)
*** 633,646 ****
  }
  
  static void
  store_register (int tid, int regno)
  {
    struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
    /* This isn't really an address.  But ptrace thinks of it as one.  */
    CORE_ADDR regaddr = ppc_register_u_addr (regno);
!   int i;
!   size_t bytes_to_transfer;
    char buf[MAX_REGISTER_SIZE];
  
    if (altivec_register_p (regno))
      {
--- 661,681 ----
  }
  
  static void
+ ptrace_write_u (int cmd, int tid, CORE_ADDR regaddr, char *buf)
+ {
+   syscall (SYS_ptrace, cmd, tid, (PTRACE_ARG3_TYPE)(size_t)regaddr,
+  	   *(PTRACE_XFER_TYPE *)buf);
+ }
+ 
+ static void
  store_register (int tid, int regno)
  {
    struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
    /* This isn't really an address.  But ptrace thinks of it as one.  */
    CORE_ADDR regaddr = ppc_register_u_addr (regno);
!   int regsize = register_size (current_gdbarch, regno);
    char buf[MAX_REGISTER_SIZE];
+   int bytes_to_transfer;
  
    if (altivec_register_p (regno))
      {
*************** store_register (int tid, int regno)
*** 660,667 ****
       idea of the register's size may not be a multiple of sizeof
       (PTRACE_XFER_TYPE).  */
    memset (buf, 0, sizeof buf);
!   bytes_to_transfer = align_up (register_size (current_gdbarch, regno),
!                                 sizeof (PTRACE_XFER_TYPE));
    if (TARGET_BYTE_ORDER == BFD_ENDIAN_LITTLE)
      {
        /* Little-endian values always sit at the left end of the buffer.  */
--- 695,701 ----
       idea of the register's size may not be a multiple of sizeof
       (PTRACE_XFER_TYPE).  */
    memset (buf, 0, sizeof buf);
!   bytes_to_transfer = align_up (regsize, sizeof (PTRACE_XFER_TYPE));
    if (TARGET_BYTE_ORDER == BFD_ENDIAN_LITTLE)
      {
        /* Little-endian values always sit at the left end of the buffer.  */
*************** store_register (int tid, int regno)
*** 670,701 ****
    else if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
      {
        /* Big-endian values sit at the right end of the buffer.  */
!       size_t padding = (bytes_to_transfer
!                         - register_size (current_gdbarch, regno));
        regcache_raw_collect (current_regcache, regno, buf + padding);
      }
  
!   for (i = 0; i < bytes_to_transfer; i += sizeof (PTRACE_XFER_TYPE))
      {
!       errno = 0;
!       ptrace (PT_WRITE_U, tid, (PTRACE_ARG3_TYPE) regaddr,
! 	      *(PTRACE_XFER_TYPE *) & buf[i]);
!       regaddr += sizeof (PTRACE_XFER_TYPE);
  
!       if (errno == EIO 
!           && regno == tdep->ppc_fpscr_regnum)
! 	{
! 	  /* Some older kernel versions don't allow fpscr to be written.  */
! 	  continue;
! 	}
  
!       if (errno != 0)
! 	{
!           char message[128];
! 	  sprintf (message, "writing register %s (#%d)", 
! 		   REGISTER_NAME (regno), regno);
! 	  perror_with_name (message);
! 	}
      }
  }
  
--- 704,740 ----
    else if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
      {
        /* Big-endian values sit at the right end of the buffer.  */
!       size_t padding = bytes_to_transfer - regsize;
        regcache_raw_collect (current_regcache, regno, buf + padding);
      }
  
!   /* We need to distinguish between 32/64-bit gdb and 32/64-bit target.  */
! 
!   errno = 0;
!   if (regsize <= sizeof (PTRACE_XFER_TYPE))
!     ptrace_write_u (PT_WRITE_U, tid, regaddr, buf);
!   else
      {
!       int cmd;
  
!       gdb_assert (sizeof (PTRACE_XFER_TYPE) == 4 && regsize == 8);
  
!       if (regsize == tdep->wordsize)
! 	cmd = PPC_PTRACE_POKEUSR_3264;
!       else
! 	cmd = PT_WRITE_U;
! 
!       ptrace_write_u (cmd, tid, regaddr, buf);
!       ptrace_write_u (cmd, tid, regaddr + 4, buf + 4);
!     }
!   if (errno == EIO && regno == tdep->ppc_fpscr_regnum)
!     /* Some older kernel versions don't allow fpscr to be written.  */;
!   else if (errno != 0)
!     {
!       char message[128];
!       sprintf (message, "writing register %s (#%d)",
! 	       REGISTER_NAME (regno), regno);
!       perror_with_name (message);
      }
  }
  
*************** store_ppc_registers (int tid)
*** 750,760 ****
--- 789,801 ----
    int i;
    struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
    
+   /* ??? Use PPC_PTRACE_SETREGS / PPC_PTRACE_SETFPREGS.  */
    for (i = 0; i < ppc_num_gprs; i++)
      store_register (tid, tdep->ppc_gp0_regnum + i);
    if (tdep->ppc_fp0_regnum >= 0)
      for (i = 0; i < ppc_num_fprs; i++)
        store_register (tid, tdep->ppc_fp0_regnum + i);
+ 
    store_register (tid, PC_REGNUM);
    if (tdep->ppc_ps_regnum != -1)
      store_register (tid, tdep->ppc_ps_regnum);
*************** fill_fpregset (gdb_fpregset_t *fpregsetp
*** 886,891 ****
--- 927,1014 ----
      }
  }
  
+ /* Hold a pointer to inf_ptrace_xfer_partial.  */
+ static LONGEST (*default_xfer_partial) (struct target_ops *ops,
+ 					enum target_object object,
+ 					const char *annex, gdb_byte *readbuf,
+ 					const gdb_byte *writebuf,
+ 					ULONGEST offset, LONGEST len);
+ 
+ static LONGEST
+ ppc32_linux_xfer_partial (struct target_ops *ops, enum target_object object,
+                           const char *annex, gdb_byte *readbuf,
+                           const gdb_byte *writebuf,
+                           ULONGEST offset, LONGEST len)
+ {
+   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch); 
+   pid_t pid;
+   CORE_ADDR rounded_offset;
+   LONGEST partial_len;
+   PTRACE_XFER_TYPE buf;
+ 
+   /* If the target is also 32-bit, we can avoid double indirection by
+      using the default implementation.  Plus, the PTRACE_*_3264 commands
+      are only present on 64-bit kernels.  */
+   if (tdep->wordsize == 4)
+     return default_xfer_partial (ops, object, annex, readbuf,
+ 				 writebuf, offset, len);
+ 
+   if (object != TARGET_OBJECT_MEMORY)
+     return -1;
+ 
+   gdb_assert (sizeof (CORE_ADDR) == 8);
+ 
+   pid = ptid_get_pid (inferior_ptid);
+ 
+   /* Round the start offset down to the next long word boundary.  */
+   rounded_offset = offset & -(ULONGEST) sizeof (PTRACE_XFER_TYPE);
+ 
+   /* Since ptrace will transfer a single word starting at that
+      rounded_offset the partial_len needs to be adjusted down to
+      that (remember this function only does a single transfer).
+      Should the required length be even less, adjust it down again.  */
+   partial_len = rounded_offset + sizeof (PTRACE_XFER_TYPE) - offset;
+   if (partial_len > len)
+     partial_len = len;
+ 
+   if (writebuf)
+     {
+       /* If OFFSET:PARTIAL_LEN is smaller than ROUNDED_OFFSET:WORDSIZE
+ 	 then a read/modify write will be needed.  Read in the entire word.  */
+       if (rounded_offset < offset
+ 	  || (offset + partial_len
+ 	      < rounded_offset + sizeof (PTRACE_XFER_TYPE)))
+ 	{
+ 	  /* Need part of initial word -- fetch it.  */
+ 	  syscall (SYS_ptrace, PPC_PTRACE_PEEKDATA_3264, pid,
+ 		   &rounded_offset, &buf);
+ 	}
+ 
+       /* Copy data to be written over corresponding part of buffer.  */
+       memcpy ((char *)&buf + offset - rounded_offset, writebuf, partial_len);
+ 
+       errno = 0;
+       syscall (SYS_ptrace, PPC_PTRACE_POKEDATA_3264, pid,
+ 	       &rounded_offset, buf);
+       if (errno)
+ 	return 0;
+     }
+ 
+   if (readbuf)
+     {
+       errno = 0;
+       syscall (SYS_ptrace, PPC_PTRACE_PEEKDATA_3264, pid,
+ 	       &rounded_offset, &buf);
+       if (errno)
+ 	return 0;
+ 
+       /* Copy appropriate bytes out of the buffer.  */
+       memcpy (readbuf, (char *)&buf + offset - rounded_offset, partial_len);
+     }
+ 
+   return partial_len;
+ }
+ 
  void _initialize_ppc_linux_nat (void);
  
  void
*************** _initialize_ppc_linux_nat (void)
*** 900,905 ****
--- 1023,1036 ----
    t->to_fetch_registers = ppc_linux_fetch_inferior_registers;
    t->to_store_registers = ppc_linux_store_inferior_registers;
  
+   /* We need a special version of xfer_partial to handle 32-bit gdb
+      debugging a 64-bit process.  */
+   if (sizeof (PTRACE_XFER_TYPE) == 4)
+     {
+       default_xfer_partial = t->to_xfer_partial;
+       t->to_xfer_partial = ppc32_linux_xfer_partial;
+     }
+ 
    /* Register the target.  */
    add_target (t);
  }

Re: ppc32 debugging ppc64, part 1

[Mark Kettenis]

> Date: Mon, 12 Sep 2005 05:50:47 -0700
> From: Richard Henderson <rth@twiddle.net>
> 
> This is just enough modifications to not immediately fail 
> debugging a 64-bit process from a 32-bit debugger.
> 
> We don't get very far in the actual debugging:
> 
> (gdb) b main
> warning: Breakpoint address adjusted from 0x10093aa0 to 0x10000370.
> Breakpoint 1 at 0x10000370
> (gdb) run
> Starting program: /home/rth/work/gcc/bld-binu64/gdb/z
> warning: Breakpoint address adjusted from 0x10093a40 to 0x100001c0.
> warning: Breakpoint 1 address previously adjusted from 0x10093aa0 to 0x10000370.
> Breakpoint 1, 0x0000000010000370 in ?? ()
> (gdb) ppc
> 0x10000370 <__libc_tsd_CTYPE_B+268436264>:      std     r31,-8(r1)
> 
> But this is better than the original:
> 
> Starting program: /home/rth/work/gcc/bld-binu64/gdb/z
> warning: Breakpoint address adjusted from 0x10093a40 to 0x100001c0.
> Failed to read a valid object file image from memory.
> 
> Program received signal SIGTRAP, Trace/breakpoint trap.
> 0x100001c00002d932 in ?? ()
> 
> 
> Comments?

Hmm, this is really odd.  From what I see above and the changes to the
code you made, the implementation of ptrace seems to be just plain
broken, either in the kernel or in glibc, probably both.

Anyway, I'd really like to see people moving away from using
PTRACE_XFER_TYPE and PTRACE_ARG3_TYPE in favour of PTRACE_TYPE_RET and
PTRACE_TYPE_ARG3.  I wouldn't be surprised if it became clear what's
wrong with ptrace(2) on Linux ppc if you realize that PTRACE_XFER_TYPE
really is the return type of ptrace(2).

This code really should be using PTRACE_GETREGS and friends (like you
indicate in the patch) but those are not implemented I assume?

I'd really wish this would be fixed in the kernel, instead of being
worked around in GDB :-(.

Mark

Re: ppc32 debugging ppc64, part 1

[Daniel Jacobowitz]

On Mon, Sep 12, 2005 at 09:47:29PM +0200, Mark Kettenis wrote:
> Hmm, this is really odd.  From what I see above and the changes to the
> code you made, the implementation of ptrace seems to be just plain
> broken, either in the kernel or in glibc, probably both.
> 
> Anyway, I'd really like to see people moving away from using
> PTRACE_XFER_TYPE and PTRACE_ARG3_TYPE in favour of PTRACE_TYPE_RET and
> PTRACE_TYPE_ARG3.  I wouldn't be surprised if it became clear what's
> wrong with ptrace(2) on Linux ppc if you realize that PTRACE_XFER_TYPE
> really is the return type of ptrace(2).
> 
> This code really should be using PTRACE_GETREGS and friends (like you
> indicate in the patch) but those are not implemented I assume?
> 
> I'd really wish this would be fixed in the kernel, instead of being
> worked around in GDB :-(.

Mark, you seem to be very big on assuming GNU/Linux systems are broken;
I'm sensing a real recurring theme here.  Could you explain exactly
what it is that you think is broken now?

Richard's trying to do something fairly different from GDB's ordinary
usage model of ptrace here.  PTRACE_PEEKDATA_3264 allows a 32-bit
process to request four bytes of memory from the inferior by specifying
a full 64-bit address.  If I'm reading it right, it does this by
passing the address by reference, instead of in arg3.  Similarly
there's a way to read the 64-bit registers in two different 32-bit
pieces.

Hmm, this is a much cleaner way than I'd been using for MIPS n32.  That
bears some thinking about.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: ppc32 debugging ppc64, part 1

[David S. Miller]

From: Daniel Jacobowitz <drow@false.org>
Date: Mon, 12 Sep 2005 16:32:20 -0400

> Hmm, this is a much cleaner way than I'd been using for MIPS n32.
> That bears some thinking about.

It's even better than some of the ideas I had, and were partially
implemented, to support this on sparc64/Linux as well.

Re: ppc32 debugging ppc64, part 1

[Mark Kettenis]

> Date: Mon, 12 Sep 2005 16:32:20 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Mon, Sep 12, 2005 at 09:47:29PM +0200, Mark Kettenis wrote:
> > Hmm, this is really odd.  From what I see above and the changes to the
> > code you made, the implementation of ptrace seems to be just plain
> > broken, either in the kernel or in glibc, probably both.
> > 
> > Anyway, I'd really like to see people moving away from using
> > PTRACE_XFER_TYPE and PTRACE_ARG3_TYPE in favour of PTRACE_TYPE_RET and
> > PTRACE_TYPE_ARG3.  I wouldn't be surprised if it became clear what's
> > wrong with ptrace(2) on Linux ppc if you realize that PTRACE_XFER_TYPE
> > really is the return type of ptrace(2).
> > 
> > This code really should be using PTRACE_GETREGS and friends (like you
> > indicate in the patch) but those are not implemented I assume?
> > 
> > I'd really wish this would be fixed in the kernel, instead of being
> > worked around in GDB :-(.
> 
> Mark, you seem to be very big on assuming GNU/Linux systems are broken;
> I'm sensing a real recurring theme here.  Could you explain exactly
> what it is that you think is broken now?

From Richards patch I inferred that the prototype for ptrace(2)
doesn't actually match the actual system call in some cases.  That's
bad.  But I may be wrong here...

...but in that case Richard is making things hopelessly complicated by
doing using syscall() instead of ptrace().

> Richard's trying to do something fairly different from GDB's ordinary
> usage model of ptrace here.  PTRACE_PEEKDATA_3264 allows a 32-bit
> process to request four bytes of memory from the inferior by specifying
> a full 64-bit address.  If I'm reading it right, it does this by
> passing the address by reference, instead of in arg3.  Similarly
> there's a way to read the 64-bit registers in two different 32-bit
> pieces.

Fair enough.  But this code is really getting difficult to read.  It
uses several constructs that really only made sense in the generic
code where this was copied from.

What really frustrates me is that the different Linux ports are
reinventing the wheel, all in a different way.  There are several
architectures that have the same 32x64 issues, yet there's nobody who
steps back, notices the pattern, and tries to solve the problem in a
unified way.  This makes us end up with a lot of native-dependent code
that's really hard to maintain.

Mark

Re: ppc32 debugging ppc64, part 1

[Richard Henderson]

On Mon, Sep 12, 2005 at 11:19:13PM +0200, Mark Kettenis wrote:
> ...but in that case Richard is making things hopelessly complicated by
> doing using syscall() instead of ptrace().

I'm undecided as to whether it's cleaner or more complicated.  On
the one hand, I don't have to add odd casts and can just use a 
pointer as the real interface allows.

But for the record, I was seeing odd things and wanted to get the
libc code out of the way.  I later discovered the typo in my own
code, but I never got around to reverting the use of syscall.

> Fair enough.  But this code is really getting difficult to read.  It
> uses several constructs that really only made sense in the generic
> code where this was copied from.

Huh?  I removed the bulk of those.  What in particular did you have
in mind?

> What really frustrates me is that the different Linux ports are
> reinventing the wheel, all in a different way.

I appreciate your frustration, but it's not something that's in my
purvey to think about.  Ptrace is so hopelessly obtuse, slow, and
indirect that any effort fixing it would be better spent designing
a proper interface.  And in the meantime there's the issue of what's
actually present in extant kernels.


r~

Re: ppc32 debugging ppc64, part 1

[Richard Henderson]

On Mon, Sep 12, 2005 at 09:47:29PM +0200, Mark Kettenis wrote:
> This code really should be using PTRACE_GETREGS and friends (like you
> indicate in the patch) but those are not implemented I assume?

It's implemented in the kernel, but since it only covers the general
and floating-point registers, it wouldn't have helped my problem.
We would still need to go through this code path for the instruction
pointer, so there was no point not fixing this first.


r~

Re: ppc32 debugging ppc64, part 1

[Richard Henderson]

On Mon, Sep 12, 2005 at 03:36:37PM -0700, Richard Henderson wrote:
> But for the record, I was seeing odd things and wanted to get the
> libc code out of the way.  I later discovered the typo in my own
> code, but I never got around to reverting the use of syscall.

Oh, yes, one could in fact argue that libc is broken.  It doesn't
handle the PTRACE_PEEK*_3264 commands specially, as it does with
PTRACE_PEEK*.  So you'd have to write 

  if (cmd == *_3264)
    ptrace (cmd, tid, addr, buf)
  else
    *buf = ptrace (cmd, tid, addr, 0);

... and then rely on libc never being fixed.  And in this case I
don't see what going through libc buys us besides frustration.



r~

Re: ppc32 debugging ppc64, part 1

[Daniel Jacobowitz]

On Mon, Sep 12, 2005 at 05:50:47AM -0700, Richard Henderson wrote:
> This is just enough modifications to not immediately fail 
> debugging a 64-bit process from a 32-bit debugger.

> 	* ppc-linux-nat.c (PTRACE_XFER_TYPE): Set to long.

You don't need this; PTRACE_TYPE_RET should work OK.  We get that from
autoconf (even if the autoconf bits are a bit broken on Linux at the
moment, they'll choose long anyway).

> 	(ppc_register_u_addr): Compute wordsize correctly for 64-bit target.
> 	(ptrace_read_u, ptrace_write_u): New.
> 	(fetch_register): Simplify.  Use PPC_PTRACE_PEEKUSR_3264 when needed.
> 	(store_register): Similarly with PPC_PTRACE_POKEUSR_3264.
> 	(default_xfer_partial, ppc32_linux_xfer_partial): New.
> 	(_initialize_ppc_linux_nat): Use them.

The rest of this I'm basically OK with, although it needs to go past
Kevin or Andrew.

I think it's safe to do away with the syscalls; the current userspace
ptrace interface where glibc has to special-case PEEKTEXT/PEEKDATA/PEEKUSER
is nasty enough, so I'm willing to declare glibc broken if it is ever
"fixed" to handle the PEEK*_3264 operations specially.  Just my two
cents.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: ppc32 debugging ppc64, part 1

[Richard Henderson]

On Sat, Sep 17, 2005 at 05:24:03PM -0400, Daniel Jacobowitz wrote:
> You don't need this; PTRACE_TYPE_RET should work OK.  We get that from
> autoconf (even if the autoconf bits are a bit broken on Linux at the
> moment, they'll choose long anyway).

I've purged PTRACE_XFER_TYPE from the file entirely.

> I think it's safe to do away with the syscalls; the current userspace
> ptrace interface where glibc has to special-case PEEKTEXT/PEEKDATA/PEEKUSER
> is nasty enough, so I'm willing to declare glibc broken if it is ever
> "fixed" to handle the PEEK*_3264 operations specially.

What about the argument that it causes source uglification because
PEEKUSER and PEEKUSER_3464 have different calling sequences in the
libc funtion, but not the syscall?

For the record, my current source follows.

I'll get rid of the use of the syscall if required, but my 2 cents
say it looks cleaner this way.


r~



Index: ppc-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-nat.c,v
retrieving revision 1.55
diff -u -p -r1.55 ppc-linux-nat.c
--- ppc-linux-nat.c	10 Sep 2005 18:11:04 -0000	1.55
+++ ppc-linux-nat.c	18 Sep 2005 01:56:58 -0000
@@ -39,6 +39,7 @@
 #include <fcntl.h>
 #include <sys/procfs.h>
 #include <sys/ptrace.h>
+#include <sys/syscall.h>
 
 /* Prototypes for supply_gregset etc. */
 #include "gregset.h"
@@ -51,10 +52,6 @@
 #define PT_WRITE_U PTRACE_POKEUSR
 #endif
 
-/* Default the type of the ptrace transfer to int.  */
-#ifndef PTRACE_XFER_TYPE
-#define PTRACE_XFER_TYPE int
-#endif
 
 /* Glibc's headers don't define PTRACE_GETVRREGS so we cannot use a
    configure time check.  Some older glibc's (for instance 2.2.1)
@@ -180,7 +177,15 @@ ppc_register_u_addr (int regno)
   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
   /* NOTE: cagney/2003-11-25: This is the word size used by the ptrace
      interface, and not the wordsize of the program's ABI.  */
-  int wordsize = sizeof (PTRACE_XFER_TYPE);
+  /* NOTE: rth/2005-09-12: Which is complicated by the fact that there
+     are in fact two ptrace interfaces: the normal one, and a different
+     one for 32-bit gdb debugging 64-bit target.  */
+  int wordsize;
+
+  if (sizeof (PTRACE_TYPE_RET) == 8)
+    wordsize = sizeof (PTRACE_TYPE_RET);
+  else
+    wordsize = tdep->wordsize;
 
   /* General purpose registers occupy 1 slot each in the buffer */
   if (regno >= tdep->ppc_gp0_regnum 
@@ -336,15 +341,27 @@ fetch_spe_register (int tid, int regno)
                          &evrregs.spefscr);
 }
 
-static void
+/* A helper function for fetch_register to avoid too much casting.  Note
+   that glibc through 2.3.5 treats PTRACE_PEEKUSR specially, but does not
+   treat PPC_PTRACE_PEEKUSR_3264 the same.  To avoid depending on glibc
+   fixing this (or not), and to reduce code alternatives, bypass glibc
+   entirely and go straight to the kernel.  */
+
+static inline void
+ptrace_read_u (int cmd, int tid, CORE_ADDR regaddr, PTRACE_TYPE_RET *buf)
+{
+  syscall (SYS_ptrace, cmd, tid, (PTRACE_TYPE_ARG3) regaddr, buf);
+}
+
+static inline void
 fetch_register (int tid, int regno)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
   /* This isn't really an address.  But ptrace thinks of it as one.  */
   CORE_ADDR regaddr = ppc_register_u_addr (regno);
+  int regsize = register_size (current_gdbarch, regno);
+  PTRACE_TYPE_RET buf[MAX_REGISTER_SIZE / sizeof(PTRACE_TYPE_RET)];
   int bytes_transferred;
-  unsigned int offset;         /* Offset of registers within the u area. */
-  char buf[MAX_REGISTER_SIZE];
 
   if (altivec_register_p (regno))
     {
@@ -367,36 +384,48 @@ fetch_register (int tid, int regno)
       return;
     }
 
+  /* Supply zeroes for unknown registers.  */
   if (regaddr == -1)
     {
-      memset (buf, '\0', register_size (current_gdbarch, regno));   /* Supply zeroes */
+      memset (buf, '\0', sizeof (buf));
       regcache_raw_supply (current_regcache, regno, buf);
       return;
     }
 
-  /* Read the raw register using PTRACE_XFER_TYPE sized chunks.  On a
-     32-bit platform, 64-bit floating-point registers will require two
-     transfers.  */
-  for (bytes_transferred = 0;
-       bytes_transferred < register_size (current_gdbarch, regno);
-       bytes_transferred += sizeof (PTRACE_XFER_TYPE))
+  /* We need to distinguish between 32/64-bit gdb and 32/64-bit target.  */
+
+  errno = 0;
+  if (regsize <= sizeof (PTRACE_TYPE_RET))
     {
-      errno = 0;
-      *(PTRACE_XFER_TYPE *) & buf[bytes_transferred]
-        = ptrace (PT_READ_U, tid, (PTRACE_ARG3_TYPE) regaddr, 0);
-      regaddr += sizeof (PTRACE_XFER_TYPE);
-      if (errno != 0)
-	{
-          char message[128];
-	  sprintf (message, "reading register %s (#%d)", 
-		   REGISTER_NAME (regno), regno);
-	  perror_with_name (message);
-	}
+      ptrace_read_u (PT_READ_U, tid, regaddr, buf);
+      bytes_transferred = sizeof (PTRACE_TYPE_RET);
+    }
+  else
+    {
+      int cmd;
+
+      gdb_assert (sizeof (PTRACE_TYPE_RET) == 4 && regsize == 8);
+      bytes_transferred = 2 * sizeof (PTRACE_TYPE_RET);
+
+      if (tdep->wordsize == 8)
+	cmd = PPC_PTRACE_PEEKUSR_3264;
+      else
+	cmd = PT_READ_U;
+
+      ptrace_read_u (cmd, tid, regaddr, buf);
+      ptrace_read_u (cmd, tid, regaddr + 4, buf + 1);
+    }
+  if (errno != 0)
+    {
+      char message[128];
+      sprintf (message, "reading register %s (#%d)", 
+	       REGISTER_NAME (regno), regno);
+      perror_with_name (message);
     }
 
   /* Now supply the register.  Keep in mind that the regcache's idea
      of the register's size may not be a multiple of sizeof
-     (PTRACE_XFER_TYPE).  */
+     (PTRACE_TYPE_RET).  */
   if (gdbarch_byte_order (current_gdbarch) == BFD_ENDIAN_LITTLE)
     {
       /* Little-endian values are always found at the left end of the
@@ -407,9 +436,9 @@ fetch_register (int tid, int regno)
     {
       /* Big-endian values are found at the right end of the bytes
          transferred.  */
-      size_t padding = (bytes_transferred
-                        - register_size (current_gdbarch, regno));
-      regcache_raw_supply (current_regcache, regno, buf + padding);
+      size_t padding = bytes_transferred - regsize;
+      regcache_raw_supply (current_regcache, regno,
+			   (char *)buf + padding);
     }
   else 
     internal_error (__FILE__, __LINE__,
@@ -466,11 +495,13 @@ fetch_ppc_registers (int tid)
   int i;
   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
 
+  /* ??? Use PPC_PTRACE_GETREGS / PPC_PTRACE_GETFPREGS.  */
   for (i = 0; i < ppc_num_gprs; i++)
     fetch_register (tid, tdep->ppc_gp0_regnum + i);
   if (tdep->ppc_fp0_regnum >= 0)
     for (i = 0; i < ppc_num_fprs; i++)
       fetch_register (tid, tdep->ppc_fp0_regnum + i);
+
   fetch_register (tid, PC_REGNUM);
   if (tdep->ppc_ps_regnum != -1)
     fetch_register (tid, tdep->ppc_ps_regnum);
@@ -632,15 +663,23 @@ store_spe_register (int tid, int regno)
   set_spe_registers (tid, &evrregs);
 }
 
+/* A helper function for store_register to avoid too much casting.  */
+
+static inline void
+ptrace_write_u (int cmd, int tid, CORE_ADDR regaddr, PTRACE_TYPE_RET *buf)
+{
+  ptrace (cmd, tid, (PTRACE_TYPE_ARG3) regaddr, *buf);
+}
+
 static void
 store_register (int tid, int regno)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
   /* This isn't really an address.  But ptrace thinks of it as one.  */
   CORE_ADDR regaddr = ppc_register_u_addr (regno);
-  int i;
-  size_t bytes_to_transfer;
-  char buf[MAX_REGISTER_SIZE];
+  int regsize = register_size (current_gdbarch, regno);
+  PTRACE_TYPE_RET buf[MAX_REGISTER_SIZE / sizeof(PTRACE_TYPE_RET)];
+  int bytes_to_transfer;
 
   if (altivec_register_p (regno))
     {
@@ -658,10 +697,9 @@ store_register (int tid, int regno)
 
   /* First collect the register.  Keep in mind that the regcache's
      idea of the register's size may not be a multiple of sizeof
-     (PTRACE_XFER_TYPE).  */
-  memset (buf, 0, sizeof buf);
-  bytes_to_transfer = align_up (register_size (current_gdbarch, regno),
-                                sizeof (PTRACE_XFER_TYPE));
+     (PTRACE_TYPE_RET).  */
+  memset (buf, '\0', sizeof (buf));
+  bytes_to_transfer = align_up (regsize, sizeof (PTRACE_TYPE_RET));
   if (TARGET_BYTE_ORDER == BFD_ENDIAN_LITTLE)
     {
       /* Little-endian values always sit at the left end of the buffer.  */
@@ -670,32 +708,37 @@ store_register (int tid, int regno)
   else if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
     {
       /* Big-endian values sit at the right end of the buffer.  */
-      size_t padding = (bytes_to_transfer
-                        - register_size (current_gdbarch, regno));
-      regcache_raw_collect (current_regcache, regno, buf + padding);
+      size_t padding = bytes_to_transfer - regsize;
+      regcache_raw_collect (current_regcache, regno, (char *)buf + padding);
     }
 
-  for (i = 0; i < bytes_to_transfer; i += sizeof (PTRACE_XFER_TYPE))
+  /* We need to distinguish between 32/64-bit gdb and 32/64-bit target.  */
+
+  errno = 0;
+  if (regsize <= sizeof (PTRACE_TYPE_RET))
+    ptrace_write_u (PT_WRITE_U, tid, regaddr, buf);
+  else
     {
-      errno = 0;
-      ptrace (PT_WRITE_U, tid, (PTRACE_ARG3_TYPE) regaddr,
-	      *(PTRACE_XFER_TYPE *) & buf[i]);
-      regaddr += sizeof (PTRACE_XFER_TYPE);
+      int cmd;
 
-      if (errno == EIO 
-          && regno == tdep->ppc_fpscr_regnum)
-	{
-	  /* Some older kernel versions don't allow fpscr to be written.  */
-	  continue;
-	}
+      gdb_assert (sizeof (PTRACE_TYPE_RET) == 4 && regsize == 8);
 
-      if (errno != 0)
-	{
-          char message[128];
-	  sprintf (message, "writing register %s (#%d)", 
-		   REGISTER_NAME (regno), regno);
-	  perror_with_name (message);
-	}
+      if (regsize == tdep->wordsize)
+	cmd = PPC_PTRACE_POKEUSR_3264;
+      else
+	cmd = PT_WRITE_U;
+
+      ptrace_write_u (cmd, tid, regaddr, buf);
+      ptrace_write_u (cmd, tid, regaddr + 4, buf + 1);
+    }
+  if (errno == EIO && regno == tdep->ppc_fpscr_regnum)
+    /* Some older kernel versions don't allow fpscr to be written.  */;
+  else if (errno != 0)
+    {
+      char message[128];
+      sprintf (message, "writing register %s (#%d)",
+	       REGISTER_NAME (regno), regno);
+      perror_with_name (message);
     }
 }
 
@@ -750,11 +793,13 @@ store_ppc_registers (int tid)
   int i;
   struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
   
+  /* ??? Use PPC_PTRACE_SETREGS / PPC_PTRACE_SETFPREGS.  */
   for (i = 0; i < ppc_num_gprs; i++)
     store_register (tid, tdep->ppc_gp0_regnum + i);
   if (tdep->ppc_fp0_regnum >= 0)
     for (i = 0; i < ppc_num_fprs; i++)
       store_register (tid, tdep->ppc_fp0_regnum + i);
+
   store_register (tid, PC_REGNUM);
   if (tdep->ppc_ps_regnum != -1)
     store_register (tid, tdep->ppc_ps_regnum);
@@ -798,7 +843,7 @@ supply_gregset (gdb_gregset_t *gregsetp)
 {
   /* NOTE: cagney/2003-11-25: This is the word size used by the ptrace
      interface, and not the wordsize of the program's ABI.  */
-  int wordsize = sizeof (PTRACE_XFER_TYPE);
+  int wordsize = sizeof (PTRACE_TYPE_RET);
   ppc_linux_supply_gregset (current_regcache, -1, gregsetp,
 			    sizeof (gdb_gregset_t), wordsize);
 }
@@ -808,7 +853,7 @@ right_fill_reg (int regnum, void *reg)
 {
   /* NOTE: cagney/2003-11-25: This is the word size used by the ptrace
      interface, and not the wordsize of the program's ABI.  */
-  int wordsize = sizeof (PTRACE_XFER_TYPE);
+  int wordsize = sizeof (PTRACE_TYPE_RET);
   /* Right fill the register.  */
   regcache_raw_collect (current_regcache, regnum,
 			((bfd_byte *) reg
@@ -886,6 +931,103 @@ fill_fpregset (gdb_fpregset_t *fpregsetp
     }
 }
 
+/* Helper functions for ppc32_linux_xfer_partial to avoid too much casting.
+   Note that glibc through 2.3.5 treats PTRACE_PEEKDATA specially, but does
+   not treat PPC_PTRACE_PEEKDATA_3264 the same.  To avoid depending on glibc
+   fixing this (or not), bypass glibc entirely.  */
+
+static inline void
+ptrace_read64_d (int tid, CORE_ADDR addr, PTRACE_TYPE_RET *buf)
+{
+  syscall (SYS_ptrace, PPC_PTRACE_PEEKDATA_3264, tid,
+	   (PTRACE_TYPE_ARG3) &addr, buf);
+}
+
+static inline void
+ptrace_write64_d (int tid, CORE_ADDR addr, PTRACE_TYPE_RET *buf)
+{
+  ptrace (PPC_PTRACE_POKEDATA_3264, tid, (PTRACE_TYPE_ARG3) &addr, *buf);
+}
+
+/* Hold a pointer to inf_ptrace_xfer_partial.  */
+static LONGEST (*default_xfer_partial) (struct target_ops *ops,
+					enum target_object object,
+					const char *annex, gdb_byte *readbuf,
+					const gdb_byte *writebuf,
+					ULONGEST offset, LONGEST len);
+
+static LONGEST
+ppc32_linux_xfer_partial (struct target_ops *ops, enum target_object object,
+                          const char *annex, gdb_byte *readbuf,
+                          const gdb_byte *writebuf,
+                          ULONGEST offset, LONGEST len)
+{
+  struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch); 
+  pid_t pid;
+  CORE_ADDR rounded_offset;
+  LONGEST partial_len;
+  PTRACE_TYPE_RET buf;
+
+  /* If the target is also 32-bit, we can avoid double indirection by
+     using the default implementation.  Plus, the PTRACE_*_3264 commands
+     are only present on 64-bit kernels.  */
+  if (tdep->wordsize == 4)
+    return default_xfer_partial (ops, object, annex, readbuf,
+				 writebuf, offset, len);
+
+  if (object != TARGET_OBJECT_MEMORY)
+    return -1;
+
+  gdb_assert (sizeof (CORE_ADDR) == 8);
+
+  pid = ptid_get_pid (inferior_ptid);
+
+  /* Round the start offset down to the next long word boundary.  */
+  rounded_offset = offset & -(ULONGEST) sizeof (PTRACE_TYPE_RET);
+
+  /* Since ptrace will transfer a single word starting at that
+     rounded_offset the partial_len needs to be adjusted down to
+     that (remember this function only does a single transfer).
+     Should the required length be even less, adjust it down again.  */
+  partial_len = rounded_offset + sizeof (PTRACE_TYPE_RET) - offset;
+  if (partial_len > len)
+    partial_len = len;
+
+  if (writebuf)
+    {
+      /* If OFFSET:PARTIAL_LEN is smaller than ROUNDED_OFFSET:WORDSIZE
+	 then a read/modify write will be needed.  Read in the entire word.  */
+      if (rounded_offset < offset
+	  || (offset + partial_len
+	      < rounded_offset + sizeof (PTRACE_TYPE_RET)))
+	{
+	  /* Need part of initial word -- fetch it.  */
+	  ptrace_read64_d (pid, rounded_offset, &buf);
+	}
+
+      /* Copy data to be written over corresponding part of buffer.  */
+      memcpy ((char *)&buf + offset - rounded_offset, writebuf, partial_len);
+
+      errno = 0;
+      ptrace_write64_d (pid, rounded_offset, &buf);
+      if (errno)
+	return 0;
+    }
+
+  if (readbuf)
+    {
+      errno = 0;
+      ptrace_read64_d (pid, rounded_offset, &buf);
+      if (errno)
+	return 0;
+
+      /* Copy appropriate bytes out of the buffer.  */
+      memcpy (readbuf, (char *)&buf + offset - rounded_offset, partial_len);
+    }
+
+  return partial_len;
+}
+
 void _initialize_ppc_linux_nat (void);
 
 void
@@ -900,6 +1042,14 @@ _initialize_ppc_linux_nat (void)
   t->to_fetch_registers = ppc_linux_fetch_inferior_registers;
   t->to_store_registers = ppc_linux_store_inferior_registers;
 
+  /* We need a special version of xfer_partial to handle 32-bit gdb
+     debugging a 64-bit process.  */
+  if (sizeof (PTRACE_TYPE_RET) == 4)
+    {
+      default_xfer_partial = t->to_xfer_partial;
+      t->to_xfer_partial = ppc32_linux_xfer_partial;
+    }
+
   /* Register the target.  */
   add_target (t);
 }

Re: ppc32 debugging ppc64, part 1

[Daniel Jacobowitz]

On Sat, Sep 17, 2005 at 07:01:23PM -0700, Richard Henderson wrote:
> On Sat, Sep 17, 2005 at 05:24:03PM -0400, Daniel Jacobowitz wrote:
> > You don't need this; PTRACE_TYPE_RET should work OK.  We get that from
> > autoconf (even if the autoconf bits are a bit broken on Linux at the
> > moment, they'll choose long anyway).
> 
> I've purged PTRACE_XFER_TYPE from the file entirely.

Thanks.

> > I think it's safe to do away with the syscalls; the current userspace
> > ptrace interface where glibc has to special-case PEEKTEXT/PEEKDATA/PEEKUSER
> > is nasty enough, so I'm willing to declare glibc broken if it is ever
> > "fixed" to handle the PEEK*_3264 operations specially.
> 
> What about the argument that it causes source uglification because
> PEEKUSER and PEEKUSER_3464 have different calling sequences in the
> libc funtion, but not the syscall?
> 
> For the record, my current source follows.
> 
> I'll get rid of the use of the syscall if required, but my 2 cents
> say it looks cleaner this way.

Yes, that's a more compelling argument.  Works for me.

-- 
Daniel Jacobowitz
CodeSourcery, LLC