[PATCH 0/5 V8] MI notification on trace started/stopped

Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed

* [PATCH 0/5 V8] MI notification on trace started/stopped
@ 2014-01-24  9:43 Yao Qi
  2014-01-24  9:43 ` [PATCH 2/5] Query supported notifications by qSupported Yao Qi
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Yao Qi @ 2014-01-24  9:43 UTC (permalink / raw)
  To: gdb-patches

Hello,
This is the V8 of the patch series for "MI notification on trace
started and stopped".  In V8, the changes are:

 - Treat notification as a feature in qSupported, to get to know
   a certain notification is supported or not.  It saves much
   code.
 - Call observer_notify_trace_changed only when GDB gets async
   remote notification Trace, so that the notification order is
   consistent in two sides (GDB side and GDBserver side).
 - Consume some notification events on some points in all stop
   mode.  See patch 1/5.

Patch 2/5 is about querying supported notifications.  Patch 3/5
is to add new async remote notification trace.  Patch 4/5 is to
add new MI notification on trace started and stopped.  Patch 5/5
is to add the test cases.

They are tested on x86_64-linux with
{unix, native-gdbserver} x {sync, async}.  Is it OK?

V7 can be found https://sourceware.org/ml/gdb-patches/2013-12/msg00495.html

*** BLURB HERE ***

Yao Qi (5):
  Process notifications when reply is got
  Query supported notifications by qSupported
  async remote notification 'Trace'.
  MI notification on trace started/stopped
  New test case mi-trace-changed.exp

 gdb/Makefile.in                              |    5 +-
 gdb/NEWS                                     |    5 +
 gdb/doc/gdb.texinfo                          |   26 +++++
 gdb/doc/observer.texi                        |    6 +
 gdb/gdbserver/notif.c                        |   44 ++++++++
 gdb/gdbserver/notif.h                        |    6 +
 gdb/gdbserver/server.c                       |   15 +++-
 gdb/gdbserver/tracepoint.c                   |   35 ++++++
 gdb/mi/mi-cmds.c                             |    6 +-
 gdb/mi/mi-interp.c                           |   22 ++++
 gdb/mi/mi-main.h                             |    2 +
 gdb/remote-notif-trace.c                     |   79 ++++++++++++++
 gdb/remote-notif.c                           |   57 +++++++++-
 gdb/remote-notif.h                           |   14 +++-
 gdb/remote.c                                 |   47 +++++++-
 gdb/testsuite/gdb.trace/mi-trace-changed.exp |  149 ++++++++++++++++++++++++++
 16 files changed, 504 insertions(+), 14 deletions(-)
 create mode 100644 gdb/remote-notif-trace.c
 create mode 100644 gdb/testsuite/gdb.trace/mi-trace-changed.exp

-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/5] Query supported notifications by qSupported
  2014-01-24  9:43 [PATCH 0/5 V8] MI notification on trace started/stopped Yao Qi
@ 2014-01-24  9:43 ` Yao Qi
  2014-01-24  9:43 ` [PATCH 5/5] New test case mi-trace-changed.exp Yao Qi
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Yao Qi @ 2014-01-24  9:43 UTC (permalink / raw)
  To: gdb-patches

V8: Treat each notification as feature.

V7: remove annex.

V6: the supported flag of each notification is saved in
'struct remote_notif_state' in GDB.  In GDBserver, the supported flag
is still associated with each notification.

As we we adding more notifications, both GDB and GDBserver
has to know what notifications are supported in the other
side.  This is what this patch does.  When GDB connects to GDBserver,
it will happen:

  --> qSupported:XXX;NotifN1+;NotifN2+;NotifN3+
      (GDB supports notification N1, N2 and N3)
  <-- XXX;NotifN1;NotifN2;NotifN4
      (GDBsever supports notification N1, N2 and N4)

after this, GDB knows what notifications GDBserver is able to send,
and GDBservers knows what notifications GDB doesn't support.

gdb/gdbserver:

	* notif.c (notif_qsupported_record): New function.
	(notif_qsupported_reply): New function.
	* notif.h (struct notif_server) <supported>: New field.
	(notif_qsupported_reply): Declare.
	(notif_qsupported_record): Declare.
	* server.c (notif_stop): Update.
	(handle_query): Call notif_qsupported_record and
	notif_qsupported_reply.

gdb:

	* remote-notif.c (remote_notif_ack): Add argument 'state'.
	Callers update.
	(remote_notif_parse): Likewise.
	(remote_notif_find): New function.
	(remote_notif_qsupported): New function.
	(remote_notif_state_allocate): Initialize field 'supported'.
	(remote_notif_state_xfree): Free field 'supported'.
	* remote-notif.h (struct remote_notif_state) <supported>: New
	field.
	(remote_notif_ack, remote_notif_parse): Update declarations.
	(remote_notif_qsupported): Declare.
	* remote.c (PACKET_notifications): New enum.
	(remote_notif_feature): New function.
	(remote_protocol_features): Add new element.
	(remote_query_supported): Call remote_notif_qsupported.
---
 gdb/gdbserver/notif.c  |   43 ++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/notif.h  |    5 ++++
 gdb/gdbserver/server.c |   15 ++++++++++++-
 gdb/remote-notif.c     |   56 +++++++++++++++++++++++++++++++++++++++++++++--
 gdb/remote-notif.h     |   12 +++++++++-
 gdb/remote.c           |   27 +++++++++++++++++++---
 6 files changed, 149 insertions(+), 9 deletions(-)

diff --git a/gdb/gdbserver/notif.c b/gdb/gdbserver/notif.c
index 8bc66dc..3847110 100644
--- a/gdb/gdbserver/notif.c
+++ b/gdb/gdbserver/notif.c
@@ -159,6 +159,49 @@ notif_event_xfree (struct notif_event *event)
   xfree (event);
 }
 
+/* Record the notification NOTIF supported by GDB.  */
+
+void
+notif_qsupported_record (char *notif)
+{
+  size_t i;
+
+  for (i = 0; i < ARRAY_SIZE (notifs); i++)
+    if (strcmp (notif, notifs[i]->notif_name) == 0)
+      break;
+
+  if (i < ARRAY_SIZE (notifs))
+    notifs[i]->supported = 1;
+}
+
+/* Return a string about notifications that GDBserver supports.
+   Return NULL if no notification is supported.  The caller is
+   responsible to free the returned string.  Suppose GDBserver
+   supports notifications N1, N2, and N3.  The returned string is
+   ";NotifN1+;NotifN2+;NotifN3+".  */
+
+void
+notif_qsupported_reply (char *buf)
+{
+  size_t i;
+
+#define BUF_LEN 128
+
+  for (i = 0; i < ARRAY_SIZE (notifs); i++)
+    {
+      struct notif_server *nb = notifs[i];
+      char s[BUF_LEN];
+
+      /* Skip stop notification because it has been supported
+	 unconditionally.   */
+      if (nb == &notif_stop)
+	continue;
+
+      xsnprintf (s, BUF_LEN, ";Notif%s+", nb->notif_name);
+      strcat (buf, s);
+    }
+}
+
 void
 initialize_notif (void)
 {
diff --git a/gdb/gdbserver/notif.h b/gdb/gdbserver/notif.h
index 3d233a1..671ef32 100644
--- a/gdb/gdbserver/notif.h
+++ b/gdb/gdbserver/notif.h
@@ -52,12 +52,17 @@ typedef struct notif_server
 
   /* Write event EVENT to OWN_BUF.  */
   void (*write) (struct notif_event *event, char *own_buf);
+
+  /* This notification is supported by GDB or not.  */
+  int supported;
 } *notif_server_p;
 
 extern struct notif_server notif_stop;
 
 int handle_notif_ack (char *own_buf, int packet_len);
 void notif_write_event (struct notif_server *notif, char *own_buf);
+void notif_qsupported_reply (char *buf);
+void notif_qsupported_record (char *notif);
 
 void notif_push (struct notif_server *np, struct notif_event *event);
 void notif_event_enque (struct notif_server *notif,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 88354be..0d8f545 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -185,7 +185,7 @@ vstop_notif_reply (struct notif_event *event, char *own_buf)
 
 struct notif_server notif_stop =
 {
-  "vStopped", "Stop", NULL, vstop_notif_reply,
+  "vStopped", "Stop", NULL, vstop_notif_reply, 1,
 };
 
 static int
@@ -1832,6 +1832,17 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 		  /* GDB supports relocate instruction requests.  */
 		  gdb_supports_qRelocInsn = 1;
 		}
+	      else if (strncmp (p, "Notif", 5) == 0)
+		{
+		  char *notif_name = xstrdup (&p[5]);
+
+		  /* Remove '+' at the end.  */
+		  notif_name[strlen(notif_name) - 1] = 0;
+		  /* Record what notifications GDB supports.  */
+		  notif_qsupported_record (notif_name);
+
+		  xfree (notif_name);
+		}
 	      else
 		target_process_qsupported (p);
 
@@ -1921,6 +1932,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 	  strcat (own_buf, ";qXfer:btrace:read+");
 	}
 
+      notif_qsupported_reply (own_buf);
+
       return;
     }
 
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index d3e1b26..ee2c2c4 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -59,7 +59,8 @@ static void do_notif_event_xfree (void *arg);
    acknowledge.  */
 
 void
-remote_notif_ack (struct notif_client *nc, char *buf)
+remote_notif_ack (struct notif_client *nc,
+		  struct remote_notif_state *state, char *buf)
 {
   struct notif_event *event = nc->alloc_event ();
   struct cleanup *old_chain
@@ -78,7 +79,8 @@ remote_notif_ack (struct notif_client *nc, char *buf)
 /* Parse the BUF for the expected notification NC.  */
 
 struct notif_event *
-remote_notif_parse (struct notif_client *nc, char *buf)
+remote_notif_parse (struct notif_client *nc,
+		    struct remote_notif_state *state, char *buf)
 {
   struct notif_event *event = nc->alloc_event ();
   struct cleanup *old_chain
@@ -121,6 +123,20 @@ remote_async_get_pending_events_handler (gdb_client_data data)
   remote_notif_process (data, NULL);
 }
 
+/* Find notification by NAME.  Return NULL if not found.  */
+
+struct notif_client *
+remote_notif_find (const char *name)
+{
+  size_t i;
+
+  for (i = 0; i < ARRAY_SIZE (notifs); i++)
+    if (strcmp (name, notifs[i]->name) == 0)
+      return notifs[i];
+
+  return NULL;
+}
+
 /* Remote notification handler.  Parse BUF, queue notification and
    update STATE.  */
 
@@ -158,7 +174,7 @@ handle_notification (struct remote_notif_state *state, char *buf)
   else
     {
       struct notif_event *event
-	= remote_notif_parse (nc, buf + strlen (nc->name) + 1);
+	= remote_notif_parse (nc, state, buf + strlen (nc->name) + 1);
 
       /* Be careful to only set it after parsing, since an error
 	 may be thrown then.  */
@@ -234,6 +250,36 @@ do_notif_event_xfree (void *arg)
   notif_event_xfree (arg);
 }
 
+/* Return a string about notifications that GDB supports.  The caller
+   is responsible to free the returned string.  Suppose GDB supports
+   notifications N1, N2, and N3.  The returned string is
+   ";NotifN1+;NotifN2+;NotifN3+".  */
+
+char *
+remote_notif_qsupported (char *q)
+{
+  int i;
+  char * p = q;
+#define BUF_LEN 128
+  char buf[BUF_LEN];
+
+  memcpy (buf, "Notif", 5);
+  for (i = 0; i < ARRAY_SIZE (notifs); i++)
+    {
+      struct notif_client *nb = notifs[i];
+
+      /* Skip stop notification because it has been supported
+	 unconditionally.   */
+      if (nb == &notif_client_stop)
+	continue;
+
+      strcpy (&buf[5], nb->name);
+      p = reconcat (p, p, ";", buf, "+", (char *) NULL);
+    }
+
+  return p;
+}
+
 /* Return an allocated remote_notif_state.  */
 
 struct remote_notif_state *
@@ -249,6 +295,8 @@ remote_notif_state_allocate (void)
     = create_async_event_handler (remote_async_get_pending_events_handler,
 				  notif_state);
 
+  notif_state->supported = xcalloc (ARRAY_SIZE (notifs), sizeof (int));
+
   return notif_state;
 }
 
@@ -268,6 +316,8 @@ remote_notif_state_xfree (struct remote_notif_state *state)
   for (i = 0; i < REMOTE_NOTIF_LAST; i++)
     notif_event_xfree (state->pending_event[i]);
 
+  xfree (state->supported);
+
   xfree (state);
 }
 
diff --git a/gdb/remote-notif.h b/gdb/remote-notif.h
index 0124ce0..6fd58bf 100644
--- a/gdb/remote-notif.h
+++ b/gdb/remote-notif.h
@@ -96,10 +96,17 @@ struct remote_notif_state
    remote.c:remote_notif_pending_replies).  */
 
   struct notif_event *pending_event[REMOTE_NOTIF_LAST];
+
+  /* Each element indicates the notification is supported by the
+     remote  stub or not.  The index is the field 'id' in
+     'struct notif_client'.  */
+  int *supported;
 };
 
-void remote_notif_ack (struct notif_client *nc, char *buf);
+void remote_notif_ack (struct notif_client *nc,
+		       struct remote_notif_state *state, char *buf);
 struct notif_event *remote_notif_parse (struct notif_client *nc,
+					struct remote_notif_state *state,
 					char *buf);
 
 void notif_event_xfree (struct notif_event *event);
@@ -111,6 +118,9 @@ void remote_notif_process (struct remote_notif_state *state,
 			   struct notif_client *except);
 struct remote_notif_state *remote_notif_state_allocate (void);
 void remote_notif_state_xfree (struct remote_notif_state *state);
+struct notif_client *remote_notif_find (const char *name);
+
+char *remote_notif_qsupported (char *buf);
 
 extern struct notif_client notif_client_stop;
 
diff --git a/gdb/remote.c b/gdb/remote.c
index 7110e0a..098ad8f 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3631,7 +3631,7 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
 	  /* remote_notif_get_pending_replies acks this one, and gets
 	     the rest out.  */
 	  rs->notif_state->pending_event[notif_client_stop.id]
-	    = remote_notif_parse (notif, rs->buf);
+	    = remote_notif_parse (notif, rs->notif_state, rs->buf);
 	  remote_notif_get_pending_events (notif);
 
 	  /* Make sure that threads that were stopped remain
@@ -4051,6 +4051,20 @@ remote_augmented_libraries_svr4_read_feature
   rs->augmented_libraries_svr4_read = (support == PACKET_ENABLE);
 }
 
+/* Function to call when the remote notification, as a feature, is
+   supported.  */
+
+static void
+remote_notif_feature (const struct protocol_feature *feature,
+		      enum packet_support support, const char *value)
+{
+  struct remote_state *rs = get_remote_state ();
+  struct notif_client *nc = remote_notif_find (&feature->name[5]);
+
+  if (nc != NULL)
+    rs->notif_state->supported[nc->id] = (support == PACKET_ENABLE);
+}
+
 static const struct protocol_feature remote_protocol_features[] = {
   { "PacketSize", PACKET_DISABLE, remote_packet_size, -1 },
   { "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet,
@@ -4125,7 +4139,8 @@ static const struct protocol_feature remote_protocol_features[] = {
   { "Qbtrace:off", PACKET_DISABLE, remote_supported_packet, PACKET_Qbtrace_off },
   { "Qbtrace:bts", PACKET_DISABLE, remote_supported_packet, PACKET_Qbtrace_bts },
   { "qXfer:btrace:read", PACKET_DISABLE, remote_supported_packet,
-    PACKET_qXfer_btrace }
+    PACKET_qXfer_btrace },
+  { "NotifTrace", PACKET_DISABLE, remote_notif_feature, -1 },
 };
 
 static char *remote_support_xml;
@@ -4199,6 +4214,8 @@ remote_query_supported (void)
 
       q = remote_query_supported_append (q, "qRelocInsn+");
 
+      q = remote_notif_qsupported (q);
+
       q = reconcat (q, "qSupported:", q, (char *) NULL);
       putpkt (q);
 
@@ -4654,7 +4671,8 @@ extended_remote_attach_1 (struct target_ops *target, char *args, int from_tty)
       if (target_can_async_p ())
 	{
 	  struct notif_event *reply
-	    =  remote_notif_parse (&notif_client_stop, wait_status);
+	    =  remote_notif_parse (&notif_client_stop, rs->notif_state,
+				   wait_status);
 
 	  push_stop_reply ((struct stop_reply *) reply);
 
@@ -5916,7 +5934,7 @@ remote_notif_get_pending_events (struct notif_client *nc)
 	  if (strcmp (rs->buf, "OK") == 0)
 	    break;
 	  else
-	    remote_notif_ack (nc, rs->buf);
+	    remote_notif_ack (nc, rs->notif_state, rs->buf);
 	}
     }
   else
@@ -6120,6 +6138,7 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
       {
 	struct stop_reply *stop_reply
 	  = (struct stop_reply *) remote_notif_parse (&notif_client_stop,
+						      rs->notif_state,
 						      rs->buf);
 
 	event_ptid = process_stop_reply (stop_reply, status);
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 5/5] New test case mi-trace-changed.exp
  2014-01-24  9:43 [PATCH 0/5 V8] MI notification on trace started/stopped Yao Qi
  2014-01-24  9:43 ` [PATCH 2/5] Query supported notifications by qSupported Yao Qi
@ 2014-01-24  9:43 ` Yao Qi
  2014-01-24  9:43 ` [PATCH 1/5] Process notifications when reply is got Yao Qi
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Yao Qi @ 2014-01-24  9:43 UTC (permalink / raw)
  To: gdb-patches

This test case tests the MI notifications are triggered by either CLI
commands or remote target.

gdb/testsuite:

	* gdb.trace/mi-trace-changed.exp: New test case.
---
 gdb/testsuite/gdb.trace/mi-trace-changed.exp |  149 ++++++++++++++++++++++++++
 1 files changed, 149 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/gdb.trace/mi-trace-changed.exp

diff --git a/gdb/testsuite/gdb.trace/mi-trace-changed.exp b/gdb/testsuite/gdb.trace/mi-trace-changed.exp
new file mode 100644
index 0000000..eec1e1e
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/mi-trace-changed.exp
@@ -0,0 +1,149 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+load_lib trace-support.exp
+
+standard_testfile status-stop.c
+
+if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
+	  executable {debug nowarnings}] != "" } {
+    untested mi-record-changed.exp
+    return -1
+}
+
+clean_restart $testfile
+
+if ![runto_main] {
+    fail "Can't run to main to check for trace support"
+    return -1
+}
+
+if ![gdb_target_supports_trace] {
+    unsupported "target does not support trace"
+    return -1;
+}
+
+gdb_exit
+
+# Verify that MI notification '=trace-started' and '=trace-stopped' are
+# emitted for normal 'tstart' and 'tstart' command.
+
+proc test_normal_tstart_stop { } {
+    with_test_prefix "tstart_tstop" {
+	global decimal hex
+
+	if [mi_gdb_start] {
+	    return
+	}
+	mi_run_to_main
+
+	mi_gdb_test "-break-insert -a main" {.*\^done,bkpt=.*} \
+	    "insert tracepoint on main"
+
+	# No =trace-started notification.
+	mi_gdb_test "-trace-start" "-trace-start\r\n=breakpoint-modified\[^\n\]+\r\n\\^done" \
+	    "start trace without notification"
+	mi_gdb_test "-trace-stop" \
+	    "-trace-stop\r\n\\^done,stop-reason=\"request\".*" \
+	    "stop trace without notification"
+
+	mi_gdb_test "tstart" \
+	    ".*=trace-started.*\\^done" "start trace notification"
+	mi_gdb_test "tstop" ".*=trace-stopped\\\\n\r\n\\^done" \
+	    "stop trace notification"
+
+	mi_gdb_exit
+    }
+}
+
+test_normal_tstart_stop
+
+# Verify that MI notification '=trace-stopped' is emitted when trace
+# buffer is full.
+
+proc test_trace_buffer_full { } {
+    with_test_prefix "tracebuffer full" {
+	global mi_gdb_prompt
+
+	if [mi_gdb_start] {
+	    return
+	}
+	mi_run_to_main
+
+	mi_gdb_test "-break-insert -a func2" {.*\^done,bkpt=.*} \
+	    "insert tracepoint on func2"
+
+	send_gdb "actions\n"
+	gdb_expect {
+	    -re "End with" {
+	    }
+	}
+
+	send_gdb "collect buf\nend\n"
+	set test "define actions"
+	gdb_expect {
+	    -re ".*${mi_gdb_prompt}$" {
+		pass $test
+	    }
+	    timeout {
+		fail "$test (timeout)"
+	    }
+	}
+
+	# No =trace-started notification.
+	mi_gdb_test "-trace-start" "-trace-start\r\n=breakpoint-modified\[^\n\]+\r\n\\^done" \
+	    "start trace without notification"
+	mi_gdb_test "-break-insert end" {.*\^done,bkpt=.*} \
+	    "insert breakpoint on end"
+
+	mi_send_resuming_command "exec-continue" \
+	    "continuing execution to end"
+
+	set test "trace-stopped triggered by bufferfull"
+	gdb_expect {
+	    # We don't set stop-notes.
+	    -re "=trace-stopped\\\\n" {
+		pass "$test"
+	    }
+	    timeout {
+		fail "$test (timeout)"
+	    }
+	}
+
+	global async
+	# In sync mode, eat all the output.  Don't have to do so in
+	# async mode.
+	if {!$async} {
+	    gdb_expect {
+		-re ".*${mi_gdb_prompt}$" {
+		}
+	    }
+	}
+	# GDB has got the rsp notifcation from remote stub that trace
+	# is stopped.
+	mi_gdb_test "tstop" ".*Trace is not running.*" \
+	    "tstop on stopped"
+
+	mi_gdb_test "-trace-status" ".*\\^done.*stop-reason=\"overflow\".*" \
+	    "trace-status"
+
+	mi_gdb_exit
+    }
+}
+
+test_trace_buffer_full
+
+return 0
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/5] Process notifications when reply is got
  2014-01-24  9:43 [PATCH 0/5 V8] MI notification on trace started/stopped Yao Qi
  2014-01-24  9:43 ` [PATCH 2/5] Query supported notifications by qSupported Yao Qi
  2014-01-24  9:43 ` [PATCH 5/5] New test case mi-trace-changed.exp Yao Qi
