Re: [RFA] handle android bionic ptrace in gdbserver.

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Hi Mark,

On Tue, 2012-03-27 at 18:11 +0200, Mark Kettenis wrote:
> > From: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> > Date: Mon, 26 Mar 2012 19:46:42 -0300
> Sad to see all this compatibility goo just a different prototype.
> This could almost certainly be fixed by just fixing the header.

I completely agree. IMHO in this case the header to be fixed is actually
glibc's, since it declares ptrace in a different way than what is
supported by the kernel.

> Anyway, you should test this on a 64-bit platform.  I have a strong
> suspicion things will break there because of plain integers that are
> being cast to void *.

Good catch, that's what happened indeed. I don't know what would be the
correct fix. I changed the types of the variables that are passed to
ptrace from int to long. At least it hurts the eyes less than two casts
in a row.

These arguments are currently passed as int but glibc reads them as void
* (with a va_arg (ap, void *) call). My impression is that on 64 bit
Linux platforms the code currently works either by chance, or by relying
on implementation-specific behavior.

-- 
[]'s
Thiago Jung Bauermann
Linaro Toolchain Working Group


2012-03-27  Thiago Jung Bauermann  <thiago.bauermann@linaro.org>
  
	* linux-low.h (PTRACE_ARG3_TYPE): Move macro from linux-low.c.
	(PTRACE_ARG4_TYPE): Likewise.
	(PTRACE_XFER_TYPE): Likewise.
	* linux-arm-low.c (arm_prepare_to_resume): Cast third argument of
	ptrace to PTRACE_ARG3_TYPE.
	* linux-low.c (PTRACE_ARG3_TYPE): Move macro to linux-low.h.
	(PTRACE_ARG4_TYPE): Likewise.
	(PTRACE_XFER_TYPE): Likewise.
	(linux_detach_one_lwp): Cast fourth argument of
	ptrace to PTRACE_ARG4_TYPE.  Change sig to long.
	(regsets_fetch_inferior_registers): Cast third argument of
	ptrace to PTRACE_ARG3_TYPE.  Change nt_type to long.
	(regsets_store_inferior_registers): Likewise.


diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index 01208ef..9490d7d 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -709,13 +709,15 @@ arm_prepare_to_resume (struct lwp_info *lwp)
 	errno = 0;
 
 	if (arm_hwbp_control_is_enabled (proc_info->bpts[i].control))
-	  if (ptrace (PTRACE_SETHBPREGS, pid, ((i << 1) + 1),
-	      &proc_info->bpts[i].address) < 0)
+	  if (ptrace (PTRACE_SETHBPREGS, pid,
+		      (PTRACE_ARG3_TYPE) ((i << 1) + 1),
+		      &proc_info->bpts[i].address) < 0)
 	    perror_with_name ("Unexpected error setting breakpoint address");
 
 	if (arm_hwbp_control_is_initialized (proc_info->bpts[i].control))
-	  if (ptrace (PTRACE_SETHBPREGS, pid, ((i << 1) + 2),
-	      &proc_info->bpts[i].control) < 0)
+	  if (ptrace (PTRACE_SETHBPREGS, pid,
+		      (PTRACE_ARG3_TYPE) ((i << 1) + 2),
+		      &proc_info->bpts[i].control) < 0)
 	    perror_with_name ("Unexpected error setting breakpoint");
 
 	lwp_info->bpts_changed[i] = 0;
@@ -727,13 +729,15 @@ arm_prepare_to_resume (struct lwp_info *lwp)
 	errno = 0;
 
 	if (arm_hwbp_control_is_enabled (proc_info->wpts[i].control))
-	  if (ptrace (PTRACE_SETHBPREGS, pid, -((i << 1) + 1),
-	      &proc_info->wpts[i].address) < 0)
+	  if (ptrace (PTRACE_SETHBPREGS, pid,
+		      (PTRACE_ARG3_TYPE) -((i << 1) + 1),
+		      &proc_info->wpts[i].address) < 0)
 	    perror_with_name ("Unexpected error setting watchpoint address");
 
 	if (arm_hwbp_control_is_initialized (proc_info->wpts[i].control))
