[patch 5/9]#2 Move siginfo_fixup linux-nat -> inf-ptrace

[patch 5/9]#2 Move siginfo_fixup linux-nat -> inf-ptrace

[Jan Kratochvil]

Hi,

as the siginfo biarch conversion needs to be done now even during inf-ptrace
ptrace calls the interface had to be moved there.  There are no code changes
of it in this patch.


Thanks,
Jan


gdb/
2010-08-30  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* amd64-linux-nat.c: Include inf-ptrace.h.
	(_initialize_amd64_linux_nat): Rename linux_nat_set_siginfo_fixup to
	inf_ptrace_set_siginfo_fixup.
	* linux-nat.c (linux_nat_siginfo_fixup, siginfo_fixup)
	(linux_nat_set_siginfo_fixup): Move ...
	* inf-ptrace.c (inf_ptrace_siginfo_fixup, siginfo_fixup)
	(inf_ptrace_set_siginfo_fixup): ... here and rename them.  Remove
	static from siginfo_fixup.
	* linux-nat.h (linux_nat_set_siginfo_fixup): Move ...
	* inf-ptrace.h (inf_ptrace_set_siginfo_fixup): ... here and rename it.
	New declaration siginfo_fixup.

--- a/gdb/amd64-linux-nat.c
+++ b/gdb/amd64-linux-nat.c
@@ -26,6 +26,7 @@
 #include "regset.h"
 #include "linux-nat.h"
 #include "amd64-linux-tdep.h"
+#include "inf-ptrace.h"
 
 #include "gdb_assert.h"
 #include "gdb_string.h"
@@ -815,5 +816,5 @@ _initialize_amd64_linux_nat (void)
   /* Register the target.  */
   linux_nat_add_target (t);
   linux_nat_set_new_thread (t, amd64_linux_new_thread);
-  linux_nat_set_siginfo_fixup (t, amd64_linux_siginfo_fixup);
+  inf_ptrace_set_siginfo_fixup (t, amd64_linux_siginfo_fixup);
 }
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -796,3 +796,42 @@ inf_ptrace_trad_target (CORE_ADDR (*register_u_offset)
 
   return t;
 }
+
+/* The method to call, if any, when the siginfo object needs to be
+   converted between the layout returned by ptrace, and the layout in
+   the architecture of the inferior.  */
+static int (*inf_ptrace_siginfo_fixup) (struct siginfo *, gdb_byte *, int);
+
+/* Register a method that converts a siginfo object between the layout
+   that ptrace returns, and the layout in the architecture of the
+   inferior.  */
+void
+inf_ptrace_set_siginfo_fixup (struct target_ops *t,
+			      int (*siginfo_fixup) (struct siginfo *,
+						    gdb_byte *, int))
+{
+  /* Save the pointer.  */
+  inf_ptrace_siginfo_fixup = siginfo_fixup;
+}
+
+/* Convert a native/host siginfo object, into/from the siginfo in the
+   layout of the inferiors' architecture.  */
+
+void
+siginfo_fixup (struct siginfo *siginfo, gdb_byte *inf_siginfo, int direction)
+{
+  int done = 0;
+
+  if (inf_ptrace_siginfo_fixup != NULL)
+    done = inf_ptrace_siginfo_fixup (siginfo, inf_siginfo, direction);
+
+  /* If there was no callback, or the callback didn't do anything,
+     then just do a straight memcpy.  */
+  if (!done)
+    {
+      if (direction == 1)
+	memcpy (siginfo, inf_siginfo, sizeof (struct siginfo));
+      else
+	memcpy (inf_siginfo, siginfo, sizeof (struct siginfo));
+    }
+}
--- a/gdb/inf-ptrace.h
+++ b/gdb/inf-ptrace.h
@@ -34,4 +34,14 @@ extern struct target_ops *
   inf_ptrace_trad_target (CORE_ADDR (*register_u_offset)
 					(struct gdbarch *, int, int));
 
+struct siginfo;
+extern void siginfo_fixup (struct siginfo *siginfo, gdb_byte *inf_siginfo,
+			   int direction);
+
+extern
+  void inf_ptrace_set_siginfo_fixup (struct target_ops *t,
+				     int (*siginfo_fixup) (struct siginfo *,
+							   gdb_byte *,
+							   int));
+
 #endif
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -210,13 +210,6 @@ static struct target_ops linux_ops_saved;
 /* The method to call, if any, when a new thread is attached.  */
 static void (*linux_nat_new_thread) (ptid_t);
 
-/* The method to call, if any, when the siginfo object needs to be
-   converted between the layout returned by ptrace, and the layout in
-   the architecture of the inferior.  */
-static int (*linux_nat_siginfo_fixup) (struct siginfo *,
-				       gdb_byte *,
-				       int);
-
 /* The saved to_xfer_partial method, inherited from inf-ptrace.c.
    Called by our to_xfer_partial.  */
 static LONGEST (*super_xfer_partial) (struct target_ops *, 
@@ -3899,28 +3892,6 @@ linux_nat_mourn_inferior (struct target_ops *ops)
     linux_fork_mourn_inferior ();
 }
 
