[RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Yao Qi]

[-- Attachment #1: Type: text/plain, Size: 379 bytes --]

This patch is derived from bits from CodeSourcery SH-2A toolchain

  http://sourceware.org/ml/gdb-patches/2010-12/msg00291.html

DSBT's loadmap is similar to FDPIC's, so they can be handled in a
unified approach.  This patch adds a mechanism from gdb to gdbserver to
read loadmap.  This part is not c6x-specific, and can be reused by other
similar ports.

-- 
Yao (齐尧)

[-- Attachment #2: 0004-fdpic-packet-and-dsbt.patch --]
[-- Type: text/x-patch, Size: 7245 bytes --]

2011-07-19  Andrew Stubbs <ams@codesourcery.com>
            Yao Qi <yao@codesourcery.com>

        gdb/
        * remote.c (PACKET_qXfer_fdpic): New enum value.
        (remote_protocol_features): Add qXfer:fdpic:read packet.
        (remote_xfer_partial): Support TARGET_OBJECT_FDPIC.
        (_initialize_remote): Add set/show remote read-fdpic-loadmap command.
        * target.h (enum target_object): Add TARGET_OBJECT_FDPIC.

        gdb/gdbserver:
        * linux-low.c (struct target_loadseg): New type.
        (struct target_loadmap): New type.
        (linux_read_loadmap): New function.
        (linux_target_ops): Add linux_read_loadmap.
        * server.c (handle_query): Support qXfer:fdpic:read packet.
        * target.h (struct target_ops): Add read_loadmap.
---
 gdb/gdbserver/linux-low.c |   60 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/server.c    |   19 ++++++++++++++
 gdb/gdbserver/target.h    |    4 +++
 gdb/remote.c              |   10 +++++++
 gdb/target.h              |    2 +
 5 files changed, 95 insertions(+), 0 deletions(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index d84fd68..1e9a4fb 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4964,6 +4964,61 @@ linux_core_of_thread (ptid_t ptid)
   return core;
 }
 
+#if defined __DSBT__
+struct target_loadseg
+{
+  /* Core address to which the segment is mapped.  */
+  Elf32_Addr addr;
+  /* VMA recorded in the program header.  */
+  Elf32_Addr p_vaddr;
+  /* Size of this segment in memory.  */
+  Elf32_Word p_memsz;
+};
+
+struct target_loadmap {
+  /* Protocol version number, must be zero.  */
+  Elf32_Word version;
+  /* Pointer to the DSBT table, its size, and the DSBT index.  */
+  unsigned *dsbt_table;
+  unsigned dsbt_size, dsbt_index;
+  /* Number of segments in this map.  */
+  Elf32_Word nsegs;
+  /* The actual memory map.  */
+  struct target_loadseg segs[/*nsegs*/];
+};
+#endif
+
+#if defined __DSBT__
+static int
+linux_read_loadmap (const char *annex, CORE_ADDR offset,
+                          unsigned char *myaddr, unsigned int len)
+{
+  int pid = lwpid_of (get_thread_lwp (current_inferior));
+  int addr = (strcmp (annex, "exec") == 0 ? (int) PTRACE_GETDSBT_EXEC :
+	      (strcmp (annex, "interp") == 0 ? (int) PTRACE_GETDSBT_INTERP :
+	       -1));
+  struct target_loadmap *data = NULL;
+  unsigned int actual_length, copy_length;
+
+  if (addr == -1)
+    return -1;
+
+  ptrace (PTRACE_GETDSBT, pid, addr, &data);
+
+  if (data == NULL)
+    return -1;
+
+  actual_length = sizeof (struct target_loadmap)
+    + sizeof (struct target_loadseg) * data->nsegs;
+
+  if (offset < 0 || offset > actual_length)
+    return -1;
+
+  copy_length = actual_length - offset < len ? actual_length - offset : len;
+  memcpy (myaddr, ((char *)data) + offset, copy_length);
+  return copy_length;
+}
+#endif
 static void
 linux_process_qsupported (const char *query)
 {
@@ -5112,6 +5167,11 @@ static struct target_ops linux_target_ops = {
   NULL,
 #endif
   linux_core_of_thread,
+#if defined __DSBT__
+  linux_read_loadmap,
+#else
+  NULL,
+#endif
   linux_process_qsupported,
   linux_supports_tracepoints,
   linux_read_pc,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index fd06c8f..7991307 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1171,6 +1171,21 @@ handle_qxfer_traceframe_info (const char *annex,
   return len;
 }
 
+/* Handle qXfer:fdpic:read.  */
+
+static int
+handle_qxfer_fdpic (const char *annex, gdb_byte *readbuf,
+		    const gdb_byte *writebuf, ULONGEST offset, LONGEST len)
+{
+  if (the_target->read_fdpic_loadmap == NULL)
+    return -2;
+
+  if (!target_running ())
+    return -1;
+
+  return (*the_target->read_fdpic_loadmap) (annex, offset, readbuf, len);
+}
+
 static const struct qxfer qxfer_packets[] =
   {
     { "auxv", handle_qxfer_auxv },
@@ -1182,6 +1197,7 @@ static const struct qxfer qxfer_packets[] =
     { "statictrace", handle_qxfer_statictrace },
     { "threads", handle_qxfer_threads },
     { "traceframe-info", handle_qxfer_traceframe_info },
+    {"fdpic", handle_qxfer_fdpic}
   };
 
 static int
@@ -1509,6 +1525,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
       if (the_target->qxfer_siginfo != NULL)
 	strcat (own_buf, ";qXfer:siginfo:read+;qXfer:siginfo:write+");
 
+      if (the_target->read_fdpic_loadmap != NULL)
+	strcat (own_buf, ";qXfer:fdpic:read+");
+
       /* We always report qXfer:features:read, as targets may
 	 install XML files on a subsequent call to arch_setup.
 	 If we reported to GDB on startup that we don't support
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 00214db..60f4f34 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -311,6 +311,10 @@ struct target_ops
   /* Returns the core given a thread, or -1 if not known.  */
   int (*core_of_thread) (ptid_t);
 
+  /* Read FDPIC loadmaps.  Read LEN bytes at OFFSET into a buffer at MYADDR.  */
+  int (*read_fdpic_loadmap) (const char *annex, CORE_ADDR offset,
+			     unsigned char *myaddr, unsigned int len);
+
   /* Target specific qSupported support.  */
   void (*process_qsupported) (const char *);
 
diff --git a/gdb/remote.c b/gdb/remote.c
index b03ef59..9a1c653 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1262,6 +1262,7 @@ enum {
   PACKET_bs,
   PACKET_TracepointSource,
   PACKET_QAllow,
+  PACKET_qXfer_fdpic,
   PACKET_MAX
 };
 
@@ -3769,6 +3770,8 @@ static struct protocol_feature remote_protocol_features[] = {
     PACKET_QAllow },
   { "EnableDisableTracepoints", PACKET_DISABLE,
     remote_enable_disable_tracepoint_feature, -1 },
+  { "qXfer:fdpic:read", PACKET_DISABLE, remote_supported_packet,
+    PACKET_qXfer_fdpic },
 };
 
 static char *remote_support_xml;
@@ -8299,6 +8302,10 @@ remote_xfer_partial (struct target_ops *ops, enum target_object object,
       return remote_read_qxfer
 	(ops, "traceframe-info", annex, readbuf, offset, len,
 	 &remote_protocol_packets[PACKET_qXfer_traceframe_info]);
+
+    case TARGET_OBJECT_FDPIC:
+      return remote_read_qxfer (ops, "fdpic", annex, readbuf, offset, len,
+				&remote_protocol_packets[PACKET_qXfer_fdpic]);
     default:
       return -1;
     }
@@ -10903,6 +10910,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_statictrace_read],
                          "qXfer:statictrace:read", "read-sdata-object", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_fdpic],
+			 "qXfer:fdpic:read", "read-fdpic-loadmap", 0);
+
   /* Keep the old ``set remote Z-packet ...'' working.  Each individual
      Z sub-packet has its own set and show commands, but users may
      have sets to this variable in their .gdbinit files (or in their
diff --git a/gdb/target.h b/gdb/target.h
index 3b39d6f..f1bb12d 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -274,6 +274,8 @@ enum target_object
   TARGET_OBJECT_HPUX_SOLIB_GOT,
   /* Traceframe info, in XML format.  */
   TARGET_OBJECT_TRACEFRAME_INFO,