@ 2014-01-24  9:43 ` Yao Qi
  2014-01-24  9:43 ` [PATCH 3/5] async remote notification 'Trace' Yao Qi
  2014-01-24  9:43 ` [PATCH 4/5] MI notification on trace started/stopped Yao Qi
  4 siblings, 0 replies; 15+ messages in thread
From: Yao Qi @ 2014-01-24  9:43 UTC (permalink / raw)
  To: gdb-patches

GDB needs some points to consume notification events in all-stop mode.
The end of remote_get_noisy_reply is good place, because GDB has got
a reply from stub, and isn't expecting any others.  At this moment,
GDB is safe to ack notification events one by one.  Likewise, it is a
good time ack notification event after GDB receives a stop reply.

With this change, GDB is able to get Trace notification when command
tstart or tstop is executed.

gdb:

2014-01-24  Yao Qi  <yao@codesourcery.com>

	* remote.c (get_remote_state): Declare.
	(remote_get_noisy_reply): Call remote_notif_process if
	non_stop is false.
	(remote_wait_as): Likewise.
---
 gdb/remote.c |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 1c914ba..7110e0a 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -233,6 +233,8 @@ static int remote_supports_cond_breakpoints (void);
 
 static int remote_can_run_breakpoint_commands (void);
 
+static struct remote_state *get_remote_state (void);
+
 /* For "remote".  */
 
 static struct cmd_list_element *remote_cmdlist;
@@ -600,7 +602,17 @@ remote_get_noisy_reply (char **buf_p,
       else if (buf[0] == 'O' && buf[1] != 'K')
 	remote_console_output (buf + 1);	/* 'O' message from stub */
       else
-	return buf;		/* Here's the actual reply.  */
+	{
+	  struct remote_state *rs = get_remote_state ();
+
+	  /* GDB has got the reply from the remote stub.  It is safe
+	     to query notification events, because GDB isn't expecting
+	     any reply at this moment.  */
+	  if (!non_stop)
+	    remote_notif_process (rs->notif_state, &notif_client_stop);
+
+	  return buf;		/* Here's the actual reply.  */
+	}
     }
   while (1);
 }
@@ -6111,6 +6123,12 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
 						      rs->buf);
 
 	event_ptid = process_stop_reply (stop_reply, status);
+
+	/* GDB has got the reply from the remote stub.  It is safe
+	   to query notification events, because GDB isn't expecting
+	   any reply at this moment.  */
+	if (!non_stop)
+	  remote_notif_process (rs->notif_state, &notif_client_stop);
 	break;
       }
     case 'O':		/* Console output.  */
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/5] async remote notification 'Trace'.
  2014-01-24  9:43 [PATCH 0/5 V8] MI notification on trace started/stopped Yao Qi
                   ` (2 preceding siblings ...)
  2014-01-24  9:43 ` [PATCH 1/5] Process notifications when reply is got Yao Qi
@ 2014-01-24  9:43 ` Yao Qi
  2014-01-24  9:43 ` [PATCH 4/5] MI notification on trace started/stopped Yao Qi
  4 siblings, 0 replies; 15+ messages in thread
From: Yao Qi @ 2014-01-24  9:43 UTC (permalink / raw)
  To: gdb-patches

Hi,
This patch adds a new async remote notification 'Trace' to report the
trace-status-related changes in the stub.  So far, it is only used for
reporting 'tracing status change', like this,

%Trace:status:T0;tstop::0;tframes:0;tcreated:0;tfree:500000;tsize:500000;circular:0;disconn:0;starttime:001355728912543287;stoptime:001355728912543768;username::;notes:666f6f:\n

V8:
GDBserver sends async notification Trace when tracing is started or
stopped, in order to keep the order of events correct in GDB side.

gdb/gdbserver:

	* notif.c (notifs): Add "notif_trace".
	* notif.h (ontif_trace): Declare.
	* tracepoint.c [!IN_PROCESS_AGENT]: Include "notif.h".
	[!IN_PROCESS_AGENT] (notif_reply_trace): New function.
	[!IN_PROCESS_AGENT] (notif_trace): New variable.
	(stop_tracing): Call notif_push.
	(start_tracing): Call notif_push.

gdb:

	* Makefile.in (REMOTE_OBS): Append remote-notif-trace.o
	(SFILES): Add remote-notif-trace.c
	* remote-notif.c (notifs): Add "notif_client_trace".
	* remote-notif.h (notif_client_trace): Declare.
	(REMOTE_NOTIF_TRACE): New enum
	* remote-notif-trace.c: New.

gdb/doc:

	* gdb.texinfo (Packets): Add "vTraced".
	(Notification Packets): Add doc about "Trace" notification
	and "vTraced" packet.
	(General Query Packets): Document NotifTrace feature and reply of
	qSupported.
---
 gdb/Makefile.in            |    5 ++-
 gdb/doc/gdb.texinfo        |   22 +++++++++++++
 gdb/gdbserver/notif.c      |    1 +
 gdb/gdbserver/notif.h      |    1 +
 gdb/gdbserver/tracepoint.c |   35 ++++++++++++++++++++
 gdb/remote-notif-trace.c   |   76 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/remote-notif.c         |    1 +
 gdb/remote-notif.h         |    2 +
 8 files changed, 141 insertions(+), 2 deletions(-)
 create mode 100644 gdb/remote-notif-trace.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index ed84e35..0978440 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -531,7 +531,7 @@ SER_HARDWIRE = @SER_HARDWIRE@
 # The `remote' debugging target is supported for most architectures,
 # but not all (e.g. 960)
 REMOTE_OBS = remote.o dcache.o tracepoint.o ax-general.o ax-gdb.o remote-fileio.o \
-	remote-notif.o ctf.o
+	remote-notif.o ctf.o remote-notif-trace.o
 
 # This is remote-sim.o if a simulator is to be linked in.
 SIM_OBS = @SIM_OBS@
@@ -762,7 +762,8 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	p-exp.y p-lang.c p-typeprint.c p-valprint.c parse.c printcmd.c \
 	proc-service.list progspace.c \
 	prologue-value.c psymtab.c \
-	regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c reverse.c \
+	regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c \
+	remote-notif-trace.c reverse.c \
 	sentinel-frame.c \
 	serial.c ser-base.c ser-unix.c skip.c \
 	solib.c solib-target.c source.c \
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a990d2a..573bc83 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -38658,6 +38658,8 @@ for success (@pxref{Stop Reply Packets})
 
 @item vStopped
 @cindex @samp{vStopped} packet
+@itemx vTraced
+@cindex @samp{vTraced} packet
 @xref{Notification Packets}.
 
 @item X @var{addr},@var{length}:@var{XX@dots{}}
@@ -39491,6 +39493,10 @@ description.
 This feature indicates whether @value{GDBN} supports the
 @samp{qRelocInsn} packet (@pxref{Tracepoint Packets,,Relocate
 instruction reply packet}).
+
+@item NotifTrace
+This feature indicates that @value{GDBN} supports the async remote
+notification trace (@pxref{Notification Packets}).
 @end table
 
 Stubs should ignore any unknown values for
@@ -39709,6 +39715,11 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab No
 
+@item @samp{NotifTrace}
+@tab No
+@tab @samp{-}
+@tab No
+
 @end multitable
 
 These are the currently defined stub features, in more detail:
@@ -39876,6 +39887,10 @@ See @ref{Bytecode Descriptions} for details about the bytecode.
 The remote stub supports running a breakpoint's command list itself,
 rather than reporting the hit to @value{GDBN}.
 
+@item NotifTrace
+@cindex async notification trace, in remote protocol
+The remote stub supports the async notification trace.
+
 @item Qbtrace:off
 The remote stub understands the @samp{Qbtrace:off} packet.
 
@@ -40668,6 +40683,7 @@ continue the tracing run, while 0 tells the target to stop tracing if
 @value{GDBN} is no longer in the picture.
 
 @item qTStatus
+@anchor{qTStatus packet}
 @cindex @samp{qTStatus} packet
 Ask the stub if there is a trace experiment running right now.
 
@@ -41198,6 +41214,12 @@ for information on how these notifications are acknowledged by
 @value{GDBN}.
 @tab Report an asynchronous stop event in non-stop mode.
 
+@item Trace
+@tab vTraced
+@tab @var{status}.  The @var{reply} has the form of the reply to
+packet @samp{qTStatus}, as described in @ref{qTStatus packet}.
+@tab Report an asynchronous event related to trace status.
+
 @end multitable
 
 @node Remote Non-Stop
diff --git a/gdb/gdbserver/notif.c b/gdb/gdbserver/notif.c
index 3847110..437a9d0 100644
--- a/gdb/gdbserver/notif.c
+++ b/gdb/gdbserver/notif.c
@@ -52,6 +52,7 @@
 static struct notif_server *notifs[] =
 {
   &notif_stop,
+  &notif_trace,
 };
 
 /* Write another event or an OK, if there are no more left, to
diff --git a/gdb/gdbserver/notif.h b/gdb/gdbserver/notif.h
index 671ef32..2934d97 100644
--- a/gdb/gdbserver/notif.h
+++ b/gdb/gdbserver/notif.h
@@ -58,6 +58,7 @@ typedef struct notif_server
 } *notif_server_p;
 
 extern struct notif_server notif_stop;
+extern struct notif_server notif_trace;
 
 int handle_notif_ack (char *own_buf, int packet_len);
 void notif_write_event (struct notif_server *notif, char *own_buf);
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 3706577..22f88d9 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -3201,6 +3201,25 @@ install_tracepoint (struct tracepoint *tpoint, char *own_buf)
     write_ok (own_buf);
 }
 
+#ifndef IN_PROCESS_AGENT
+#include "notif.h"
+
+static void cmd_qtstatus (char *packet);
+
+static void
+notif_reply_trace (struct notif_event *event, char *own_buf)
+{
+  sprintf (own_buf, "status:");
+  cmd_qtstatus (own_buf + 7);
+}
+
+struct notif_server notif_trace =
+{
+  "vTraced", "Trace", NULL, notif_reply_trace, 0,
+};
+
+#endif
+
 static void download_tracepoint_1 (struct tracepoint *tpoint);
 
 static void
@@ -3394,6 +3413,14 @@ cmd_qtstart (char *packet)
   unpause_all (1);
 
   write_ok (packet);
+
+  if (notif_trace.supported)
+    {
+      struct notif_event *event
+	= xmalloc (sizeof (struct notif_event));
+
+      notif_push (&notif_trace, event);
+    }
 }
 
 /* End a tracing run, filling in a stop reason to report back to GDB,
@@ -3496,6 +3523,14 @@ stop_tracing (void)
     }
 
   unpause_all (1);
+
+  if (notif_trace.supported)
+    {
+      struct notif_event *event
+	= xmalloc (sizeof (struct notif_event));
+
+      notif_push (&notif_trace, event);
+    }
 }
 
 static int
diff --git a/gdb/remote-notif-trace.c b/gdb/remote-notif-trace.c
new file mode 100644
index 0000000..9ae1d33
--- /dev/null
+++ b/gdb/remote-notif-trace.c
@@ -0,0 +1,76 @@
+/* Async remote notification on trace.
+
+   Copyright (C) 2014 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include <string.h>
+#include "remote.h"
+#include "tracepoint.h"
+#include "remote-notif.h"
+
+static void
+remote_notif_trace_parse (struct notif_client *self, char *buf,
+			  struct notif_event *event)
+{
+  if (strncmp (buf, "status:", 7) == 0)
+    {
+      struct trace_status *ts = current_trace_status ();
+
+      if (buf[7] != 'T')
+	error (_("Unknown trace status in trace notification."));
+      parse_trace_status (buf + 7 + 1, ts);
+    }
+  else
+    error (_("Unknown trace notification."));
+}
+
+static void
+remote_notif_trace_ack (struct notif_client *self, char *buf,
+			struct notif_event *event)
+{
+  /* acknowledge */
+  putpkt ((char *) self->ack_command);
+}
+
+static int
+remote_notif_trace_can_get_pending_events (struct notif_client *self)
+{
+  return 1;
+}
+
+static struct notif_event *
+remote_notif_trace_alloc_event (void)
+{
+  struct notif_event *event = xmalloc (sizeof (struct notif_event));
+
+  event->dtr = NULL;
+
+  return event;
+}
+
+/* A client of notification 'Trace'.  */
+
+struct notif_client notif_client_trace =
+{
+  "Trace", "vTraced",
+  remote_notif_trace_parse,
+  remote_notif_trace_ack,
+  remote_notif_trace_can_get_pending_events,
+  remote_notif_trace_alloc_event,
+  REMOTE_NOTIF_TRACE,
+};
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index ee2c2c4..8672696 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -49,6 +49,7 @@ int notif_debug = 0;
 static struct notif_client *notifs[] =
 {
   &notif_client_stop,
+  &notif_client_trace,
 };
 
 gdb_static_assert (ARRAY_SIZE (notifs) == REMOTE_NOTIF_LAST);
diff --git a/gdb/remote-notif.h b/gdb/remote-notif.h
index 6fd58bf..1b55aab 100644
--- a/gdb/remote-notif.h
+++ b/gdb/remote-notif.h
@@ -36,6 +36,7 @@ struct notif_event
 enum REMOTE_NOTIF_ID
 {
   REMOTE_NOTIF_STOP = 0,
+  REMOTE_NOTIF_TRACE,
   REMOTE_NOTIF_LAST,
 };
 
@@ -123,6 +124,7 @@ struct notif_client *remote_notif_find (const char *name);
 char *remote_notif_qsupported (char *buf);
 
 extern struct notif_client notif_client_stop;
+extern struct notif_client notif_client_trace;
 
 extern int notif_debug;
 
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/5] MI notification on trace started/stopped
  2014-01-24  9:43 [PATCH 0/5 V8] MI notification on trace started/stopped Yao Qi
                   ` (3 preceding siblings ...)
  2014-01-24  9:43 ` [PATCH 3/5] async remote notification 'Trace' Yao Qi
@ 2014-01-24  9:43 ` Yao Qi
  4 siblings, 0 replies; 15+ messages in thread
From: Yao Qi @ 2014-01-24  9:43 UTC (permalink / raw)
  To: gdb-patches

Hi,
This patch adds the notifications of 'trace-started' and
'trace-stopped', which are emitted when trace is started or stopped by
command 'tstart' and 'tstop', so that when trace is started or stopped
in console, MI frontend can be notified.

The documentation patch was approved by Eli here
<http://sourceware.org/ml/gdb-patches/2013-02/msg00441.html>

V8:
 Only calls observer_notify_trace_changed when GDB gets an async
remote notification, in order to keep events order correct.

gdb/doc:

	* gdb.texinfo (GDB/MI Async Records): New MI notifications
	'trace-changed'.
	* observer.texi (GDB Observers): New observer 'trace-changed'.

gdb:

	* mi/mi-cmds.c (mi_cmds): Adjust for commands 'trace-start'
	and 'trace-stop'.
	* mi/mi-interp.c: Declare mi_trace_changed.
	(mi_interpreter_init): Install mi_trace_changed to observer.
	(mi_trace_changed): New.
	* mi/mi-main.h (struct mi_suppress_notification) <trace>:
	New field.
	* remote-notif-trace.c: Include "observer.h".
	(remote_notif_trace_parse): Call
	observer_notify_trace_changed.
	* NEWS: Mention it.
---
 gdb/NEWS                 |    5 +++++
 gdb/doc/gdb.texinfo      |    4 ++++
 gdb/doc/observer.texi    |    6 ++++++
 gdb/mi/mi-cmds.c         |    6 ++++--
 gdb/mi/mi-interp.c       |   22 ++++++++++++++++++++++
 gdb/mi/mi-main.h         |    2 ++
 gdb/remote-notif-trace.c |    3 +++
 7 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 6b1aacb..b8aaa2d 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -42,6 +42,11 @@ qXfer:btrace:read's annex
   The qXfer:btrace:read packet supports a new annex 'delta' to read
   branch trace incrementally.
 
+* MI changes
+
+  ** The start and stop of trace are now notified using new async records
+     "=trace-started" and "=trace-stopped".
+
 *** Changes in GDB 7.7
 
 * Improved support for process record-replay and reverse debugging on
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 573bc83..d0e13b7 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -29736,6 +29736,10 @@ written in an inferior.  The @var{id} is the identifier of the
 thread group corresponding to the affected inferior.  The optional
 @code{type="code"} part is reported if the memory written to holds
 executable code.
+
+@item =trace-started
+@itemx =trace-stopped
+Reports that trace was started or stopped.
 @end table
 
 @node GDB/MI Breakpoint Information
diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index 61acbb2..fa8bbaa 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -253,6 +253,12 @@ The trace state variable @var{tsv} is deleted.  If @var{tsv} is
 The trace state value @var{tsv} is modified.
 @end deftypefun
 
+@deftypefun void trace_changed (int @var{started})
+The status of trace in @value{GDBN} has changed.  The trace is started
+if @var{started} is non-zero, and the trace is stopped if
+@var{started} is zero.
+@end deftypefun
+
 @deftypefun void test_notification (int @var{somearg})
 This observer is used for internal testing.  Do not use.  
 See testsuite/gdb.gdb/observer.exp.
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 87a536f..81ecc46 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -158,9 +158,11 @@ static struct mi_cmd mi_cmds[] =
 		 mi_cmd_trace_frame_collected),
   DEF_MI_CMD_MI ("trace-list-variables", mi_cmd_trace_list_variables),
   DEF_MI_CMD_MI ("trace-save", mi_cmd_trace_save),
-  DEF_MI_CMD_MI ("trace-start", mi_cmd_trace_start),
+  DEF_MI_CMD_MI_1 ("trace-start", mi_cmd_trace_start,
+		   &mi_suppress_notification.trace),
   DEF_MI_CMD_MI ("trace-status", mi_cmd_trace_status),
-  DEF_MI_CMD_MI ("trace-stop", mi_cmd_trace_stop),
+  DEF_MI_CMD_MI_1 ("trace-stop", mi_cmd_trace_stop,
+		   &mi_suppress_notification.trace),
   DEF_MI_CMD_MI ("var-assign", mi_cmd_var_assign),
   DEF_MI_CMD_MI ("var-create", mi_cmd_var_create),
   DEF_MI_CMD_MI ("var-delete", mi_cmd_var_delete),
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 25bf0a1..44e3f35 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -64,6 +64,7 @@ static void mi_inferior_appeared (struct inferior *inf);
 static void mi_inferior_exit (struct inferior *inf);
 static void mi_inferior_removed (struct inferior *inf);
 static void mi_on_resume (ptid_t ptid);
+static void mi_trace_changed (int started);
 static void mi_solib_loaded (struct so_list *solib);
 static void mi_solib_unloaded (struct so_list *solib);
 static void mi_about_to_proceed (void);
@@ -129,6 +130,7 @@ mi_interpreter_init (struct interp *interp, int top_level)
       observer_attach_record_changed (mi_record_changed);
       observer_attach_normal_stop (mi_on_normal_stop);
       observer_attach_target_resumed (mi_on_resume);
+      observer_attach_trace_changed (mi_trace_changed);
       observer_attach_solib_loaded (mi_solib_loaded);
       observer_attach_solib_unloaded (mi_solib_unloaded);
       observer_attach_about_to_proceed (mi_about_to_proceed);
@@ -582,6 +584,26 @@ mi_tsv_modified (const struct trace_state_variable *tsv)
   gdb_flush (mi->event_channel);
 }
 
+/* Emit notification on trace was started or stopped.  */
+
+static void
+mi_trace_changed (int started)
+{
+  struct mi_interp *mi = top_level_interpreter_data ();
+
+  if (mi_suppress_notification.trace)
+    return;
+
+  target_terminal_ours ();
+
+  if (started)
+    fprintf_unfiltered (mi->event_channel, "trace-started\n");
+  else
+    fprintf_unfiltered (mi->event_channel, "trace-stopped\n");
+
+  gdb_flush (mi->event_channel);
+}
+
 /* Emit notification about a created breakpoint.  */
 
 static void
diff --git a/gdb/mi/mi-main.h b/gdb/mi/mi-main.h
index c32845d..343c8bb 100644
--- a/gdb/mi/mi-main.h
+++ b/gdb/mi/mi-main.h
@@ -43,6 +43,8 @@ struct mi_suppress_notification
   int traceframe;
   /* Memory changed notification suppressed?  */
   int memory;
+  /* Trace started/stopped notification suppressed?  */
+  int trace;
 };
 extern struct mi_suppress_notification mi_suppress_notification;
 
diff --git a/gdb/remote-notif-trace.c b/gdb/remote-notif-trace.c
index 9ae1d33..75b6300 100644
--- a/gdb/remote-notif-trace.c
+++ b/gdb/remote-notif-trace.c
@@ -22,6 +22,7 @@
 #include "remote.h"
 #include "tracepoint.h"
 #include "remote-notif.h"
+#include "observer.h"
 
 static void
 remote_notif_trace_parse (struct notif_client *self, char *buf,
@@ -34,6 +35,8 @@ remote_notif_trace_parse (struct notif_client *self, char *buf,
       if (buf[7] != 'T')
 	error (_("Unknown trace status in trace notification."));
       parse_trace_status (buf + 7 + 1, ts);
+
+      observer_notify_trace_changed (ts->running);
     }
   else
     error (_("Unknown trace notification."));
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 0/5] MI notification on trace started/stopped (V3)
@ 2013-01-22  7:05 Yao Qi
  2013-01-22  7:05 ` [PATCH 2/5] Query supported notifications by qSupported Yao Qi
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Yao Qi @ 2013-01-22  7:05 UTC (permalink / raw)
  To: gdb-patches

