[6/7] Implement qXfer:exec-file:read in gdbserver
Commit Message
This commit implements the "qXfer:exec-file:read" packet in gdbserver.
gdb/gdbserver/ChangeLog:
* target.h (struct target_ops) <pid_to_exec_file>: New field.
* linux-low.c (linux_target_ops): Initialize pid_to_exec_file.
* server.c (handle_qxfer_exec_file): New function.
(qxfer_packets): Add exec-file entry.
(handle_query): Report qXfer:exec-file:read as supported packet.
---
gdb/gdbserver/ChangeLog | 8 ++++++++
gdb/gdbserver/linux-low.c | 1 +
gdb/gdbserver/server.c | 40 ++++++++++++++++++++++++++++++++++++++++
gdb/gdbserver/target.h | 8 ++++++++
4 files changed, 57 insertions(+), 0 deletions(-)
Comments
Gary Benson <gbenson@redhat.com> writes:
> This commit implements the "qXfer:exec-file:read" packet in gdbserver.
>
> gdb/gdbserver/ChangeLog:
>
> * target.h (struct target_ops) <pid_to_exec_file>: New field.
> * linux-low.c (linux_target_ops): Initialize pid_to_exec_file.
> * server.c (handle_qxfer_exec_file): New function.
> (qxfer_packets): Add exec-file entry.
> (handle_query): Report qXfer:exec-file:read as supported packet.
>
> ...
>
> diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
> index 126c861..dc7802d 100644
> --- a/gdb/gdbserver/target.h
> +++ b/gdb/gdbserver/target.h
> @@ -394,6 +394,14 @@ struct target_ops
>
> /* Return true if target supports range stepping. */
> int (*supports_range_stepping) (void);
> +
> + /* Return the pathname of the executable file that was run to
> + create the process PID. If the executable file cannot be
> + determined, NULL is returned. Otherwise, a pointer to a
> + character string containing the pathname is returned. This
> + string should be copied into a buffer by the client if the
> + string will not be immediately used, or if it must persist. */
> + char *(*pid_to_exec_file) (int pid);
> };
>
> extern struct target_ops *the_target;
IWBN to have some clarity on what the pathname result can and cannot be.
Perhaps nitpicky, but the less ambiguity the better.
I think(!) the intent is that the path name is the full
path name, but I could be wrong.
Another issue is whether the path has been real-path'd.
[all symlinks resolved] I don't have a strong opinion on that,
but I do think we should at least require full paths to be returned here.
const char * result?
Also, I was going to say we need to pick a type for "pid" and consistently
use it, but that's a whole 'nother discussion, and this patch set
needn't be bogged down by it.
Doug Evans wrote:
> Gary Benson <gbenson@redhat.com> writes:
> > diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
> > index 126c861..dc7802d 100644
> > --- a/gdb/gdbserver/target.h
> > +++ b/gdb/gdbserver/target.h
> > @@ -394,6 +394,14 @@ struct target_ops
> >
> > /* Return true if target supports range stepping. */
> > int (*supports_range_stepping) (void);
> > +
> > + /* Return the pathname of the executable file that was run to
> > + create the process PID. If the executable file cannot be
> > + determined, NULL is returned. Otherwise, a pointer to a
> > + character string containing the pathname is returned. This
> > + string should be copied into a buffer by the client if the
> > + string will not be immediately used, or if it must persist. */
> > + char *(*pid_to_exec_file) (int pid);
> > };
> >
> > extern struct target_ops *the_target;
>
> IWBN to have some clarity on what the pathname result can and cannot be.
>
> Perhaps nitpicky, but the less ambiguity the better. I think(!) the
> intent is that the path name is the full path name, but I could be
> wrong.
>
> Another issue is whether the path has been real-path'd. [all
> symlinks resolved] I don't have a strong opinion on that, but I do
> think we should at least require full paths to be returned here.
I will use the "the full absolute name of the file" as Eli suggested
for the documentation. I will also change the gdb/target.h version
to match.
It's not necessary for all symlinks to be resolved. I think the
intention is for the function to return what the operating system
says the executable's filename is, but of course non of these old
parts of GDB are documented to that extent so I'm guessing.
> const char * result?
Not const for the same reason as the GDB side.
> Also, I was going to say we need to pick a type for "pid" and
> consistently use it, but that's a whole 'nother discussion, and
> this patch set needn't be bogged down by it.
Quite :)
Thanks,
Gary
On Wednesday, April 01 2015, Gary Benson wrote:
> This commit implements the "qXfer:exec-file:read" packet in gdbserver.
Hi Gary,
I haven't really investigated further, but I saw that this commit
introduced a regression on the gdb.base/attach.exp test. This happened
on some builders; for example:
<http://gdb-build.sergiodj.net/builders/Fedora-x86_64-native-extended-gdbserver-m32/builds/887/steps/regressions/logs/regressions>
As far as I have checked, this failure manifests only when testing the
native-extended-gdbserver variant.
Sergio Durigan Junior wrote:
> On Wednesday, April 01 2015, Gary Benson wrote:
> > This commit implements the "qXfer:exec-file:read" packet in gdbserver.
>
> I haven't really investigated further, but I saw that this commit
> introduced a regression on the gdb.base/attach.exp test. This happened
> on some builders; for example:
>
> <http://gdb-build.sergiodj.net/builders/Fedora-x86_64-native-extended-gdbserver-m32/builds/887/steps/regressions/logs/regressions>
>
> As far as I have checked, this failure manifests only when testing the
> native-extended-gdbserver variant.
Ok, looking into it.
Thanks,
Gary
@@ -6429,6 +6429,7 @@ static struct target_ops linux_target_ops = {
NULL,
#endif
linux_supports_range_stepping,
+ linux_pid_to_exec_file,
};
static void
@@ -1137,6 +1137,42 @@ handle_qxfer_auxv (const char *annex,
return (*the_target->read_auxv) (offset, readbuf, len);
}
+/* Handle qXfer:exec-file:read. */
+
+static int
+handle_qxfer_exec_file (const char *const_annex,
+ gdb_byte *readbuf, const gdb_byte *writebuf,
+ ULONGEST offset, LONGEST len)
+{
+ char *annex, *file;
+ ULONGEST pid;
+ int total_len;
+
+ if (the_target->pid_to_exec_file == NULL || writebuf != NULL)
+ return -2;
+
+ annex = alloca (strlen (const_annex) + 1);
+ strcpy (annex, const_annex);
+ annex = unpack_varlen_hex (annex, &pid);
+ if (annex[0] != '\0' || pid == 0)
+ return -1;
+
+ file = (*the_target->pid_to_exec_file) (pid);
+ if (file == NULL)
+ return -1;
+
+ total_len = strlen (file);
+
+ if (offset > total_len)
+ return -1;
+
+ if (offset + len > total_len)
+ len = total_len - offset;
+
+ memcpy (readbuf, file + offset, len);
+ return len;
+}
+
/* Handle qXfer:features:read. */
static int
@@ -1638,6 +1674,7 @@ static const struct qxfer qxfer_packets[] =
{ "auxv", handle_qxfer_auxv },
{ "btrace", handle_qxfer_btrace },
{ "btrace-conf", handle_qxfer_btrace_conf },
+ { "exec-file", handle_qxfer_exec_file},
{ "fdpic", handle_qxfer_fdpic},
{ "features", handle_qxfer_features },
{ "libraries", handle_qxfer_libraries },
@@ -2082,6 +2119,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
if (target_supports_stopped_by_hw_breakpoint ())
strcat (own_buf, ";hwbreak+");
+ if (the_target->pid_to_exec_file != NULL)
+ strcat (own_buf, ";qXfer:exec-file:read+");
+
return;
}
@@ -394,6 +394,14 @@ struct target_ops
/* Return true if target supports range stepping. */
int (*supports_range_stepping) (void);
+
+ /* Return the pathname of the executable file that was run to
+ create the process PID. If the executable file cannot be
+ determined, NULL is returned. Otherwise, a pointer to a
+ character string containing the pathname is returned. This
+ string should be copied into a buffer by the client if the
+ string will not be immediately used, or if it must persist. */
+ char *(*pid_to_exec_file) (int pid);
};
extern struct target_ops *the_target;