Locate executables on remote stubs without multiprocess extensions

Message ID 1430932230-12551-1-git-send-email-gbenson@redhat.com
State New, archived
Headers

Commit Message

Gary Benson May 6, 2015, 5:10 p.m. UTC
  Hi all,

This commit allows GDB to determine filenames of main executables
when debugging using remote stubs without multiprocess extensions.
The qXfer:exec-file:read packet is extended to allow an empty
annex, with the meaning that the remote stub should supply the
filename of whatever it thinks is the current process.

Built and regtested on RHEL6.6 x86_64.

Is this ok to commit?

Cheers,
Gary


gdb/ChangeLog:

	* remote.c (remote_add_inferior): Call exec_file_locate_attach
	for fake PIDs as well as real ones.
	(remote_pid_to_exec_file): Send empty annex if PID is fake.

gdb/doc/ChangeLog:

	* gdb.texinfo (General Query Packets): Document
	qXfer:exec-file:read with empty annex.

gdb/gdbserver/ChangeLog:

	* server.c (handle_qxfer_exec_file): Use current process
	if annex is empty.
---
 gdb/ChangeLog           |    6 ++++++
 gdb/doc/ChangeLog       |    5 +++++
 gdb/doc/gdb.texinfo     |    3 ++-
 gdb/gdbserver/ChangeLog |    5 +++++
 gdb/gdbserver/server.c  |   25 ++++++++++++++++++++-----
 gdb/remote.c            |   15 ++++++++++++---
 6 files changed, 50 insertions(+), 9 deletions(-)
  

Comments

Eli Zaretskii May 6, 2015, 5:15 p.m. UTC | #1
> From: Gary Benson <gbenson@redhat.com>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Wed,  6 May 2015 18:10:30 +0100
> 
> This commit allows GDB to determine filenames of main executables
> when debugging using remote stubs without multiprocess extensions.
> The qXfer:exec-file:read packet is extended to allow an empty
> annex, with the meaning that the remote stub should supply the
> filename of whatever it thinks is the current process.
> 
> Built and regtested on RHEL6.6 x86_64.
> 
> Is this ok to commit?
> 
> Cheers,
> Gary
> 
> 
> gdb/ChangeLog:
> 
> 	* remote.c (remote_add_inferior): Call exec_file_locate_attach
> 	for fake PIDs as well as real ones.
> 	(remote_pid_to_exec_file): Send empty annex if PID is fake.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (General Query Packets): Document
> 	qXfer:exec-file:read with empty annex.
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* server.c (handle_qxfer_exec_file): Use current process
> 	if annex is empty.

The documentation part is OK.

Thanks.
  
Gary Benson May 6, 2015, 5:16 p.m. UTC | #2
Gary Benson wrote:
> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index d2e20d9..516a311 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -1144,17 +1144,32 @@ handle_qxfer_exec_file (const char *const_annex,
>  			gdb_byte *readbuf, const gdb_byte *writebuf,
>  			ULONGEST offset, LONGEST len)
>  {
> -  char *annex, *file;
> +  char *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)
> +  if (const_annex[0] == '\0')
> +    {
> +      if (current_thread == NULL)
> +	return -1;
> +
> +      pid = pid_of (current_thread);
> +    }
> +  else
> +    {
> +      char *annex = alloca (strlen (const_annex) + 1);
> +
> +      strcpy (annex, const_annex);
> +      annex = unpack_varlen_hex (annex, &pid);
> +
> +      if (annex[0] != '\0')
> +	return -1;
> +    }
> +
> +  if (pid < 0)
>      return -1;

Oops, this should be "<=".

Cheers,
Gary

--
http://gbenson.net/
  
Pedro Alves May 11, 2015, 2:37 p.m. UTC | #3
On 05/06/2015 06:16 PM, Gary Benson wrote:
> Gary Benson wrote:

> @@ -11718,7 +11719,15 @@ remote_pid_to_exec_file (struct target_ops *self, int pid)
>    if (filename != NULL)
>      xfree (filename);
>
> -  xsnprintf (annex, sizeof (annex), "%x", pid);
> +  inf = find_inferior_pid (pid);
> +  if (inf != NULL && !inf->fake_pid_p)