Hello,
This patch series adds the MI notifications of 'trace-started' and
'trace-stopped', which are emitted when

  1) trace is started or stopped by commands in GDB,
  2) trace is stopped due to some reasons in the remote stub, such as
trace buffer full.

With these notifications, MI front-end can show the status of trace
up to date.

Patch 4/5 is to address #1, adding new MI notifications and
notifying observers when the commands are called.  #2 needs more work
here, because GDB doesn't know the trace is stopped in the remote
stub.  So we need an async remote notification 'Trace' to tell GDB.
That is what patch 3/5 about.  Patch 5/5 is to use this async remote
notification 'Trace' and notify trace_changed observer.

Patch 1/5 and 2/5 are the enhancement to the async remote
notification, which is needed by the rest of patches in this series.
Patch 1/5 adds "annex" for notification, which is helpful 1) to give
more information on each event of notification, 2) to query supported
notifications on "annex" level.  Patch 2/5 teaches both GDB and
GDBserver to query supported notifications and annexes in the other
side so that 1) GDBserver doesn't send notifications that GDB doesn't
understand, 2) GDB doesn't have to fetch status from the GDBserver if
a certain notification is supported by GDBserver.

This patch series is the V3 of this series.  V2 was posted
<http://sourceware.org/ml/gdb-patches/2012-12/msg00687.html>.  Here
are some changes in V3,

  - Introduce query mechanism on supported notifications
  - Introduce concept "annex" into remote notification,
  - Remove trace notes in MI notifications, suggested by Pedro in the
    review.
  - Combine observers 'trace-started' and 'trace-stoped' into one
    observer 'trace-changed'.

Each patch is tested cumulatively on x86_64-linux with the combination
{unix, native-gdbserver} x {sync, async}.  Is it OK?

-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/5] Query supported notifications by qSupported
  2013-01-22  7:05 [PATCH 0/5] MI notification on trace started/stopped (V3) Yao Qi
@ 2013-01-22  7:05 ` Yao Qi
  2013-01-22  8:09   ` Eli Zaretskii
  2013-02-16  4:01 ` [PATCH 0/5] MI notification on trace started/stopped (V3.1) Yao Qi
  2013-04-02  2:33 ` [PATCH v4 0/5] MI notification on trace started/stopped Yao Qi
  2 siblings, 1 reply; 15+ messages in thread
From: Yao Qi @ 2013-01-22  7:05 UTC (permalink / raw)
  To: gdb-patches

As we we adding more notifications and annexes, both GDB and GDBserver
has to know what notifications and annexes are supported in the other
side.  This is what this patch does.  When GDB connects to GDBserver,
it will happen:

  --> qSupported:XXX;notifications=N1,N2.A1.A2,N3
      (GDB supports notification N1, N2 with annexes A1,A2, and N3)
  <-- XXX;Notifications=N1,N2.A1,N4
      (GDBsever supports notification N1, N2 with annex A1 and N4)

after this, GDB knows what notifications GDBserver is able to send,
and GDBservers knows what notifications GDB doesn't support.

gdb/gdbserver:

2013-01-21  Yao Qi  <yao@codesourcery.com>

	* Makefile.in (SFILES): Add "common-notif.c".
	(OBS): Add common-notif.o.
	(common-notif.o): New rule.

	* notif.c (notif_find_annex): New.
	(notif_qsupported_record): New.
	(+notif_qsupported_reply): New.
	* notif.h (notif_qsupported_reply): Declare.
	(notif_qsupported_record): Declare.
	* server.c (notif_annex_stop): Update.
	(handle_query): Call notif_qsupported_record and
	notif_qsupported_reply.
gdb:

2013-01-21  Yao Qi  <yao@codesourcery.com>

	* common/common-notif.c: New.
	* common/common-notif.h (struct notif_annex) <supported>: New
	field.
	* Makefile.in (iREMOTE_OBS): Append "common-notif.o".
	(SFILES): Add "common-notif.c".

	* remote-notif.c (remote_notif_parse_1): Call the parser of
	annex if it is supported.
	(remote_notif_qsupported): New.
	(remote_notif_qsupported_reply): New.
	* remote-notif.h (remote_notif_qsupported): Declare.
	(remote_notif_qsupported_reply): Declare.

	* remote.c (PACKET_notifications): New enum.
	(remote_notifications_feature): New.
	(remote_protocol_features): Add new element.
	(remote_query_supported): Call remote_notif_qsupported and
	append supported notifications to qSupported feature.
	(notif_client_annex_stop): Update.
	(_initialize_remote): Call add_packet_config_cmd.

gdb/doc:

2013-01-21  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (General Query Packets): Document the new feature
	'notifications' of 'qSupported' packet.
	Document the new feature in qSupported reply.
---
 gdb/Makefile.in           |    9 ++-
 gdb/common/common-notif.c |  181 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/common/common-notif.h |   15 ++++
 gdb/doc/gdb.texinfo       |   19 +++++
 gdb/gdbserver/Makefile.in |    7 +-
 gdb/gdbserver/notif.c     |   21 +++++
 gdb/gdbserver/notif.h     |    2 +
 gdb/gdbserver/server.c    |   15 ++++-
 gdb/remote-notif.c        |   32 +++++++-
 gdb/remote-notif.h        |    4 +
 gdb/remote.c              |   24 ++++++-
 11 files changed, 320 insertions(+), 9 deletions(-)
 create mode 100644 gdb/common/common-notif.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3bd4363..2e22ed0 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -508,7 +508,7 @@ SER_HARDWIRE = @SER_HARDWIRE@
 # The `remote' debugging target is supported for most architectures,
 # but not all (e.g. 960)
 REMOTE_OBS = remote.o dcache.o tracepoint.o ax-general.o ax-gdb.o remote-fileio.o \
-	remote-notif.o
+	remote-notif.o common-notif.o
 
 # This is remote-sim.o if a simulator is to be linked in.
 SIM_OBS = @SIM_OBS@
@@ -732,7 +732,8 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	p-exp.y p-lang.c p-typeprint.c p-valprint.c parse.c printcmd.c \
 	proc-service.list progspace.c \
 	prologue-value.c psymtab.c \
-	regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c reverse.c \
+	regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c \
+	common-notif.c reverse.c \
 	sentinel-frame.c \
 	serial.c ser-base.c ser-unix.c skip.c \
 	solib.c solib-target.c source.c \
@@ -1918,6 +1919,10 @@ common-utils.o: ${srcdir}/common/common-utils.c
 	$(COMPILE) $(srcdir)/common/common-utils.c
 	$(POSTCOMPILE)
 
+common-notif.o: ${srcdir}/common/common-notif.c
+	$(COMPILE) $(srcdir)/common/common-notif.c
+	$(POSTCOMPILE)
+
 gdb_vecs.o: ${srcdir}/common/gdb_vecs.c
 	$(COMPILE) $(srcdir)/common/gdb_vecs.c
 	$(POSTCOMPILE)
diff --git a/gdb/common/common-notif.c b/gdb/common/common-notif.c
new file mode 100644
index 0000000..4f23865
--- /dev/null
+++ b/gdb/common/common-notif.c
@@ -0,0 +1,181 @@
+/* Copyright (C) 2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+#include <string.h>
+#include "common-notif.h"
+#include "gdb_assert.h"
+
+/* Return a string about the notifications in array NOTIFS.  NUM is
+   the number of elements in array NOTIFS.  */
+
+char *
+notif_supported (struct notif_base *notifs[], int num)
+{
+  int i;
+  char * p = NULL;
+
+#define BUF_LEN 128
+
+  for (i = 0; i < num; i++)
+    {
+      struct notif_base *nb = notifs[i];
+
+      if (p == NULL)
+	{
+	  p = xmalloc (BUF_LEN);
+	  strcpy (p, nb->notif_name);
+	}
+      else
+	xsnprintf (p + strlen (p), BUF_LEN - strlen (p), ",%s",
+		   nb->notif_name);
+
+      if (NOTIF_HAS_ANNEX (nb))
+	{
+	  int j;
+
+	  NOTIF_ITER_ANNEX (nb, j)
+	    {
+	      xsnprintf (p + strlen (p), BUF_LEN - strlen (p),
+			 ".%s", nb->annexes[j].name);
+	    }
+	}
+    }
+
+  return p;
+}
+
+/* Find annex in notification NB by name NAME and length LEN.
+   If found, return annex, otherwise return NULL.  */
+
+static struct notif_annex *
+remote_notif_find_annex (struct notif_base *nb,
+			 const char *name, int len)
+{
+  if (NOTIF_HAS_ANNEX (nb))
+    {
+      int j;
+
+      NOTIF_ITER_ANNEX (nb, j)
+	if (strncmp (name, nb->annexes[j].name, len) == 0
+	    && len == strlen (nb->annexes[j].name))
+	  return &nb->annexes[j];
+    }
+  return NULL;
+}
+
+/* Parse the REPLY, which is about supported annexes and
+   notifications in peer, and disable some annexes
+   if the remote stub doesn't support.  */
+
+void
+notif_parse_supported (const char *reply,
+		       struct notif_base *notifs[], int num)
+{
+  const char *p = reply;
+  int notif_num = 1;
+  char **notif_str;
+  int i;
+
+  for (i = 0; reply[i] != '\0'; i++)
+    if (reply[i] == ',')
+      notif_num++;
+
+  notif_str = xmalloc (notif_num * sizeof (char *));
+  for (i = 0; i < notif_num; i++)
+    {
+      char *end = strchr (p, ',');
+
+      if (end == NULL)
+	notif_str[i] = xstrdup (p);
+      else
+	{
+	  /* Can't use xstrndup in GDBserver.  */
+	  notif_str[i] = strndup (p, end - p);
+	  p = end + 1;
+	}
+    }
+
+  for (i = 0; i < notif_num; i++)
+    {
+      int j;
+      struct notif_base *nb = NULL;
+
+      p = notif_str[i];
+
+      for (j = 0; j < num; j++)
+	{
+	  int name_len = strlen (notifs[j]->notif_name);
+
+	  if (0 == strncmp (notifs[j]->notif_name, p, name_len)
+	      && (p[name_len] == '.' || p[name_len] == 0))
+	    {
+	      nb = notifs[j];
+	      p += name_len;
+	      break;
+	    }
+	}
+
+      if (nb != NULL)
+	{
+	  if (p[0] == 0)
+	    {
+	      /* No annex.  */
+	      gdb_assert (!NOTIF_HAS_ANNEX (nb));
+	      nb->annexes[0].supported = 1;
+	    }
+	  else if (p[0] == '.')
+	    {
+	      gdb_assert (NOTIF_HAS_ANNEX (nb));
+
+	      p++;
+
+	      /* Parse the rest of P and look for annexes.  */
+	      while (p != NULL)
+		{
+		  char *end = strchr (p, '.');
+		  struct notif_annex *annex = NULL;
+
+		  if (end != NULL)
+		    {
+		      annex = remote_notif_find_annex (nb, p,
+						       end - p);
+		      p = end + 1;
+		    }
+		  else
+		    {
+		      annex = remote_notif_find_annex (nb, p,
+						       strlen (p));
+		      p = end;
+		    }
+
+		  /* If annex is known, mark it supported, otherwise
+		     skip it because the peer knows the annex but we
+		     don't know.  */
+		  if (annex != NULL)
+		    annex->supported = 1;
+		}
+	    }
+	  else
+	    warning (_("Unknown supported notification"));
+	}
+    }
+}
diff --git a/gdb/common/common-notif.h b/gdb/common/common-notif.h
index 1ffa26c..8e0e0c1 100644
--- a/gdb/common/common-notif.h
+++ b/gdb/common/common-notif.h
@@ -32,6 +32,11 @@ struct notif_annex
      notification this annex belongs to.  */
   const char *name;
 
+  /* This annex is supported by the peer (GDB or GDBserver)?  A
+     notification may have multiple annexes and some of them are
+     supported.  Annex is the smallest unit of support.  */
+  int supported;
+
 #ifdef GDBSERVER
   /* Write event EVENT to OWN_BUF.  */
   void (*write) (struct notif_event *event, char *own_buf);
@@ -53,6 +58,10 @@ struct notif_annex
 
 #define NOTIF_HAS_ANNEX(NOTIF) ((NOTIF)->annexes[0].name != NULL)
 
+/* Whether the annex of notification N is supported.  */
+
+#define NOTIF_ANNEX_SUPPORTED_P(N, INDEX) \
+  ((N).annexes[INDEX].supported)
 
 /* "Base class" of a notification.  It can be extended in both GDB
    and GDBserver to represent a type of notification.  */
@@ -71,3 +80,9 @@ struct notif_base
      of the notification.  */
   struct notif_annex *annexes;
 };
+
+char *notif_supported (struct notif_base *notifs[], int num);
+
+void notif_parse_supported (const char *reply,
+			    struct notif_base *notifs[], int num);
+
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8f08e82..bc5a8c7 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -37113,6 +37113,14 @@ description.
 This feature indicates whether @value{GDBN} supports the
 @samp{qRelocInsn} packet (@pxref{Tracepoint Packets,,Relocate
 instruction reply packet}).
+
+@item notifications
+@anchor{notifications feature}
+This feature indicates that @value{GDBN} supports the async remote
+notifications (@pxref{Notification Packets}).  If the stub sees
+@samp{notifications=} with a string of supported notifications,
+separated by comma, it will report notifications supported by the
+stub.
 @end table
 
 Stubs should ignore any unknown values for
@@ -37301,6 +37309,11 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab No
 
+@item @samp{Notifications}
+@tab Yes
+@tab @samp{-}
+@tab No
+
 @end multitable
 
 These are the currently defined stub features, in more detail:
@@ -37455,6 +37468,12 @@ See @ref{Bytecode Descriptions} for details about the bytecode.
 The remote stub supports running a breakpoint's command list itself,
 rather than reporting the hit to @value{GDBN}.
 
+@item Notifications=@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{[},@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{]}@dots{}
+@cindex notifications, in remote protocol
+The remote stub supports a string of notifications which the remote stub
+supports.  @var{name} is the name of the notification and @var{annex}
+is the name of the annex, if the notification has the annex.
+
 @end table
 
 @item qSymbol::
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index f8b1794..8533e54 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -155,7 +155,7 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/common/vec.c $(srcdir)/common/gdb_vecs.c \
 	$(srcdir)/common/common-utils.c $(srcdir)/common/xml-utils.c \
 	$(srcdir)/common/linux-osdata.c $(srcdir)/common/ptid.c \
-	$(srcdir)/common/buffer.c
+	$(srcdir)/common/buffer.c $(srcdir)/common/common-notif.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -165,7 +165,7 @@ SOURCES = $(SFILES)
 TAGFILES = $(SOURCES) ${HFILES} ${ALLPARAM} ${POSSLIBS}
 
 OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o target.o \
-	utils.o version.o vec.o gdb_vecs.o \
+	utils.o version.o vec.o gdb_vecs.o common-notif.o \
 	mem-break.o hostio.o event-loop.o tracepoint.o \
 	xml-utils.o common-utils.o ptid.o buffer.o format.o \
 	dll.o notif.o \
@@ -517,6 +517,9 @@ linux-ptrace.o: ../common/linux-ptrace.c
 common-utils.o: ../common/common-utils.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+common-notif.o: ../common/common-notif.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 vec.o: ../common/vec.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdb/gdbserver/notif.c b/gdb/gdbserver/notif.c
index 168f77d..e6b3756 100644
--- a/gdb/gdbserver/notif.c
+++ b/gdb/gdbserver/notif.c
@@ -182,6 +182,27 @@ notif_event_xfree (struct notif_event *event)
   xfree (event);
 }
 
+/* Record the notifications supported by GDB.  GDB_NOTIFICATIONS is a
+   string about notifications GDB supports.  */
+
+void
+notif_qsupported_record (char *gdb_notifications)
+{
+  return notif_parse_supported (gdb_notifications,
+				(struct notif_base **) notifs,
+				ARRAY_SIZE (notifs));
+}
+
+/* Return a string about notifications that GDBserver supports.
+   Return NULL if no notification is supported.  */
+
+char *
+notif_qsupported_reply (void)
+{
+  return notif_supported ((struct notif_base **) notifs,
+			  ARRAY_SIZE (notifs));
+}
+
 void
 initialize_notif (void)
 {
diff --git a/gdb/gdbserver/notif.h b/gdb/gdbserver/notif.h
index 5f9b6bc..72417aa 100644
--- a/gdb/gdbserver/notif.h
+++ b/gdb/gdbserver/notif.h
@@ -58,6 +58,8 @@ extern struct notif_server notif_stop;
 
 int handle_notif_ack (char *own_buf, int packet_len);
 void notif_write_event (struct notif_server *notif, char *own_buf);
+char* notif_qsupported_reply (void);
+void notif_qsupported_record (char *gdb_notifications);
 
 void notif_push (struct notif_server *np, struct notif_event *event);
 void notif_event_enque (struct notif_server *notif,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index f98a780..61f08bd 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -185,7 +185,7 @@ vstop_notif_reply (struct notif_event *event, char *own_buf)
 
 static struct notif_annex notif_annex_stop[] =
 {
-  { NULL, vstop_notif_reply, },
+  { NULL, 1, vstop_notif_reply, },
 };
 
 struct notif_server notif_stop =
@@ -1578,6 +1578,11 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 		  /* GDB supports relocate instruction requests.  */
 		  gdb_supports.qRelocInsn = 1;
 		}
+	      else if (strncmp (p, "notifications=", 14) == 0)
+		{
+		  /* Record what notifications GDB supports.  */
+		  notif_qsupported_record (&p[14]);
+		}
 	      else
 		target_process_qsupported (p);
 
@@ -1659,6 +1664,14 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
       if (target_supports_agent ())
 	strcat (own_buf, ";QAgent+");
 
+      p = notif_qsupported_reply ();
+
+      if (p != NULL)
+	{
+	  strcat (own_buf, ";Notifications=");
+	  strcat (own_buf, p);
+	  xfree (p);
+	}
       return;
     }
 
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index e0bc745..ae05bbf 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -75,9 +75,16 @@ remote_notif_parse_1 (struct notif_client *nc,
 		 contents in BUF.  */
 	      && buf[strlen (m->name)] == ':')
 	    {
-	      /* Pass BUF without annex and ':'.  */
-	      m->parse (nc, buf + strlen (m->name) + 1, event);
-	      break;
+	      if (m->supported)
+		{
+		  /* Pass BUF without annex and ':'.  */
+		  m->parse (nc, buf + strlen (m->name) + 1, event);
+		  break;
+		}
+	      else
+		warning (_("GDB gets annex '%s' of notification '%s'"
+			   "but remote stub doesn't support"),
+			 base->notif_name, m->name);
 	    }
 	  m = NULL;
 	}
