From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19591 invoked by alias); 14 Jun 2012 14:50:33 -0000 Received: (qmail 19570 invoked by uid 22791); 14 Jun 2012 14:50:28 -0000 X-SWARE-Spam-Status: No, hits=-4.2 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_CP,TW_QT X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 14 Jun 2012 14:50:09 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1SfBNA-0003yY-D7 from Yao_Qi@mentor.com ; Thu, 14 Jun 2012 07:50:08 -0700 Received: from SVR-ORW-FEM-02.mgc.mentorg.com ([147.34.96.206]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Thu, 14 Jun 2012 07:49:32 -0700 Received: from yaolp.localnet (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.1.289.1; Thu, 14 Jun 2012 07:50:03 -0700 From: Yao Qi To: Pedro Alves Subject: Re: [PATCH 3/4] New agent command 'kill' and used by gdbserver Date: Thu, 14 Jun 2012 14:50:00 -0000 User-Agent: KMail/1.13.5 (Linux/2.6.32-41-generic; KDE/4.4.5; i686; ; ) CC: References: <1339246002-1987-1-git-send-email-yao@codesourcery.com> <1339246002-1987-4-git-send-email-yao@codesourcery.com> <4FD76ADB.7090302@redhat.com> In-Reply-To: <4FD76ADB.7090302@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-ID: <201206142250.01331.yao@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-06/txt/msg00468.txt.bz2 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 * 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 * 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 . */ #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