[PATCH] gdbserver: add support for FDPIC loadmaps

[PATCH] gdbserver: add support for FDPIC loadmaps

[Mike Frysinger]

The DSBT support is very close to the FDPIC code, so extend the existing
loadmap support to work with FDPIC loadmaps too.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>

gdb/gdbserver/:
2011-09-27  Mike Frysinger  <vapier@gentoo.org>

	* linux-low.c (target_loadseg): Add defined PTRACE_GETFDPIC to the
	ifdef check.
	(target_loadmap, PT_GETDSBT): Wrap in a defined PT_GETDSBT check.
	(target_loadmap, !PT_GETDSBT): New definition.
	(LINUX_LOADMAP, LINUX_LOADMAP_EXEC, LINUX_LOADMAP_INTERP): Define.
	(linux_read_loadmap): Change PTRACE_GETDSBT_EXEC to
	LINUX_LOADMAP_EXEC, PTRACE_GETDSBT_INTERP to LINUX_LOADMAP_INTERP,
	and PT_GETDSBT to LINUX_LOADMAP.
	(linux_read_loadmap, !PT_GETDSBT): Define to NULL.
	(linux_target_ops): Delete unnecessary ifdef PT_GETDSBT check.

gdb/common/:
2011-09-27  Mike Frysinger  <vapier@gentoo.org>

	* common/linux-ptrace.h (PTRACE_GETFDPIC, PTRACE_GETFDPIC_EXEC,
	PTRACE_GETFDPIC_INTERP): Define.
---
 gdb/common/linux-ptrace.h |    7 +++++++
 gdb/gdbserver/linux-low.c |   36 +++++++++++++++++++++++++-----------
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/gdb/common/linux-ptrace.h b/gdb/common/linux-ptrace.h
index ea4ee0d..0f30430 100644
--- a/gdb/common/linux-ptrace.h
+++ b/gdb/common/linux-ptrace.h
@@ -51,6 +51,13 @@
 
 #endif /* PTRACE_EVENT_FORK */
 
+#if (defined __bfin__ || defined __frv__ || defined __sh__) && \
+    !defined PTRACE_GETFDPIC
+#define PTRACE_GETFDPIC		31
+#define PTRACE_GETFDPIC_EXEC	0
+#define PTRACE_GETFDPIC_INTERP	1
+#endif
+
 /* We can't always assume that this flag is available, but all systems
    with the ptrace event handlers also have __WALL, so it's safe to use
    in some contexts.  */
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 94f785c..5dfa59a 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4727,7 +4727,7 @@ linux_qxfer_spu (const char *annex, unsigned char *readbuf,
   return ret;
 }
 
-#if defined PT_GETDSBT
+#if defined PT_GETDSBT || defined PTRACE_GETFDPIC
 struct target_loadseg
 {
   /* Core address to which the segment is mapped.  */
@@ -4738,6 +4738,7 @@ struct target_loadseg
   Elf32_Word p_memsz;
 };
 
+# if defined PT_GETDSBT
 struct target_loadmap
 {
   /* Protocol version number, must be zero.  */
@@ -4750,9 +4751,24 @@ struct target_loadmap
   /* The actual memory map.  */
   struct target_loadseg segs[/*nsegs*/];
 };
-#endif
+#  define LINUX_LOADMAP		PT_GETDSBT
+#  define LINUX_LOADMAP_EXEC	PTRACE_GETDSBT_EXEC
+#  define LINUX_LOADMAP_INTERP	PTRACE_GETDSBT_INTERP
+# else
+struct target_loadmap
+{
+  /* Protocol version number, must be zero.  */
+  Elf32_Half version;
+  /* Number of segments in this map.  */
+  Elf32_Half nsegs;
+  /* The actual memory map.  */
+  struct target_loadseg segs[/*nsegs*/];
+};
+#  define LINUX_LOADMAP		PTRACE_GETFDPIC
+#  define LINUX_LOADMAP_EXEC	PTRACE_GETFDPIC_EXEC
+#  define LINUX_LOADMAP_INTERP	PTRACE_GETFDPIC_INTERP
+# endif
 
-#if defined PT_GETDSBT
 static int
 linux_read_loadmap (const char *annex, CORE_ADDR offset,
 		    unsigned char *myaddr, unsigned int len)