@@ -305,6 +312,25 @@ notif_xfree (struct notif_client *notif)
   xfree (notif);
 }
 
+/* Return a string of GDB supported features.  */
+
+char *
+remote_notif_qsupported (void)
+{
+  return notif_supported ((struct notif_base **) notifs,
+			  ARRAY_SIZE (notifs));
+}
+
+/* Parse the qSupported reply REPLY from the remote stub and disable
+   some notifications if the remote stub doesn't support.  */
+
+void
+remote_notif_qsupported_reply (const char *reply)
+{
+  notif_parse_supported (reply, (struct notif_base **) notifs,
+			 ARRAY_SIZE (notifs));
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_notif;
 
diff --git a/gdb/remote-notif.h b/gdb/remote-notif.h
index cb1636f..4859254 100644
--- a/gdb/remote-notif.h
+++ b/gdb/remote-notif.h
@@ -69,6 +69,10 @@ void remote_notif_register_async_event_handler (void);
 void remote_notif_unregister_async_event_handler (void);
 
 void remote_notif_process (struct notif_client *except);
+
+char * remote_notif_qsupported (void);
+void remote_notif_qsupported_reply (const char *reply);
+
 extern struct notif_client notif_client_stop;
 
 extern unsigned int notif_debug;
diff --git a/gdb/remote.c b/gdb/remote.c
index 92a0f27..aeb93ff 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1284,6 +1284,7 @@ enum {
   PACKET_qXfer_fdpic,
   PACKET_QDisableRandomization,
   PACKET_QAgent,
+  PACKET_notifications,
   PACKET_MAX
 };
 
@@ -3873,6 +3874,14 @@ remote_string_tracing_feature (const struct protocol_feature *feature,
   rs->string_tracing = (support == PACKET_ENABLE);
 }
 
+static void
+remote_notifications_feature (const struct protocol_feature *feature,
+			      enum packet_support support,
+			      const char *value)
+{
+  remote_notif_qsupported_reply (value);
+}
+
 static struct protocol_feature remote_protocol_features[] = {
   { "PacketSize", PACKET_DISABLE, remote_packet_size, -1 },
   { "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet,
@@ -3938,6 +3947,8 @@ static struct protocol_feature remote_protocol_features[] = {
   { "QDisableRandomization", PACKET_DISABLE, remote_supported_packet,
     PACKET_QDisableRandomization },
   { "QAgent", PACKET_DISABLE, remote_supported_packet, PACKET_QAgent},
+  { "Notifications", PACKET_DISABLE, remote_notifications_feature,
+    PACKET_notifications },
   { "tracenz", PACKET_DISABLE,
     remote_string_tracing_feature, -1 },
 };
@@ -4004,6 +4015,7 @@ remote_query_supported (void)
   if (remote_protocol_packets[PACKET_qSupported].support != PACKET_DISABLE)
     {
       char *q = NULL;
+      char *notifications = remote_notif_qsupported ();
       struct cleanup *old_chain = make_cleanup (free_current_contents, &q);
 
       q = remote_query_supported_append (q, "multiprocess+");
@@ -4013,6 +4025,10 @@ remote_query_supported (void)
 
       q = remote_query_supported_append (q, "qRelocInsn+");
 
+      q = reconcat (q, q, ";notifications=", notifications,
+		    (char *) NULL);
+      xfree (notifications);
+
       q = reconcat (q, "qSupported:", q, (char *) NULL);
       putpkt (q);
 
@@ -5211,7 +5227,10 @@ remote_notif_stop_alloc_reply (void)
 
 static struct notif_annex notif_client_annex_stop[] =
 {
-  { NULL, remote_notif_stop_parse, },
+  /* Even the remote stub doesn't understand
+     'qSupported:notifications=', it may still support notification
+     stop if it supports non-stop.  */
+  { NULL, 1, remote_notif_stop_parse, },
 };
 
 /* A client of notification Stop.  */
@@ -11696,6 +11715,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_QAgent],
 			 "QAgent", "agent", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_notifications],
+			 "Notifications", "read notifications", 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
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/5] Query supported notifications by qSupported
  2013-01-22  7:05 ` [PATCH 2/5] Query supported notifications by qSupported Yao Qi
@ 2013-01-22  8:09   ` Eli Zaretskii
  2013-01-22  8:41     ` Yao Qi
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2013-01-22  8:09 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

> From: Yao Qi <yao@codesourcery.com>
> Date: Tue, 22 Jan 2013 15:03:49 +0800
> 
> +@item notifications
> +@anchor{notifications feature}
> +This feature indicates that @value{GDBN} supports the async remote
> +notifications (@pxref{Notification Packets}).  If the stub sees
> +@samp{notifications=} with a string of supported notifications,
> +separated by comma, it will report notifications supported by the
> +stub.     ^^^^^^^^

"by commas", in plural.

> +@cindex notifications, in remote protocol
> +The remote stub supports a string of notifications which the remote stub
> +supports.

Something is wrong with this sentence...


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/5] Query supported notifications by qSupported
  2013-01-22  8:09   ` Eli Zaretskii
@ 2013-01-22  8:41     ` Yao Qi
  2013-01-22 10:26       ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Yao Qi @ 2013-01-22  8:41 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 01/22/2013 04:08 PM, Eli Zaretskii wrote:
> "by commas", in plural.
> 
Fixed.

>> >+@cindex notifications, in remote protocol
>> >+The remote stub supports a string of notifications which the remote stub
>> >+supports.
> Something is wrong with this sentence...

How about this?

@cindex notifications, in remote protocol
The remote stub supports a string of notifications.  @var{name} is
the name of the notification and @var{annex} is the name of the annex,
if the notification has the annex.

Below is the updated version with some minor non-doc cleanups.

-- 
Yao (é½å°§)

gdb/gdbserver:

2013-01-21  Yao Qi  <yao@codesourcery.com>

	* Makefile.in (SFILES): Add "common-notif.c".
	(OBS): Add common-notif.o.
	(common-notif.o): New rule.

	* notif.c (notif_find_annex): New.
	(notif_qsupported_record): New.
	(+notif_qsupported_reply): New.
	* notif.h (notif_qsupported_reply): Declare.
	(notif_qsupported_record): Declare.
	* server.c (notif_annex_stop): Update.
	(handle_query): Call notif_qsupported_record and
	notif_qsupported_reply.
gdb:

2013-01-21  Yao Qi  <yao@codesourcery.com>

	* common/common-notif.c: New.
	* common/common-notif.h (struct notif_annex) <supported>: New
	field.
	* Makefile.in (iREMOTE_OBS): Append "common-notif.o".
	(SFILES): Add "common-notif.c".

	* remote-notif.c (remote_notif_parse_1): Call the parser of
	annex if it is supported.
	(remote_notif_qsupported): New.
	(remote_notif_qsupported_reply): New.
	* remote-notif.h (remote_notif_qsupported): Declare.
	(remote_notif_qsupported_reply): Declare.

	* remote.c (PACKET_notifications): New enum.
	(remote_notifications_feature): New.
	(remote_protocol_features): Add new element.
	(remote_query_supported): Call remote_notif_qsupported and
	append supported notifications to qSupported feature.
	(notif_client_annex_stop): Update.

gdb/doc:

2013-01-21  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (General Query Packets): Document the new feature
	'notifications' of 'qSupported' packet.
	Document the new feature in qSupported reply.
---
 gdb/Makefile.in           |    9 ++-
 gdb/common/common-notif.c |  181 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/common/common-notif.h |   15 ++++
 gdb/doc/gdb.texinfo       |   19 +++++
 gdb/gdbserver/Makefile.in |    7 +-
 gdb/gdbserver/notif.c     |   21 +++++
 gdb/gdbserver/notif.h     |    2 +
 gdb/gdbserver/server.c    |   15 ++++-
 gdb/remote-notif.c        |   32 +++++++-
 gdb/remote-notif.h        |    4 +
 gdb/remote.c              |   22 +++++-
 11 files changed, 318 insertions(+), 9 deletions(-)
 create mode 100644 gdb/common/common-notif.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 3bd4363..2e22ed0 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -508,7 +508,7 @@ SER_HARDWIRE = @SER_HARDWIRE@
 # The `remote' debugging target is supported for most architectures,
 # but not all (e.g. 960)
 REMOTE_OBS = remote.o dcache.o tracepoint.o ax-general.o ax-gdb.o remote-fileio.o \
-	remote-notif.o
+	remote-notif.o common-notif.o
 
 # This is remote-sim.o if a simulator is to be linked in.
 SIM_OBS = @SIM_OBS@
@@ -732,7 +732,8 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	p-exp.y p-lang.c p-typeprint.c p-valprint.c parse.c printcmd.c \
 	proc-service.list progspace.c \
 	prologue-value.c psymtab.c \
-	regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c reverse.c \
+	regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c \
+	common-notif.c reverse.c \
 	sentinel-frame.c \
 	serial.c ser-base.c ser-unix.c skip.c \
 	solib.c solib-target.c source.c \
@@ -1918,6 +1919,10 @@ common-utils.o: ${srcdir}/common/common-utils.c
 	$(COMPILE) $(srcdir)/common/common-utils.c
 	$(POSTCOMPILE)
 
+common-notif.o: ${srcdir}/common/common-notif.c
+	$(COMPILE) $(srcdir)/common/common-notif.c
+	$(POSTCOMPILE)
+
 gdb_vecs.o: ${srcdir}/common/gdb_vecs.c
 	$(COMPILE) $(srcdir)/common/gdb_vecs.c
 	$(POSTCOMPILE)
diff --git a/gdb/common/common-notif.c b/gdb/common/common-notif.c
new file mode 100644
index 0000000..4f23865
--- /dev/null
+++ b/gdb/common/common-notif.c
@@ -0,0 +1,181 @@
+/* Copyright (C) 2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+#include <string.h>
+#include "common-notif.h"
+#include "gdb_assert.h"
+
+/* Return a string about the notifications in array NOTIFS.  NUM is
+   the number of elements in array NOTIFS.  */
+
+char *
+notif_supported (struct notif_base *notifs[], int num)
+{
+  int i;
+  char * p = NULL;
+
+#define BUF_LEN 128
+
+  for (i = 0; i < num; i++)
+    {
+      struct notif_base *nb = notifs[i];
+
+      if (p == NULL)
+	{
+	  p = xmalloc (BUF_LEN);
+	  strcpy (p, nb->notif_name);
+	}
+      else
+	xsnprintf (p + strlen (p), BUF_LEN - strlen (p), ",%s",
+		   nb->notif_name);
+
+      if (NOTIF_HAS_ANNEX (nb))
+	{
+	  int j;
+
+	  NOTIF_ITER_ANNEX (nb, j)
+	    {
+	      xsnprintf (p + strlen (p), BUF_LEN - strlen (p),
+			 ".%s", nb->annexes[j].name);
+	    }
+	}
+    }
+
+  return p;
+}
+
+/* Find annex in notification NB by name NAME and length LEN.
+   If found, return annex, otherwise return NULL.  */
+
+static struct notif_annex *
+remote_notif_find_annex (struct notif_base *nb,
+			 const char *name, int len)
+{
+  if (NOTIF_HAS_ANNEX (nb))
+    {
+      int j;
+
+      NOTIF_ITER_ANNEX (nb, j)
+	if (strncmp (name, nb->annexes[j].name, len) == 0
+	    && len == strlen (nb->annexes[j].name))
+	  return &nb->annexes[j];
+    }
+  return NULL;
+}
+
+/* Parse the REPLY, which is about supported annexes and
+   notifications in peer, and disable some annexes
+   if the remote stub doesn't support.  */
+
+void
+notif_parse_supported (const char *reply,
+		       struct notif_base *notifs[], int num)
+{
+  const char *p = reply;
+  int notif_num = 1;
+  char **notif_str;
+  int i;
+
+  for (i = 0; reply[i] != '\0'; i++)
+    if (reply[i] == ',')
+      notif_num++;
+
+  notif_str = xmalloc (notif_num * sizeof (char *));
+  for (i = 0; i < notif_num; i++)
+    {
+      char *end = strchr (p, ',');
+
+      if (end == NULL)
+	notif_str[i] = xstrdup (p);
+      else
+	{
+	  /* Can't use xstrndup in GDBserver.  */
+	  notif_str[i] = strndup (p, end - p);
+	  p = end + 1;
+	}
+    }
+
+  for (i = 0; i < notif_num; i++)
+    {
+      int j;
+      struct notif_base *nb = NULL;
+
+      p = notif_str[i];
+
+      for (j = 0; j < num; j++)
+	{
+	  int name_len = strlen (notifs[j]->notif_name);
+
+	  if (0 == strncmp (notifs[j]->notif_name, p, name_len)
+	      && (p[name_len] == '.' || p[name_len] == 0))
+	    {
+	      nb = notifs[j];
+	      p += name_len;
+	      break;
+	    }
+	}
+
+      if (nb != NULL)
+	{
+	  if (p[0] == 0)
+	    {
+	      /* No annex.  */
+	      gdb_assert (!NOTIF_HAS_ANNEX (nb));
+	      nb->annexes[0].supported = 1;
+	    }
+	  else if (p[0] == '.')
+	    {
+	      gdb_assert (NOTIF_HAS_ANNEX (nb));
+
+	      p++;
+
+	      /* Parse the rest of P and look for annexes.  */
+	      while (p != NULL)
+		{
+		  char *end = strchr (p, '.');
+		  struct notif_annex *annex = NULL;
+
+		  if (end != NULL)
+		    {
+		      annex = remote_notif_find_annex (nb, p,
+						       end - p);
+		      p = end + 1;
+		    }
+		  else
+		    {
+		      annex = remote_notif_find_annex (nb, p,
+						       strlen (p));
+		      p = end;
+		    }
+
+		  /* If annex is known, mark it supported, otherwise
+		     skip it because the peer knows the annex but we
+		     don't know.  */
+		  if (annex != NULL)
+		    annex->supported = 1;
+		}
+	    }
+	  else
+	    warning (_("Unknown supported notification"));
+	}
+    }
+}
diff --git a/gdb/common/common-notif.h b/gdb/common/common-notif.h
index 1ffa26c..8e0e0c1 100644
--- a/gdb/common/common-notif.h
+++ b/gdb/common/common-notif.h
@@ -32,6 +32,11 @@ struct notif_annex
      notification this annex belongs to.  */
   const char *name;
 
+  /* This annex is supported by the peer (GDB or GDBserver)?  A
+     notification may have multiple annexes and some of them are
+     supported.  Annex is the smallest unit of support.  */
+  int supported;
+
 #ifdef GDBSERVER
   /* Write event EVENT to OWN_BUF.  */
   void (*write) (struct notif_event *event, char *own_buf);
@@ -53,6 +58,10 @@ struct notif_annex
 
 #define NOTIF_HAS_ANNEX(NOTIF) ((NOTIF)->annexes[0].name != NULL)
 
+/* Whether the annex of notification N is supported.  */
+
+#define NOTIF_ANNEX_SUPPORTED_P(N, INDEX) \
+  ((N).annexes[INDEX].supported)
 
 /* "Base class" of a notification.  It can be extended in both GDB
    and GDBserver to represent a type of notification.  */
@@ -71,3 +80,9 @@ struct notif_base
      of the notification.  */
   struct notif_annex *annexes;
 };
+
+char *notif_supported (struct notif_base *notifs[], int num);
+
+void notif_parse_supported (const char *reply,
+			    struct notif_base *notifs[], int num);
+
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8f08e82..0c25772 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -37113,6 +37113,14 @@ description.
 This feature indicates whether @value{GDBN} supports the
 @samp{qRelocInsn} packet (@pxref{Tracepoint Packets,,Relocate
 instruction reply packet}).
+
+@item notifications
+@anchor{notifications feature}
+This feature indicates that @value{GDBN} supports the async remote
+notifications (@pxref{Notification Packets}).  If the stub sees
+@samp{notifications=} with a string of supported notifications,
+separated by commas, it will report notifications supported by the
+stub.
 @end table
 
 Stubs should ignore any unknown values for
@@ -37301,6 +37309,11 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab No
 
+@item @samp{Notifications}
+@tab Yes
+@tab @samp{-}
+@tab No
+
 @end multitable
 
 These are the currently defined stub features, in more detail:
@@ -37455,6 +37468,12 @@ See @ref{Bytecode Descriptions} for details about the bytecode.
 The remote stub supports running a breakpoint's command list itself,
 rather than reporting the hit to @value{GDBN}.
 
+@item Notifications=@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{[},@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{]}@dots{}
+@cindex notifications, in remote protocol
+The remote stub supports a string of notifications.  @var{name} is
+the name of the notification and @var{annex} is the name of the annex,
+if the notification has the annex.
+
 @end table
 
 @item qSymbol::
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index f8b1794..8533e54 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -155,7 +155,7 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/common/vec.c $(srcdir)/common/gdb_vecs.c \
 	$(srcdir)/common/common-utils.c $(srcdir)/common/xml-utils.c \
 	$(srcdir)/common/linux-osdata.c $(srcdir)/common/ptid.c \
-	$(srcdir)/common/buffer.c
+	$(srcdir)/common/buffer.c $(srcdir)/common/common-notif.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -165,7 +165,7 @@ SOURCES = $(SFILES)
 TAGFILES = $(SOURCES) ${HFILES} ${ALLPARAM} ${POSSLIBS}
 
 OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o target.o \
-	utils.o version.o vec.o gdb_vecs.o \
+	utils.o version.o vec.o gdb_vecs.o common-notif.o \
 	mem-break.o hostio.o event-loop.o tracepoint.o \
 	xml-utils.o common-utils.o ptid.o buffer.o format.o \
 	dll.o notif.o \
@@ -517,6 +517,9 @@ linux-ptrace.o: ../common/linux-ptrace.c
 common-utils.o: ../common/common-utils.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+common-notif.o: ../common/common-notif.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 vec.o: ../common/vec.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdb/gdbserver/notif.c b/gdb/gdbserver/notif.c
index 168f77d..e6b3756 100644
--- a/gdb/gdbserver/notif.c
+++ b/gdb/gdbserver/notif.c
@@ -182,6 +182,27 @@ notif_event_xfree (struct notif_event *event)
   xfree (event);
 }
 