+  /* Load maps for FDPIC systems.  */
+  TARGET_OBJECT_FDPIC
   /* Possible future objects: TARGET_OBJECT_FILE, ...  */
 };
 
-- 
1.7.0.4

Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> This patch is derived from bits from CodeSourcery SH-2A toolchain
Yao>   http://sourceware.org/ml/gdb-patches/2010-12/msg00291.html

Yao> DSBT's loadmap is similar to FDPIC's, so they can be handled in a
Yao> unified approach.  This patch adds a mechanism from gdb to gdbserver to
Yao> read loadmap.  This part is not c6x-specific, and can be reused by other
Yao> similar ports.

I can't really comment on the patch; but all additions to the remote
protocol should come with a documentation patch.

Tom

[RFA 9/8] New port: TI C6x: Document on qXfer:fdpic:read packet

[Yao Qi]

[-- Attachment #1: Type: text/plain, Size: 251 bytes --]

On 07/20/2011 10:15 PM, Tom Tromey wrote:
> I can't really comment on the patch; but all additions to the remote
> protocol should come with a documentation patch.

Right.  A documentation patch on this new packet is attached.

-- 
Yao (齐尧)

[-- Attachment #2: 0009-fdpic-packet-doc.patch --]
[-- Type: text/x-patch, Size: 1987 bytes --]

From 9db4e71b5d686ea22e9c7b4f0710094d3530977f Mon Sep 17 00:00:00 2001
From: Yao Qi <yao@codesourcery.com>
Date: Thu, 21 Jul 2011 11:42:20 +0800
Subject: [PATCH 09/15] fdpic packet doc

        gdb/doc/
        * gdb.texinfo : Document qXfer:fdpic:read packet.
---
 gdb/doc/gdb.texinfo |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 129a46b..37c81b8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -33654,6 +33654,10 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab Yes
 
+@item @samp{qXfer:fdpic:read}
+@tab No
+@tab @samp{-}
+@tab Yes
 
 @item @samp{QNonStop}
 @tab No
@@ -33765,6 +33769,10 @@ The remote stub understands the @samp{qXfer:threads:read} packet
 The remote stub understands the @samp{qXfer:traceframe-info:read}
 packet (@pxref{qXfer traceframe info read}).
 
+@item qXfer:fdpic:read
+The remote stub understands the @samp{qXfer:fdpic:read}
+packet (@pxref{qXfer fdpic loadmap read}).
+
 @item QNonStop
 The remote stub understands the @samp{QNonStop} packet
 (@pxref{QNonStop}).
@@ -34020,6 +34028,15 @@ Return a description of the current traceframe's contents.
 This packet is not probed by default; the remote stub must request it,
 by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
 
+@item qXfer:fdpic:read:@var{annex}:@var{offset},@var{length}
+@anchor{qXfer fdpic loadmap read}
+Read contents of @code{loadmap}s on the target system.  The
+annex, either @samp{exec} or @samp{interp}, specifies which @code{loadmap},
+executable @code{loadmap} or interpreter @code{loadmap} to read.
+
+This packet is not probed by default; the remote stub must request it,
+by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
+
 @item qXfer:osdata:read::@var{offset},@var{length}
 @anchor{qXfer osdata read}
 Access the target's @dfn{operating system information}.  
-- 
1.7.0.4

Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Mike Frysinger]

On Tue, Jul 19, 2011 at 22:07, Yao Qi wrote:
> +#if defined __DSBT__
> +static int

rather than being tied to the exec format that *gdbserver* is being
built as, shouldnt this be bound to the ptrace defines being available
?  how abut using "#ifdef PTRACE_GETDSBT" ?

> +linux_read_loadmap (const char *annex, CORE_ADDR offset,
> +                          unsigned char *myaddr, unsigned int len)

indentation looks wrong wrt GNU style.  needs tabs and spaces to
properly align it.

> +  int addr = (strcmp (annex, "exec") == 0 ? (int) PTRACE_GETDSBT_EXEC :
> +	      (strcmp (annex, "interp") == 0 ? (int) PTRACE_GETDSBT_INTERP :
> +	       -1));
> ...
> +  if (addr == -1)
> +    return -1;

style with add init is off here too.  but seems like it'd be cleaner
to have this be a series of if statements incorporated into the -1
check to make the -1 value unnecessary.

> +  copy_length = actual_length - offset < len ? actual_length - offset : len;

does gdb really not have min/max helpers ?

> +  ptrace (PTRACE_GETDSBT, pid, addr, &data);

what if it fails ?

> +    {"fdpic", handle_qxfer_fdpic}

style is broken.  needs space after "{", space before "}", and comma after "}"

> +  /* Load maps for FDPIC systems.  */
> +  TARGET_OBJECT_FDPIC

i think this needs a trailing comma
-mike

Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Yao Qi]