@@ -4763,13 +4779,13 @@ linux_read_loadmap (const char *annex, CORE_ADDR offset,
   unsigned int actual_length, copy_length;
 
   if (strcmp (annex, "exec") == 0)
-    addr= (int) PTRACE_GETDSBT_EXEC;
+    addr = (int) LINUX_LOADMAP_EXEC;
   else if (strcmp (annex, "interp") == 0)
-    addr = (int) PTRACE_GETDSBT_INTERP;
+    addr = (int) LINUX_LOADMAP_INTERP;
   else
     return -1;
 
-  if (ptrace (PT_GETDSBT, pid, addr, &data) != 0)
+  if (ptrace (LINUX_LOADMAP, pid, addr, &data) != 0)
     return -1;
 
   if (data == NULL)
@@ -4785,7 +4801,9 @@ linux_read_loadmap (const char *annex, CORE_ADDR offset,
   memcpy (myaddr, (char *) data + offset, copy_length);
   return copy_length;
 }
-#endif /* defined PT_GETDSBT */
+#else
+# define linux_read_loadmap NULL
+#endif /* defined PT_GETDSBT || defined PTRACE_GETFDPIC */
 
 static void
 linux_process_qsupported (const char *query)
@@ -4935,11 +4953,7 @@ static struct target_ops linux_target_ops = {
   NULL,
 #endif
   linux_common_core_of_thread,
-#if defined PT_GETDSBT
   linux_read_loadmap,
-#else
-  NULL,
-#endif
   linux_process_qsupported,
   linux_supports_tracepoints,
   linux_read_pc,
-- 
1.7.6.1

Re: [PATCH] gdbserver: add support for FDPIC loadmaps

[Yao Qi]

On 09/29/2011 11:58 AM, Mike Frysinger wrote:
> The DSBT support is very close to the FDPIC code, so extend the existing
> loadmap support to work with FDPIC loadmaps too.
> 

Mike, patch looks quite good.

> ---
>  gdb/common/linux-ptrace.h |    7 +++++++
>  gdb/gdbserver/linux-low.c |   36 +++++++++++++++++++++++++-----------
>  2 files changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/gdb/common/linux-ptrace.h b/gdb/common/linux-ptrace.h
> index ea4ee0d..0f30430 100644
> --- a/gdb/common/linux-ptrace.h
> +++ b/gdb/common/linux-ptrace.h
> @@ -51,6 +51,13 @@
>  
>  #endif /* PTRACE_EVENT_FORK */
>  
> +#if (defined __bfin__ || defined __frv__ || defined __sh__) && \
> +    !defined PTRACE_GETFDPIC
> +#define PTRACE_GETFDPIC		31
> +#define PTRACE_GETFDPIC_EXEC	0
> +#define PTRACE_GETFDPIC_INTERP	1
> +#endif
> +

Do you have some reasons to define these macros in common/?  I don't see
these macros are/will be used in GDB, so maybe, we can move them in
gdbserver.  Files in common/ are to have contents shared between gdb and
gdbserver.

>  /* We can't always assume that this flag is available, but all systems
>     with the ptrace event handlers also have __WALL, so it's safe to use
>     in some contexts.  */
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 94f785c..5dfa59a 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -4727,7 +4727,7 @@ linux_qxfer_spu (const char *annex, unsigned char *readbuf,
>    return ret;
>  }
>  
> -#if defined PT_GETDSBT
> +#if defined PT_GETDSBT || defined PTRACE_GETFDPIC
>  struct target_loadseg
>  {
>    /* Core address to which the segment is mapped.  */
> @@ -4738,6 +4738,7 @@ struct target_loadseg
>    Elf32_Word p_memsz;
>  };
>  
> +# if defined PT_GETDSBT
>  struct target_loadmap
>  {
>    /* Protocol version number, must be zero.  */
> @@ -4750,9 +4751,24 @@ struct target_loadmap
>    /* The actual memory map.  */
>    struct target_loadseg segs[/*nsegs*/];
>  };
> -#endif
> +#  define LINUX_LOADMAP		PT_GETDSBT
> +#  define LINUX_LOADMAP_EXEC	PTRACE_GETDSBT_EXEC
> +#  define LINUX_LOADMAP_INTERP	PTRACE_GETDSBT_INTERP
> +# else

The spaces between '#' and 'define' are not needed.

The changes related to dsbt look correct to me.  I applied this patch to
trunk, and build c6x-uclinux gdb and gdbserver.  I didn't run test
because the board is in use.

-- 
Yao (齐尧)

Re: [PATCH] gdbserver: add support for FDPIC loadmaps

[Pedro Alves]

On Thursday 29 September 2011 09:53:39, Yao Qi wrote:
> > -#endif
> > +#  define LINUX_LOADMAP              PT_GETDSBT
> > +#  define LINUX_LOADMAP_EXEC PTRACE_GETDSBT_EXEC
> > +#  define LINUX_LOADMAP_INTERP       PTRACE_GETDSBT_INTERP
> > +# else
> 
> The spaces between '#' and 'define' are not needed.

Please leave them in.  They're useful to follow the
indentation level.

-- 
Pedro Alves

Re: [PATCH] gdbserver: add support for FDPIC loadmaps

[Pedro Alves]

On Thursday 29 September 2011 04:58:25, Mike Frysinger wrote:
>         (target_loadmap, !PT_GETDSBT): New definition.

The standard way to write change logs of conditionally compiled
bits is:
          [!PT_GETDSBT] (target_loadmap): New definition.

See <http://www.gnu.org/prep/standards/html_node/Conditional-Changes.html#Conditional-Changes>.

> +#if (defined __bfin__ || defined __frv__ || defined __sh__) && \
> +    !defined PTRACE_GETFDPIC

&& on start of next line.

Otherwise OK.  Thanks.

-- 
Pedro Alves

Re: [PATCH] gdbserver: add support for FDPIC loadmaps

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 883 bytes --]

On Thursday, September 29, 2011 04:53:39 Yao Qi wrote:
> On 09/29/2011 11:58 AM, Mike Frysinger wrote:
> > --- a/gdb/common/linux-ptrace.h
> > +++ b/gdb/common/linux-ptrace.h
> > 
> > +#if (defined __bfin__ || defined __frv__ || defined __sh__) && \
> > +    !defined PTRACE_GETFDPIC
> > +#define PTRACE_GETFDPIC		31
> > +#define PTRACE_GETFDPIC_EXEC	0
> > +#define PTRACE_GETFDPIC_INTERP	1
> > +#endif
> 
> Do you have some reasons to define these macros in common/?  I don't see
> these macros are/will be used in GDB, so maybe, we can move them in
> gdbserver.  Files in common/ are to have contents shared between gdb and
> gdbserver.

it isn't out of the realm of possibility to have a native gdb running which 
would need these ptrace defines.  unlikely, but not impossible.  this file 
seemed to be the best place for fallback linux ptrace defines.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] gdbserver: add support for FDPIC loadmaps