+/* Record the notifications supported by GDB.  GDB_NOTIFICATIONS is a
+   string about notifications GDB supports.  */
+
+void
+notif_qsupported_record (char *gdb_notifications)
+{
+  return notif_parse_supported (gdb_notifications,
+				(struct notif_base **) notifs,
+				ARRAY_SIZE (notifs));
+}
+
+/* Return a string about notifications that GDBserver supports.
+   Return NULL if no notification is supported.  */
+
+char *
+notif_qsupported_reply (void)
+{
+  return notif_supported ((struct notif_base **) notifs,
+			  ARRAY_SIZE (notifs));
+}
+
 void
 initialize_notif (void)
 {
diff --git a/gdb/gdbserver/notif.h b/gdb/gdbserver/notif.h
index 5f9b6bc..72417aa 100644
--- a/gdb/gdbserver/notif.h
+++ b/gdb/gdbserver/notif.h
@@ -58,6 +58,8 @@ extern struct notif_server notif_stop;
 
 int handle_notif_ack (char *own_buf, int packet_len);
 void notif_write_event (struct notif_server *notif, char *own_buf);
+char* notif_qsupported_reply (void);
+void notif_qsupported_record (char *gdb_notifications);
 
 void notif_push (struct notif_server *np, struct notif_event *event);
 void notif_event_enque (struct notif_server *notif,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index f98a780..61f08bd 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -185,7 +185,7 @@ vstop_notif_reply (struct notif_event *event, char *own_buf)
 
 static struct notif_annex notif_annex_stop[] =
 {
-  { NULL, vstop_notif_reply, },
+  { NULL, 1, vstop_notif_reply, },
 };
 
 struct notif_server notif_stop =
@@ -1578,6 +1578,11 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 		  /* GDB supports relocate instruction requests.  */
 		  gdb_supports.qRelocInsn = 1;
 		}
+	      else if (strncmp (p, "notifications=", 14) == 0)
+		{
+		  /* Record what notifications GDB supports.  */
+		  notif_qsupported_record (&p[14]);
+		}
 	      else
 		target_process_qsupported (p);
 
@@ -1659,6 +1664,14 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
       if (target_supports_agent ())
 	strcat (own_buf, ";QAgent+");
 
+      p = notif_qsupported_reply ();
+
+      if (p != NULL)
+	{
+	  strcat (own_buf, ";Notifications=");
+	  strcat (own_buf, p);
+	  xfree (p);
+	}
       return;
     }
 
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index e0bc745..ae05bbf 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -75,9 +75,16 @@ remote_notif_parse_1 (struct notif_client *nc,
 		 contents in BUF.  */
 	      && buf[strlen (m->name)] == ':')
 	    {
-	      /* Pass BUF without annex and ':'.  */
-	      m->parse (nc, buf + strlen (m->name) + 1, event);
-	      break;
+	      if (m->supported)
+		{
+		  /* Pass BUF without annex and ':'.  */
+		  m->parse (nc, buf + strlen (m->name) + 1, event);
+		  break;
+		}
+	      else
+		warning (_("GDB gets annex '%s' of notification '%s'"
+			   "but remote stub doesn't support"),
+			 base->notif_name, m->name);
 	    }
 	  m = NULL;
 	}
@@ -305,6 +312,25 @@ notif_xfree (struct notif_client *notif)
   xfree (notif);
 }
 
+/* Return a string of GDB supported features.  */
+
+char *
+remote_notif_qsupported (void)
+{
+  return notif_supported ((struct notif_base **) notifs,
+			  ARRAY_SIZE (notifs));
+}
+
+/* Parse the qSupported reply REPLY from the remote stub and disable
+   some notifications if the remote stub doesn't support.  */
+
+void
+remote_notif_qsupported_reply (const char *reply)
+{
+  notif_parse_supported (reply, (struct notif_base **) notifs,
+			 ARRAY_SIZE (notifs));
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_notif;
 
diff --git a/gdb/remote-notif.h b/gdb/remote-notif.h
index cb1636f..4859254 100644
--- a/gdb/remote-notif.h
+++ b/gdb/remote-notif.h
@@ -69,6 +69,10 @@ void remote_notif_register_async_event_handler (void);
 void remote_notif_unregister_async_event_handler (void);
 
 void remote_notif_process (struct notif_client *except);
+
+char * remote_notif_qsupported (void);
+void remote_notif_qsupported_reply (const char *reply);
+
 extern struct notif_client notif_client_stop;
 
 extern unsigned int notif_debug;
diff --git a/gdb/remote.c b/gdb/remote.c
index 92a0f27..e20ac80 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1284,6 +1284,7 @@ enum {
   PACKET_qXfer_fdpic,
   PACKET_QDisableRandomization,
   PACKET_QAgent,
+  PACKET_notifications,
   PACKET_MAX
 };
 
@@ -3873,6 +3874,15 @@ remote_string_tracing_feature (const struct protocol_feature *feature,
   rs->string_tracing = (support == PACKET_ENABLE);
 }
 
+static void
+remote_notifications_feature (const struct protocol_feature *feature,
+			      enum packet_support support,
+			      const char *value)
+{
+  if (support == PACKET_ENABLE)
+    remote_notif_qsupported_reply (value);
+}
+
 static struct protocol_feature remote_protocol_features[] = {
   { "PacketSize", PACKET_DISABLE, remote_packet_size, -1 },
   { "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet,
@@ -3938,6 +3948,8 @@ static struct protocol_feature remote_protocol_features[] = {
   { "QDisableRandomization", PACKET_DISABLE, remote_supported_packet,
     PACKET_QDisableRandomization },
   { "QAgent", PACKET_DISABLE, remote_supported_packet, PACKET_QAgent},
+  { "Notifications", PACKET_DISABLE, remote_notifications_feature,
+    -1 },
   { "tracenz", PACKET_DISABLE,
     remote_string_tracing_feature, -1 },
 };
@@ -4004,6 +4016,7 @@ remote_query_supported (void)
   if (remote_protocol_packets[PACKET_qSupported].support != PACKET_DISABLE)
     {
       char *q = NULL;
+      char *notifications = remote_notif_qsupported ();
       struct cleanup *old_chain = make_cleanup (free_current_contents, &q);
 
       q = remote_query_supported_append (q, "multiprocess+");
@@ -4013,6 +4026,10 @@ remote_query_supported (void)
 
       q = remote_query_supported_append (q, "qRelocInsn+");
 
+      q = reconcat (q, q, ";notifications=", notifications,
+		    (char *) NULL);
+      xfree (notifications);
+
       q = reconcat (q, "qSupported:", q, (char *) NULL);
       putpkt (q);
 
@@ -5211,7 +5228,10 @@ remote_notif_stop_alloc_reply (void)
 
 static struct notif_annex notif_client_annex_stop[] =
 {
-  { NULL, remote_notif_stop_parse, },
+  /* Even the remote stub doesn't understand
+     'qSupported:notifications=', it may still support notification
+     stop if it supports non-stop.  */
+  { NULL, 1, remote_notif_stop_parse, },
 };
 
 /* A client of notification Stop.  */
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/5] Query supported notifications by qSupported
  2013-01-22  8:41     ` Yao Qi
@ 2013-01-22 10:26       ` Eli Zaretskii
  0 siblings, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2013-01-22 10:26 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

> Date: Tue, 22 Jan 2013 16:39:48 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
> 
> >> >+@cindex notifications, in remote protocol
> >> >+The remote stub supports a string of notifications which the remote stub
> >> >+supports.
> > Something is wrong with this sentence...
> 
> How about this?
> 
> @cindex notifications, in remote protocol
> The remote stub supports a string of notifications.  @var{name} is
> the name of the notification and @var{annex} is the name of the annex,
> if the notification has the annex.

Fine with me, thanks.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 0/5] MI notification on trace started/stopped (V3.1)
  2013-01-22  7:05 [PATCH 0/5] MI notification on trace started/stopped (V3) Yao Qi
  2013-01-22  7:05 ` [PATCH 2/5] Query supported notifications by qSupported Yao Qi
@ 2013-02-16  4:01 ` Yao Qi
  2013-02-16  4:02   ` [PATCH 2/5] Query supported notifications by qSupported Yao Qi
  2013-04-02  2:33 ` [PATCH v4 0/5] MI notification on trace started/stopped Yao Qi
  2 siblings, 1 reply; 15+ messages in thread
From: Yao Qi @ 2013-02-16  4:01 UTC (permalink / raw)
  To: gdb-patches

On 01/22/2013 03:03 PM, Yao Qi wrote:
> This patch series is the V3 of this series.  V2 was posted
> <http://sourceware.org/ml/gdb-patches/2012-12/msg00687.html>.  Here
> are some changes in V3,
> 
>    - Introduce query mechanism on supported notifications
>    - Introduce concept "annex" into remote notification,
>    - Remove trace notes in MI notifications, suggested by Pedro in the
>      review.
>    - Combine observers 'trace-started' and 'trace-stoped' into one
>      observer 'trace-changed'.

The V3 can't be applied to CVS trunk without conflicts, so I resolved
these conflicts, rebase patches to CVS trunk, and post them again.

Beside this, I also add some comments here and there in the patches.
Regression tested them on  x86_64-linux with the combination
{unix, native-gdbserver} x {sync, async}.  Is it OK?

All doc bits were reviewed by Eli in V3.

-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/5] Query supported notifications by qSupported
  2013-02-16  4:01 ` [PATCH 0/5] MI notification on trace started/stopped (V3.1) Yao Qi
@ 2013-02-16  4:02   ` Yao Qi
  2013-02-16  9:33     ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Yao Qi @ 2013-02-16  4:02 UTC (permalink / raw)
  To: gdb-patches

As we we adding more notifications and annexes, both GDB and GDBserver
has to know what notifications and annexes are supported in the other
side.  This is what this patch does.  When GDB connects to GDBserver,
it will happen:

  --> qSupported:XXX;notifications=N1,N2.A1.A2,N3
      (GDB supports notification N1, N2 with annexes A1,A2, and N3)
  <-- XXX;Notifications=N1,N2.A1,N4
      (GDBsever supports notification N1, N2 with annex A1 and N4)

after this, GDB knows what notifications GDBserver is able to send,
and GDBservers knows what notifications GDB doesn't support.

gdb/gdbserver:

2013-02-16  Yao Qi  <yao@codesourcery.com>

	* Makefile.in (SFILES): Add "common-notif.c".
	(OBS): Add common-notif.o.
	(common-notif.o): New rule.

	* notif.c (notif_find_annex): New.
	(notif_qsupported_record): New.
	(notif_qsupported_reply): New.
	* notif.h (notif_qsupported_reply): Declare.
	(notif_qsupported_record): Declare.
	* server.c (notif_annex_stop): Update.
	(handle_query): Call notif_qsupported_record and
	notif_qsupported_reply.
gdb:

2013-02-16  Yao Qi  <yao@codesourcery.com>

	* common/common-notif.c: New.
	* common/common-notif.h (struct notif_annex) <supported>: New
	field.
	* Makefile.in (iREMOTE_OBS): Append "common-notif.o".
	(SFILES): Add "common-notif.c".

	* remote-notif.c (remote_notif_parse_1): Call the parser of
	annex if it is supported.
	(remote_notif_qsupported): New.
	(remote_notif_qsupported_reply): New.
	* remote-notif.h (remote_notif_qsupported): Declare.
	(remote_notif_qsupported_reply): Declare.

	* remote.c (PACKET_notifications): New enum.
	(remote_notifications_feature): New.
	(remote_protocol_features): Add new element.
	(remote_query_supported): Call remote_notif_qsupported and
	append supported notifications to qSupported feature.
	(notif_client_annex_stop): Update.

gdb/doc:

2013-02-16  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (General Query Packets): Document the new feature
	'notifications' of 'qSupported' packet.
	Document the new feature in qSupported reply.
---
 gdb/Makefile.in           |    9 ++-
 gdb/common/common-notif.c |  186 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/common/common-notif.h |   15 ++++
 gdb/doc/gdb.texinfo       |   19 +++++
 gdb/gdbserver/Makefile.in |    7 +-
 gdb/gdbserver/notif.c     |   21 +++++
 gdb/gdbserver/notif.h     |    2 +
 gdb/gdbserver/server.c    |   15 ++++-
 gdb/remote-notif.c        |   32 +++++++-
 gdb/remote-notif.h        |    4 +
 gdb/remote.c              |   22 +++++-
 11 files changed, 323 insertions(+), 9 deletions(-)
 create mode 100644 gdb/common/common-notif.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index a6ff9c3..4eb1548 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -510,7 +510,7 @@ SER_HARDWIRE = @SER_HARDWIRE@
 # The `remote' debugging target is supported for most architectures,
 # but not all (e.g. 960)
 REMOTE_OBS = remote.o dcache.o tracepoint.o ax-general.o ax-gdb.o remote-fileio.o \
-	remote-notif.o
+	remote-notif.o common-notif.o
 
 # This is remote-sim.o if a simulator is to be linked in.
 SIM_OBS = @SIM_OBS@
@@ -736,7 +736,8 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	p-exp.y p-lang.c p-typeprint.c p-valprint.c parse.c printcmd.c \
 	proc-service.list progspace.c \
 	prologue-value.c psymtab.c \
-	regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c reverse.c \
+	regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c \
+	common-notif.c reverse.c \
 	sentinel-frame.c \
 	serial.c ser-base.c ser-unix.c skip.c \
 	solib.c solib-target.c source.c \
@@ -1925,6 +1926,10 @@ common-utils.o: ${srcdir}/common/common-utils.c
 	$(COMPILE) $(srcdir)/common/common-utils.c
 	$(POSTCOMPILE)
 
+common-notif.o: ${srcdir}/common/common-notif.c
+	$(COMPILE) $(srcdir)/common/common-notif.c
+	$(POSTCOMPILE)
+
 gdb_vecs.o: ${srcdir}/common/gdb_vecs.c
 	$(COMPILE) $(srcdir)/common/gdb_vecs.c
 	$(POSTCOMPILE)
diff --git a/gdb/common/common-notif.c b/gdb/common/common-notif.c
new file mode 100644
index 0000000..e53729f
--- /dev/null
+++ b/gdb/common/common-notif.c
@@ -0,0 +1,186 @@
+/* Copyright (C) 2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+#include <string.h>
+#include "common-notif.h"
+#include "gdb_assert.h"
+
+/* Return a string about the notifications in array NOTIFS.  NUM is
+   the number of elements in array NOTIFS.  The caller is responsible
+   to free the returned string.  Suppose array NOTIFS has
+   notifications N1, N2, and N3.  Only N2 has annexes A1 and A2.  The
+   returned string is "N1,N2.A2.A2,N3".  */
+
+char *
+notif_supported (struct notif_base *notifs[], int num)
+{
+  int i;
+  char * p = NULL;
+
+#define BUF_LEN 128
+
+  for (i = 0; i < num; i++)
+    {
+      struct notif_base *nb = notifs[i];
+
+      if (p == NULL)
+	{
+	  p = xmalloc (BUF_LEN);
+	  strcpy (p, nb->notif_name);
+	}
+      else
+	xsnprintf (p + strlen (p), BUF_LEN - strlen (p), ",%s",
+		   nb->notif_name);
+
+      if (NOTIF_HAS_ANNEX (nb))
+	{
+	  int j;
+
+	  NOTIF_ITER_ANNEX (nb, j)
+	    {
+	      xsnprintf (p + strlen (p), BUF_LEN - strlen (p),
+			 ".%s", nb->annexes[j].name);
+	    }
+	}
+    }
+
+  return p;
+}
+
+/* Find annex in notification NB by name NAME and length LEN.
+   If found, return annex, otherwise return NULL.  */
+
+static struct notif_annex *
+notif_find_annex (struct notif_base *nb, const char *name, int len)
+{
+  if (NOTIF_HAS_ANNEX (nb))
+    {
+      int j;
+
+      NOTIF_ITER_ANNEX (nb, j)
+	if (strncmp (name, nb->annexes[j].name, len) == 0
+	    && len == strlen (nb->annexes[j].name))
+	  return &nb->annexes[j];
+    }
+  return NULL;
+}
+
+/* Parse the REPLY, which is about supported annexes and
+   notifications from the peer, and disable some annexes of
+   notification in array NOTIFS if the peer doesn't support.  NUM is
+   the number of elements in array NOTIFS.  */
+
+void
+notif_parse_supported (const char *reply,
+		       struct notif_base *notifs[], int num)
+{
+  const char *p = reply;
+  int notif_num = 1;
+  char **notif_str;
+  int i;
+
+  /* Count how many notifications in REPLY.  */
+  for (i = 0; reply[i] != '\0'; i++)
+    if (reply[i] == ',')
+      notif_num++;
+
+  /* Copy contents of each notification in REPLY to each slot of
+     NOTIF_STR.  */
+  notif_str = xmalloc (notif_num * sizeof (char *));
+  for (i = 0; i < notif_num; i++)
+    {
+      char *end = strchr (p, ',');
+
+      if (end == NULL)
+	notif_str[i] = xstrdup (p);
+      else
+	{
+	  /* Can't use xstrndup in GDBserver.  */
+	  notif_str[i] = strndup (p, end - p);
+	  p = end + 1;
+	}
+    }
+
+  /* Iterate each element in NOTIF_STR and parse annex in it.  */
+  for (i = 0; i < notif_num; i++)
+    {
+      int j;
+      struct notif_base *nb = NULL;
+
+      p = notif_str[i];
+
+      for (j = 0; j < num; j++)
+	{
+	  int name_len = strlen (notifs[j]->notif_name);
+
+	  if (0 == strncmp (notifs[j]->notif_name, p, name_len)
+	      && (p[name_len] == '.' || p[name_len] == 0))
+	    {
+	      nb = notifs[j];
+	      p += name_len;
+	      break;
+	    }
+	}
+
+      if (nb != NULL)
+	{
+	  if (p[0] == 0)
+	    {
+	      /* No annex.  */
+	      gdb_assert (!NOTIF_HAS_ANNEX (nb));
+	      nb->annexes[0].supported = 1;
+	    }
+	  else if (p[0] == '.')
+	    {
+	      gdb_assert (NOTIF_HAS_ANNEX (nb));
+
+	      p++;
+
+	      /* Parse the rest of P and look for annexes.  */
+	      while (p != NULL)
+		{
+		  char *end = strchr (p, '.');
+		  struct notif_annex *annex = NULL;
+
+		  if (end != NULL)
+		    {
+		      annex = notif_find_annex (nb, p, end - p);
+		      p = end + 1;
+		    }
+		  else
+		    {
+		      annex = notif_find_annex (nb, p, strlen (p));
+		      p = end;
+		    }
+
+		  /* If annex is known, mark it supported, otherwise
+		     skip it because the peer knows the annex but we
+		     don't know.  */
+		  if (annex != NULL)
+		    annex->supported = 1;
+		}
+	    }
+	  else
+	    warning (_("Unknown supported notification"));
+	}
+    }
+}
diff --git a/gdb/common/common-notif.h b/gdb/common/common-notif.h
index 1ffa26c..8e0e0c1 100644
--- a/gdb/common/common-notif.h
+++ b/gdb/common/common-notif.h
@@ -32,6 +32,11 @@ struct notif_annex
      notification this annex belongs to.  */
   const char *name;
 
+  /* This annex is supported by the peer (GDB or GDBserver)?  A
+     notification may have multiple annexes and some of them are
+     supported.  Annex is the smallest unit of support.  */
+  int supported;
+
 #ifdef GDBSERVER
   /* Write event EVENT to OWN_BUF.  */
   void (*write) (struct notif_event *event, char *own_buf);
@@ -53,6 +58,10 @@ struct notif_annex
 
 #define NOTIF_HAS_ANNEX(NOTIF) ((NOTIF)->annexes[0].name != NULL)
 
+/* Whether the annex of notification N is supported.  */
+
+#define NOTIF_ANNEX_SUPPORTED_P(N, INDEX) \
+  ((N).annexes[INDEX].supported)
 
 /* "Base class" of a notification.  It can be extended in both GDB
    and GDBserver to represent a type of notification.  */
@@ -71,3 +80,9 @@ struct notif_base
      of the notification.  */
   struct notif_annex *annexes;
 };
+
+char *notif_supported (struct notif_base *notifs[], int num);
+
+void notif_parse_supported (const char *reply,
+			    struct notif_base *notifs[], int num);
+
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 68b81a2..c94a203 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -37202,6 +37202,14 @@ description.
 This feature indicates whether @value{GDBN} supports the
 @samp{qRelocInsn} packet (@pxref{Tracepoint Packets,,Relocate
 instruction reply packet}).
+
+@item notifications
+@anchor{notifications feature}
+This feature indicates that @value{GDBN} supports the async remote
+notifications (@pxref{Notification Packets}).  If the stub sees
+@samp{notifications=} with a string of supported notifications,
+separated by commas, it will report notifications supported by the
+stub.
 @end table
 
 Stubs should ignore any unknown values for
@@ -37390,6 +37398,11 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab No
 
+@item @samp{Notifications}
+@tab Yes
+@tab @samp{-}
+@tab No
+
 @end multitable
 
 These are the currently defined stub features, in more detail:
