qXfer:exec-file:read and non multiprocess target

Message ID 20150505110207.GA17684@blade.nx
State New, archived
Headers

Commit Message

Gary Benson May 5, 2015, 11:02 a.m. UTC
  Philippe Waroquiers wrote:
> I am busy adding qXfer:exec-file:read  to the Valgrind gdbserver.
> 
> Even if Valgrind reports it supports qXfer:exec-file, GDB does
> not query it.  This is due to the fact that GDB does not query
> the exec-file when the pid is a fake pid, which is the case for
> Valgrind, as the target command to use is:
>    target remote | vgdb
> 
> The following change in remote.c ensures GDB queries the
> remote exec-file:
> 1561,1562c1561,1562
> <   if (try_open_exec && !fake_pid_p && get_exec_file (0) == NULL)
> <     exec_file_locate_attach (pid, 1);
> ---
> >   if (try_open_exec && get_exec_file (0) == NULL)
> >     exec_file_locate_attach (fake_pid_p ? 0 : pid, 1);
> 
> With that change, GDB can use a Valgrind target without having
> to specify the exec file.  The idea is that when the stub gets
> a pid 0 in this request, it replies with the exec file of the
> current process.

The PID is fake because vgdb does not support multiprocess extensions.
I don't like sending a fake/zero PID over the wire, but how about I
change qXfer:exec-file:read to send a NULL annex if the remote does
not have multiprocess extensions?  Can you make your side work with
the patch inlined below?  If so I'll tidy and document it and submit
it for review.

Thanks,
Gary

--
  

Comments

Philippe Waroquiers May 5, 2015, 8:45 p.m. UTC | #1
On Tue, 2015-05-05 at 12:02 +0100, Gary Benson wrote:
> The PID is fake because vgdb does not support multiprocess extensions.
> I don't like sending a fake/zero PID over the wire, but how about I
> change qXfer:exec-file:read to send a NULL annex if the remote does
> not have multiprocess extensions?  Can you make your side work with
> the patch inlined below?  If so I'll tidy and document it and submit
> it for review.
It looks effectively better to not send the fake or 0 pid
(e.g. similar to the qAttached packet).

The valgrind gdbserver side works ok with the patch to send an empty
annex.
This is the resulting exchange, traced at Valgrind side:
  ...
  --4980:3:  gdbsrv     getpkt ("qAttached");  [no ack] 
  --4980:3:  gdbsrv     putpkt ("$1#31"); [no ack]
  --4980:3:  gdbsrv     getpkt ("qXfer:exec-file:read::0,fff");  [no ack] 
  --4980:3:  gdbsrv     putpkt ("$l/home/philippe/valgrind/better_stats/memcheck/tests/trivialleak#2c"); [no ack]
  --4980:3:  gdbsrv     getpkt ("vFile:open:2f686f6d652f7068696c697070652f76616c6772696e642f6265747465725f73746174732f6d656d636865636b2f74657374732f7472697669616c6c65616b,0,0");  [no ack] 
  --4980:3:  gdbsrv     putpkt ("$#00"); [no ack]
  ...

and then GDB could properly use the returned exec-file
(even if vFile is not supported by Valgrind gdbserver).

Thanks

Philippe
  
Gary Benson May 6, 2015, 10:31 a.m. UTC | #2
Philippe Waroquiers wrote:
> On Tue, 2015-05-05 at 12:02 +0100, Gary Benson wrote:
> > The PID is fake because vgdb does not support multiprocess
> > extensions.  I don't like sending a fake/zero PID over the wire,
> > but how about I change qXfer:exec-file:read to send a NULL annex
> > if the remote does not have multiprocess extensions?  Can you make
> > your side work with the patch inlined below?  If so I'll tidy and
> > document it and submit it for review.
> 
> It looks effectively better to not send the fake or 0 pid
> (e.g. similar to the qAttached packet).
> 
> The valgrind gdbserver side works ok with the patch to send an empty
> annex.  This is the resulting exchange, traced at Valgrind side:
>   ...
>   --4980:3:  gdbsrv     getpkt ("qAttached");  [no ack] 
>   --4980:3:  gdbsrv     putpkt ("$1#31"); [no ack]
>   --4980:3:  gdbsrv     getpkt ("qXfer:exec-file:read::0,fff");  [no ack] 
>   --4980:3:  gdbsrv     putpkt ("$l/home/philippe/valgrind/better_stats/memcheck/tests/trivialleak#2c"); [no ack]
>   --4980:3:  gdbsrv     getpkt ("vFile:open:2f686f6d652f7068696c697070652f76616c6772696e642f6265747465725f73746174732f6d656d636865636b2f74657374732f7472697669616c6c65616b,0,0");  [no ack] 
>   --4980:3:  gdbsrv     putpkt ("$#00"); [no ack]
>   ...

Great, I will get that turned into a proper patch today.

> and then GDB could properly use the returned exec-file
> (even if vFile is not supported by Valgrind gdbserver).

Yes, there is a special workaround just for vgdb ;)

Cheers,
Gary
  

Patch

diff --git a/gdb/remote.c b/gdb/remote.c
index 099ddbb..42d990a 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1558,7 +1558,7 @@ 
 
   /* 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,7 @@ 
 remote_pid_to_exec_file (struct target_ops *self, int pid)
 {
   static char *filename = NULL;
-  char annex[9];
+  char *annex = NULL;
 
   if (packet_support (PACKET_qXfer_exec_file) != PACKET_ENABLE)
     return NULL;
@@ -11718,7 +11718,14 @@ 
   if (filename != NULL)
     xfree (filename);
 
-  xsnprintf (annex, sizeof (annex), "%x", pid);
+  if (remote_multi_process_p (get_remote_state ()))
+    {
+      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);