This will silently do the wrong thing (retrieve the exec file
of the server's current thread/process) if this method is ever
used to try to fetch the exec out of a process that we're
_not_ currently attached to.  Maybe this should be:

  if (inf == NULL)
    internal_error (__FILE__, __LINE__,
                    "attempt to retrieve exec-file of not-debugged process");
  if (!inf->fake_pid_p)

>> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
>> index d2e20d9..516a311 100644
>> --- a/gdb/gdbserver/server.c
>> +++ b/gdb/gdbserver/server.c
>> @@ -1144,17 +1144,32 @@ handle_qxfer_exec_file (const char *const_annex,
>>  			gdb_byte *readbuf, const gdb_byte *writebuf,
>>  			ULONGEST offset, LONGEST len)
>>  {
>> -  char *annex, *file;
>> +  char *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)
>> +  if (const_annex[0] == '\0')
>> +    {
>> +      if (current_thread == NULL)
>> +	return -1;
>> +
>> +      pid = pid_of (current_thread);
>> +    }
>> +  else
>> +    {
>> +      char *annex = alloca (strlen (const_annex) + 1);
>> +
>> +      strcpy (annex, const_annex);
>> +      annex = unpack_varlen_hex (annex, &pid);
>> +
>> +      if (annex[0] != '\0')
>> +	return -1;
>> +    }
>> +
>> +  if (pid < 0)
>>      return -1;
> 
> Oops, this should be "<=".

This is OK with that change and the point above addressed.

Thanks,
Pedro Alves
  
Gary Benson May 12, 2015, 11:03 a.m. UTC | #4
Pedro Alves wrote:
> On 05/06/2015 06:16 PM, Gary Benson wrote:
> > Gary Benson wrote:
> > @@ -11718,7 +11719,15 @@ remote_pid_to_exec_file
> >    if (filename != NULL)
> >      xfree (filename);
> >
> > -  xsnprintf (annex, sizeof (annex), "%x", pid);
> > +  inf = find_inferior_pid (pid);
> > +  if (inf != NULL && !inf->fake_pid_p)
> 
> This will silently do the wrong thing (retrieve the exec file of
> the server's current thread/process) if this method is ever used
> to try to fetch the exec out of a process that we're _not_ currently
> attached to.  Maybe this should be:
> 
>   if (inf == NULL)
>     internal_error (__FILE__, __LINE__,
>                     "attempt to retrieve exec-file of not-debugged process");
>   if (!inf->fake_pid_p)

Good catch, thanks!

> > > @@ -1144,17 +1144,32 @@ handle_qxfer_exec_file
[snip]
> > > +
> > > +  if (pid < 0)
> > >      return -1;
> > 
> > Oops, this should be "<=".
> 
> This is OK with that change and the point above addressed.

Ok, I've pushed this, thanks Pedro and Eli for the reviews.

Cheers,
Gary
  

Patch

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9e2787d..63e063a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -36558,7 +36558,8 @@  by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
 Return the full absolute name of the file that was executed to create
 a process running on the remote system.  The annex specifies the
 numeric process ID of the process to query, encoded as a hexadecimal
-number.
+number.  If the annex part is empty the remote stub should return the
+filename corresponding to the currently executing process.
 
 This packet is not probed by default; the remote stub must request it,
 by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index d2e20d9..516a311 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1144,17 +1144,32 @@  handle_qxfer_exec_file (const char *const_annex,
 			gdb_byte *readbuf, const gdb_byte *writebuf,
 			ULONGEST offset, LONGEST len)
 {
-  char *annex, *file;
+  char *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)
+  if (const_annex[0] == '\0')
+    {
+      if (current_thread == NULL)
+	return -1;
+
+      pid = pid_of (current_thread);
+    }
+  else
+    {
+      char *annex = alloca (strlen (const_annex) + 1);
+
+      strcpy (annex, const_annex);
+      annex = unpack_varlen_hex (annex, &pid);
+
+      if (annex[0] != '\0')
+	return -1;
+    }
+
+  if (pid < 0)
     return -1;
 
   file = (*the_target->pid_to_exec_file) (pid);
diff --git a/gdb/remote.c b/gdb/remote.c
index 099ddbb..6b18960 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1558,7 +1558,7 @@  remote_add_inferior (int fake_pid_p, int pid, int attached,
 
   /* If no main executable is currently open then attempt to
      open the file that was executed to create this inferior.  */
-  if (try_open_exec && !fake_pid_p && get_exec_file (0) == NULL)
+  if (try_open_exec && get_exec_file (0) == NULL)
     exec_file_locate_attach (pid, 1);
 
   return inf;
@@ -11710,7 +11710,8 @@  static char *
 remote_pid_to_exec_file (struct target_ops *self, int pid)
 {
   static char *filename = NULL;
-  char annex[9];
+  struct inferior *inf;
+  char *annex = NULL;
 
   if (packet_support (PACKET_qXfer_exec_file) != PACKET_ENABLE)
     return NULL;
@@ -11718,7 +11719,15 @@  remote_pid_to_exec_file (struct target_ops *self, int pid)
   if (filename != NULL)
     xfree (filename);
 
-  xsnprintf (annex, sizeof (annex), "%x", pid);
+  inf = find_inferior_pid (pid);
+  if (inf != NULL && !inf->fake_pid_p)
+    {
+      const int annex_size = 9;
+
+      annex = alloca (annex_size);
+      xsnprintf (annex, annex_size, "%x", pid);
+    }
+
   filename = target_read_stralloc (&current_target,
 				   TARGET_OBJECT_EXEC_FILE, annex);