@@ -37544,6 +37557,12 @@ See @ref{Bytecode Descriptions} for details about the bytecode.
 The remote stub supports running a breakpoint's command list itself,
 rather than reporting the hit to @value{GDBN}.
 
+@item Notifications=@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{[},@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{]}@dots{}
+@cindex notifications, in remote protocol
+The remote stub supports a string of notifications.  @var{name} is
+the name of the notification and @var{annex} is the name of the annex,
+if the notification has the annex.
+
 @end table
 
 @item qSymbol::
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index dffe8ae..346e8c8 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -155,7 +155,7 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/common/vec.c $(srcdir)/common/gdb_vecs.c \
 	$(srcdir)/common/common-utils.c $(srcdir)/common/xml-utils.c \
 	$(srcdir)/common/linux-osdata.c $(srcdir)/common/ptid.c \
-	$(srcdir)/common/buffer.c
+	$(srcdir)/common/buffer.c $(srcdir)/common/common-notif.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -165,7 +165,7 @@ SOURCES = $(SFILES)
 TAGFILES = $(SOURCES) ${HFILES} ${ALLPARAM} ${POSSLIBS}
 
 OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o target.o \
-	utils.o version.o vec.o gdb_vecs.o \
+	utils.o version.o vec.o gdb_vecs.o common-notif.o \
 	mem-break.o hostio.o event-loop.o tracepoint.o \
 	xml-utils.o common-utils.o ptid.o buffer.o format.o \
 	dll.o notif.o \
@@ -518,6 +518,9 @@ linux-ptrace.o: ../common/linux-ptrace.c
 common-utils.o: ../common/common-utils.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+common-notif.o: ../common/common-notif.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 vec.o: ../common/vec.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdb/gdbserver/notif.c b/gdb/gdbserver/notif.c
index 168f77d..e6b3756 100644
--- a/gdb/gdbserver/notif.c
+++ b/gdb/gdbserver/notif.c
@@ -182,6 +182,27 @@ notif_event_xfree (struct notif_event *event)
   xfree (event);
 }
 
+/* Record the notifications supported by GDB.  GDB_NOTIFICATIONS is a
+   string about notifications GDB supports.  */
+
+void
+notif_qsupported_record (char *gdb_notifications)
+{
+  return notif_parse_supported (gdb_notifications,
+				(struct notif_base **) notifs,
+				ARRAY_SIZE (notifs));
+}
+
+/* Return a string about notifications that GDBserver supports.
+   Return NULL if no notification is supported.  */
+
+char *
+notif_qsupported_reply (void)
+{
+  return notif_supported ((struct notif_base **) notifs,
+			  ARRAY_SIZE (notifs));
+}
+
 void
 initialize_notif (void)
 {
diff --git a/gdb/gdbserver/notif.h b/gdb/gdbserver/notif.h
index 5f9b6bc..72417aa 100644
--- a/gdb/gdbserver/notif.h
+++ b/gdb/gdbserver/notif.h
@@ -58,6 +58,8 @@ extern struct notif_server notif_stop;
 
 int handle_notif_ack (char *own_buf, int packet_len);
 void notif_write_event (struct notif_server *notif, char *own_buf);
+char* notif_qsupported_reply (void);
+void notif_qsupported_record (char *gdb_notifications);
 
 void notif_push (struct notif_server *np, struct notif_event *event);
 void notif_event_enque (struct notif_server *notif,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 3e8ef0a..d20c1a8 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -185,7 +185,7 @@ vstop_notif_reply (struct notif_event *event, char *own_buf)
 
 static struct notif_annex notif_annex_stop[] =
 {
-  { NULL, vstop_notif_reply, },
+  { NULL, 1, vstop_notif_reply, },
 };
 
 struct notif_server notif_stop =
@@ -1572,6 +1572,11 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 		  /* GDB supports relocate instruction requests.  */
 		  gdb_supports_qRelocInsn = 1;
 		}
+	      else if (strncmp (p, "notifications=", 14) == 0)
+		{
+		  /* Record what notifications GDB supports.  */
+		  notif_qsupported_record (&p[14]);
+		}
 	      else
 		target_process_qsupported (p);
 
@@ -1652,6 +1657,14 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
       if (target_supports_agent ())
 	strcat (own_buf, ";QAgent+");
 
+      p = notif_qsupported_reply ();
+
+      if (p != NULL)
+	{
+	  strcat (own_buf, ";Notifications=");
+	  strcat (own_buf, p);
+	  xfree (p);
+	}
       return;
     }
 
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index e0bc745..ae05bbf 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -75,9 +75,16 @@ remote_notif_parse_1 (struct notif_client *nc,
 		 contents in BUF.  */
 	      && buf[strlen (m->name)] == ':')
 	    {
-	      /* Pass BUF without annex and ':'.  */
-	      m->parse (nc, buf + strlen (m->name) + 1, event);
-	      break;
+	      if (m->supported)
+		{
+		  /* Pass BUF without annex and ':'.  */
+		  m->parse (nc, buf + strlen (m->name) + 1, event);
+		  break;
+		}
+	      else
+		warning (_("GDB gets annex '%s' of notification '%s'"
+			   "but remote stub doesn't support"),
+			 base->notif_name, m->name);
 	    }
 	  m = NULL;
 	}
@@ -305,6 +312,25 @@ notif_xfree (struct notif_client *notif)
   xfree (notif);
 }
 
+/* Return a string of GDB supported features.  */
+
+char *
+remote_notif_qsupported (void)
+{
+  return notif_supported ((struct notif_base **) notifs,
+			  ARRAY_SIZE (notifs));
+}
+
+/* Parse the qSupported reply REPLY from the remote stub and disable
+   some notifications if the remote stub doesn't support.  */
+
+void
+remote_notif_qsupported_reply (const char *reply)
+{
+  notif_parse_supported (reply, (struct notif_base **) notifs,
+			 ARRAY_SIZE (notifs));
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_notif;
 
diff --git a/gdb/remote-notif.h b/gdb/remote-notif.h
index cb1636f..4859254 100644
--- a/gdb/remote-notif.h
+++ b/gdb/remote-notif.h
@@ -69,6 +69,10 @@ void remote_notif_register_async_event_handler (void);
 void remote_notif_unregister_async_event_handler (void);
 
 void remote_notif_process (struct notif_client *except);
+
+char * remote_notif_qsupported (void);
+void remote_notif_qsupported_reply (const char *reply);
+
 extern struct notif_client notif_client_stop;
 
 extern unsigned int notif_debug;
diff --git a/gdb/remote.c b/gdb/remote.c
index b4a8d24..cf5cb3f 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1284,6 +1284,7 @@ enum {
   PACKET_qXfer_fdpic,
   PACKET_QDisableRandomization,
   PACKET_QAgent,
+  PACKET_notifications,
   PACKET_MAX
 };
 
@@ -3928,6 +3929,15 @@ remote_string_tracing_feature (const struct protocol_feature *feature,
   rs->string_tracing = (support == PACKET_ENABLE);
 }
 
+static void
+remote_notifications_feature (const struct protocol_feature *feature,
+			      enum packet_support support,
+			      const char *value)
+{
+  if (support == PACKET_ENABLE)
+    remote_notif_qsupported_reply (value);
+}
+
 static struct protocol_feature remote_protocol_features[] = {
   { "PacketSize", PACKET_DISABLE, remote_packet_size, -1 },
   { "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet,
@@ -3993,6 +4003,8 @@ static struct protocol_feature remote_protocol_features[] = {
   { "QDisableRandomization", PACKET_DISABLE, remote_supported_packet,
     PACKET_QDisableRandomization },
   { "QAgent", PACKET_DISABLE, remote_supported_packet, PACKET_QAgent},
+  { "Notifications", PACKET_DISABLE, remote_notifications_feature,
+    -1 },
   { "tracenz", PACKET_DISABLE,
     remote_string_tracing_feature, -1 },
 };
@@ -4059,6 +4071,7 @@ remote_query_supported (void)
   if (remote_protocol_packets[PACKET_qSupported].support != PACKET_DISABLE)
     {
       char *q = NULL;
+      char *notifications = remote_notif_qsupported ();
       struct cleanup *old_chain = make_cleanup (free_current_contents, &q);
 
       q = remote_query_supported_append (q, "multiprocess+");
@@ -4068,6 +4081,10 @@ remote_query_supported (void)
 
       q = remote_query_supported_append (q, "qRelocInsn+");
 
+      q = reconcat (q, q, ";notifications=", notifications,
+		    (char *) NULL);
+      xfree (notifications);
+
       q = reconcat (q, "qSupported:", q, (char *) NULL);
       putpkt (q);
 
@@ -5266,7 +5283,10 @@ remote_notif_stop_alloc_reply (void)
 
 static struct notif_annex notif_client_annex_stop[] =
 {
-  { NULL, remote_notif_stop_parse, },
+  /* Even the remote stub doesn't understand
+     'qSupported:notifications=', it may still support notification
+     stop if it supports non-stop.  */
+  { NULL, 1, remote_notif_stop_parse, },
 };
 
 /* A client of notification Stop.  */
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/5] Query supported notifications by qSupported
  2013-02-16  4:02   ` [PATCH 2/5] Query supported notifications by qSupported Yao Qi
@ 2013-02-16  9:33     ` Eli Zaretskii
  2013-03-11  9:26       ` Yao Qi
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2013-02-16  9:33 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

> From: Yao Qi <yao@codesourcery.com>
> Date: Sat, 16 Feb 2013 12:00:11 +0800
> 
> As we we adding more notifications and annexes, both GDB and GDBserver
> has to know what notifications and annexes are supported in the other
> side.  This is what this patch does.  When GDB connects to GDBserver,
> it will happen:
> 
>   --> qSupported:XXX;notifications=N1,N2.A1.A2,N3
>       (GDB supports notification N1, N2 with annexes A1,A2, and N3)

This form is not described in your patch for the manual.

> +@item notifications
> +@anchor{notifications feature}
> +This feature indicates that @value{GDBN} supports the async remote
> +notifications (@pxref{Notification Packets}).  If the stub sees
> +@samp{notifications=} with a string of supported notifications,
> +separated by commas, it will report notifications supported by the
> +stub.

See above: I think you should show here the form of the
"notifications=" part in more detail.  In particular, the fact that
each notification is of the form NOTIFICATION.ANNEX1.ANNEX2... is
important and should be described and the meaning of the multiple
ANNEX parts explained.

> +@item Notifications=@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{[},@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{]}@dots{}
> +@cindex notifications, in remote protocol
> +The remote stub supports a string of notifications.  @var{name} is
> +the name of the notification and @var{annex} is the name of the annex,
> +if the notification has the annex.

Here, too, we should say explicitly that there could be several
@var{annex} parts separated by periods.

OK with those changes.

Thanks.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/5] Query supported notifications by qSupported
  2013-02-16  9:33     ` Eli Zaretskii
@ 2013-03-11  9:26       ` Yao Qi
  2013-03-11 16:59         ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Yao Qi @ 2013-03-11  9:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 02/16/2013 05:32 PM, Eli Zaretskii wrote:
> See above: I think you should show here the form of the
> "notifications=" part in more detail.  In particular, the fact that
> each notification is of the form NOTIFICATION.ANNEX1.ANNEX2... is
> important and should be described and the meaning of the multiple
> ANNEX parts explained.

How about this below,

.... supported by the
stub.  Each notification in the string is composed by the name of the
notification and the annexes, if any, which are separated by periods.

> 
>> >+@item Notifications=@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{[},@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{]}@dots{}
>> >+@cindex notifications, in remote protocol
>> >+The remote stub supports a string of notifications.  @var{name} is
>> >+the name of the notification and @var{annex} is the name of the annex,
>> >+if the notification has the annex.
> Here, too, we should say explicitly that there could be several
> @var{annex} parts separated by periods.

and this,

if the notification has the annex.  Note that the notification may
have several annexes, so here could be several @var{annex} parts
separated by periods.

Patch 2/5 can't be applied to current trunk (again) because of some
changes recently.  I resolved the conflicts and post a updated one.

I sincerely ask maintainers to review the non-doc part of this patch
set.  The previous review happened on Jan. 8th, two months ago.
I kept posting updated versions to resolve conflicts and sending ping
mail.  I even tried to ping this patch set on IRC, but I got no
response.  Usually, my patches are reviewed in 1~5 days, thanks to
maintainers' hard work, but don't know why this patch set can't
catch people's eyeball.

-- 
Yao (é½å°§)


gdb/gdbserver:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* Makefile.in (SFILES): Add "common-notif.c".
	(OBS): Add common-notif.o.
	(common-notif.o): New rule.

	* notif.c (notif_find_annex): New.
	(notif_qsupported_record): New.
	(notif_qsupported_reply): New.
	* notif.h (notif_qsupported_reply): Declare.
	(notif_qsupported_record): Declare.
	* server.c (notif_annex_stop): Update.
	(handle_query): Call notif_qsupported_record and
	notif_qsupported_reply.
gdb:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* common/common-notif.c: New.
	* common/common-notif.h (struct notif_annex) <supported>: New
	field.
	* Makefile.in (iREMOTE_OBS): Append "common-notif.o".
	(SFILES): Add "common-notif.c".

	* remote-notif.c (remote_notif_parse_1): Call the parser of
	annex if it is supported.
	(remote_notif_qsupported): New.
	(remote_notif_qsupported_reply): New.
	* remote-notif.h (remote_notif_qsupported): Declare.
	(remote_notif_qsupported_reply): Declare.

	* remote.c (PACKET_notifications): New enum.
	(remote_notifications_feature): New.
	(remote_protocol_features): Add new element.
	(remote_query_supported): Call remote_notif_qsupported and
	append supported notifications to qSupported feature.
	(notif_client_annex_stop): Update.

gdb/doc:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (General Query Packets): Document the new feature
	'notifications' of 'qSupported' packet.
	Document the new feature in qSupported reply.
---
 gdb/Makefile.in           |    9 ++-
 gdb/common/common-notif.c |  186 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/common/common-notif.h |   15 ++++
 gdb/doc/gdb.texinfo       |   22 +++++
 gdb/gdbserver/Makefile.in |    7 +-
 gdb/gdbserver/notif.c     |   21 +++++
 gdb/gdbserver/notif.h     |    2 +
 gdb/gdbserver/server.c    |   15 ++++-
 gdb/remote-notif.c        |   32 +++++++-
 gdb/remote-notif.h        |    4 +
 gdb/remote.c              |   22 +++++-
 11 files changed, 326 insertions(+), 9 deletions(-)
 create mode 100644 gdb/common/common-notif.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index a6ff9c3..4eb1548 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -510,7 +510,7 @@ SER_HARDWIRE = @SER_HARDWIRE@
 # The `remote' debugging target is supported for most architectures,
 # but not all (e.g. 960)
 REMOTE_OBS = remote.o dcache.o tracepoint.o ax-general.o ax-gdb.o remote-fileio.o \
-	remote-notif.o
+	remote-notif.o common-notif.o
 
 # This is remote-sim.o if a simulator is to be linked in.
 SIM_OBS = @SIM_OBS@
@@ -736,7 +736,8 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	p-exp.y p-lang.c p-typeprint.c p-valprint.c parse.c printcmd.c \
 	proc-service.list progspace.c \
 	prologue-value.c psymtab.c \
-	regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c reverse.c \
+	regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c \
+	common-notif.c reverse.c \
 	sentinel-frame.c \
 	serial.c ser-base.c ser-unix.c skip.c \
 	solib.c solib-target.c source.c \
@@ -1925,6 +1926,10 @@ common-utils.o: ${srcdir}/common/common-utils.c
 	$(COMPILE) $(srcdir)/common/common-utils.c
 	$(POSTCOMPILE)
 
+common-notif.o: ${srcdir}/common/common-notif.c
+	$(COMPILE) $(srcdir)/common/common-notif.c
+	$(POSTCOMPILE)
+
 gdb_vecs.o: ${srcdir}/common/gdb_vecs.c
 	$(COMPILE) $(srcdir)/common/gdb_vecs.c
 	$(POSTCOMPILE)
diff --git a/gdb/common/common-notif.c b/gdb/common/common-notif.c
new file mode 100644
index 0000000..e53729f
--- /dev/null
+++ b/gdb/common/common-notif.c
@@ -0,0 +1,186 @@
+/* Copyright (C) 2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+#include <string.h>
+#include "common-notif.h"
+#include "gdb_assert.h"
+
+/* Return a string about the notifications in array NOTIFS.  NUM is
+   the number of elements in array NOTIFS.  The caller is responsible
+   to free the returned string.  Suppose array NOTIFS has
+   notifications N1, N2, and N3.  Only N2 has annexes A1 and A2.  The
+   returned string is "N1,N2.A2.A2,N3".  */
+
+char *
+notif_supported (struct notif_base *notifs[], int num)
+{
+  int i;
+  char * p = NULL;
+
+#define BUF_LEN 128
+
+  for (i = 0; i < num; i++)
+    {
+      struct notif_base *nb = notifs[i];
+
+      if (p == NULL)
+	{
+	  p = xmalloc (BUF_LEN);
+	  strcpy (p, nb->notif_name);
+	}
+      else
+	xsnprintf (p + strlen (p), BUF_LEN - strlen (p), ",%s",
+		   nb->notif_name);
+
+      if (NOTIF_HAS_ANNEX (nb))
+	{
+	  int j;
+
+	  NOTIF_ITER_ANNEX (nb, j)
+	    {
+	      xsnprintf (p + strlen (p), BUF_LEN - strlen (p),
+			 ".%s", nb->annexes[j].name);
+	    }
+	}
+    }
+
+  return p;
+}
+
+/* Find annex in notification NB by name NAME and length LEN.
+   If found, return annex, otherwise return NULL.  */
+
+static struct notif_annex *
+notif_find_annex (struct notif_base *nb, const char *name, int len)
+{
+  if (NOTIF_HAS_ANNEX (nb))
+    {
+      int j;
+
+      NOTIF_ITER_ANNEX (nb, j)
+	if (strncmp (name, nb->annexes[j].name, len) == 0
+	    && len == strlen (nb->annexes[j].name))
+	  return &nb->annexes[j];
+    }
+  return NULL;
+}
+
+/* Parse the REPLY, which is about supported annexes and
+   notifications from the peer, and disable some annexes of
+   notification in array NOTIFS if the peer doesn't support.  NUM is
+   the number of elements in array NOTIFS.  */
+
+void
+notif_parse_supported (const char *reply,
+		       struct notif_base *notifs[], int num)
+{
+  const char *p = reply;
+  int notif_num = 1;
+  char **notif_str;
+  int i;
+
+  /* Count how many notifications in REPLY.  */
+  for (i = 0; reply[i] != '\0'; i++)
+    if (reply[i] == ',')
+      notif_num++;
+
+  /* Copy contents of each notification in REPLY to each slot of
+     NOTIF_STR.  */
+  notif_str = xmalloc (notif_num * sizeof (char *));
+  for (i = 0; i < notif_num; i++)
+    {
+      char *end = strchr (p, ',');
+
+      if (end == NULL)
+	notif_str[i] = xstrdup (p);
+      else
+	{
+	  /* Can't use xstrndup in GDBserver.  */
+	  notif_str[i] = strndup (p, end - p);
+	  p = end + 1;
+	}
+    }
+
+  /* Iterate each element in NOTIF_STR and parse annex in it.  */
+  for (i = 0; i < notif_num; i++)
+    {
+      int j;
+      struct notif_base *nb = NULL;
+
+      p = notif_str[i];
+
+      for (j = 0; j < num; j++)
+	{
+	  int name_len = strlen (notifs[j]->notif_name);
+
+	  if (0 == strncmp (notifs[j]->notif_name, p, name_len)
+	      && (p[name_len] == '.' || p[name_len] == 0))
+	    {
+	      nb = notifs[j];
+	      p += name_len;
+	      break;
+	    }
+	}
+
+      if (nb != NULL)
+	{
+	  if (p[0] == 0)
+	    {
+	      /* No annex.  */
+	      gdb_assert (!NOTIF_HAS_ANNEX (nb));
+	      nb->annexes[0].supported = 1;
+	    }
+	  else if (p[0] == '.')
+	    {
+	      gdb_assert (NOTIF_HAS_ANNEX (nb));
+
+	      p++;
+
+	      /* Parse the rest of P and look for annexes.  */
+	      while (p != NULL)
+		{
+		  char *end = strchr (p, '.');
+		  struct notif_annex *annex = NULL;
+
+		  if (end != NULL)
+		    {
+		      annex = notif_find_annex (nb, p, end - p);
+		      p = end + 1;
+		    }
+		  else
+		    {
+		      annex = notif_find_annex (nb, p, strlen (p));
+		      p = end;
+		    }
+
+		  /* If annex is known, mark it supported, otherwise
+		     skip it because the peer knows the annex but we
+		     don't know.  */
+		  if (annex != NULL)
+		    annex->supported = 1;
+		}
+	    }
+	  else
+	    warning (_("Unknown supported notification"));
+	}
+    }
+}
diff --git a/gdb/common/common-notif.h b/gdb/common/common-notif.h
index 1ffa26c..8e0e0c1 100644
--- a/gdb/common/common-notif.h
+++ b/gdb/common/common-notif.h
@@ -32,6 +32,11 @@ struct notif_annex
      notification this annex belongs to.  */
   const char *name;
 
+  /* This annex is supported by the peer (GDB or GDBserver)?  A
+     notification may have multiple annexes and some of them are
+     supported.  Annex is the smallest unit of support.  */
+  int supported;
+
 #ifdef GDBSERVER
   /* Write event EVENT to OWN_BUF.  */
   void (*write) (struct notif_event *event, char *own_buf);
@@ -53,6 +58,10 @@ struct notif_annex
 
 #define NOTIF_HAS_ANNEX(NOTIF) ((NOTIF)->annexes[0].name != NULL)
 
+/* Whether the annex of notification N is supported.  */
+
+#define NOTIF_ANNEX_SUPPORTED_P(N, INDEX) \
+  ((N).annexes[INDEX].supported)
 
 /* "Base class" of a notification.  It can be extended in both GDB
    and GDBserver to represent a type of notification.  */
@@ -71,3 +80,9 @@ struct notif_base
      of the notification.  */
   struct notif_annex *annexes;
 };
+
+char *notif_supported (struct notif_base *notifs[], int num);
+
+void notif_parse_supported (const char *reply,
+			    struct notif_base *notifs[], int num);
+
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 400c794..50520b0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -37214,6 +37214,15 @@ description.
 This feature indicates whether @value{GDBN} supports the
 @samp{qRelocInsn} packet (@pxref{Tracepoint Packets,,Relocate
 instruction reply packet}).
+
+@item notifications
+@anchor{notifications feature}
+This feature indicates that @value{GDBN} supports the async remote
+notifications (@pxref{Notification Packets}).  If the stub sees
+@samp{notifications=} with a string of supported notifications,
+separated by commas, it will report notifications supported by the
+stub.  Each notification in the string is composed by the name of the
+notification and the annexes, if any, which are separated by periods.
 @end table
 
 Stubs should ignore any unknown values for
@@ -37407,6 +37416,11 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab No
 
+@item @samp{Notifications}
+@tab Yes
+@tab @samp{-}
+@tab No
+
 @end multitable
 
 These are the currently defined stub features, in more detail:
@@ -37565,6 +37579,14 @@ See @ref{Bytecode Descriptions} for details about the bytecode.
 The remote stub supports running a breakpoint's command list itself,
 rather than reporting the hit to @value{GDBN}.
 
+@item Notifications=@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{[},@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{]}@dots{}
+@cindex notifications, in remote protocol
+The remote stub supports a string of notifications.  @var{name} is
+the name of the notification and @var{annex} is the name of the annex,
+if the notification has the annex.  Note that the notification may
+have several annexes, so here could be several @var{annex} parts
+separated by periods.
+
 @end table
 
 @item qSymbol::
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index ee2c400..fb0f604 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -155,7 +155,7 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/common/vec.c $(srcdir)/common/gdb_vecs.c \
 	$(srcdir)/common/common-utils.c $(srcdir)/common/xml-utils.c \
 	$(srcdir)/common/linux-osdata.c $(srcdir)/common/ptid.c \
-	$(srcdir)/common/buffer.c
+	$(srcdir)/common/buffer.c $(srcdir)/common/common-notif.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -165,7 +165,7 @@ SOURCES = $(SFILES)
 TAGFILES = $(SOURCES) ${HFILES} ${ALLPARAM} ${POSSLIBS}
 
 OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o target.o \
-	utils.o version.o vec.o gdb_vecs.o \
+	utils.o version.o vec.o gdb_vecs.o common-notif.o \
 	mem-break.o hostio.o event-loop.o tracepoint.o \
 	xml-utils.o common-utils.o ptid.o buffer.o format.o \
 	dll.o notif.o \
@@ -519,6 +519,9 @@ linux-ptrace.o: ../common/linux-ptrace.c
 common-utils.o: ../common/common-utils.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+common-notif.o: ../common/common-notif.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 vec.o: ../common/vec.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdb/gdbserver/notif.c b/gdb/gdbserver/notif.c
index 168f77d..e6b3756 100644
--- a/gdb/gdbserver/notif.c
+++ b/gdb/gdbserver/notif.c
@@ -182,6 +182,27 @@ notif_event_xfree (struct notif_event *event)
   xfree (event);
 }
 
+/* Record the notifications supported by GDB.  GDB_NOTIFICATIONS is a
+   string about notifications GDB supports.  */
+
+void
+notif_qsupported_record (char *gdb_notifications)
+{
+  return notif_parse_supported (gdb_notifications,
+				(struct notif_base **) notifs,
+				ARRAY_SIZE (notifs));
+}
+
+/* Return a string about notifications that GDBserver supports.
+   Return NULL if no notification is supported.  */
+
+char *
+notif_qsupported_reply (void)
+{
+  return notif_supported ((struct notif_base **) notifs,
+			  ARRAY_SIZE (notifs));
+}
+
 void
 initialize_notif (void)
 {
diff --git a/gdb/gdbserver/notif.h b/gdb/gdbserver/notif.h
index 5f9b6bc..72417aa 100644
--- a/gdb/gdbserver/notif.h
+++ b/gdb/gdbserver/notif.h
@@ -58,6 +58,8 @@ extern struct notif_server notif_stop;
 
 int handle_notif_ack (char *own_buf, int packet_len);
 void notif_write_event (struct notif_server *notif, char *own_buf);
+char* notif_qsupported_reply (void);
+void notif_qsupported_record (char *gdb_notifications);
 
 void notif_push (struct notif_server *np, struct notif_event *event);
 void notif_event_enque (struct notif_server *notif,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 3722ed6..be76378 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -185,7 +185,7 @@ vstop_notif_reply (struct notif_event *event, char *own_buf)
 
 static struct notif_annex notif_annex_stop[] =
 {
-  { NULL, vstop_notif_reply, },
+  { NULL, 1, vstop_notif_reply, },
 };
 
 struct notif_server notif_stop =
@@ -1572,6 +1572,11 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 		  /* GDB supports relocate instruction requests.  */
 		  gdb_supports_qRelocInsn = 1;
 		}
+	      else if (strncmp (p, "notifications=", 14) == 0)
+		{
+		  /* Record what notifications GDB supports.  */
+		  notif_qsupported_record (&p[14]);
+		}
 	      else
 		target_process_qsupported (p);
 
@@ -1653,6 +1658,14 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
       if (target_supports_agent ())
 	strcat (own_buf, ";QAgent+");
 
+      p = notif_qsupported_reply ();
+
+      if (p != NULL)
+	{
+	  strcat (own_buf, ";Notifications=");
+	  strcat (own_buf, p);
+	  xfree (p);
+	}
       return;
     }
 
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index 41bcc3e..9768143 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -75,9 +75,16 @@ remote_notif_parse_1 (struct notif_client *nc,
 		 contents in BUF.  */
 	      && buf[strlen (m->name)] == ':')
 	    {
-	      /* Pass BUF without annex and ':'.  */
-	      m->parse (nc, buf + strlen (m->name) + 1, event);
-	      break;
+	      if (m->supported)
+		{
+		  /* Pass BUF without annex and ':'.  */
+		  m->parse (nc, buf + strlen (m->name) + 1, event);
+		  break;
+		}
+	      else
+		warning (_("GDB gets annex '%s' of notification '%s'"
+			   "but remote stub doesn't support"),
+			 base->notif_name, m->name);
 	    }
 	  m = NULL;
 	}