[-- Attachment #1: Type: text/plain, Size: 1681 bytes --]

On 07/22/2011 10:16 AM, Mike Frysinger wrote:

Mike, thanks for the review.

>> +#if defined __DSBT__
>> +static int
> 
> rather than being tied to the exec format that *gdbserver* is being
> built as, shouldnt this be bound to the ptrace defines being available
> ?  how abut using "#ifdef PTRACE_GETDSBT" ?
> 

Yeah, that makes sense.  Done.

>> +linux_read_loadmap (const char *annex, CORE_ADDR offset,
>> +                          unsigned char *myaddr, unsigned int len)
> 
> indentation looks wrong wrt GNU style.  needs tabs and spaces to
> properly align it.
> 

Fixed.

>> +  int addr = (strcmp (annex, "exec") == 0 ? (int) PTRACE_GETDSBT_EXEC :
>> +	      (strcmp (annex, "interp") == 0 ? (int) PTRACE_GETDSBT_INTERP :
>> +	       -1));
>> ...
>> +  if (addr == -1)
>> +    return -1;
> 
> style with add init is off here too.  but seems like it'd be cleaner
> to have this be a series of if statements incorporated into the -1
> check to make the -1 value unnecessary.
> 

Agreed.  Fixed.

>> +  copy_length = actual_length - offset < len ? actual_length - offset : len;
> 
> does gdb really not have min/max helpers ?
> 

gdb has, but gdbserver doesn't.  I'll leave the code here.  A follow-up
patch can move min/max helper into common/ and use them in gdbserver.

>> +  ptrace (PTRACE_GETDSBT, pid, addr, &data);
> 
> what if it fails ?
> 

We can return -1 if it fails.  Done in new patch.

>> +    {"fdpic", handle_qxfer_fdpic}
> 
> style is broken.  needs space after "{", space before "}", and comma after "}"
> 

Fixed.

>> +  /* Load maps for FDPIC systems.  */
>> +  TARGET_OBJECT_FDPIC
> 
> i think this needs a trailing comma

Fixed.

-- 
Yao (齐尧)

[-- Attachment #2: 0004-fdpic-packet-and-dsbt.patch --]
[-- Type: text/x-patch, Size: 9036 bytes --]

2011-07-19  Andrew Stubbs <ams@codesourcery.com>
            Yao Qi <yao@codesourcery.com>

        gdb/
        * remote.c (PACKET_qXfer_fdpic): New enum value.
        (remote_protocol_features): Add qXfer:fdpic:read packet.
        (remote_xfer_partial): Support TARGET_OBJECT_FDPIC.
        (_initialize_remote): Add set/show remote read-fdpic-loadmap command.
        * target.h (enum target_object): Add TARGET_OBJECT_FDPIC.

        gdb/gdbserver:
        * linux-low.c (struct target_loadseg): New type.
        (struct target_loadmap): New type.
        (linux_read_loadmap): New function.
        (linux_target_ops): Add linux_read_loadmap.
        * server.c (handle_query): Support qXfer:fdpic:read packet.
        * target.h (struct target_ops): Add read_loadmap.

        gdb/doc/
        * gdb.texinfo : Document qXfer:fdpic:read packet.
---
 gdb/doc/gdb.texinfo       |   17 ++++++++++++
 gdb/gdbserver/linux-low.c |   65 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/server.c    |   19 +++++++++++++
 gdb/gdbserver/target.h    |    4 +++
 gdb/remote.c              |   10 +++++++
 gdb/target.h              |    2 +
 6 files changed, 117 insertions(+), 0 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 129a46b..37c81b8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -33654,6 +33654,10 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab Yes
 
+@item @samp{qXfer:fdpic:read}
+@tab No
+@tab @samp{-}
+@tab Yes
 
 @item @samp{QNonStop}
 @tab No
@@ -33765,6 +33769,10 @@ The remote stub understands the @samp{qXfer:threads:read} packet
 The remote stub understands the @samp{qXfer:traceframe-info:read}
 packet (@pxref{qXfer traceframe info read}).
 
+@item qXfer:fdpic:read
+The remote stub understands the @samp{qXfer:fdpic:read}
+packet (@pxref{qXfer fdpic loadmap read}).
+
 @item QNonStop
 The remote stub understands the @samp{QNonStop} packet
 (@pxref{QNonStop}).
@@ -34020,6 +34028,15 @@ Return a description of the current traceframe's contents.
 This packet is not probed by default; the remote stub must request it,
 by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
 
+@item qXfer:fdpic:read:@var{annex}:@var{offset},@var{length}
+@anchor{qXfer fdpic loadmap read}
+Read contents of @code{loadmap}s on the target system.  The
+annex, either @samp{exec} or @samp{interp}, specifies which @code{loadmap},
+executable @code{loadmap} or interpreter @code{loadmap} to read.
+
+This packet is not probed by default; the remote stub must request it,
+by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
+
 @item qXfer:osdata:read::@var{offset},@var{length}
 @anchor{qXfer osdata read}
 Access the target's @dfn{operating system information}.  
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index d84fd68..f3641d0 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4964,6 +4964,66 @@ linux_core_of_thread (ptid_t ptid)
   return core;
 }
 
+#if defined __DSBT__
+struct target_loadseg
+{
+  /* Core address to which the segment is mapped.  */
+  Elf32_Addr addr;
+  /* VMA recorded in the program header.  */
+  Elf32_Addr p_vaddr;
+  /* Size of this segment in memory.  */
+  Elf32_Word p_memsz;
+};
+
+struct target_loadmap
+{
+  /* Protocol version number, must be zero.  */
+  Elf32_Word version;
+  /* Pointer to the DSBT table, its size, and the DSBT index.  */
+  unsigned *dsbt_table;
+  unsigned dsbt_size, dsbt_index;
+  /* Number of segments in this map.  */
+  Elf32_Word nsegs;
+  /* The actual memory map.  */
+  struct target_loadseg segs[/*nsegs*/];
+};
+#endif
+
+#if defined PTRACE_GETDSBT
+static int
+linux_read_loadmap (const char *annex, CORE_ADDR offset,
+		    unsigned char *myaddr, unsigned int len)
+{
+  int pid = lwpid_of (get_thread_lwp (current_inferior));
+  int addr = -1;
+  struct target_loadmap *data = NULL;
+  unsigned int actual_length, copy_length;
+
+  if (strcmp (annex, "exec") == 0)
+    addr= (int) PTRACE_GETDSBT_EXEC;
+  else if (strcmp (annex, "interp") == 0)
+    addr =(int) PTRACE_GETDSBT_INTERP;
+  else
+    return -1;
+
+  if (ptrace (PTRACE_GETDSBT, pid, addr, &data) != 0)
+    return -1;
+
+  if (data == NULL)
+    return -1;
+
+  actual_length = sizeof (struct target_loadmap)
+    + sizeof (struct target_loadseg) * data->nsegs;
+
+  if (offset < 0 || offset > actual_length)
+    return -1;
+
+  copy_length = actual_length - offset < len ? actual_length - offset : len;
+  memcpy (myaddr, ((char *)data) + offset, copy_length);
+  return copy_length;
+}
+#endif /* defined PTRACE_GETDSBT */
+
 static void
 linux_process_qsupported (const char *query)
 {
@@ -5112,6 +5172,11 @@ static struct target_ops linux_target_ops = {
   NULL,
 #endif
   linux_core_of_thread,
+#if defined PTRACE_GETDSBT
+  linux_read_loadmap,
+#else
+  NULL,
+#endif
   linux_process_qsupported,
   linux_supports_tracepoints,
   linux_read_pc,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index fd06c8f..7b19745 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1171,6 +1171,21 @@ handle_qxfer_traceframe_info (const char *annex,
   return len;
 }
 
+/* Handle qXfer:fdpic:read.  */
+
+static int
+handle_qxfer_fdpic (const char *annex, gdb_byte *readbuf,
+		    const gdb_byte *writebuf, ULONGEST offset, LONGEST len)
+{
+  if (the_target->read_fdpic_loadmap == NULL)
+    return -2;
+
+  if (!target_running ())
+    return -1;
+
+  return (*the_target->read_fdpic_loadmap) (annex, offset, readbuf, len);
+}
+
 static const struct qxfer qxfer_packets[] =
   {
     { "auxv", handle_qxfer_auxv },
@@ -1182,6 +1197,7 @@ static const struct qxfer qxfer_packets[] =
     { "statictrace", handle_qxfer_statictrace },
     { "threads", handle_qxfer_threads },
     { "traceframe-info", handle_qxfer_traceframe_info },
+    { "fdpic", handle_qxfer_fdpic},
   };
 
 static int
@@ -1509,6 +1525,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
       if (the_target->qxfer_siginfo != NULL)
 	strcat (own_buf, ";qXfer:siginfo:read+;qXfer:siginfo:write+");
 
+      if (the_target->read_fdpic_loadmap != NULL)
+	strcat (own_buf, ";qXfer:fdpic:read+");
+
       /* We always report qXfer:features:read, as targets may
 	 install XML files on a subsequent call to arch_setup.
 	 If we reported to GDB on startup that we don't support
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 00214db..60f4f34 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -311,6 +311,10 @@ struct target_ops
   /* Returns the core given a thread, or -1 if not known.  */
   int (*core_of_thread) (ptid_t);
 
+  /* Read FDPIC loadmaps.  Read LEN bytes at OFFSET into a buffer at MYADDR.  */
+  int (*read_fdpic_loadmap) (const char *annex, CORE_ADDR offset,
+			     unsigned char *myaddr, unsigned int len);
+
   /* Target specific qSupported support.  */
   void (*process_qsupported) (const char *);
 
diff --git a/gdb/remote.c b/gdb/remote.c
index b03ef59..9a1c653 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1262,6 +1262,7 @@ enum {
   PACKET_bs,
   PACKET_TracepointSource,
   PACKET_QAllow,
+  PACKET_qXfer_fdpic,
   PACKET_MAX
 };
 
@@ -3769,6 +3770,8 @@ static struct protocol_feature remote_protocol_features[] = {
     PACKET_QAllow },
   { "EnableDisableTracepoints", PACKET_DISABLE,
     remote_enable_disable_tracepoint_feature, -1 },
+  { "qXfer:fdpic:read", PACKET_DISABLE, remote_supported_packet,
+    PACKET_qXfer_fdpic },
 };
 
 static char *remote_support_xml;
@@ -8299,6 +8302,10 @@ remote_xfer_partial (struct target_ops *ops, enum target_object object,
       return remote_read_qxfer
 	(ops, "traceframe-info", annex, readbuf, offset, len,
 	 &remote_protocol_packets[PACKET_qXfer_traceframe_info]);
+
+    case TARGET_OBJECT_FDPIC:
+      return remote_read_qxfer (ops, "fdpic", annex, readbuf, offset, len,
+				&remote_protocol_packets[PACKET_qXfer_fdpic]);
     default:
       return -1;
     }
@@ -10903,6 +10910,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_statictrace_read],
                          "qXfer:statictrace:read", "read-sdata-object", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_fdpic],
