From patchwork Fri Feb 27 00:46:12 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Don Breazeal X-Patchwork-Id: 5327 Received: (qmail 20179 invoked by alias); 27 Feb 2015 00:46:53 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 20166 invoked by uid 89); 27 Feb 2015 00:46:52 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL, BAYES_00, LIKELY_SPAM_BODY, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=no version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 27 Feb 2015 00:46:50 +0000 Received: from svr-orw-fem-05.mgc.mentorg.com ([147.34.97.43]) by relay1.mentorg.com with esmtp id 1YR94p-0006Hp-LT from Don_Breazeal@mentor.com ; Thu, 26 Feb 2015 16:46:47 -0800 Received: from build4-lucid-cs (147.34.91.1) by svr-orw-fem-05.mgc.mentorg.com (147.34.97.43) with Microsoft SMTP Server id 14.3.224.2; Thu, 26 Feb 2015 16:46:47 -0800 Received: by build4-lucid-cs (Postfix, from userid 1905) id 1670740FB6; Thu, 26 Feb 2015 16:46:47 -0800 (PST) From: Don Breazeal To: , Subject: [PATCH v5 01/06] Identify remote fork event support Date: Thu, 26 Feb 2015 16:46:12 -0800 Message-ID: <1424997977-13316-2-git-send-email-donb@codesourcery.com> In-Reply-To: <1424997977-13316-1-git-send-email-donb@codesourcery.com> References: <54C566F2.2020302@codesourcery.com> <1424997977-13316-1-git-send-email-donb@codesourcery.com> MIME-Version: 1.0 X-IsSubscribed: yes This is an updated version of the patch last reviewed here: https://sourceware.org/ml/gdb-patches/2015-02/msg00260.html Hi Pedro, here are responses to your review comments: On 2/10/2015 8:34 AM, Pedro Alves wrote: > On 01/25/2015 09:46 PM, Don Breazeal wrote: ... >> There is an #if that has been added temporarily to allow the code for >>... > (Nit: I'd prefer that these code bits / hunks were simply moved > to the patches that make use of them.) Done. >> + linux_ptrace_set_additional_flags (0); > > Likewise, seems pointless add this call now. Removed. >> - current_ptrace_options |= (additional_flags & (PTRACE_O_TRACEFORK >> - | PTRACE_O_TRACEVFORK >> - | PTRACE_O_TRACEEXEC)); >> + current_ptrace_options |= (additional_flags >> + & (PTRACE_O_TRACEFORK >> + | PTRACE_O_TRACEVFORK >> + | PTRACE_O_TRACEEXEC)); > > This looks like a spurious change? Yes. Fixed. >> - linux_check_ptrace_features (); >> + linux_test_for_event_reporting (); > > I don't really understand the motivation for the renaming. > The function still tests all kinds of ptrace features, not just > event-related features. E.g., it tests PTRACE_O_EXITKILL. And the > variable it controls is called "ptrace_options". How about > leaving the function's name as is? It was an attempt to give functions managing the same mechanisms to have a common naming convention. But it is unnecessary and I have restored the original name. > The patch should also include a GDB manual change describing the > new RSP features, and a NEWS change mentioning them. Added. An updated patch, including updated commit message, follow. The only substantive change to the commit message was to remove the paragraph about the temporary #if statements, which don't appear in this version of the patch. Thanks, --Don This patch implements a mechanism for GDB to determine whether fork events are supported in gdbserver. This is a preparatory patch for remote fork and exec event support. Two new RSP packets are defined to represent fork and vfork event support. These packets are used just like PACKET_multiprocess_feature to denote whether the corresponding event is supported. GDB sends fork-events+ and vfork-events+ to gdbserver to inquire about fork event support. If the response enables these packets, then GDB knows that gdbserver supports the corresponding events and will enable them. Target functions used to query for support are included along with each new packet. In order for gdbserver to know whether the events are supported at the point where the qSupported packet arrives, the code in nat/linux-ptrace.c had to be reorganized. Previously it would test for fork/exec event support, then enable the events using the pid of the inferior. When the qSupported packet arrives there may not be an inferior. So the mechanism was split into two parts: a function that checks whether the events are supported, called when gdbserver starts up, and another that enables the events when the inferior stops for the first time. Another gdbserver change was to add some global variables similar to multi_process, one per new packet. These are used to control whether the corresponding fork events are enabled. If GDB does not inquire about the event support in the qSupported packet, then gdbserver will not set these "report the event" flags. If the flags are not set, the events are ignored like they were in the past. Thus, gdbserver will never send fork event notification to an older GDB that doesn't recognize fork events. Tested on Ubuntu x64, native/remote/extended-remote, and as part of subsequent patches in the series. gdb/doc/ 2015-02-25 Don Breazeal * gdb.texinfo (General Query Packets): Add remote protocol features 'fork-events' and 'vfork-events'. gdb/gdbserver/ 2015-02-25 Don Breazeal * linux-low.c (linux_supports_fork_events): New function. (linux_supports_vfork_events): New function. (linux_target_ops): Initialize new structure members. (initialize_low): Call linux_check_ptrace_features. * lynx-low.c (lynx_target_ops): Initialize new structure members. * server.c (report_fork_events, report_vfork_events): New global flags. (handle_query): Add new features to qSupported packet. (captured_main): Initialize new global variables. * target.h (struct target_ops) : New member. : New member. (target_supports_fork_events): New macro. (target_supports_vfork_events): New macro. * win32-low.c (win32_target_ops): Initialize new structure members. gdb/ 2015-02-25 Don Breazeal * NEWS: Announce new fork event feature support in qSupported packet. * nat/linux-ptrace.c (linux_check_ptrace_features): Change from static to extern. * nat/linux-ptrace.h (linux_check_ptrace_features): Declare. * remote.c (anonymous enum): : New enumeration constants. (remote_query_supported): Add new feature queries to qSupported packet. (remote_protocol_features): Add table entries for new packets. (_initialize_remote): Exempt new packets from the requirement to have 'set remote' commands. --- gdb/NEWS | 3 +++ gdb/doc/gdb.texinfo | 12 ++++++++++++ gdb/gdbserver/linux-low.c | 21 +++++++++++++++++++++ gdb/gdbserver/lynx-low.c | 2 ++ gdb/gdbserver/server.c | 22 ++++++++++++++++++++++ gdb/gdbserver/target.h | 14 ++++++++++++++ gdb/gdbserver/win32-low.c | 2 ++ gdb/nat/linux-ptrace.c | 2 +- gdb/nat/linux-ptrace.h | 1 + gdb/remote.c | 23 +++++++++++++++++++++-- 10 files changed, 99 insertions(+), 3 deletions(-) diff --git a/gdb/NEWS b/gdb/NEWS index b79b162..d7fe430 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -71,6 +71,9 @@ Qbtrace-conf:bts:size * GDB now has support for DTrace USDT (Userland Static Defined Tracing) probes. The supported targets are x86_64-*-linux-gnu. +* The remote stub now reports support for fork and vfork events to GDB's +qSupported query. + *** Changes in GDB 7.9 * GDB now supports hardware watchpoints on x86 GNU Hurd. diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index e84a251..9834709 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -35762,6 +35762,18 @@ extensions unless the stub also reports that it supports them by including @samp{multiprocess+} in its @samp{qSupported} reply. @xref{multiprocess extensions}, for details. +@item fork-events +This feature indicates whether @value{GDBN} supports fork event +extensions to the remote protocol. @value{GDBN} does not use such +extensions unless the stub also reports that it supports them by +including @samp{fork-events+} in its @samp{qSupported} reply. + +@item vfork-events +This feature indicates whether @value{GDBN} supports vfork event +extensions to the remote protocol. @value{GDBN} does not use such +extensions unless the stub also reports that it supports them by +including @samp{vfork-events+} in its @samp{qSupported} reply. + @item xmlRegisters This feature indicates that @value{GDBN} supports the XML target description. If the stub sees @samp{xmlRegisters=} with target diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 1c66985..5eb7576 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -5154,6 +5154,22 @@ linux_supports_multi_process (void) return 1; } +/* Check if fork events are supported. */ + +static int +linux_supports_fork_events (void) +{ + return linux_supports_tracefork (); +} + +/* Check if vfork events are supported. */ + +static int +linux_supports_vfork_events (void) +{ + return linux_supports_tracefork (); +} + static int linux_supports_disable_randomization (void) { @@ -6114,6 +6130,8 @@ static struct target_ops linux_target_ops = { linux_async, linux_start_non_stop, linux_supports_multi_process, + linux_supports_fork_events, + linux_supports_vfork_events, #ifdef USE_THREAD_DB thread_db_handle_monitor_command, #else @@ -6190,4 +6208,7 @@ initialize_low (void) sigaction (SIGCHLD, &sigchld_action, NULL); initialize_low_arch (); + + /* Enable extended ptrace events. */ + linux_check_ptrace_features (); } diff --git a/gdb/gdbserver/lynx-low.c b/gdb/gdbserver/lynx-low.c index 98797ba..1126103 100644 --- a/gdb/gdbserver/lynx-low.c +++ b/gdb/gdbserver/lynx-low.c @@ -754,6 +754,8 @@ static struct target_ops lynx_target_ops = { NULL, /* async */ NULL, /* start_non_stop */ NULL, /* supports_multi_process */ + NULL, /* supports_fork_events */ + NULL, /* supports_vfork_events */ NULL, /* handle_monitor_command */ }; diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index 156fcc8..1efcbc0 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -57,6 +57,8 @@ static int exit_requested; int run_once; int multi_process; +int report_fork_events; +int report_vfork_events; int non_stop; /* Whether we should attempt to disable the operating system's address @@ -1977,6 +1979,18 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) /* GDB supports relocate instruction requests. */ gdb_supports_qRelocInsn = 1; } + if (strcmp (p, "fork-events+") == 0) + { + /* GDB supports and wants fork events if possible. */ + if (target_supports_fork_events ()) + report_fork_events = 1; + } + if (strcmp (p, "vfork-events+") == 0) + { + /* GDB supports and wants vfork events if possible. */ + if (target_supports_vfork_events ()) + report_vfork_events = 1; + } else target_process_qsupported (p); @@ -2027,6 +2041,12 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) if (target_supports_multi_process ()) strcat (own_buf, ";multiprocess+"); + if (target_supports_fork_events ()) + strcat (own_buf, ";fork-events+"); + + if (target_supports_vfork_events ()) + strcat (own_buf, ";vfork-events+"); + if (target_supports_non_stop ()) strcat (own_buf, ";QNonStop+"); @@ -3373,6 +3393,8 @@ captured_main (int argc, char *argv[]) noack_mode = 0; multi_process = 0; + report_fork_events = 0; + report_vfork_events = 0; /* Be sure we're out of tfind mode. */ current_traceframe = -1; cont_thread = null_ptid; diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h index 05feb36..74ed07c 100644 --- a/gdb/gdbserver/target.h +++ b/gdb/gdbserver/target.h @@ -262,6 +262,12 @@ struct target_ops /* Returns true if the target supports multi-process debugging. */ int (*supports_multi_process) (void); + /* Returns true if fork events are supported. */ + int (*supports_fork_events) (void); + + /* Returns true if vfork events are supported. */ + int (*supports_vfork_events) (void); + /* If not NULL, target-specific routine to process monitor command. Returns 1 if handled, or 0 to perform default processing. */ int (*handle_monitor_command) (char *); @@ -393,6 +399,14 @@ void set_target_ops (struct target_ops *); int kill_inferior (int); +#define target_supports_fork_events() \ + (the_target->supports_fork_events ? \ + (*the_target->supports_fork_events) () : 0) + +#define target_supports_vfork_events() \ + (the_target->supports_vfork_events ? \ + (*the_target->supports_vfork_events) () : 0) + #define detach_inferior(pid) \ (*the_target->detach) (pid) diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c index e3fb618..210a747 100644 --- a/gdb/gdbserver/win32-low.c +++ b/gdb/gdbserver/win32-low.c @@ -1823,6 +1823,8 @@ static struct target_ops win32_target_ops = { NULL, /* async */ NULL, /* start_non_stop */ NULL, /* supports_multi_process */ + NULL, /* supports_fork_events */ + NULL, /* supports_vfork_events */ NULL, /* handle_monitor_command */ NULL, /* core_of_thread */ NULL, /* read_loadmap */ diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c index 1c67819..ba934af 100644 --- a/gdb/nat/linux-ptrace.c +++ b/gdb/nat/linux-ptrace.c @@ -337,7 +337,7 @@ static void linux_test_for_exitkill (int child_pid); /* Determine ptrace features available on this target. */ -static void +void linux_check_ptrace_features (void) { int child_pid, ret, status; diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h index c5b0f14..c7bcfb0 100644 --- a/gdb/nat/linux-ptrace.h +++ b/gdb/nat/linux-ptrace.h @@ -98,6 +98,7 @@ extern void linux_ptrace_attach_fail_reason (pid_t pid, struct buffer *buffer); extern char *linux_ptrace_attach_fail_reason_string (ptid_t ptid, int err); extern void linux_ptrace_init_warnings (void); +extern void linux_check_ptrace_features (void); extern void linux_enable_event_reporting (pid_t pid, int attached); extern void linux_disable_event_reporting (pid_t pid); extern int linux_supports_tracefork (void); diff --git a/gdb/remote.c b/gdb/remote.c index 3479140..50b6d45 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -1338,6 +1338,12 @@ enum { /* Support for the Qbtrace-conf:bts:size packet. */ PACKET_Qbtrace_conf_bts_size, + /* Support for fork events. */ + PACKET_fork_event_feature, + + /* Support for vfork events. */ + PACKET_vfork_event_feature, + PACKET_MAX }; @@ -4025,7 +4031,11 @@ static const struct protocol_feature remote_protocol_features[] = { { "qXfer:btrace-conf:read", PACKET_DISABLE, remote_supported_packet, PACKET_qXfer_btrace_conf }, { "Qbtrace-conf:bts:size", PACKET_DISABLE, remote_supported_packet, - PACKET_Qbtrace_conf_bts_size } + PACKET_Qbtrace_conf_bts_size }, + { "fork-events", PACKET_DISABLE, remote_supported_packet, + PACKET_fork_event_feature }, + { "vfork-events", PACKET_DISABLE, remote_supported_packet, + PACKET_vfork_event_feature } }; static char *remote_support_xml; @@ -4099,6 +4109,12 @@ remote_query_supported (void) q = remote_query_supported_append (q, "qRelocInsn+"); + if (rs->extended) + { + q = remote_query_supported_append (q, "fork-events+"); + q = remote_query_supported_append (q, "vfork-events+"); + } + q = reconcat (q, "qSupported:", q, (char *) NULL); putpkt (q); @@ -12319,7 +12335,8 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL, add_packet_config_cmd (&remote_protocol_packets[PACKET_Qbtrace_conf_bts_size], "Qbtrace-conf:bts:size", "btrace-conf-bts-size", 0); - /* Assert that we've registered commands for all packet configs. */ + /* Assert that we've registered "set remote foo-packet" commands + for all packet configs. */ { int i; @@ -12338,6 +12355,8 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL, case PACKET_DisconnectedTracing_feature: case PACKET_augmented_libraries_svr4_read_feature: case PACKET_qCRC: + case PACKET_fork_event_feature: + case PACKET_vfork_event_feature: /* Additions to this list need to be well justified: pre-existing packets are OK; new packets are not. */ excepted = 1;