From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9533 invoked by alias); 4 Mar 2009 01:17:48 -0000 Received: (qmail 9524 invoked by uid 22791); 4 Mar 2009 01:17:46 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 04 Mar 2009 01:17:39 +0000 Received: (qmail 19708 invoked from network); 4 Mar 2009 01:17:33 -0000 Received: from unknown (HELO orlando) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 4 Mar 2009 01:17:33 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: [remote/rfc] Let GDB know if a remote server originally attached to a process. Date: Wed, 04 Mar 2009 01:17:00 -0000 User-Agent: KMail/1.9.10 MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200903040117.32166.pedro@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: 2009-03/txt/msg00050.txt.bz2 Hi folks, When gdbserver is used to attach to a running process ('gdbserver --attach PID'), exiting GDB kills the inferior. By contrast, when GDB is attached to a running process, exiting GDB detaches. There's an obvious workaround for this (detach explicitly, then quit GDB), but this ought to be changed for consistency (*). The issue here, is that GDB has no idea that gdbserver originaly attached to the remote process, instead of having created it (vs gdbserver :9999 foo). This suggests that we need a new remote protocol query. I've came up with the attached patch as a sort of proof-of-concept. This is what happens with the patch installed: $ gdbserver :9999 --attach 10211 (gdb) tar remote :9999 Remote debugging using :9999 (...) 0x00007fc77a1b6796 in pthread_join () from /lib/libpthread.so.0 (gdb) q The program is running. Quit anyway (and detach it)? (y or n) ^^^^^^^^^^^^^ Whereas with current GDB one gets: (gdb) q The program is running. Quit anyway (and kill it)? (y or n) ^^^^^^^^^^^ (another example, is, connecting to gdbserver with 'target extended-remote', then issue an "attach" command. In this case GDB *does* know that the inferior was attached to. Now, issue a "disconnect", and reconnect again. This time, although gdbserver is still controlling the same inferior, GDB doesn't know anymore that gdbserver originally attached to the inferior, and will now offer to "kill" it. ) This works by simply defining a new "qAttached:PID"/"qAttached" query to GDB, to which the stub returns "1" --- attached, "0" --- not attached, or "" --- not supported, as for all other optional packets. This is simple enough --- the patch is smaller than this whole description of it :-) I was considering if we want to generalize a mechanism for this or not, say similar to target descriptions, but I don't have any other use case for such a thing. The target description mechanism as is currently isn't exactly right for this case. E.g., there's only a single description for a target interface, not one per inferior. That *may* need to change in the future, but I'm not certain yet. What do you think? Should I just stick with the below and go write documentation for it? Or...? (*) - That was the simple version, which actually did come through a customer request. Here's another, albeit more complicated use case, which ends up needing the same solution: I have a multi-process aware remote stub, for this OS where code sections are shared between processes. One of the features this enabled was support for so called "global breakpoints". That is, we insert a breakpoint at a given address, as usual, but, what can happen is that a process that GDB and the stub wasn't attached to yet, hits this breakpoint. When this happens, the stub "auto-attaches" to the process that hit the breakpoint, and reports the SIGTRAP as usual to GDB, which on its end adds this process to its internal inferior table. So far, so good, all is simple in this regard. Thing is, GDB has no way to know that we had "attached" to these processes to begin with, and that we want "quit" to *not* kill these processes, but to instead "detach" from them. This is seriously annoying when we add non-stop mode to the mix, since in that case, we have to carefully and manually remove all breakpoints (in case a process auto attaches just while we were exiting), then detach from all inferiors manually, and only can we safelly "quit" from GDB. -- Pedro Alves 2009-03-04 Pedro Alves gdb/ * remote.c (PACKET_qAttached): New. (remote_query_attached): New. (remote_add_inferior): New. (notice_new_inferiors): Use it. (remote_start_remote): Use it. (_initialize_remote): Add "set remote query-attached-packet" command. 2009-03-04 Pedro Alves gdb/gdbserver/ * server.c (handle_query): Handle "qAttached". --- gdb/gdbserver/server.c | 7 ++++++ gdb/remote.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 61 insertions(+), 3 deletions(-) Index: src/gdb/remote.c =================================================================== --- src.orig/gdb/remote.c 2009-03-04 00:38:28.000000000 +0000 +++ src/gdb/remote.c 2009-03-04 00:41:14.000000000 +0000 @@ -992,6 +992,7 @@ enum { PACKET_vKill, PACKET_qXfer_siginfo_read, PACKET_qXfer_siginfo_write, + PACKET_qAttached, PACKET_MAX }; @@ -1118,6 +1119,53 @@ static ptid_t any_thread_ptid; static ptid_t general_thread; static ptid_t continue_thread; +/* Try to find out if the stub attached to PID (and hence GDB should + detach instead of killing it when bailing out). */ + +static int +remote_query_attached (int pid) +{ + struct remote_state *rs = get_remote_state (); + + if (remote_protocol_packets[PACKET_qAttached].support == PACKET_DISABLE) + return 0; + + if (remote_multi_process_p (rs)) + sprintf (rs->buf, "qAttached:%x", pid); + else + sprintf (rs->buf, "qAttached"); + + putpkt (rs->buf); + getpkt (&rs->buf, &rs->buf_size, 0); + + if (packet_ok (rs->buf, + &remote_protocol_packets[PACKET_qAttached]) == PACKET_OK) + { + if (strcmp (rs->buf, "1") == 0) + return 1; + } + + return 0; +} + +static void +remote_add_inferior (int pid) +{ + struct remote_state *rs = get_remote_state (); + struct inferior *inf; + int attached; + + /* Check whether this process we're learning about is to be + considered attached, or if is to be considered to have been + spawned by the stub. Do this before adding the inferior to the + tables, in case we throw an unexpected error, to avoid leaving + things inconsistent. */ + attached = remote_query_attached (pid); + + inf = add_inferior (pid); + inf->attach_flag = attached; +} + static void notice_new_inferiors (ptid_t currthread) { @@ -1161,7 +1209,7 @@ notice_new_inferiors (ptid_t currthread) may not know about it yet. Add it before adding its child thread, so notifications are emitted in a sensible order. */ if (!in_inferior_list (ptid_get_pid (currthread))) - add_inferior (ptid_get_pid (currthread)); + remote_add_inferior (ptid_get_pid (currthread)); /* This is really a new thread. Add it. */ add_thread (currthread); @@ -2157,7 +2205,7 @@ remote_threads_info (struct target_ops * threads, so notifications are emitted in a sensible order. */ if (!in_inferior_list (ptid_get_pid (new_thread))) - add_inferior (ptid_get_pid (new_thread)); + remote_add_inferior (ptid_get_pid (new_thread)); add_thread (new_thread); @@ -2628,7 +2676,7 @@ remote_start_remote (struct ui_out *uiou /* Now, if we have thread information, update inferior_ptid. */ inferior_ptid = remote_current_thread (inferior_ptid); - add_inferior (ptid_get_pid (inferior_ptid)); + remote_add_inferior (ptid_get_pid (inferior_ptid)); /* Always add the main thread. */ add_thread_silent (inferior_ptid); @@ -9127,6 +9175,9 @@ Show the maximum size of the address (in add_packet_config_cmd (&remote_protocol_packets[PACKET_vKill], "vKill", "kill", 0); + add_packet_config_cmd (&remote_protocol_packets[PACKET_qAttached], + "qAttached", "query-attached", 0); + /* Keep the old ``set remote Z-packet ...'' working. Each individual Z sub-packet has its own set and show commands, but users may have sets to this variable in their .gdbinit files (or in their Index: src/gdb/gdbserver/server.c =================================================================== --- src.orig/gdb/gdbserver/server.c 2009-03-04 00:38:28.000000000 +0000 +++ src/gdb/gdbserver/server.c 2009-03-04 00:41:14.000000000 +0000 @@ -1040,6 +1040,13 @@ handle_query (char *own_buf, int packet_ return; } + if (strcmp (own_buf, "qAttached") == 0) + { + require_running (own_buf); + strcpy (own_buf, attached ? "1" : "0"); + return; + } + /* Otherwise we didn't know what packet it was. Say we didn't understand it. */ own_buf[0] = 0;