@@ -305,6 +312,25 @@ notif_xfree (struct notif_client *notif)
   xfree (notif);
 }
 
+/* Return a string of GDB supported features.  */
+
+char *
+remote_notif_qsupported (void)
+{
+  return notif_supported ((struct notif_base **) notifs,
+			  ARRAY_SIZE (notifs));
+}
+
+/* Parse the qSupported reply REPLY from the remote stub and disable
+   some notifications if the remote stub doesn't support.  */
+
+void
+remote_notif_qsupported_reply (const char *reply)
+{
+  notif_parse_supported (reply, (struct notif_base **) notifs,
+			 ARRAY_SIZE (notifs));
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_notif;
 
diff --git a/gdb/remote-notif.h b/gdb/remote-notif.h
index fd38ea4..088713a 100644
--- a/gdb/remote-notif.h
+++ b/gdb/remote-notif.h
@@ -69,6 +69,10 @@ void remote_notif_register_async_event_handler (void);
 void remote_notif_unregister_async_event_handler (void);
 
 void remote_notif_process (struct notif_client *except);
+
+char * remote_notif_qsupported (void);
+void remote_notif_qsupported_reply (const char *reply);
+
 extern struct notif_client notif_client_stop;
 
 extern int notif_debug;
diff --git a/gdb/remote.c b/gdb/remote.c
index 51f9726..c200d06 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1281,6 +1281,7 @@ enum {
   PACKET_QDisableRandomization,
   PACKET_QAgent,
   PACKET_QTBuffer_size,
+  PACKET_notifications,
   PACKET_MAX
 };
 
@@ -3925,6 +3926,15 @@ remote_string_tracing_feature (const struct protocol_feature *feature,
   rs->string_tracing = (support == PACKET_ENABLE);
 }
 
+static void
+remote_notifications_feature (const struct protocol_feature *feature,
+			      enum packet_support support,
+			      const char *value)
+{
+  if (support == PACKET_ENABLE)
+    remote_notif_qsupported_reply (value);
+}
+
 static struct protocol_feature remote_protocol_features[] = {
   { "PacketSize", PACKET_DISABLE, remote_packet_size, -1 },
   { "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet,
@@ -3992,6 +4002,8 @@ static struct protocol_feature remote_protocol_features[] = {
   { "QAgent", PACKET_DISABLE, remote_supported_packet, PACKET_QAgent},
   { "QTBuffer:size", PACKET_DISABLE,
     remote_supported_packet, PACKET_QTBuffer_size},
+  { "Notifications", PACKET_DISABLE, remote_notifications_feature,
+    -1 },
   { "tracenz", PACKET_DISABLE,
     remote_string_tracing_feature, -1 },
 };
@@ -4058,6 +4070,7 @@ remote_query_supported (void)
   if (remote_protocol_packets[PACKET_qSupported].support != PACKET_DISABLE)
     {
       char *q = NULL;
+      char *notifications = remote_notif_qsupported ();
       struct cleanup *old_chain = make_cleanup (free_current_contents, &q);
 
       q = remote_query_supported_append (q, "multiprocess+");
@@ -4067,6 +4080,10 @@ remote_query_supported (void)
 
       q = remote_query_supported_append (q, "qRelocInsn+");
 
+      q = reconcat (q, q, ";notifications=", notifications,
+		    (char *) NULL);
+      xfree (notifications);
+
       q = reconcat (q, "qSupported:", q, (char *) NULL);
       putpkt (q);
 
@@ -5265,7 +5282,10 @@ remote_notif_stop_alloc_reply (void)
 
 static struct notif_annex notif_client_annex_stop[] =
 {
-  { NULL, remote_notif_stop_parse, },
+  /* Even the remote stub doesn't understand
+     'qSupported:notifications=', it may still support notification
+     stop if it supports non-stop.  */
+  { NULL, 1, remote_notif_stop_parse, },
 };
 
 /* A client of notification Stop.  */
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2/5] Query supported notifications by qSupported
  2013-03-11  9:26       ` Yao Qi
@ 2013-03-11 16:59         ` Eli Zaretskii
  0 siblings, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2013-03-11 16:59 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

> Date: Mon, 11 Mar 2013 17:25:33 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
> 
> On 02/16/2013 05:32 PM, Eli Zaretskii wrote:
> > See above: I think you should show here the form of the
> > "notifications=" part in more detail.  In particular, the fact that
> > each notification is of the form NOTIFICATION.ANNEX1.ANNEX2... is
> > important and should be described and the meaning of the multiple
> > ANNEX parts explained.
> 
> How about this below,
> 
> .... supported by the
> stub.  Each notification in the string is composed by the name of the
> notification and the annexes, if any, which are separated by periods.
> 
> > 
> >> >+@item Notifications=@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{[},@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{]}@dots{}
> >> >+@cindex notifications, in remote protocol
> >> >+The remote stub supports a string of notifications.  @var{name} is
> >> >+the name of the notification and @var{annex} is the name of the annex,
> >> >+if the notification has the annex.
> > Here, too, we should say explicitly that there could be several
> > @var{annex} parts separated by periods.
> 
> and this,
> 
> if the notification has the annex.  Note that the notification may
> have several annexes, so here could be several @var{annex} parts
> separated by periods.

Thanks, I'm happy now.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4 0/5] MI notification on trace started/stopped
  2013-01-22  7:05 [PATCH 0/5] MI notification on trace started/stopped (V3) Yao Qi
  2013-01-22  7:05 ` [PATCH 2/5] Query supported notifications by qSupported Yao Qi
  2013-02-16  4:01 ` [PATCH 0/5] MI notification on trace started/stopped (V3.1) Yao Qi
@ 2013-04-02  2:33 ` Yao Qi
  2013-04-02  3:15   ` [PATCH 2/5] Query supported notifications by qSupported Yao Qi
  2 siblings, 1 reply; 15+ messages in thread
From: Yao Qi @ 2013-04-02  2:33 UTC (permalink / raw)
  To: gdb-patches

Hi,
The V3.1 can't be applied to CVS trunk clearly, so I resolved these
conflicts and submit them again.

All doc bits were approved by Eli in V3.1.  Regression tested them
on x86_64-linux with {unix, native-gdbserver} x {sync, async}.  Is
it OK?  Below is the introduction of this series.  People who are
familiar with this series, please skip it.

This patch series adds the MI notifications of 'trace-started' and
'trace-stopped', which are emitted when

  1) trace is started or stopped by commands in GDB,
  2) trace is stopped due to some reasons in the remote stub, such as
trace buffer full.

With these notifications, MI front-end can show the status of trace
up to date.

Patch 4/5 is to address #1, adding new MI notifications and
notifying observers when the commands are called.  #2 needs more work
here, because GDB doesn't know the trace is stopped in the remote
stub.  So we need an async remote notification 'Trace' to tell GDB.
That is what patch 3/5 about.  Patch 5/5 is to use this async remote
notification 'Trace' and notify trace_changed observer.

Patch 1/5 and 2/5 are the enhancement to the async remote
notification, which is needed by the rest of patches in this series.
Patch 1/5 adds "annex" for notification, which is helpful 1) to give
more information on each event of notification, 2) to query supported
notifications on "annex" level.  Patch 2/5 teaches both GDB and
GDBserver to query supported notifications and annexes in the other
side so that 1) GDBserver doesn't send notifications that GDB doesn't
understand, 2) GDB doesn't have to fetch status from the GDBserver if
a certain notification is supported by GDBserver.

 gdb/Makefile.in                              |   11 +-
 gdb/NEWS                                     |    2 +
 gdb/common/common-notif.c                    |  186 ++++++++++++++++++++++++++
 gdb/common/common-notif.h                    |   92 +++++++++++++
 gdb/doc/gdb.texinfo                          |   48 ++++++-
 gdb/doc/observer.texi                        |    6 +
 gdb/gdbserver/Makefile.in                    |    8 +-
 gdb/gdbserver/notif.c                        |   63 ++++++++-
 gdb/gdbserver/notif.h                        |   23 ++--
 gdb/gdbserver/server.c                       |   20 +++-
 gdb/gdbserver/tracepoint.c                   |   36 +++++
 gdb/mi/mi-cmds.c                             |    6 +-
 gdb/mi/mi-interp.c                           |   22 +++
 gdb/mi/mi-main.h                             |    2 +
 gdb/remote-notif-trace.c                     |   86 ++++++++++++
 gdb/remote-notif.c                           |   86 +++++++++++-
 gdb/remote-notif.h                           |   18 +--
 gdb/remote.c                                 |   37 ++++-
 gdb/testsuite/gdb.trace/mi-trace-changed.exp |  149 +++++++++++++++++++++
 gdb/tracepoint.c                             |    4 +
 20 files changed, 851 insertions(+), 54 deletions(-)
 create mode 100644 gdb/common/common-notif.c
 create mode 100644 gdb/common/common-notif.h
 create mode 100644 gdb/remote-notif-trace.c
 create mode 100644 gdb/testsuite/gdb.trace/mi-trace-changed.exp

-- 
1.7.7.6

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/5] Query supported notifications by qSupported
  2013-04-02  2:33 ` [PATCH v4 0/5] MI notification on trace started/stopped Yao Qi
@ 2013-04-02  3:15   ` Yao Qi
  0 siblings, 0 replies; 15+ messages in thread
From: Yao Qi @ 2013-04-02  3:15 UTC (permalink / raw)
  To: gdb-patches

As we we adding more notifications and annexes, both GDB and GDBserver
has to know what notifications and annexes are supported in the other
side.  This is what this patch does.  When GDB connects to GDBserver,
it will happen:

  --> qSupported:XXX;notifications=N1,N2.A1.A2,N3
      (GDB supports notification N1, N2 with annexes A1,A2, and N3)
  <-- XXX;Notifications=N1,N2.A1,N4
      (GDBsever supports notification N1, N2 with annex A1 and N4)

after this, GDB knows what notifications GDBserver is able to send,
and GDBservers knows what notifications GDB doesn't support.

The documentation patch was approved by Eli here
<http://sourceware.org/ml/gdb-patches/2013-03/msg00474.html>

gdb/gdbserver:

2013-04-02  Yao Qi  <yao@codesourcery.com>

	* Makefile.in (SFILES): Add "common-notif.c".
	(OBS): Add common-notif.o.
	(common-notif.o): New rule.

	* notif.c (notif_qsupported_record): New.
	(notif_qsupported_reply): New.
	* notif.h (notif_qsupported_reply): Declare.
	(notif_qsupported_record): Declare.
	* server.c (notif_annex_stop): Update.
	(handle_query): Call notif_qsupported_record and
	notif_qsupported_reply.
gdb:

2013-04-02  Yao Qi  <yao@codesourcery.com>

	* common/common-notif.c: New.
	* common/common-notif.h (struct notif_annex) <supported>: New
	field.
	(NOTIF_ANNEX_SUPPORTED_P): New macro.
	(notif_supported, notif_parse_supported): Declare.
	* Makefile.in (REMOTE_OBS): Append "common-notif.o".
	(SFILES): Add "common-notif.c".
	* remote-notif.c (remote_notif_parse_1): Call the parser of
	annex if it is supported.
	(remote_notif_qsupported): New.
	(remote_notif_qsupported_reply): New.
	* remote-notif.h (remote_notif_qsupported): Declare.
	(remote_notif_qsupported_reply): Declare.

	* remote.c (PACKET_notifications): New enum.
	(remote_notifications_feature): New.
	(remote_protocol_features): Add new element.
	(remote_query_supported): Call remote_notif_qsupported and
	append supported notifications to qSupported feature.
	(notif_client_annex_stop): Update.

gdb/doc:

2013-04-02  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (General Query Packets): Document the new feature
	'notifications' of 'qSupported' packet.
	Document the new feature in qSupported reply.
---
 gdb/Makefile.in           |    9 ++-
 gdb/common/common-notif.c |  186 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/common/common-notif.h |   15 ++++
 gdb/doc/gdb.texinfo       |   22 +++++
 gdb/gdbserver/Makefile.in |    8 ++-
 gdb/gdbserver/notif.c     |   21 +++++
 gdb/gdbserver/notif.h     |    2 +
 gdb/gdbserver/server.c    |   15 ++++-
 gdb/remote-notif.c        |   32 +++++++-
 gdb/remote-notif.h        |    4 +
 gdb/remote.c              |   24 +++++-
 11 files changed, 328 insertions(+), 10 deletions(-)
 create mode 100644 gdb/common/common-notif.c

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index fa4c302..3fe3e34 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -511,7 +511,7 @@ SER_HARDWIRE = @SER_HARDWIRE@
 # The `remote' debugging target is supported for most architectures,
 # but not all (e.g. 960)
 REMOTE_OBS = remote.o dcache.o tracepoint.o ax-general.o ax-gdb.o remote-fileio.o \
