Re: [PATCH 3/4] New agent command 'kill' and used by gdbserver

[Yao Qi <yao@codesourcery.com>]

On Wednesday 13 June 2012 00:14:19 Pedro Alves wrote:
> > +  run_inferior_command ("kill", strlen ("kill"));
> 
> - Missing space before parens.
> 
> - What happens if there's no IPA to talk to?
> 
> - Also, all callers of run_inferior_command command currently
>   pass "cmd, strlen (cmd) + 1".
> 
> - The kill_inferior function takes a PID as argument.  It's not
>   right to assume that PID is the current process.  IOW, in a
>   multi-process scenario, you may well run the inferior command
>   on the wrong inferior as is.

This new patch addresses all your comments except this one above.  IMO, the
 whole agent work in GDBserver doesn't consider much about multi-process, so
 this problem shall be addressed as a whole, when we start to support agent in
 multi-process.  I leave a FIXME in comment there.  WDYT?

gdb/gdbserver:

2012-06-14  Yao Qi  <yao@codesourcery.com>

	* server.h: Declare run_inferior_command and
	maybe_write_ipa_not_loaded.
	* target.c (kill_inferior): Include "agent.h".
	New.  Send command 'kill'.
	* target.h (kill_inferior): Removed macro.
	* tracepoint.c (run_inferior_command): Remove 'static'.
	(kill_inferior): New.
	(maybe_write_ipa_not_loaded): Remove 'static'.
	(gdb_agent_helper_thread): Handle command 'close'.
	Wait endlessly until the inferior stops.

gdb/doc:

2012-06-14  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (IPA Protocol Commands): Document new command
	'close'.
---
 gdb/doc/gdb.texinfo        |    4 ++++
 gdb/gdbserver/server.h     |    3 +++
 gdb/gdbserver/target.c     |   18 ++++++++++++++++++
 gdb/gdbserver/target.h     |    3 +--
 gdb/gdbserver/tracepoint.c |   27 ++++++++++++++++++++++-----
 5 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index ad227a4..8dd1bf7 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -33585,6 +33585,10 @@ for an error
 
 @end table
 
+@item close
+Closes the in-process agent.  This command is sent when @value{GDBN} or GDBserver
+is about to kill inferiors.
+
 @item qTfSTM
 @xref{qTfSTM}.
 @item qTsSTM
diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
index 02dfa29..d5fa867 100644
--- a/gdb/gdbserver/server.h
+++ b/gdb/gdbserver/server.h
@@ -467,6 +467,8 @@ void stop_tracing (void);
 
 int claim_trampoline_space (ULONGEST used, CORE_ADDR *trampoline);
 int have_fast_tracepoint_trampoline_buffer (char *msgbuf);
+
+int maybe_write_ipa_not_loaded (char *buffer);
 #endif
 
 struct traceframe;
@@ -558,4 +560,5 @@ int emit_error;
 extern const char version[];
 extern const char host_name[];
 
+int run_inferior_command (char *cmd, int len);
 #endif /* SERVER_H */
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 7539476..6148e1c 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -20,6 +20,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "server.h"
+#include "agent.h"
 
 struct target_ops *the_target;
 
@@ -182,3 +183,20 @@ target_waitstatus_to_string (const struct target_waitstatus *ws)
 
   return buf;
 }
+
+int
+kill_inferior (int pid)
+{
+  char buf[IPA_CMD_BUF_SIZE];
+
+  if (!maybe_write_ipa_not_loaded (buf))
+    {
+      strcpy (buf, "close");
+      /* FIXME: It is wrong to assume PID is the current process.  In
+	 a multiple-process scenario, we may run inferior command on
+	 the wrong process.  */
+      run_inferior_command (buf, strlen (buf) + 1);
+    }
+
+  return (*the_target->kill) (pid);
+}
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 0cf5687..9f96e04 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -409,8 +409,7 @@ void set_target_ops (struct target_ops *);
 #define myattach(pid) \
   (*the_target->attach) (pid)
 
-#define kill_inferior(pid) \
-  (*the_target->kill) (pid)
+int kill_inferior (int);
 
 #define detach_inferior(pid) \
   (*the_target->detach) (pid)
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 8fcaa46..df69508 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -281,7 +281,7 @@ write_e_ust_not_loaded (char *buffer)
 /* If the in-process agent library isn't loaded in the inferior, write
    an error to BUFFER, and return 1.  Otherwise, return 0.  */
 
-static int
+int
 maybe_write_ipa_not_loaded (char *buffer)
 {
   if (!agent_loaded_p ())
@@ -412,8 +412,6 @@ static int flush_trace_buffer_handler (CORE_ADDR);
 static void download_trace_state_variables (void);
 static void upload_fast_traceframes (void);
 
-static int run_inferior_command (char *cmd, int len);
-
 static int
 read_inferior_integer (CORE_ADDR symaddr, int *val)
 {
@@ -6662,7 +6660,7 @@ static struct ltt_available_probe gdb_ust_probe =
    thread by means of direct memory xfering, and a socket for
    synchronization.  */
 
-static int
+int
 run_inferior_command (char *cmd, int len)
 {
   int err = -1;
@@ -7033,6 +7031,7 @@ gdb_agent_helper_thread (void *arg)
 	  int fd;
 	  char buf[1];
 	  int ret;
+	  int stop_loop = 0;
 
 	  tmp = sizeof (sockaddr);
 
@@ -7065,8 +7064,12 @@ gdb_agent_helper_thread (void *arg)
 
 	  if (cmd_buf[0])
 	    {
+	      if (strncmp ("close", cmd_buf, 5) == 0)
+		{
+		  stop_loop = 1;
+		}
 #ifdef HAVE_UST
-	      if (strcmp ("qTfSTM", cmd_buf) == 0)
+	      else if (strcmp ("qTfSTM", cmd_buf) == 0)
 		{
 		  cmd_qtfstm (cmd_buf);
 		}
@@ -7098,6 +7101,20 @@ gdb_agent_helper_thread (void *arg)
 	  /* Fix compiler's warning: ignoring return value of 'write'.  */
 	  ret = write (fd, buf, 1);
 	  close (fd);
+
+	  if (stop_loop)
+	    {
+	      close (listen_fd);
+	      unlink (agent_socket_name);
+
+	      /* Sleep endlessly to wait the whole inferior stops.  This
+		 thread can not exit because GDB or GDBserver may still need
+		 'current_inferior' (representing this thread) to access
+		 inferior memory.  Otherwise, this thread exits earlier than
+		 other threads, and 'current_inferior' is set to NULL.  */
+	      while (1)
+		sleep (10);
+	    }
 	}
     }
 
-- 
1.7.0.4