+			 "qXfer:fdpic:read", "read-fdpic-loadmap", 0);
+
   /* Keep the old ``set remote Z-packet ...'' working.  Each individual
      Z sub-packet has its own set and show commands, but users may
      have sets to this variable in their .gdbinit files (or in their
diff --git a/gdb/target.h b/gdb/target.h
index 3b39d6f..e264657 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -274,6 +274,8 @@ enum target_object
   TARGET_OBJECT_HPUX_SOLIB_GOT,
   /* Traceframe info, in XML format.  */
   TARGET_OBJECT_TRACEFRAME_INFO,
+  /* Load maps for FDPIC systems.  */
+  TARGET_OBJECT_FDPIC,
   /* Possible future objects: TARGET_OBJECT_FILE, ...  */
 };
 
-- 
1.7.0.4

ping: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Yao Qi]

On 07/25/2011 12:27 PM, Yao Qi wrote:
> 2011-07-19  Andrew Stubbs <ams@codesourcery.com>
>             Yao Qi <yao@codesourcery.com>
> 
>         gdb/
>         * remote.c (PACKET_qXfer_fdpic): New enum value.
>         (remote_protocol_features): Add qXfer:fdpic:read packet.
>         (remote_xfer_partial): Support TARGET_OBJECT_FDPIC.
>         (_initialize_remote): Add set/show remote read-fdpic-loadmap command.
>         * target.h (enum target_object): Add TARGET_OBJECT_FDPIC.
> 
>         gdb/gdbserver:
>         * linux-low.c (struct target_loadseg): New type.
>         (struct target_loadmap): New type.
>         (linux_read_loadmap): New function.
>         (linux_target_ops): Add linux_read_loadmap.
>         * server.c (handle_query): Support qXfer:fdpic:read packet.
>         * target.h (struct target_ops): Add read_loadmap.
>

Ping.  http://sourceware.org/ml/gdb-patches/2011-07/msg00679.html

>         gdb/doc/
>         * gdb.texinfo : Document qXfer:fdpic:read packet.

	gdb/doc/
	* gdb.texinfo (General Query Packets): Document
	qXfer:fdpic:read packet.

Eli, how about documentation change in this patch?  ChangeLog entry is
updated a little bit with the name of the node.

-- 
Yao (齐尧)

Re: ping: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Eli Zaretskii]

> Date: Wed, 03 Aug 2011 09:28:44 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: Eli Zaretskii <eliz@gnu.org>
> 
> Ping.  http://sourceware.org/ml/gdb-patches/2011-07/msg00679.html
> 
> >         gdb/doc/
> >         * gdb.texinfo : Document qXfer:fdpic:read packet.
> 
> 	gdb/doc/
> 	* gdb.texinfo (General Query Packets): Document
> 	qXfer:fdpic:read packet.
> 
> Eli, how about documentation change in this patch?  ChangeLog entry is
> updated a little bit with the name of the node.

It's okay, thanks.  And sorry for the delay.

Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Mike Frysinger]

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

On Monday, July 25, 2011 00:27:50 Yao Qi wrote:
> On 07/22/2011 10:16 AM, Mike Frysinger wrote:
> >> +#if defined __DSBT__
> >> +static int
> > 
> > rather than being tied to the exec format that *gdbserver* is being
> > built as, shouldnt this be bound to the ptrace defines being available
> > ?  how abut using "#ifdef PTRACE_GETDSBT" ?
> 
> Yeah, that makes sense.  Done.

i think you missed a spot.  one place uses __DSBT__ while another uses 
PTRACE_GETDSBT.

> +    addr= (int) PTRACE_GETDSBT_EXEC;
> +  else if (strcmp (annex, "interp") == 0)
> +    addr =(int) PTRACE_GETDSBT_INTERP;

there needs to be one space before and after the "=" here
-mike

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

Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Yao Qi]