[Mike Frysinger]

On Thursday, September 29, 2011 07:22:50 Pedro Alves wrote:
> On Thursday 29 September 2011 04:58:25, Mike Frysinger wrote:
> >         (target_loadmap, !PT_GETDSBT): New definition.
> 
> The standard way to write change logs of conditionally compiled
> bits is:
>           [!PT_GETDSBT] (target_loadmap): New definition.

thanks ... wasn't quite sure

> > +#if (defined __bfin__ || defined __frv__ || defined __sh__) && \
> > +    !defined PTRACE_GETFDPIC
> 
> && on start of next line.
> 
> Otherwise OK.  Thanks.

i've committed the following change then:

gdbserver: add support for FDPIC loadmaps

The DSBT support is very close to the FDPIC code, so extend the existing
loadmap support to work with FDPIC loadmaps too.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>

gdb/gdbserver/:
2011-09-29  Mike Frysinger  <vapier@gentoo.org>

	* linux-low.c (target_loadseg): Add defined PTRACE_GETFDPIC to the
	ifdef check.
	[PT_GETDSBT] (target_loadmap): Wrap in a defined PT_GETDSBT check.
	[!PT_GETDSBT] (target_loadmap): New definition.
	(LINUX_LOADMAP, LINUX_LOADMAP_EXEC, LINUX_LOADMAP_INTERP): Define.
	(linux_read_loadmap): Change PTRACE_GETDSBT_EXEC to
	LINUX_LOADMAP_EXEC, PTRACE_GETDSBT_INTERP to LINUX_LOADMAP_INTERP,
	and PT_GETDSBT to LINUX_LOADMAP.
	[!PT_GETDSBT] (linux_read_loadmap): Define to NULL.
	(linux_target_ops): Delete unnecessary ifdef PT_GETDSBT check.

gdb/common/:
2011-09-29  Mike Frysinger  <vapier@gentoo.org>

	* common/linux-ptrace.h (PTRACE_GETFDPIC, PTRACE_GETFDPIC_EXEC,
	PTRACE_GETFDPIC_INTERP): Define.
---
 gdb/common/linux-ptrace.h |    7 +++++++
 gdb/gdbserver/linux-low.c |   36 +++++++++++++++++++++++++-----------
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/gdb/common/linux-ptrace.h b/gdb/common/linux-ptrace.h
index ea4ee0d..841775e 100644
--- a/gdb/common/linux-ptrace.h
+++ b/gdb/common/linux-ptrace.h
@@ -51,6 +51,13 @@
 
 #endif /* PTRACE_EVENT_FORK */
 