-	  if (ptrace (PTRACE_SETHBPREGS, pid, -((i << 1) + 2),
-	      &proc_info->wpts[i].control) < 0)
+	  if (ptrace (PTRACE_SETHBPREGS, pid,
+		      (PTRACE_ARG3_TYPE) -((i << 1) + 2),
+		      &proc_info->wpts[i].control) < 0)
 	    perror_with_name ("Unexpected error setting watchpoint");
 
 	lwp_info->wpts_changed[i] = 0;
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 1caff5a..52ae96c 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -239,10 +239,6 @@ struct pending_signals
   struct pending_signals *prev;
 };
 
-#define PTRACE_ARG3_TYPE void *
-#define PTRACE_ARG4_TYPE void *
-#define PTRACE_XFER_TYPE long
-
 #ifdef HAVE_LINUX_REGSETS
 static char *disabled_regsets;
 static int num_regsets;
@@ -1122,7 +1118,7 @@ linux_detach_one_lwp (struct inferior_list_entry *entry, void *args)
   struct thread_info *thread = (struct thread_info *) entry;
   struct lwp_info *lwp = get_thread_lwp (thread);
   int pid = * (int *) args;
-  int sig;
+  long sig;
 
   if (ptid_get_pid (entry->id) != pid)
     return 0;
@@ -1149,7 +1145,7 @@ linux_detach_one_lwp (struct inferior_list_entry *entry, void *args)
   /* Finally, let it resume.  */
   if (the_low_target.prepare_to_resume != NULL)
     the_low_target.prepare_to_resume (lwp);
-  if (ptrace (PTRACE_DETACH, lwpid_of (lwp), 0, sig) < 0)
+  if (ptrace (PTRACE_DETACH, lwpid_of (lwp), 0, (PTRACE_ARG4_TYPE) sig) < 0)
     error (_("Can't detach %s: %s"),
 	   target_pid_to_str (ptid_of (lwp)),
 	   strerror (errno));
@@ -3968,7 +3964,7 @@ regsets_fetch_inferior_registers (struct regcache *regcache)
   while (regset->size >= 0)
     {
       void *buf, *data;
-      int nt_type, res;
+      long nt_type, res;
 
       if (regset->size == 0 || disabled_regsets[regset - target_regsets])
 	{
@@ -3989,7 +3985,8 @@ regsets_fetch_inferior_registers (struct regcache *regcache)
 	data = buf;
 
 #ifndef __sparc__
-      res = ptrace (regset->get_request, pid, nt_type, data);
+      res = ptrace (regset->get_request, pid,
+		    (PTRACE_ARG3_TYPE) nt_type, data);
 #else
       res = ptrace (regset->get_request, pid, data, nt_type);
 #endif
@@ -4037,7 +4034,7 @@ regsets_store_inferior_registers (struct regcache *regcache)
   while (regset->size >= 0)
     {
       void *buf, *data;
-      int nt_type, res;
+      long nt_type, res;
 
       if (regset->size == 0 || disabled_regsets[regset - target_regsets])
 	{
@@ -4062,7 +4059,8 @@ regsets_store_inferior_registers (struct regcache *regcache)
 	data = buf;
 
 #ifndef __sparc__
-      res = ptrace (regset->get_request, pid, nt_type, data);
+      res = ptrace (regset->get_request, pid,
+		    (PTRACE_ARG3_TYPE) nt_type, data);
 #else
       res = ptrace (regset->get_request, pid, &iov, data);
 #endif
@@ -4074,7 +4072,8 @@ regsets_store_inferior_registers (struct regcache *regcache)
 
 	  /* Only now do we write the register set.  */
 #ifndef __sparc__
-	  res = ptrace (regset->set_request, pid, nt_type, data);
+	  res = ptrace (regset->set_request, pid,
+			(PTRACE_ARG3_TYPE) nt_type, data);
 #else
 	  res = ptrace (regset->set_request, pid, data, nt_type);
 #endif
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 3aeae70..cb26ebb 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -24,6 +24,10 @@
 
 #include "gdb_proc_service.h"
 
+#define PTRACE_ARG3_TYPE void *
+#define PTRACE_ARG4_TYPE void *
+#define PTRACE_XFER_TYPE long
+
 #ifdef HAVE_LINUX_REGSETS
 typedef void (*regset_fill_func) (struct regcache *, void *);
 typedef void (*regset_store_func) (struct regcache *, const void *);