[-- Attachment #1: Type: text/plain, Size: 1049 bytes --]

On 08/08/2011 08:30 AM, Mike Frysinger wrote:
> On Monday, July 25, 2011 00:27:50 Yao Qi wrote:
>> On 07/22/2011 10:16 AM, Mike Frysinger wrote:
>>>> +#if defined __DSBT__
>>>> +static int
>>>
>>> rather than being tied to the exec format that *gdbserver* is being
>>> built as, shouldnt this be bound to the ptrace defines being available
>>> ?  how abut using "#ifdef PTRACE_GETDSBT" ?
>>
>> Yeah, that makes sense.  Done.
> 
> i think you missed a spot.  one place uses __DSBT__ while another uses 
> PTRACE_GETDSBT.

Yes, I thought it is not quite related to PTRACE stuff, so I didn't
change that.  I agree that we should use macros in a consistent way.  I
replace PTRACE_GETDSBT and __DSBT__ with PT_GETDSBT, because PT_GETDSBT
is defined in sys/ptrace.h, but PTRACE_GETDSBT is defined in asm/ptrace.h.

> 
>> +    addr= (int) PTRACE_GETDSBT_EXEC;
>> +  else if (strcmp (annex, "interp") == 0)
>> +    addr =(int) PTRACE_GETDSBT_INTERP;
> 
> there needs to be one space before and after the "=" here

oh, thanks, fixed.

-- 
Yao (齐尧)

[-- Attachment #2: 0004-fdpic-packet-and-dsbt.patch --]
[-- Type: text/x-patch, Size: 9028 bytes --]


2011-07-19  Andrew Stubbs <ams@codesourcery.com>
            Yao Qi <yao@codesourcery.com>

        gdb/
        * remote.c (PACKET_qXfer_fdpic): New enum value.
        (remote_protocol_features): Add qXfer:fdpic:read packet.
        (remote_xfer_partial): Support TARGET_OBJECT_FDPIC.
        (_initialize_remote): Add set/show remote read-fdpic-loadmap command.
        * target.h (enum target_object): Add TARGET_OBJECT_FDPIC.

        gdb/gdbserver:
        * linux-low.c (struct target_loadseg): New type.
        (struct target_loadmap): New type.
        (linux_read_loadmap): New function.
        (linux_target_ops): Add linux_read_loadmap.
        * server.c (handle_query): Support qXfer:fdpic:read packet.
        * target.h (struct target_ops): Add read_loadmap.

        gdb/doc/
        * gdb.texinfo : Document qXfer:fdpic:read packet.
---
 gdb/doc/gdb.texinfo       |   17 ++++++++++++
 gdb/gdbserver/linux-low.c |   65 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/server.c    |   19 +++++++++++++
 gdb/gdbserver/target.h    |    4 +++
 gdb/remote.c              |   10 +++++++
 gdb/target.h              |    2 +
 6 files changed, 117 insertions(+), 0 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 129a46b..37c81b8 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -33654,6 +33654,10 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab Yes
 
+@item @samp{qXfer:fdpic:read}
+@tab No
+@tab @samp{-}
+@tab Yes
 
 @item @samp{QNonStop}
 @tab No
@@ -33765,6 +33769,10 @@ The remote stub understands the @samp{qXfer:threads:read} packet
 The remote stub understands the @samp{qXfer:traceframe-info:read}
 packet (@pxref{qXfer traceframe info read}).
 
+@item qXfer:fdpic:read
+The remote stub understands the @samp{qXfer:fdpic:read}
+packet (@pxref{qXfer fdpic loadmap read}).
+
 @item QNonStop
 The remote stub understands the @samp{QNonStop} packet
 (@pxref{QNonStop}).
@@ -34020,6 +34028,15 @@ Return a description of the current traceframe's contents.
 This packet is not probed by default; the remote stub must request it,
 by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
 
+@item qXfer:fdpic:read:@var{annex}:@var{offset},@var{length}
+@anchor{qXfer fdpic loadmap read}
+Read contents of @code{loadmap}s on the target system.  The
+annex, either @samp{exec} or @samp{interp}, specifies which @code{loadmap},
+executable @code{loadmap} or interpreter @code{loadmap} to read.
+
+This packet is not probed by default; the remote stub must request it,
+by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
+
 @item qXfer:osdata:read::@var{offset},@var{length}
 @anchor{qXfer osdata read}
 Access the target's @dfn{operating system information}.  
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index d84fd68..f8c3170 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4964,6 +4964,66 @@ linux_core_of_thread (ptid_t ptid)
   return core;
 }
 
+#if defined PT_GETDSBT
+struct target_loadseg
+{
+  /* Core address to which the segment is mapped.  */
+  Elf32_Addr addr;
+  /* VMA recorded in the program header.  */
+  Elf32_Addr p_vaddr;
+  /* Size of this segment in memory.  */
+  Elf32_Word p_memsz;
+};
+
+struct target_loadmap
+{
+  /* Protocol version number, must be zero.  */
+  Elf32_Word version;
+  /* Pointer to the DSBT table, its size, and the DSBT index.  */
+  unsigned *dsbt_table;
+  unsigned dsbt_size, dsbt_index;
+  /* Number of segments in this map.  */
+  Elf32_Word nsegs;
+  /* The actual memory map.  */
+  struct target_loadseg segs[/*nsegs*/];
+};
+#endif
+
+#if defined PT_GETDSBT
+static int
+linux_read_loadmap (const char *annex, CORE_ADDR offset,
+		    unsigned char *myaddr, unsigned int len)
+{
+  int pid = lwpid_of (get_thread_lwp (current_inferior));
+  int addr = -1;
+  struct target_loadmap *data = NULL;
+  unsigned int actual_length, copy_length;
+
+  if (strcmp (annex, "exec") == 0)
+    addr= (int) PTRACE_GETDSBT_EXEC;
+  else if (strcmp (annex, "interp") == 0)
+    addr = (int) PTRACE_GETDSBT_INTERP;
+  else
+    return -1;
+
+  if (ptrace (PTRACE_GETDSBT, pid, addr, &data) != 0)
+    return -1;
+
+  if (data == NULL)
+    return -1;
+
+  actual_length = sizeof (struct target_loadmap)
+    + sizeof (struct target_loadseg) * data->nsegs;
+
+  if (offset < 0 || offset > actual_length)
+    return -1;
+
+  copy_length = actual_length - offset < len ? actual_length - offset : len;
+  memcpy (myaddr, ((char *)data) + offset, copy_length);
+  return copy_length;
+}
+#endif /* defined PT_GETDSBT */
+
 static void
 linux_process_qsupported (const char *query)
 {
@@ -5112,6 +5172,11 @@ static struct target_ops linux_target_ops = {
   NULL,
 #endif
   linux_core_of_thread,
+#if defined PT_GETDSBT
+  linux_read_loadmap,
+#else
+  NULL,
+#endif
   linux_process_qsupported,
   linux_supports_tracepoints,
   linux_read_pc,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index fd06c8f..7b19745 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1171,6 +1171,21 @@ handle_qxfer_traceframe_info (const char *annex,
   return len;
 }
 
+/* Handle qXfer:fdpic:read.  */
+
+static int
+handle_qxfer_fdpic (const char *annex, gdb_byte *readbuf,
+		    const gdb_byte *writebuf, ULONGEST offset, LONGEST len)
+{
+  if (the_target->read_fdpic_loadmap == NULL)
+    return -2;
+
+  if (!target_running ())
+    return -1;
+
+  return (*the_target->read_fdpic_loadmap) (annex, offset, readbuf, len);
+}
+
 static const struct qxfer qxfer_packets[] =
   {
     { "auxv", handle_qxfer_auxv },
@@ -1182,6 +1197,7 @@ static const struct qxfer qxfer_packets[] =
     { "statictrace", handle_qxfer_statictrace },
     { "threads", handle_qxfer_threads },
     { "traceframe-info", handle_qxfer_traceframe_info },
+    { "fdpic", handle_qxfer_fdpic},
   };
 
 static int
@@ -1509,6 +1525,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
       if (the_target->qxfer_siginfo != NULL)
 	strcat (own_buf, ";qXfer:siginfo:read+;qXfer:siginfo:write+");
 
+      if (the_target->read_fdpic_loadmap != NULL)
+	strcat (own_buf, ";qXfer:fdpic:read+");
+
       /* We always report qXfer:features:read, as targets may
 	 install XML files on a subsequent call to arch_setup.
 	 If we reported to GDB on startup that we don't support
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 00214db..60f4f34 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -311,6 +311,10 @@ struct target_ops
   /* Returns the core given a thread, or -1 if not known.  */
   int (*core_of_thread) (ptid_t);
 
+  /* Read FDPIC loadmaps.  Read LEN bytes at OFFSET into a buffer at MYADDR.  */
+  int (*read_fdpic_loadmap) (const char *annex, CORE_ADDR offset,
+			     unsigned char *myaddr, unsigned int len);
+
   /* Target specific qSupported support.  */
   void (*process_qsupported) (const char *);
 
diff --git a/gdb/remote.c b/gdb/remote.c
index b03ef59..9a1c653 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1262,6 +1262,7 @@ enum {
   PACKET_bs,
   PACKET_TracepointSource,
   PACKET_QAllow,
+  PACKET_qXfer_fdpic,
   PACKET_MAX
 };
 
@@ -3769,6 +3770,8 @@ static struct protocol_feature remote_protocol_features[] = {
     PACKET_QAllow },
   { "EnableDisableTracepoints", PACKET_DISABLE,
     remote_enable_disable_tracepoint_feature, -1 },
+  { "qXfer:fdpic:read", PACKET_DISABLE, remote_supported_packet,
+    PACKET_qXfer_fdpic },
 };
 
 static char *remote_support_xml;
@@ -8299,6 +8302,10 @@ remote_xfer_partial (struct target_ops *ops, enum target_object object,
       return remote_read_qxfer
 	(ops, "traceframe-info", annex, readbuf, offset, len,
 	 &remote_protocol_packets[PACKET_qXfer_traceframe_info]);
+
+    case TARGET_OBJECT_FDPIC:
+      return remote_read_qxfer (ops, "fdpic", annex, readbuf, offset, len,
+				&remote_protocol_packets[PACKET_qXfer_fdpic]);
     default:
       return -1;
     }
@@ -10903,6 +10910,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_statictrace_read],
                          "qXfer:statictrace:read", "read-sdata-object", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_fdpic],
+			 "qXfer:fdpic:read", "read-fdpic-loadmap", 0);
+
   /* Keep the old ``set remote Z-packet ...'' working.  Each individual
      Z sub-packet has its own set and show commands, but users may
      have sets to this variable in their .gdbinit files (or in their
diff --git a/gdb/target.h b/gdb/target.h
index 3b39d6f..e264657 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -274,6 +274,8 @@ enum target_object
   TARGET_OBJECT_HPUX_SOLIB_GOT,
   /* Traceframe info, in XML format.  */
   TARGET_OBJECT_TRACEFRAME_INFO,
+  /* Load maps for FDPIC systems.  */
+  TARGET_OBJECT_FDPIC,
   /* Possible future objects: TARGET_OBJECT_FILE, ...  */
 };
 
-- 
1.7.0.4

Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Mark Kettenis]

> Date: Mon, 08 Aug 2011 10:46:29 +0800
> From: Yao Qi <yao@codesourcery.com>
> 
> On 08/08/2011 08:30 AM, Mike Frysinger wrote:
> > On Monday, July 25, 2011 00:27:50 Yao Qi wrote:
> >> On 07/22/2011 10:16 AM, Mike Frysinger wrote:
> >>>> +#if defined __DSBT__
> >>>> +static int
> >>>
> >>> rather than being tied to the exec format that *gdbserver* is being
> >>> built as, shouldnt this be bound to the ptrace defines being available
> >>> ?  how abut using "#ifdef PTRACE_GETDSBT" ?
> >>
> >> Yeah, that makes sense.  Done.
> > 
> > i think you missed a spot.  one place uses __DSBT__ while another uses 
> > PTRACE_GETDSBT.
> 
> Yes, I thought it is not quite related to PTRACE stuff, so I didn't
> change that.  I agree that we should use macros in a consistent way.  I
> replace PTRACE_GETDSBT and __DSBT__ with PT_GETDSBT, because PT_GETDSBT
> is defined in sys/ptrace.h, but PTRACE_GETDSBT is defined in asm/ptrace.h.

Well, that leads to the following funny situation:

> +#if defined PT_GETDSBT

You check for PT_GETDSBT here...

> +static int
> +linux_read_loadmap (const char *annex, CORE_ADDR offset,
> +		    unsigned char *myaddr, unsigned int len)
> +{
> +  int pid = lwpid_of (get_thread_lwp (current_inferior));
> +  int addr = -1;
> +  struct target_loadmap *data = NULL;
> +  unsigned int actual_length, copy_length;
> +
> +  if (strcmp (annex, "exec") == 0)
> +    addr= (int) PTRACE_GETDSBT_EXEC;
> +  else if (strcmp (annex, "interp") == 0)
> +    addr = (int) PTRACE_GETDSBT_INTERP;
> +  else
> +    return -1;
> +
> +  if (ptrace (PTRACE_GETDSBT, pid, addr, &data) != 0)

...but use PTRACE_GETDSBT here.

Now the kernel vs. libc headers issue has always been a contentious
one on Linux.  But I think you should use the PTRACE_-prefixed names
in your code since those are the "official" Linux names, since Linux
was intended to be System V compatible.  The PT_-prefixed names are
really only for compatibility with BSD (So I don't really understand
why people keep adding them for ptrace(2) requests that no BSD variant
ever had).

Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Yao Qi]

On 08/08/2011 04:28 PM, Mark Kettenis wrote:
> Now the kernel vs. libc headers issue has always been a contentious
> one on Linux.  But I think you should use the PTRACE_-prefixed names
> in your code since those are the "official" Linux names, since Linux
> was intended to be System V compatible.  The PT_-prefixed names are
> really only for compatibility with BSD (So I don't really understand
> why people keep adding them for ptrace(2) requests that no BSD variant
> ever had).

Mark, thanks for pointing this out.  If we want to use PTRACE_GETDSBT
here, we should include "asm/ptrace.h" in linux-low.c, which I am
hesitant to do.  Current convention in linux-low.c, AFAICT, is about
target-independent code, and so "asm/ptrace.h" is not included.  I don't
want to break this convention to include "asm/ptrace.h", but I am not
insist on this.  If you believe it is safe to include "asm/ptrace.h", I
am fine with it.

-- 
Yao (齐尧)

Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Pedro Alves]

On Monday 08 August 2011 03:46:29, Yao Qi wrote:
> @@ -1182,6 +1197,7 @@ static const struct qxfer qxfer_packets[] =
>      { "statictrace", handle_qxfer_statictrace },
>      { "threads", handle_qxfer_threads },
>      { "traceframe-info", handle_qxfer_traceframe_info },
> +    { "fdpic", handle_qxfer_fdpic},

Please keep the list alpha sorted.

> +  memcpy (myaddr, ((char *)data) + offset, copy_length);

  memcpy (myaddr, (char *) data + offset, copy_length);

> @@ -5112,6 +5172,11 @@ static struct target_ops linux_target_ops = {
>    NULL,
>  #endif
>    linux_core_of_thread,
> +#if defined PT_GETDSBT
> +  linux_read_loadmap,
> +#else
> +  NULL,
> +#endif
>    linux_process_qsupported,
>    linux_supports_tracepoints,
>    linux_read_pc,

Looks like you may be breaking all !tic6x ports.  Either
put the new method at the end, or update all 
struct target_ops instances to adjust for the new field.

Otherwise, with Mark's comments addressed, this looks
fine to me.

Is there any patch in the series missing review?

Please take care to apply the patches in a sequence
where none breaks the build, to avoid breaking bisects.

-- 
Pedro Alves

Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Mark Kettenis]

> Date: Mon, 08 Aug 2011 21:04:51 +0800
> From: Yao Qi <yao@codesourcery.com>
> 
> On 08/08/2011 04:28 PM, Mark Kettenis wrote:
> > Now the kernel vs. libc headers issue has always been a contentious
> > one on Linux.  But I think you should use the PTRACE_-prefixed names
> > in your code since those are the "official" Linux names, since Linux
> > was intended to be System V compatible.  The PT_-prefixed names are
> > really only for compatibility with BSD (So I don't really understand
> > why people keep adding them for ptrace(2) requests that no BSD variant
> > ever had).
> 
> Mark, thanks for pointing this out.  If we want to use PTRACE_GETDSBT
> here, we should include "asm/ptrace.h" in linux-low.c, which I am
> hesitant to do.  Current convention in linux-low.c, AFAICT, is about
> target-independent code, and so "asm/ptrace.h" is not included.  I don't
> want to break this convention to include "asm/ptrace.h", but I am not
> insist on this.  If you believe it is safe to include "asm/ptrace.h", I
> am fine with it.

Bleah.  Assuming your TI C6x system's libc headers are somewhat
similar to my amd64 system I see your problem.  The <sys/ptrace.h>
header file uses an enum for the PTRACE_XXX values, and only has
#defines for the PT_XXX values.

I think you have two choices:

1. Add a configure test for PTRACE_GETDSBT.

2. s/PTRACE_/PT_/g everywehere in your diff.

Personally I'd prefer choice #2, since we already have to many
configure checks ;).

Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Yao Qi]

[-- Attachment #1: Type: text/plain, Size: 2081 bytes --]

On 08/08/2011 09:31 PM, Pedro Alves wrote:
> On Monday 08 August 2011 03:46:29, Yao Qi wrote:
>> @@ -1182,6 +1197,7 @@ static const struct qxfer qxfer_packets[] =
>>      { "statictrace", handle_qxfer_statictrace },
>>      { "threads", handle_qxfer_threads },
>>      { "traceframe-info", handle_qxfer_traceframe_info },
>> +    { "fdpic", handle_qxfer_fdpic},
> 
> Please keep the list alpha sorted.
> 

OK.  Fixed.

>> +  memcpy (myaddr, ((char *)data) + offset, copy_length);
> 
>   memcpy (myaddr, (char *) data + offset, copy_length);
> 
>> @@ -5112,6 +5172,11 @@ static struct target_ops linux_target_ops = {
>>    NULL,
>>  #endif
>>    linux_core_of_thread,
>> +#if defined PT_GETDSBT
>> +  linux_read_loadmap,
>> +#else
>> +  NULL,
>> +#endif
>>    linux_process_qsupported,
>>    linux_supports_tracepoints,
>>    linux_read_pc,
> 
> Looks like you may be breaking all !tic6x ports.  Either
> put the new method at the end, or update all 
> struct target_ops instances to adjust for the new field.
> 

There are five instances of target_ops in gdbserver, they are in
linux-low.c, lynx-low.c, nto-low.c, spu-low.c, and win32-low.c.  Adding
new field linux_read_loadmap has only affect on target_ops instance in
win32-low.c.  The rest instances are too short to reach field
linux_read_loadmap and fields after it.  Updated target_ops instance in
win32-low.c.

> Otherwise, with Mark's comments addressed, this looks
> fine to me.
> 
I take Mark's choice #2 on the macro of PTRACE_GETDSBT.  PTRACE_GETDSBT
is replaced with PT_GETDSBT in new patch.  I leave PTRACE_GETDSBT_EXEC
and PTRACE_GETDSBT_INTERP there, because they are macros and no PT_*
counterparts defined in headers.

> Is there any patch in the series missing review?
> 

The last bit is about test case patch,

  [RFA 7/8] New port: TI C6x: test case fixes
  http://sourceware.org/ml/gdb-patches/2011-07/msg00682.html

> Please take care to apply the patches in a sequence
> where none breaks the build, to avoid breaking bisects.
> 

Thanks for reminding this.  I'll take care of it.

-- 
Yao (齐尧)

[-- Attachment #2: 0004-fdpic-packet-and-dsbt.patch --]
[-- Type: text/x-patch, Size: 9492 bytes --]


2011-08-09  Andrew Stubbs <ams@codesourcery.com>
            Yao Qi <yao@codesourcery.com>

        gdb/
        * remote.c (PACKET_qXfer_fdpic): New enum value.
        (remote_protocol_features): Add qXfer:fdpic:read packet.
        (remote_xfer_partial): Support TARGET_OBJECT_FDPIC.
        (_initialize_remote): Add set/show remote read-fdpic-loadmap command.
        * target.h (enum target_object): Add TARGET_OBJECT_FDPIC.

        gdb/gdbserver:
        * target.h (struct target_ops): Add read_loadmap.
        * linux-low.c (struct target_loadseg): New type.
        (struct target_loadmap): New type.
        (linux_read_loadmap): New function.
        (linux_target_ops): Add linux_read_loadmap.
        * server.c (handle_query): Support qXfer:fdpic:read packet.
	* win32-low.c (win32_target_ops): Initialize field `read_loadmap' to NULL.

        gdb/doc/
        * gdb.texinfo : Document qXfer:fdpic:read packet.
---
 gdb/doc/gdb.texinfo       |   17 ++++++++++++
 gdb/gdbserver/linux-low.c |   65 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/server.c    |   19 +++++++++++++
 gdb/gdbserver/target.h    |    4 +++
 gdb/gdbserver/win32-low.c |    1 +
 gdb/remote.c              |   10 +++++++
 gdb/target.h              |    2 +
 7 files changed, 118 insertions(+), 0 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 35fa075..e7ce78b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -33764,6 +33764,10 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab Yes
 
+@item @samp{qXfer:fdpic:read}
+@tab No
+@tab @samp{-}
+@tab Yes
 
 @item @samp{QNonStop}
 @tab No
@@ -33875,6 +33879,10 @@ The remote stub understands the @samp{qXfer:threads:read} packet
 The remote stub understands the @samp{qXfer:traceframe-info:read}
 packet (@pxref{qXfer traceframe info read}).
 
+@item qXfer:fdpic:read
+The remote stub understands the @samp{qXfer:fdpic:read}
+packet (@pxref{qXfer fdpic loadmap read}).
+
 @item QNonStop
 The remote stub understands the @samp{QNonStop} packet
 (@pxref{QNonStop}).
@@ -34130,6 +34138,15 @@ Return a description of the current traceframe's contents.
 This packet is not probed by default; the remote stub must request it,
 by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
 
+@item qXfer:fdpic:read:@var{annex}:@var{offset},@var{length}
+@anchor{qXfer fdpic loadmap read}
+Read contents of @code{loadmap}s on the target system.  The
+annex, either @samp{exec} or @samp{interp}, specifies which @code{loadmap},
+executable @code{loadmap} or interpreter @code{loadmap} to read.
+
+This packet is not probed by default; the remote stub must request it,
+by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
+
 @item qXfer:osdata:read::@var{offset},@var{length}
 @anchor{qXfer osdata read}
 Access the target's @dfn{operating system information}.  
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index a9cdaca..999ca92 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4654,6 +4654,66 @@ linux_qxfer_spu (const char *annex, unsigned char *readbuf,
   return ret;
 }
 
+#if defined PT_GETDSBT
+struct target_loadseg
+{
+  /* Core address to which the segment is mapped.  */
+  Elf32_Addr addr;
+  /* VMA recorded in the program header.  */
+  Elf32_Addr p_vaddr;
+  /* Size of this segment in memory.  */
+  Elf32_Word p_memsz;
+};
+
+struct target_loadmap
+{
+  /* Protocol version number, must be zero.  */
+  Elf32_Word version;
+  /* Pointer to the DSBT table, its size, and the DSBT index.  */
+  unsigned *dsbt_table;
+  unsigned dsbt_size, dsbt_index;
+  /* Number of segments in this map.  */
+  Elf32_Word nsegs;
+  /* The actual memory map.  */
+  struct target_loadseg segs[/*nsegs*/];
+};
+#endif
+
+#if defined PT_GETDSBT
+static int
+linux_read_loadmap (const char *annex, CORE_ADDR offset,
+		    unsigned char *myaddr, unsigned int len)
+{
+  int pid = lwpid_of (get_thread_lwp (current_inferior));
+  int addr = -1;
+  struct target_loadmap *data = NULL;
+  unsigned int actual_length, copy_length;
+
+  if (strcmp (annex, "exec") == 0)
+    addr= (int) PTRACE_GETDSBT_EXEC;
+  else if (strcmp (annex, "interp") == 0)
+    addr = (int) PTRACE_GETDSBT_INTERP;
+  else
+    return -1;
+
+  if (ptrace (PT_GETDSBT, pid, addr, &data) != 0)
+    return -1;
+
+  if (data == NULL)
+    return -1;
+
+  actual_length = sizeof (struct target_loadmap)
+    + sizeof (struct target_loadseg) * data->nsegs;
+
+  if (offset < 0 || offset > actual_length)
+    return -1;
+
+  copy_length = actual_length - offset < len ? actual_length - offset : len;
+  memcpy (myaddr, (char *) data + offset, copy_length);
+  return copy_length;
+}
+#endif /* defined PT_GETDSBT */
+
 static void
 linux_process_qsupported (const char *query)
 {
@@ -4802,6 +4862,11 @@ 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,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index a87aef5..a97bf86 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1171,9 +1171,25 @@ handle_qxfer_traceframe_info (const char *annex,
   return len;
 }
 
+/* Handle qXfer:fdpic:read.  */
+
+static int
+handle_qxfer_fdpic (const char *annex, gdb_byte *readbuf,
+		    const gdb_byte *writebuf, ULONGEST offset, LONGEST len)
+{
+  if (the_target->read_loadmap == NULL)
+    return -2;
+
+  if (!target_running ())
+    return -1;
+
+  return (*the_target->read_loadmap) (annex, offset, readbuf, len);
+}
+
 static const struct qxfer qxfer_packets[] =
   {
     { "auxv", handle_qxfer_auxv },
+    { "fdpic", handle_qxfer_fdpic},
     { "features", handle_qxfer_features },
     { "libraries", handle_qxfer_libraries },
     { "osdata", handle_qxfer_osdata },
@@ -1509,6 +1525,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
       if (the_target->qxfer_siginfo != NULL)
 	strcat (own_buf, ";qXfer:siginfo:read+;qXfer:siginfo:write+");
 
+      if (the_target->read_loadmap != NULL)
+	strcat (own_buf, ";qXfer:fdpic:read+");
+
       /* We always report qXfer:features:read, as targets may
 	 install XML files on a subsequent call to arch_setup.
 	 If we reported to GDB on startup that we don't support
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 00214db..3a823a1 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -311,6 +311,10 @@ struct target_ops
   /* Returns the core given a thread, or -1 if not known.  */
   int (*core_of_thread) (ptid_t);
 
+  /* Read loadmaps.  Read LEN bytes at OFFSET into a buffer at MYADDR.  */
+  int (*read_loadmap) (const char *annex, CORE_ADDR offset,
+			     unsigned char *myaddr, unsigned int len);
+
   /* Target specific qSupported support.  */
   void (*process_qsupported) (const char *);
 
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index cc4e23d..728af50 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -1811,6 +1811,7 @@ static struct target_ops win32_target_ops = {
   NULL, /* supports_multi_process */
   NULL, /* handle_monitor_command */
   NULL, /* core_of_thread */
+  NULL, /* read_fdpic_loadmap */
   NULL, /* process_qsupported */
   NULL, /* supports_tracepoints */
   NULL, /* read_pc */
diff --git a/gdb/remote.c b/gdb/remote.c
index fd4852f..7acbc71 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1262,6 +1262,7 @@ enum {
   PACKET_bs,
   PACKET_TracepointSource,
   PACKET_QAllow,
+  PACKET_qXfer_fdpic,
   PACKET_MAX
 };
 
@@ -3758,6 +3759,8 @@ static struct protocol_feature remote_protocol_features[] = {
     PACKET_QAllow },
   { "EnableDisableTracepoints", PACKET_DISABLE,
     remote_enable_disable_tracepoint_feature, -1 },
+  { "qXfer:fdpic:read", PACKET_DISABLE, remote_supported_packet,
+    PACKET_qXfer_fdpic },
 };
 
 static char *remote_support_xml;
@@ -8302,6 +8305,10 @@ remote_xfer_partial (struct target_ops *ops, enum target_object object,
       return remote_read_qxfer
 	(ops, "traceframe-info", annex, readbuf, offset, len,
 	 &remote_protocol_packets[PACKET_qXfer_traceframe_info]);
+
+    case TARGET_OBJECT_FDPIC:
+      return remote_read_qxfer (ops, "fdpic", annex, readbuf, offset, len,
+				&remote_protocol_packets[PACKET_qXfer_fdpic]);
     default:
       return -1;
     }
@@ -10930,6 +10937,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_statictrace_read],
                          "qXfer:statictrace:read", "read-sdata-object", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_fdpic],
+			 "qXfer:fdpic:read", "read-fdpic-loadmap", 0);
+
   /* Keep the old ``set remote Z-packet ...'' working.  Each individual
      Z sub-packet has its own set and show commands, but users may
      have sets to this variable in their .gdbinit files (or in their
diff --git a/gdb/target.h b/gdb/target.h
index 3b39d6f..e264657 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -274,6 +274,8 @@ enum target_object
   TARGET_OBJECT_HPUX_SOLIB_GOT,
   /* Traceframe info, in XML format.  */
   TARGET_OBJECT_TRACEFRAME_INFO,
+  /* Load maps for FDPIC systems.  */
+  TARGET_OBJECT_FDPIC,
   /* Possible future objects: TARGET_OBJECT_FILE, ...  */
 };
 
-- 
1.7.0.4

Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Mark Kettenis]

> > Otherwise, with Mark's comments addressed, this looks
> > fine to me.
> > 
> I take Mark's choice #2 on the macro of PTRACE_GETDSBT.  PTRACE_GETDSBT
> is replaced with PT_GETDSBT in new patch.  I leave PTRACE_GETDSBT_EXEC
> and PTRACE_GETDSBT_INTERP there, because they are macros and no PT_*
> counterparts defined in headers.

Sigh.  I guess there is no way we can win here; we'll just have to
accept these inconsistencies.

Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Pedro Alves]

On Tuesday 09 August 2011 04:11:33, Yao Qi wrote:
> 2011-08-09  Andrew Stubbs <ams@codesourcery.com>
>             Yao Qi <yao@codesourcery.com>
> 
>         gdb/
>         * remote.c (PACKET_qXfer_fdpic): New enum value.
>         (remote_protocol_features): Add qXfer:fdpic:read packet.
>         (remote_xfer_partial): Support TARGET_OBJECT_FDPIC.
>         (_initialize_remote): Add set/show remote read-fdpic-loadmap command.
>         * target.h (enum target_object): Add TARGET_OBJECT_FDPIC.
> 
>         gdb/gdbserver:
>         * target.h (struct target_ops): Add read_loadmap.
>         * linux-low.c (struct target_loadseg): New type.
>         (struct target_loadmap): New type.
>         (linux_read_loadmap): New function.
>         (linux_target_ops): Add linux_read_loadmap.
>         * server.c (handle_query): Support qXfer:fdpic:read packet.
>         * win32-low.c (win32_target_ops): Initialize field `read_loadmap' to NULL.
> 
>         gdb/doc/
>         * gdb.texinfo : Document qXfer:fdpic:read packet.

Thanks.  Looks good to me.

-- 
Pedro Alves

Re: [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Pedro Alves]

On Wednesday 10 August 2011 15:03:27, Pedro Alves wrote:
> On Tuesday 09 August 2011 04:11:33, Yao Qi wrote:
> > 2011-08-09  Andrew Stubbs <ams@codesourcery.com>
> >             Yao Qi <yao@codesourcery.com>
> > 
> >         gdb/
> >         * remote.c (PACKET_qXfer_fdpic): New enum value.
> >         (remote_protocol_features): Add qXfer:fdpic:read packet.
> >         (remote_xfer_partial): Support TARGET_OBJECT_FDPIC.
> >         (_initialize_remote): Add set/show remote read-fdpic-loadmap command.
> >         * target.h (enum target_object): Add TARGET_OBJECT_FDPIC.
> > 
> >         gdb/gdbserver:
> >         * target.h (struct target_ops): Add read_loadmap.
> >         * linux-low.c (struct target_loadseg): New type.
> >         (struct target_loadmap): New type.
> >         (linux_read_loadmap): New function.
> >         (linux_target_ops): Add linux_read_loadmap.
> >         * server.c (handle_query): Support qXfer:fdpic:read packet.
> >         * win32-low.c (win32_target_ops): Initialize field `read_loadmap' to NULL.
> > 
> >         gdb/doc/
> >         * gdb.texinfo : Document qXfer:fdpic:read packet.
> 
> Thanks.  Looks good to me.

Oh, just one detail:

> +  /* Read loadmaps.  Read LEN bytes at OFFSET into a buffer at MYADDR.  */
> +  int (*read_loadmap) (const char *annex, CORE_ADDR offset,
> +                            unsigned char *myaddr, unsigned int len);

...

> +  NULL, /* read_fdpic_loadmap */

Please make those coincide though.

-- 
Pedro Alves

[committed] [RFA 4/8] New port: TI C6x: Read loadmap from gdbserver

[Yao Qi]

On 08/10/2011 10:08 PM, Pedro Alves wrote:
> On Wednesday 10 August 2011 15:03:27, Pedro Alves wrote:
>> On Tuesday 09 August 2011 04:11:33, Yao Qi wrote:
>>> 2011-08-09  Andrew Stubbs <ams@codesourcery.com>
>>>             Yao Qi <yao@codesourcery.com>
>>>
>>>         gdb/
>>>         * remote.c (PACKET_qXfer_fdpic): New enum value.
>>>         (remote_protocol_features): Add qXfer:fdpic:read packet.
>>>         (remote_xfer_partial): Support TARGET_OBJECT_FDPIC.
>>>         (_initialize_remote): Add set/show remote read-fdpic-loadmap command.
>>>         * target.h (enum target_object): Add TARGET_OBJECT_FDPIC.
>>>
>>>         gdb/gdbserver:
>>>         * target.h (struct target_ops): Add read_loadmap.
>>>         * linux-low.c (struct target_loadseg): New type.
>>>         (struct target_loadmap): New type.
>>>         (linux_read_loadmap): New function.
>>>         (linux_target_ops): Add linux_read_loadmap.
>>>         * server.c (handle_query): Support qXfer:fdpic:read packet.
>>>         * win32-low.c (win32_target_ops): Initialize field `read_loadmap' to NULL.
>>>
>>>         gdb/doc/
>>>         * gdb.texinfo : Document qXfer:fdpic:read packet.
>>
>> Thanks.  Looks good to me.
> 
> Oh, just one detail:
> 
>> +  /* Read loadmaps.  Read LEN bytes at OFFSET into a buffer at MYADDR.  */
>> +  int (*read_loadmap) (const char *annex, CORE_ADDR offset,
>> +                            unsigned char *myaddr, unsigned int len);
> 
> ...
> 
>> +  NULL, /* read_fdpic_loadmap */
> 
> Please make those coincide though.
> 

Thanks for the patient review.  Committed.
http://sourceware.org/ml/gdb-cvs/2011-08/msg00069.html
http://sourceware.org/ml/gdb-cvs/2011-08/msg00073.html

-- 
Yao (齐尧)