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

Yao Qi
2011-08-09T03:12:03Z

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