-	remote-notif.o ctf.o
+	remote-notif.o ctf.o common-notif.o
 
 # This is remote-sim.o if a simulator is to be linked in.
 SIM_OBS = @SIM_OBS@
@@ -737,7 +737,8 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
 	p-exp.y p-lang.c p-typeprint.c p-valprint.c parse.c printcmd.c \
 	proc-service.list progspace.c \
 	prologue-value.c psymtab.c \
-	regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c reverse.c \
+	regcache.c reggroups.c remote.c remote-fileio.c remote-notif.c \
+	common-notif.c reverse.c \
 	sentinel-frame.c \
 	serial.c ser-base.c ser-unix.c skip.c \
 	solib.c solib-target.c source.c \
@@ -1928,6 +1929,10 @@ common-utils.o: ${srcdir}/common/common-utils.c
 	$(COMPILE) $(srcdir)/common/common-utils.c
 	$(POSTCOMPILE)
 
+common-notif.o: ${srcdir}/common/common-notif.c
+	$(COMPILE) $(srcdir)/common/common-notif.c
+	$(POSTCOMPILE)
+
 gdb_vecs.o: ${srcdir}/common/gdb_vecs.c
 	$(COMPILE) $(srcdir)/common/gdb_vecs.c
 	$(POSTCOMPILE)
diff --git a/gdb/common/common-notif.c b/gdb/common/common-notif.c
new file mode 100644
index 0000000..e53729f
--- /dev/null
+++ b/gdb/common/common-notif.c
@@ -0,0 +1,186 @@
+/* Copyright (C) 2013 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+#include <string.h>
+#include "common-notif.h"
+#include "gdb_assert.h"
+
+/* Return a string about the notifications in array NOTIFS.  NUM is
+   the number of elements in array NOTIFS.  The caller is responsible
+   to free the returned string.  Suppose array NOTIFS has
+   notifications N1, N2, and N3.  Only N2 has annexes A1 and A2.  The
+   returned string is "N1,N2.A2.A2,N3".  */
+
+char *
+notif_supported (struct notif_base *notifs[], int num)
+{
+  int i;
+  char * p = NULL;
+
+#define BUF_LEN 128
+
+  for (i = 0; i < num; i++)
+    {
+      struct notif_base *nb = notifs[i];
+
+      if (p == NULL)
+	{
+	  p = xmalloc (BUF_LEN);
+	  strcpy (p, nb->notif_name);
+	}
+      else
+	xsnprintf (p + strlen (p), BUF_LEN - strlen (p), ",%s",
+		   nb->notif_name);
+
+      if (NOTIF_HAS_ANNEX (nb))
+	{
+	  int j;
+
+	  NOTIF_ITER_ANNEX (nb, j)
+	    {
+	      xsnprintf (p + strlen (p), BUF_LEN - strlen (p),
+			 ".%s", nb->annexes[j].name);
+	    }
+	}
+    }
+
+  return p;
+}
+
+/* Find annex in notification NB by name NAME and length LEN.
+   If found, return annex, otherwise return NULL.  */
+
+static struct notif_annex *
+notif_find_annex (struct notif_base *nb, const char *name, int len)
+{
+  if (NOTIF_HAS_ANNEX (nb))
+    {
+      int j;
+
+      NOTIF_ITER_ANNEX (nb, j)
+	if (strncmp (name, nb->annexes[j].name, len) == 0
+	    && len == strlen (nb->annexes[j].name))
+	  return &nb->annexes[j];
+    }
+  return NULL;
+}
+
+/* Parse the REPLY, which is about supported annexes and
+   notifications from the peer, and disable some annexes of
+   notification in array NOTIFS if the peer doesn't support.  NUM is
+   the number of elements in array NOTIFS.  */
+
+void
+notif_parse_supported (const char *reply,
+		       struct notif_base *notifs[], int num)
+{
+  const char *p = reply;
+  int notif_num = 1;
+  char **notif_str;
+  int i;
+
+  /* Count how many notifications in REPLY.  */
+  for (i = 0; reply[i] != '\0'; i++)
+    if (reply[i] == ',')
+      notif_num++;
+
+  /* Copy contents of each notification in REPLY to each slot of
+     NOTIF_STR.  */
+  notif_str = xmalloc (notif_num * sizeof (char *));
+  for (i = 0; i < notif_num; i++)
+    {
+      char *end = strchr (p, ',');
+
+      if (end == NULL)
+	notif_str[i] = xstrdup (p);
+      else
+	{
+	  /* Can't use xstrndup in GDBserver.  */
+	  notif_str[i] = strndup (p, end - p);
+	  p = end + 1;
+	}
+    }
+
+  /* Iterate each element in NOTIF_STR and parse annex in it.  */
+  for (i = 0; i < notif_num; i++)
+    {
+      int j;
+      struct notif_base *nb = NULL;
+
+      p = notif_str[i];
+
+      for (j = 0; j < num; j++)
+	{
+	  int name_len = strlen (notifs[j]->notif_name);
+
+	  if (0 == strncmp (notifs[j]->notif_name, p, name_len)
+	      && (p[name_len] == '.' || p[name_len] == 0))
+	    {
+	      nb = notifs[j];
+	      p += name_len;
+	      break;
+	    }
+	}
+
+      if (nb != NULL)
+	{
+	  if (p[0] == 0)
+	    {
+	      /* No annex.  */
+	      gdb_assert (!NOTIF_HAS_ANNEX (nb));
+	      nb->annexes[0].supported = 1;
+	    }
+	  else if (p[0] == '.')
+	    {
+	      gdb_assert (NOTIF_HAS_ANNEX (nb));
+
+	      p++;
+
+	      /* Parse the rest of P and look for annexes.  */
+	      while (p != NULL)
+		{
+		  char *end = strchr (p, '.');
+		  struct notif_annex *annex = NULL;
+
+		  if (end != NULL)
+		    {
+		      annex = notif_find_annex (nb, p, end - p);
+		      p = end + 1;
+		    }
+		  else
+		    {
+		      annex = notif_find_annex (nb, p, strlen (p));
+		      p = end;
+		    }
+
+		  /* If annex is known, mark it supported, otherwise
+		     skip it because the peer knows the annex but we
+		     don't know.  */
+		  if (annex != NULL)
+		    annex->supported = 1;
+		}
+	    }
+	  else
+	    warning (_("Unknown supported notification"));
+	}
+    }
+}
diff --git a/gdb/common/common-notif.h b/gdb/common/common-notif.h
index 5e7e9fa..9a8fb7c 100644
--- a/gdb/common/common-notif.h
+++ b/gdb/common/common-notif.h
@@ -36,6 +36,11 @@ struct notif_annex
      notification.  */
   const char *name;
 
+  /* This annex is supported by the peer (GDB or GDBserver)?  A
+     notification may have multiple annexes and some of them are
+     supported.  Annex is the smallest unit of support.  */
+  int supported;
+
 #ifdef GDBSERVER
   /* Write event EVENT to OWN_BUF.  */
   void (*write) (struct notif_event *event, char *own_buf);
@@ -57,6 +62,10 @@ struct notif_annex
 
 #define NOTIF_HAS_ANNEX(NOTIF) ((NOTIF)->annexes[0].name != NULL)
 
+/* Whether the annex of notification N is supported.  */
+
+#define NOTIF_ANNEX_SUPPORTED_P(N, INDEX) \
+  ((N).annexes[INDEX].supported)
 
 /* "Base class" of a notification.  It can be extended in both GDB
    and GDBserver to represent a type of notification.  */
@@ -75,3 +84,9 @@ struct notif_base
      of the notification.  */
   struct notif_annex *annexes;
 };
+
+char *notif_supported (struct notif_base *notifs[], int num);
+
+void notif_parse_supported (const char *reply,
+			    struct notif_base *notifs[], int num);
+
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 6cbb8b5..fbf0e1a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -37298,6 +37298,15 @@ description.
 This feature indicates whether @value{GDBN} supports the
 @samp{qRelocInsn} packet (@pxref{Tracepoint Packets,,Relocate
 instruction reply packet}).
+
+@item notifications
+@anchor{notifications feature}
+This feature indicates that @value{GDBN} supports the async remote
+notifications (@pxref{Notification Packets}).  If the stub sees
+@samp{notifications=} with a string of supported notifications,
+separated by commas, it will report notifications supported by the
+stub.  Each notification in the string is composed by the name of the
+notification and the annexes, if any, which are separated by periods.
 @end table
 
 Stubs should ignore any unknown values for
@@ -37506,6 +37515,11 @@ These are the currently defined stub features and their properties:
 @tab @samp{-}
 @tab No
 
+@item @samp{Notifications}
+@tab Yes
+@tab @samp{-}
+@tab No
+
 @end multitable
 
 These are the currently defined stub features, in more detail:
@@ -37668,6 +37682,14 @@ See @ref{Bytecode Descriptions} for details about the bytecode.
 The remote stub supports running a breakpoint's command list itself,
 rather than reporting the hit to @value{GDBN}.
 
+@item Notifications=@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{[},@var{name}@r{[}.@var{annex}@r{]}@dots{}@r{]}@dots{}
+@cindex notifications, in remote protocol
+The remote stub supports a string of notifications.  @var{name} is
+the name of the notification and @var{annex} is the name of the annex,
+if the notification has the annex.  Note that the notification may
+have several annexes, so here could be several @var{annex} parts
+separated by periods.
+
 @item Qbtrace:off
 The remote stub understands the @samp{Qbtrace:off} packet.
 
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 08db2cc..9285bdc 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -155,7 +155,8 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/common/vec.c $(srcdir)/common/gdb_vecs.c \
 	$(srcdir)/common/common-utils.c $(srcdir)/common/xml-utils.c \
 	$(srcdir)/common/linux-osdata.c $(srcdir)/common/ptid.c \
-	$(srcdir)/common/buffer.c $(srcdir)/common/linux-btrace.c
+	$(srcdir)/common/buffer.c $(srcdir)/common/linux-btrace.c \
+	$(srcdir)/common/common-notif.c
 
 DEPFILES = @GDBSERVER_DEPFILES@
 
@@ -165,7 +166,7 @@ SOURCES = $(SFILES)
 TAGFILES = $(SOURCES) ${HFILES} ${ALLPARAM} ${POSSLIBS}
 
 OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o target.o \
-	utils.o version.o vec.o gdb_vecs.o \
+	utils.o version.o vec.o gdb_vecs.o common-notif.o \
 	mem-break.o hostio.o event-loop.o tracepoint.o \
 	xml-utils.o common-utils.o ptid.o buffer.o format.o \
 	dll.o notif.o \
@@ -521,6 +522,9 @@ linux-ptrace.o: ../common/linux-ptrace.c
 common-utils.o: ../common/common-utils.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+common-notif.o: ../common/common-notif.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 vec.o: ../common/vec.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdb/gdbserver/notif.c b/gdb/gdbserver/notif.c
index 9300697..2b09078 100644
--- a/gdb/gdbserver/notif.c
+++ b/gdb/gdbserver/notif.c
@@ -183,6 +183,27 @@ notif_event_xfree (struct notif_event *event)
   xfree (event);
 }
 
+/* Record the notifications supported by GDB.  GDB_NOTIFICATIONS is a
+   string about notifications GDB supports.  */
+
+void
+notif_qsupported_record (char *gdb_notifications)
+{
+  return notif_parse_supported (gdb_notifications,
+				(struct notif_base **) notifs,
+				ARRAY_SIZE (notifs));
+}
+
+/* Return a string about notifications that GDBserver supports.
+   Return NULL if no notification is supported.  */
+
+char *
+notif_qsupported_reply (void)
+{
+  return notif_supported ((struct notif_base **) notifs,
+			  ARRAY_SIZE (notifs));
+}
+
 void
 initialize_notif (void)
 {
diff --git a/gdb/gdbserver/notif.h b/gdb/gdbserver/notif.h
index 9bdb299..31f135f 100644
--- a/gdb/gdbserver/notif.h
+++ b/gdb/gdbserver/notif.h
@@ -58,6 +58,8 @@ extern struct notif_server notif_stop;
 
 int handle_notif_ack (char *own_buf, int packet_len);
 void notif_write_event (struct notif_server *notif, char *own_buf);
+char* notif_qsupported_reply (void);
+void notif_qsupported_record (char *gdb_notifications);
 
 void notif_push (struct notif_server *np, struct notif_event *event);
 void notif_event_enque (struct notif_server *notif,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 8c00b7c..1f9d526 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -186,7 +186,7 @@ vstop_notif_reply (struct notif_event *event, char *own_buf)
 
 static struct notif_annex notif_annex_stop[] =
 {
-  { NULL, vstop_notif_reply, },
+  { NULL, 1, vstop_notif_reply, },
 };
 
 struct notif_server notif_stop =
@@ -1734,6 +1734,11 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 		  /* GDB supports relocate instruction requests.  */
 		  gdb_supports_qRelocInsn = 1;
 		}
+	      else if (strncmp (p, "notifications=", 14) == 0)
+		{
+		  /* Record what notifications GDB supports.  */
+		  notif_qsupported_record (&p[14]);
+		}
 	      else
 		target_process_qsupported (p);
 
@@ -1821,6 +1826,14 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 	  strcat (own_buf, ";Qbtrace:off+");
 	  strcat (own_buf, ";qXfer:btrace:read+");
 	}
+      p = notif_qsupported_reply ();
+
+      if (p != NULL)
+	{
+	  strcat (own_buf, ";Notifications=");
+	  strcat (own_buf, p);
+	  xfree (p);
+	}
 
       return;
     }
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index 335572b..c2d64f6 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -75,9 +75,16 @@ remote_notif_parse_1 (struct notif_client *nc,
 		 contents in BUF.  */
 	      && buf[strlen (m->name)] == ':')
 	    {
-	      /* Pass BUF without annex and ':'.  */
-	      m->parse (nc, buf + strlen (m->name) + 1, event);
-	      break;
+	      if (m->supported)
+		{
+		  /* Pass BUF without annex and ':'.  */
+		  m->parse (nc, buf + strlen (m->name) + 1, event);
+		  break;
+		}
+	      else
+		warning (_("GDB gets annex '%s' of notification '%s'"
+			   "but remote stub doesn't support"),
+			 base->notif_name, m->name);
 	    }
 	  m = NULL;
 	}
@@ -305,6 +312,25 @@ notif_xfree (struct notif_client *notif)
   xfree (notif);
 }
 
+/* Return a string of GDB supported features.  */
+
+char *
+remote_notif_qsupported (void)
+{
+  return notif_supported ((struct notif_base **) notifs,
+			  ARRAY_SIZE (notifs));
+}
+
+/* Parse the qSupported reply REPLY from the remote stub and disable
+   some notifications if the remote stub doesn't support.  */
+
+void
+remote_notif_qsupported_reply (const char *reply)
+{
+  notif_parse_supported (reply, (struct notif_base **) notifs,
+			 ARRAY_SIZE (notifs));
+}
+
 /* -Wmissing-prototypes */
 extern initialize_file_ftype _initialize_notif;
 
diff --git a/gdb/remote-notif.h b/gdb/remote-notif.h
index fd38ea4..088713a 100644
--- a/gdb/remote-notif.h
+++ b/gdb/remote-notif.h
@@ -69,6 +69,10 @@ void remote_notif_register_async_event_handler (void);
 void remote_notif_unregister_async_event_handler (void);
 
 void remote_notif_process (struct notif_client *except);
+
+char * remote_notif_qsupported (void);
+void remote_notif_qsupported_reply (const char *reply);
+
 extern struct notif_client notif_client_stop;
 
 extern int notif_debug;
diff --git a/gdb/remote.c b/gdb/remote.c
index e435354..8bd70e6 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1284,6 +1284,7 @@ enum {
   PACKET_Qbtrace_off,
   PACKET_Qbtrace_bts,
   PACKET_qXfer_btrace,
+  PACKET_notifications,
   PACKET_MAX
 };
 
@@ -3928,6 +3929,15 @@ remote_string_tracing_feature (const struct protocol_feature *feature,
   rs->string_tracing = (support == PACKET_ENABLE);
 }
 
+static void
+remote_notifications_feature (const struct protocol_feature *feature,
+			      enum packet_support support,
+			      const char *value)
+{
+  if (support == PACKET_ENABLE)
+    remote_notif_qsupported_reply (value);
+}
+
 static struct protocol_feature remote_protocol_features[] = {
   { "PacketSize", PACKET_DISABLE, remote_packet_size, -1 },
   { "qXfer:auxv:read", PACKET_DISABLE, remote_supported_packet,
@@ -4000,7 +4010,9 @@ static struct protocol_feature remote_protocol_features[] = {
   { "Qbtrace:off", PACKET_DISABLE, remote_supported_packet, PACKET_Qbtrace_off },
   { "Qbtrace:bts", PACKET_DISABLE, remote_supported_packet, PACKET_Qbtrace_bts },
   { "qXfer:btrace:read", PACKET_DISABLE, remote_supported_packet,
-    PACKET_qXfer_btrace }
+    PACKET_qXfer_btrace },
+  { "Notifications", PACKET_DISABLE, remote_notifications_feature,
+    -1 },
 };
 
 static char *remote_support_xml;
@@ -4065,6 +4077,7 @@ remote_query_supported (void)
   if (remote_protocol_packets[PACKET_qSupported].support != PACKET_DISABLE)
     {
       char *q = NULL;
+      char *notifications = remote_notif_qsupported ();
       struct cleanup *old_chain = make_cleanup (free_current_contents, &q);
 
       q = remote_query_supported_append (q, "multiprocess+");
@@ -4074,6 +4087,10 @@ remote_query_supported (void)
 
       q = remote_query_supported_append (q, "qRelocInsn+");
 
+      q = reconcat (q, q, ";notifications=", notifications,
+		    (char *) NULL);
+      xfree (notifications);
+
       q = reconcat (q, "qSupported:", q, (char *) NULL);
       putpkt (q);
 
@@ -5268,7 +5285,10 @@ remote_notif_stop_alloc_reply (void)
 
 static struct notif_annex notif_client_annex_stop[] =
 {
-  { NULL, remote_notif_stop_parse, },
+  /* Even the remote stub doesn't understand
+     'qSupported:notifications=', it may still support notification
+     stop if it supports non-stop.  */
+  { NULL, 1, remote_notif_stop_parse, },
 };
 
 /* A client of notification Stop.  */
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-01-24  9:43 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-24  9:43 [PATCH 0/5 V8] MI notification on trace started/stopped Yao Qi
2014-01-24  9:43 ` [PATCH 2/5] Query supported notifications by qSupported Yao Qi
2014-01-24  9:43 ` [PATCH 5/5] New test case mi-trace-changed.exp Yao Qi
2014-01-24  9:43 ` [PATCH 1/5] Process notifications when reply is got Yao Qi
2014-01-24  9:43 ` [PATCH 3/5] async remote notification 'Trace' Yao Qi
2014-01-24  9:43 ` [PATCH 4/5] MI notification on trace started/stopped Yao Qi
  -- strict thread matches above, loose matches on Subject: below --
2013-01-22  7:05 [PATCH 0/5] MI notification on trace started/stopped (V3) Yao Qi
2013-01-22  7:05 ` [PATCH 2/5] Query supported notifications by qSupported Yao Qi
2013-01-22  8:09   ` Eli Zaretskii
2013-01-22  8:41     ` Yao Qi
2013-01-22 10:26       ` Eli Zaretskii
2013-02-16  4:01 ` [PATCH 0/5] MI notification on trace started/stopped (V3.1) Yao Qi
2013-02-16  4:02   ` [PATCH 2/5] Query supported notifications by qSupported Yao Qi
2013-02-16  9:33     ` Eli Zaretskii
2013-03-11  9:26       ` Yao Qi
2013-03-11 16:59         ` Eli Zaretskii
2013-04-02  2:33 ` [PATCH v4 0/5] MI notification on trace started/stopped Yao Qi
2013-04-02  3:15   ` [PATCH 2/5] Query supported notifications by qSupported Yao Qi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox