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

[Yao Qi <yao@codesourcery.com>]

[-- 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