-/* Convert a native/host siginfo object, into/from the siginfo in the
-   layout of the inferiors' architecture.  */
-
-static void
-siginfo_fixup (struct siginfo *siginfo, gdb_byte *inf_siginfo, int direction)
-{
-  int done = 0;
-
-  if (linux_nat_siginfo_fixup != NULL)
-    done = linux_nat_siginfo_fixup (siginfo, inf_siginfo, direction);
-
-  /* If there was no callback, or the callback didn't do anything,
-     then just do a straight memcpy.  */
-  if (!done)
-    {
-      if (direction == 1)
-	memcpy (siginfo, inf_siginfo, sizeof (struct siginfo));
-      else
-	memcpy (inf_siginfo, siginfo, sizeof (struct siginfo));
-    }
-}
-
 static LONGEST
 linux_xfer_siginfo (struct target_ops *ops, enum target_object object,
                     const char *annex, gdb_byte *readbuf,
@@ -5695,18 +5666,6 @@ linux_nat_set_new_thread (struct target_ops *t, void (*new_thread) (ptid_t))
   linux_nat_new_thread = new_thread;
 }
 
-/* Register a method that converts a siginfo object between the layout
-   that ptrace returns, and the layout in the architecture of the
-   inferior.  */
-void
-linux_nat_set_siginfo_fixup (struct target_ops *t,
-			     int (*siginfo_fixup) (struct siginfo *,
-						   gdb_byte *,
-						   int))
-{
-  /* Save the pointer.  */
-  linux_nat_siginfo_fixup = siginfo_fixup;
-}
 
 /* Return the saved siginfo associated with PTID.  */
 struct siginfo *
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -155,14 +155,6 @@ void linux_nat_add_target (struct target_ops *);
 /* Register a method to call whenever a new thread is attached.  */
 void linux_nat_set_new_thread (struct target_ops *, void (*) (ptid_t));
 
-/* Register a method that converts a siginfo object between the layout
-   that ptrace returns, and the layout in the architecture of the
-   inferior.  */
-void linux_nat_set_siginfo_fixup (struct target_ops *,
-				  int (*) (struct siginfo *,
-					   gdb_byte *,
-					   int));
-
 /* Update linux-nat internal state when changing from one fork
    to another.  */
 void linux_nat_switch_fork (ptid_t new_ptid);

Re: [patch 5/9]#2 Move siginfo_fixup linux-nat -> inf-ptrace

[Mark Kettenis]

> Date: Mon, 30 Aug 2010 09:13:12 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> Hi,
> 
> as the siginfo biarch conversion needs to be done now even during inf-ptrace
> ptrace calls the interface had to be moved there.  There are no code changes
> of it in this patch.

Couple of issues/questions:

* It's difficult to review this diff as is, since the part that
  actually uses this stuff in the inf-ptrace.c file is missing.  I may
  be missing something, but my gut feeling is that the siginfo
  processing is (at least for now) a Linux-specific feature and
  therefore belongs in linux-nat.c.

Assuming that my gut feeling is wrong:

* Isn't it better to make the siginfo conversion/fixup part of the
  target vector?

* The memcpy that's being done requires the size of "struct siginfo"
  to be known.  I'm not sure all platforms that use ptrace(2) actually
  have a "struct siginfo" (OpenBSD, HP-UX 10.20 and Linux have it).
  I'm not sure <signal.h> is enough to actually make the definition
  available (have some doubts here for HP-UX 10.20).

* I'd like to see siginfo_fixup() renamed into inf_ptrace_siginfo_fixup(),
  and made static if possible.

* All of this probably should be #ifdef PT_GETSIGINFO.

Cheers,

Mark

cancel: Re: [patch 5/9]#2 Move siginfo_fixup linux-nat -> inf-ptrace

[Jan Kratochvil]

Hi,

cancelling now this patch as being just a pre-requisite for:
	cancel: Re: [rfc 7/9]#2 Associate siginfo_t with any signal
	http://sourceware.org/ml/gdb-patches/2010-08/msg00545.html


On Mon, 30 Aug 2010 13:44:17 +0200, Mark Kettenis wrote:
>   but my gut feeling is that the siginfo
>   processing is (at least for now) a Linux-specific feature and
>   therefore belongs in linux-nat.c.

I see now that the siginfo corruption is more a bug in linux-nat.c only than
a general GDB bug.


> * The memcpy that's being done requires the size of "struct siginfo"
>   to be known.  I'm not sure all platforms that use ptrace(2) actually
>   have a "struct siginfo" (OpenBSD, HP-UX 10.20 and Linux have it).
>   I'm not sure <signal.h> is enough to actually make the definition
>   available (have some doubts here for HP-UX 10.20).

There was that HAVE_SIGINFO_T for it but it is offtopic now.


Thanks,
Jan