+#if (defined __bfin__ || defined __frv__ || defined __sh__) \
+    && !defined PTRACE_GETFDPIC
+#define PTRACE_GETFDPIC		31
+#define PTRACE_GETFDPIC_EXEC	0
+#define PTRACE_GETFDPIC_INTERP	1
+#endif
+
 /* We can't always assume that this flag is available, but all systems
    with the ptrace event handlers also have __WALL, so it's safe to use
    in some contexts.  */
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 94f785c..5dfa59a 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4727,7 +4727,7 @@ linux_qxfer_spu (const char *annex, unsigned char 
*readbuf,
   return ret;
 }
 
-#if defined PT_GETDSBT
+#if defined PT_GETDSBT || defined PTRACE_GETFDPIC
 struct target_loadseg
 {
   /* Core address to which the segment is mapped.  */
@@ -4738,6 +4738,7 @@ struct target_loadseg
   Elf32_Word p_memsz;
 };
 
+# if defined PT_GETDSBT
 struct target_loadmap
 {
   /* Protocol version number, must be zero.  */
@@ -4750,9 +4751,24 @@ struct target_loadmap
   /* The actual memory map.  */
   struct target_loadseg segs[/*nsegs*/];
 };
-#endif
+#  define LINUX_LOADMAP		PT_GETDSBT
+#  define LINUX_LOADMAP_EXEC	PTRACE_GETDSBT_EXEC
+#  define LINUX_LOADMAP_INTERP	PTRACE_GETDSBT_INTERP
+# else
+struct target_loadmap
+{
+  /* Protocol version number, must be zero.  */
+  Elf32_Half version;
+  /* Number of segments in this map.  */
+  Elf32_Half nsegs;
+  /* The actual memory map.  */
+  struct target_loadseg segs[/*nsegs*/];
+};
+#  define LINUX_LOADMAP		PTRACE_GETFDPIC
+#  define LINUX_LOADMAP_EXEC	PTRACE_GETFDPIC_EXEC
+#  define LINUX_LOADMAP_INTERP	PTRACE_GETFDPIC_INTERP
+# endif
 
-#if defined PT_GETDSBT
 static int
 linux_read_loadmap (const char *annex, CORE_ADDR offset,
 		    unsigned char *myaddr, unsigned int len)
@@ -4763,13 +4779,13 @@ linux_read_loadmap (const char *annex, CORE_ADDR 
offset,
   unsigned int actual_length, copy_length;
 
   if (strcmp (annex, "exec") == 0)
-    addr= (int) PTRACE_GETDSBT_EXEC;
+    addr = (int) LINUX_LOADMAP_EXEC;
   else if (strcmp (annex, "interp") == 0)
-    addr = (int) PTRACE_GETDSBT_INTERP;
+    addr = (int) LINUX_LOADMAP_INTERP;
   else
     return -1;
 
-  if (ptrace (PT_GETDSBT, pid, addr, &data) != 0)
+  if (ptrace (LINUX_LOADMAP, pid, addr, &data) != 0)
     return -1;
 
   if (data == NULL)
@@ -4785,7 +4801,9 @@ linux_read_loadmap (const char *annex, CORE_ADDR offset,
   memcpy (myaddr, (char *) data + offset, copy_length);
   return copy_length;
 }
-#endif /* defined PT_GETDSBT */
+#else
+# define linux_read_loadmap NULL
+#endif /* defined PT_GETDSBT || defined PTRACE_GETFDPIC */
 
 static void
 linux_process_qsupported (const char *query)
@@ -4935,11 +4953,7 @@ static struct target_ops linux_target_ops = {
   NULL,
 #endif
   linux_common_core_of_thread,
-#if defined PT_GETDSBT
   linux_read_loadmap,
-#else
-  NULL,
-#endif
   linux_process_qsupported,
   linux_supports_tracepoints,
   linux_read_pc,

Re: [PATCH] gdbserver: add support for FDPIC loadmaps

[Yao Qi]

On 09/29/2011 10:31 PM, Mike Frysinger wrote:
>> > Do you have some reasons to define these macros in common/?  I don't see
>> > these macros are/will be used in GDB, so maybe, we can move them in
>> > gdbserver.  Files in common/ are to have contents shared between gdb and
>> > gdbserver.
> it isn't out of the realm of possibility to have a native gdb running which 
> would need these ptrace defines.  unlikely, but not impossible.  this file 
> seemed to be the best place for fallback linux ptrace defines.

That sounds good to me.  Thanks.

-- 
Yao (齐尧)