diff mbox

[v6,1/6] Identify remote fork event support

Message ID 1426625788-4469-2-git-send-email-donb@codesourcery.com
State New
Headers show

Commit Message

Don Breazeal March 17, 2015, 8:56 p.m. UTC 
Hi Pedro,
This patch is virtually unchanged from the previous version except
for accommodating new changes on the mainline.

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-03-17  Don Breazeal  <donb@codesourcery.com>

	* gdb.texinfo (General Query Packets): Add remote protocol
	features 'fork-events' and 'vfork-events'.

gdb/gdbserver/
2015-03-17  Don Breazeal  <donb@codesourcery.com>

	* 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) <supports_fork_events>:
	New member.
	<supports_vfork_events>: 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-03-17  Don Breazeal  <donb@codesourcery.com>

	* 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): <PACKET_fork_event_feature,
	* PACKET_vfork_event_feature>: 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              | 21 ++++++++++++++++++++-
 10 files changed, 98 insertions(+), 2 deletions(-)

Comments

Pedro Alves March 24, 2015, 10:57 a.m. UTC | #1 
On 03/17/2015 08:56 PM, Don Breazeal wrote:

> @@ -12508,7 +12524,8 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
>    add_packet_config_cmd (&remote_protocol_packets[PACKET_hwbreak_feature],
>                           "hwbreak-feature", "hwbreak-feature", 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;
>  

This hunk could go immediately/separately as obvious.


> @@ -12527,6 +12544,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.  */

I think I mentioned this before: please do register commands for these
features.  As the comment says, new packets here are not OK without
good justification, and I can't think of a good justification.
Then you'll need to make sure remote_query_supported only includes the
feature in GDB's query if the feature wasn't force-disabled.  See
the new swbreak+ feature's handling in that function.

>  	    excepted = 1;

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/NEWS b/gdb/NEWS
index bda4a35..d6c8769 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -92,6 +92,9 @@  vFile:fstat:
 HP/PA running HP-UX           hppa*-*-hpux*
 Itanium running HP-UX         ia64-*-hpux*
 
+* 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 9e71642..b089b1b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35806,6 +35806,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 48d905b..c710344 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -5255,6 +5255,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)
 {
@@ -6219,6 +6235,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
@@ -6295,4 +6313,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 2f85829..687cce0 100644
--- a/gdb/gdbserver/lynx-low.c
+++ b/gdb/gdbserver/lynx-low.c
@@ -758,6 +758,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 9ff2f8e..ee865fa 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;
 int swbreak_feature;
 int hwbreak_feature;
@@ -1992,6 +1994,18 @@  handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 		  if (target_supports_stopped_by_hw_breakpoint ())
 		    hwbreak_feature = 1;
 		}
+	      else if (strcmp (p, "fork-events+") == 0)
+		{
+		  /* GDB supports and wants fork events if possible.  */
+		  if (target_supports_fork_events ())
+		    report_fork_events = 1;
+		}
+	      else 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);
 
@@ -2042,6 +2056,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+");
 
@@ -3390,6 +3410,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 126c861..dc51627 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -277,6 +277,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 *);
@@ -408,6 +414,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 6c86765..5f50e46 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -1827,6 +1827,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 2244d9d..aba3da8 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 8354a4d..03d98c9 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -149,6 +149,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 9aaee13..85ced68 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1350,6 +1350,12 @@  enum {
   /* Support for hwbreak+ feature.  */
   PACKET_hwbreak_feature,
 
+  /* Support for fork events.  */
+  PACKET_fork_event_feature,
+
+  /* Support for vfork events.  */
+  PACKET_vfork_event_feature,
+
   PACKET_MAX
 };
 
@@ -4051,6 +4057,10 @@  static const struct protocol_feature remote_protocol_features[] = {
   { "hwbreak", PACKET_DISABLE, remote_supported_packet, PACKET_hwbreak_feature },
   { "vFile:fstat", PACKET_DISABLE, remote_supported_packet,
     PACKET_vFile_fstat },
+  { "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;
@@ -4129,6 +4139,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);
 
@@ -12508,7 +12524,8 @@  Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_hwbreak_feature],
                          "hwbreak-feature", "hwbreak-feature", 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;
 
@@ -12527,6 +12544,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;