[PATCH 3/4] observer on create/modify/delete itsets

[PATCH 3/4] observer on create/modify/delete itsets

[Yao Qi]

Hi,
This patch is about adding MI notifications to creating, modifying and
deleting named itsets in console.  Originally, it is not allowed to
'modify' a named itset, however, when taking 'predefined itset from
remote target' into account, an itset may be modified time and time
again as it is changing in remote target.

gdb/doc:

2012-08-30  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (GDB/MI Async Records): Document new MI notifications
	'=itset-created' '=itset-modified' and '=itset-deleted'.
	* observer.texi (GDB Observers): Add new observers
	'itset_created' 'itset_deleted' and 'itset_modified'.
gdb:

2012-08-30  Yao Qi  <yao@codesourcery.com>

	* itset.c: Include 'observer.h'.
	(named_itset_create): Call observer_notify_itset_modified
	and observer_notify_itset_created respectively.
	(undefset_command): Call observer_notify_itset_deleted.

	* mi/mi-interp.c : Declare mi_itset_created, mi_itset_deleted
	and mi_itset_modified.
	(mi_interpreter_init): Call observer_attach_itset_created
	observer_attach_itset_deleted and
	observer_attach_itset_modified.
	(mi_itset_created, mi_itset_deleted, mi_itset_modified): New.

gdb/testsuite:

2012-08-30  Yao Qi  <yao@codesourcery.com>

	*gdb.mi/mi-itset-changed.exp: New.
---
 gdb/doc/gdb.texinfo                       |   10 ++++
 gdb/doc/observer.texi                     |   12 +++++
 gdb/itset.c                               |   15 +++++-
 gdb/mi/mi-interp.c                        |   76 +++++++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-itset-changed.exp |   50 +++++++++++++++++++
 5 files changed, 161 insertions(+), 2 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-itset-changed.exp

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c9228d3..7d2a470 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -27705,6 +27705,16 @@ thread group in whose context the library was unloaded.  If the field is
 absent, it means the library was unloaded in the context of all present
 thread groups.
 
+@item =itset-created,name=@var{name},spec=@var{spec}
+@item =itset-modified,name=@var{name},spec=@var{spec}
+Reports that a named itset is created or modified respecitvely.  The name
+of the itset is @var{name}, and the specificaiton of the itset is
+@var{spec}.
+@item =itset-deleted,name=@var{name}
+Reports that the named itset whose name is @var{name} is deleted.
+@item =itset-deleted
+Reports that all named itsets are deleted.
+
 @item =breakpoint-created,bkpt=@{...@}
 @itemx =breakpoint-modified,bkpt=@{...@}
 @itemx =breakpoint-deleted,id=@var{number}
diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index c81e137..9da915d 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -235,7 +235,19 @@ The parameter of some @code{set} commands in console are changed.  This
 method is called after a command @code{set @var{param} @var{value}}.
 @var{param} is the parameter of @code{set} command, and @var{value}
 is the value of changed parameter.
+@end deftypefun
+
+@deftypefun void itset_created (const char *@var{name}, const char *@var{spec})
+called after a named itset @var{name} is created, and its specification is @var{spec}.
+@end deftypefun
+
+@deftypefun void itset_deleted (const char *@var{name})
+called after a named itset @var{name} is deleted.  If @var{name} is equal to
+@code{NULL}, all named itsets are deleted.
+@end deftypefun
 
+@deftypefun void itset_modified (const char *@var{name}, const char *@var{spec})
+called after a named itset @var{name} is modified, and its specification is @var{spec}.
 @end deftypefun
 
 @deftypefun void test_notification (int @var{somearg})
diff --git a/gdb/itset.c b/gdb/itset.c
index d3f79f7..573f332 100644
--- a/gdb/itset.c
+++ b/gdb/itset.c
@@ -18,6 +18,7 @@
 
 #include "defs.h"
 #include "itset.h"
+#include "observer.h"
 #include "vec.h"
 #include "bfd.h"
 #include "inferior.h"
@@ -2047,8 +2048,15 @@ named_itset_create (char *set_name, char *spec)
   itset_free (itset);
   discard_cleanups (old_chain);
 
-  if (!found)
-    add_to_named_itset_chain (named_itset);
+  if (found)
+    observer_notify_itset_modified (itset_name (named_itset->set),
+				    itset_spec (named_itset->set));
+  else
+    {
+      add_to_named_itset_chain (named_itset);
+      observer_notify_itset_created (itset_name (named_itset->set),
+				     itset_spec (named_itset->set));
+    }
 }
 
 static void
@@ -2106,6 +2114,7 @@ undefset_command (char *arg, int from_tty)
 	    it_link = &it->next;
 	  it = *it_link;
 	}
+      observer_notify_itset_deleted (NULL);
       return;
     }
 
@@ -2120,6 +2129,8 @@ undefset_command (char *arg, int from_tty)
 	    error (_("cannot delete builtin I/T set"));
 
 	  *it_link = it->next;
+	  observer_notify_itset_deleted (itset_name (it->set));
+
 	  free_named_itset (it);
 	  found = 1;
 	  break;
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 94df818..a5e172b 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -58,6 +58,9 @@ static void mi_insert_notify_hooks (void);
 static void mi_remove_notify_hooks (void);
 static void mi_on_normal_stop (struct bpstats *bs, int print_frame);
 
+static void mi_itset_created (const char *name, const char *spec);
+static void mi_itset_deleted (const char *name);
+static void mi_itset_modified (const char *name, const char *spec);
 static void mi_new_thread (struct thread_info *t);
 static void mi_thread_exit (struct thread_info *t, int silent);
 static void mi_inferior_added (struct inferior *inf);
@@ -121,6 +124,9 @@ mi_interpreter_init (struct interp *interp, int top_level)
       observer_attach_inferior_appeared (mi_inferior_appeared);
       observer_attach_inferior_exit (mi_inferior_exit);
       observer_attach_inferior_removed (mi_inferior_removed);
+      observer_attach_itset_created (mi_itset_created);
+      observer_attach_itset_deleted (mi_itset_deleted);
+      observer_attach_itset_modified (mi_itset_modified);
       observer_attach_normal_stop (mi_on_normal_stop);
       observer_attach_target_resumed (mi_on_resume);
       observer_attach_solib_loaded (mi_solib_loaded);
@@ -599,6 +605,76 @@ mi_breakpoint_modified (struct breakpoint *b)
   gdb_flush (mi->event_channel);
 }
 
+/* Emit notification about the created named itset.  */
+
+static void
+mi_itset_created (const char *name, const char *spec)
+{
+  struct mi_interp *mi = top_level_interpreter_data ();
+  struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
+
+  target_terminal_ours ();
+
+  fprintf_unfiltered (mi->event_channel,
+		      "itset-created");
+
+  ui_out_redirect (mi_uiout, mi->event_channel);
+
+  ui_out_field_string (mi_uiout, "name", name);
+  ui_out_field_string (mi_uiout, "spec", spec);
+
+  ui_out_redirect (mi_uiout, NULL);
+
+  gdb_flush (mi->event_channel);
+}
+
+/* Emit notification about deleted named itset.  NAME is the name of deleted
+   itset.  If NAME is NULL, it means delete all named itsets.  */
+
+static void
+mi_itset_deleted (const char *name)
+{
+  struct mi_interp *mi = top_level_interpreter_data ();
+  struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
+
+  target_terminal_ours ();
+
+  fprintf_unfiltered (mi->event_channel,
+		      "itset-deleted");
+
+  ui_out_redirect (mi_uiout, mi->event_channel);
+
+  if (name != NULL)
+      ui_out_field_string (mi_uiout, "name", name);
+
+  ui_out_redirect (mi_uiout, NULL);
+
+  gdb_flush (mi->event_channel);
+}
+
+/* Emit notification about the modified named itset.  */
+
+static void
+mi_itset_modified (const char *name, const char *spec)
+{
+  struct mi_interp *mi = top_level_interpreter_data ();
+  struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
+
+  target_terminal_ours ();
+
+  fprintf_unfiltered (mi->event_channel,
+		      "itset-modified");
+
+  ui_out_redirect (mi_uiout, mi->event_channel);
+
+  ui_out_field_string (mi_uiout, "name", name);
+  ui_out_field_string (mi_uiout, "spec", spec);
+
+  ui_out_redirect (mi_uiout, NULL);
+
+  gdb_flush (mi->event_channel);
+}
+
 static int
 mi_output_running_pid (struct thread_info *info, void *arg)
 {
diff --git a/gdb/testsuite/gdb.mi/mi-itset-changed.exp b/gdb/testsuite/gdb.mi/mi-itset-changed.exp
new file mode 100644
index 0000000..4767547
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-itset-changed.exp
@@ -0,0 +1,50 @@
+# Copyright 2012 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
+set MIFLAGS "-i=mi"
+
+standard_testfile basics.c
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+     untested mi-itset-changed.exp
+     return -1
+}
+
+if [mi_gdb_start] {
+    return
+}
+mi_run_to_main
+
+mi_gdb_test "defset itset1 !i1.t1-2,i2\.t3-4" \
+    ".*=itset-created,name=\"itset1\",spec=\"!i1.t1-2,i2.t3-4\".*\\^done" \
+    "defset itset1"
+mi_gdb_test "defset itset2 c1-2" \
+    ".*=itset-created,name=\"itset2\",spec=\"c1-2\".*\\^done" \
+    "defset itset2"
+
+mi_gdb_test "defset itset2 c1" \
+    ".*=itset-modified,name=\"itset2\",spec=\"c1\".*\\^done" \
+    "defset itset2 again"
+
+mi_gdb_test "undefset itset2" ".*=itset-deleted,name=\"itset2\".*\\^done" \
+    "undefset itset2"
+
+mi_gdb_test "undefset -all" ".*=itset-deleted.*\\^done" \
+    "undefset all"
+
+mi_gdb_exit
+
+return 0
-- 
1.7.7.6

[PATCH 1/4] create named itset.

[Yao Qi]

Hi,
This patch refactors adding new named itset a little bit, and adding a
new command to set the preference to resolve the duplicated itset when
adding new.

gdb:

2012-08-30  Yao Qi  <yao@codesourcery.com>

	* itset.c (duplicate_style_overwrite, duplicate_style_query): New.
	(duplicate_style): New.
	(defset_command): Move some code to ...
	(named_itset_create): ... here.  New.
	(_initialize_itset): Call add_setshow_enum_cmd to register
	"duplicate" command.

gdb/doc:

2012-08-30  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (ITSET): Document on command 'set resolve-duplicate'.
---
 gdb/doc/gdb.texinfo |   12 ++++++++
 gdb/itset.c         |   79 ++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 72 insertions(+), 19 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0419e3c..5573e0d 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5215,6 +5215,18 @@ specific numbers, such as 1, 3, but also can be range, like 4-8.
 (@value{GDBP}) info itsets 1 3 4-8
 @end smallexample
 
+@kindex set resolve-duplicate
+@kindex show resolve-duplicate
+@item set resolve-duplicate query
+When a new named itset is defined, which has already been defined before,
+@value{GDBN} will query whether to overwrite the existing named itset
+or dicard the new definition.
+
+@item set resolve-duplicate overwrite
+When a new named itset is defined, which has already been defined before,
+@value{GDBN} will overwrite the existing named itset with the new
+definition.
+
 @item @ref{maint info itsets}
 
 @end table
diff --git a/gdb/itset.c b/gdb/itset.c
index 5b3098e..6c14e6c 100644
--- a/gdb/itset.c
+++ b/gdb/itset.c
@@ -1998,15 +1998,63 @@ itset_is_static (struct itset *itset)
   return 1;
 }
 
+static const char duplicate_style_overwrite[] = "overwrite";
+static const char duplicate_style_query[] = "query";
+static const char *const duplicate_style_enums[] =
+{
+  duplicate_style_overwrite,
+  duplicate_style_query,
+  NULL,
+};
+static const char *duplicate_style = duplicate_style_overwrite;
+
+static void
+named_itset_create (char *set_name, char *spec)
+{
+  struct itset *itset;
+  struct named_itset *named_itset;
+  char *name = xstrdup (set_name);
+  struct cleanup *old_chain = make_cleanup (xfree, name);
+  int found = 0;
+
+  named_itset = get_named_itset (name);
+  found = (named_itset != NULL);
+  if (found && duplicate_style == duplicate_style_query)
+    {
+      if (query (_("Overwrite '%s' from '%s' to '%s'?"), name,
+		 itset_spec (named_itset->set), spec))
+	return;
+    }
+
+  itset = itset_create (&spec);
+  make_cleanup_itset_free (itset);
+
+  if (itset_is_static (itset) && itset_is_empty (itset))
+    warning (_("static itset is empty"));
+
+  if (found)
+    {
+      itset_free (named_itset->set);
+      itset_reference (itset);
+      named_itset->set = itset;
+      itset->name = name;
+    }
+  else
+    named_itset = make_itset_named_itset (itset, name, 0);
+
+  itset_free (itset);
+  discard_cleanups (old_chain);
+
+  if (!found)
+    add_to_named_itset_chain (named_itset);
+}
+
 static void
 defset_command (char *arg, int from_tty)
 {
   char *endp;
   char *name;
   char *spec;
-  struct itset *itset;
-  struct named_itset *named_itset;
-  struct cleanup *old_chain;
 
   if (arg == NULL || *arg == '\0')
     error_no_arg (_("no args"));
@@ -2017,24 +2065,9 @@ defset_command (char *arg, int from_tty)
   spec = endp;
 
   name = xstrndup (arg, endp - arg);
-  old_chain = make_cleanup (xfree, name);
-
-  named_itset = get_named_itset (name);
-  if (named_itset != NULL)
-    error (_("itset %s already exists"), name);
-
   spec = skip_spaces (spec);
 
-  itset = itset_create (&spec);
-  make_cleanup_itset_free (itset);
-
-  if (itset_is_static (itset) && itset_is_empty (itset))
-    warning (_("static itset is empty"));
-
-  named_itset = make_itset_named_itset (itset, name, 0);
-  itset_free (itset);
-  discard_cleanups (old_chain);
-  add_to_named_itset_chain (named_itset);
+  named_itset_create (name, spec);
 }
 
 static void
@@ -2377,4 +2410,12 @@ Usage: info itsets [NUMBERS AND/OR RANGES]\n"));
   add_cmd ("itsets", class_maintenance, maintenance_info_itsets_command, _("\
 Display the list of all defined named itsets, user-defined and built-in.\n"),
   &maintenanceinfolist);
+
+  add_setshow_enum_cmd ("resolve-duplicate", class_support,
+			duplicate_style_enums, &duplicate_style, _("\
+Set the style of resolving adding duplicated itset."), _("\
+Show the style of resolving adding duplicated itset."), _("\
+This setting chooses how GDB will resolve duplicated itset."),
+			NULL, NULL,
+			&setlist, &showlist);
 }
-- 
1.7.7.6

[PATCH 2/4] save itsets into file

[Yao Qi]

Hi,
This patch adds a command 'save itsets' to a file, which can be used
later by command 'source'.  New command 'save itsets' is similar to
command 'save breakpoints'.

gdb:

2012-08-30  Yao Qi  <yao@codesourcery.com>

	* itset.c: Include "completer.h" and "readline/tilde.h".
	(itset_save_command): New
	(_initialize_itset): Call add_cmd for "save itset" command.

gdb/testsuite:

2012-08-30  Yao Qi  <yao@codesourcery.com>

	* gdb.base/itset.exp (test_itset_save_restore): New.

gdb/doc:

2012-08-30  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (ITSET): Document command 'save itsets'.
---
 gdb/doc/gdb.texinfo              |    8 ++++++
 gdb/itset.c                      |   52 ++++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/itset.exp |   37 +++++++++++++++++++++++++++
 3 files changed, 97 insertions(+), 0 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5573e0d..c9228d3 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5227,6 +5227,14 @@ When a new named itset is defined, which has already been defined before,
 @value{GDBN} will overwrite the existing named itset with the new
 definition.
 
+@kindex save itsets
+@cindex save itsets to a file for future sessions
+@item save itsets
+This command saves all current itset definitions into a file
+@file{@var{filename}} suitable for use in a later debugging session.  To
+read the saved breakpoint definitions, use the @code{source} command
+(@pxref{Command Files}).
+
 @item @ref{maint info itsets}
 
 @end table
diff --git a/gdb/itset.c b/gdb/itset.c
index 6c14e6c..d3f79f7 100644
--- a/gdb/itset.c
+++ b/gdb/itset.c
@@ -28,6 +28,8 @@
 #include "command.h"
 #include <ctype.h>
 #include "gdbcmd.h"
+#include "completer.h"
+#include "readline/tilde.h"
 
 /* Rather than creating/destroying these dynamic itsets when
    necessary, keep global copies around (itsets are refcounted).  */
@@ -2352,6 +2354,49 @@ viewset_command (char *arg, int from_tty)
 }
 
 static void
+itset_save_command (char *filename, int from_tty)
+{
+  const struct named_itset *named_itset;
+  unsigned int count = 0;
+  char *pathname;
+  struct cleanup *cleanup;
+  struct ui_file *fp;
+
+  if (filename == 0 || *filename == 0)
+    error (_("Argument required (file name in which to save)"));
+
+  ALL_NAMED_ITSETS (named_itset)
+  {
+    if (named_itset->number > 0)
+      count++;
+  }
+  if (count == 0)
+    {
+      warning (_("Nothing to save."));
+      return;
+    }
+
+  pathname = tilde_expand (filename);
+  cleanup = make_cleanup (xfree, pathname);
+  fp = gdb_fopen (pathname, "w");
+  if (fp == NULL)
+    error (_("Unable to open file '%s' for saving (%s)"),
+	   filename, safe_strerror (errno));
+  make_cleanup_ui_file_delete (fp);
+
+  ALL_NAMED_ITSETS (named_itset)
+  {
+    if (named_itset->number > 0)
+      fprintf_unfiltered (fp, "defset %s %s\n", itset_name (named_itset->set),
+			  itset_spec (named_itset->set));
+  }
+
+  do_cleanups (cleanup);
+  if (from_tty)
+    printf_filtered (_("Saved to file '%s'.\n"), filename);
+}
+
+static void
 make_internal_itset (struct itset *itset, const char *name)
 {
   struct named_itset *named_itset;
@@ -2418,4 +2463,11 @@ Show the style of resolving adding duplicated itset."), _("\
 This setting chooses how GDB will resolve duplicated itset."),
 			NULL, NULL,
 			&setlist, &showlist);
+
+  c = add_cmd ("itsets", no_class, itset_save_command, _("\
+Save the definitions of named itsets as a script.\n\
+This includes named itset\n\
+Use the 'source' command in another debug session to restore them."),
+	       &save_cmdlist);
+  set_cmd_completer (c, filename_completer);
 }
diff --git a/gdb/testsuite/gdb.base/itset.exp b/gdb/testsuite/gdb.base/itset.exp
index ba72a53..5cbbbee 100644
--- a/gdb/testsuite/gdb.base/itset.exp
+++ b/gdb/testsuite/gdb.base/itset.exp
@@ -109,6 +109,41 @@ proc test_itset_without_running_process { } \
     gdb_test_no_output "undefset -all"
 }}
 
+# Test save and restore itsets.
+
+proc test_itset_save_restore { } {
+with_test_prefix "save restore" {
+    global srcdir
+    global subdir
+    global binfile
+
+    gdb_exit
+    gdb_start
+    gdb_reinitialize_dir $srcdir/$subdir
+    gdb_load ${binfile}
+
+    gdb_test "save itsets itset.txt" ".*Nothing to save.*" "nothing to save"
+
+    gdb_test_no_output "defset itset-c1 c1"
+    gdb_test_no_output "defset itset-c2 c2"
+
+    gdb_test "save itsets itset.txt" "Saved to file \'itset\.txt\'.*"
+
+    gdb_exit
+    gdb_start
+    gdb_reinitialize_dir $srcdir/$subdir
+    gdb_load ${binfile}
+
+    gdb_test "source itset.txt"
+    gdb_test_sequence "info itsets" "info itsets" {
+	"\[\n\r\]1\[ \]+itset-c1\[ \]+c1"
+	"\[\n\r\]2\[ \]+itset-c2\[ \]+c2"
+    }
+
+    remote_file host delete "itset.txt"
+}}
+
+
 # Test various error messages from ITSET related commands.
 
 proc test_itset_check_error { } \
@@ -190,6 +225,8 @@ test_itset_check_error
 
 test_itset_check_syntax
 
+test_itset_save_restore
+
 # Test the behaviour of ITSET in single-thread.
 
 proc test_itset_with_running_single_thread { } \
-- 
1.7.7.6

[PATCH WIP 0/4] GDB enhancement on ITSET

[Yao Qi]

Hi,
This patch set includes various GDB enhancement related to ITSET.  We
used to plan to post them after ITSET is ready, but we want users to
have a chance to have a try these new enhancements (even they have to
apply various patches first), so we post them.

This patch set can't be applied to trunk cleanly, it depends on this
patch set

  [PATCH 0/3] Basic ITSET
  http://sourceware.org/ml/gdb-patches/2012-05/msg01079.html

Patch 4/4 also depends on the 'general notification' patches I posted
last week.

[PATCH 4/4] remote notification on itset

[Yao Qi]

Hi,
When GDB connects to remote target, remote target may have some
predefined or builtin itsets, and GDB has to get them uploaded.  For
example, remote target may have some predefined itset for all its
working cores or broken cores.  Since it is possible that some core
stops working, remote target will notify GDB about this with the
updated itsets.  This requires that remote target is able to send
itsets at anytime during connection, and GDB update its itsets.

This patch is to achieve this feature, and it depends on 'general
notification' patches I submitted last week.

  [RCF 0/4] A general notification in GDB RSP
  http://sourceware.org/ml/gdb-patches/2012-08/msg00703.html

The GDBserver side of this patch is a hack to get some notifications
on itsts to exercise the code in GDB side, so don't have to review the
GDBserver part of this patch.

gdb:

2012-08-30  Yao Qi  <yao@codesourcery.com>

	* itset.c (itset_reply): New struct.
	(remote_notif_parse_itset): New.
	(remote_notif_ack_itset): New.
	(remote_notif_alloc_reply_itset): New.
	(notif_packet_itset): New.
	* remote-notif.c (notifs): Add 'notif_packet_itset'.

gdb/gdbserver:

2012-08-30  Yao Qi  <yao@codesourcery.com>

	* Makefile.in (SFILES): Add itset.c.
	(OBJS): Add itset.o.
	(itset.o): New rule.
	* itset.c: New.
	* linux-low.c (linux_fetch_registers): Add event to queue for
	itset changes.
	* notif.c (notif_packets): Add 'notif_itset'.
	* notif.h (notif_type): New enum 'NOTIF_ITSET'.
	* server.c (handle_target_event): Handle NOTIF_ITSET.

gdbserver itset notif
---
 gdb/gdbserver/Makefile.in |    5 ++-
 gdb/gdbserver/itset.c     |   62 ++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/linux-low.c |   13 +++++++++
 gdb/gdbserver/notif.c     |    2 +
 gdb/gdbserver/notif.h     |    2 +-
 gdb/gdbserver/server.c    |    3 ++
 gdb/itset.c               |   66 +++++++++++++++++++++++++++++++++++++++++++++
 gdb/remote-notif.c        |    3 ++
 8 files changed, 153 insertions(+), 3 deletions(-)
 create mode 100644 gdb/gdbserver/itset.c

diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 68f6b23..9210e56 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -124,7 +124,7 @@ SFILES=	$(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
 	$(srcdir)/mem-break.c $(srcdir)/proc-service.c \
 	$(srcdir)/proc-service.list $(srcdir)/regcache.c \
 	$(srcdir)/remote-utils.c $(srcdir)/server.c $(srcdir)/target.c \
-	$(srcdir)/thread-db.c $(srcdir)/utils.c \
+	$(srcdir)/thread-db.c $(srcdir)/utils.c $(srcdir)/itset.c \
 	$(srcdir)/linux-arm-low.c $(srcdir)/linux-bfin-low.c \
 	$(srcdir)/linux-cris-low.c $(srcdir)/linux-crisv32-low.c \
 	${srcdir}/i386-low.c $(srcdir)/i387-fp.c \
@@ -153,7 +153,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 itset.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 \
@@ -495,6 +495,7 @@ tracepoint.o: tracepoint.c $(server_h) $(ax_h) $(agent_h) $(gdbthread_h)
 utils.o: utils.c $(server_h)
 gdbreplay.o: gdbreplay.c config.h
 dll.o: dll.c $(server_h)
+itset.o: $(notif_h)
 
 signals.o: ../common/signals.c $(server_h) $(signals_def)
 	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< -DGDBSERVER
diff --git a/gdb/gdbserver/itset.c b/gdb/gdbserver/itset.c
new file mode 100644
index 0000000..8c55497
--- /dev/null
+++ b/gdb/gdbserver/itset.c
@@ -0,0 +1,62 @@
+/* Builtin ITSET.  */
+#include "notif.h"
+
+struct builtin_itset
+{
+  const char *name;
+  char *spec;
+  /* The status of field <spec>.  0 means <spec> is unchanged, 1 means <spec>
+     is changed, 2 means reply has been queued. When the reply has been
+     ack'ed by GDB, it is set back to 0.  */
+  int changed;
+};
+
+static struct builtin_itset itsets[] =
+{
+  {"working-cores", NULL, 0},
+  {"broken-cores", NULL, 0},
+};
+
+static void
+notif_reply_itset (struct notif_reply *reply, char *own_buf)
+{
+  int i;
+
+  for (i = 0; i < sizeof (itsets) / sizeof (itsets[0]); i++)
+    if (itsets[i].changed)
+      {
+	sprintf (own_buf, "%s:%s;", itsets[i].name, itsets[i].spec);
+	own_buf += strlen (own_buf);
+
+	itsets[i].changed = 0;
+      }
+}
+
+/* Notification %ITSET to send updated builtin ITSETs.  */
+
+struct notif notif_itset =
+{
+  "vITSET", "ITSET", NOTIF_ITSET, NULL, notif_reply_itset,
+};
+
+extern void itset_update_itsets (void);
+
+/* Mark builtin itsets as changed.  */
+
+void
+itset_update_itsets (void)
+{
+  int i;
+  static int counter = 0;
+
+  for (i = 0; i < sizeof (itsets) / sizeof (itsets[0]); i++)
+    {
+      if (itsets[i].spec != NULL)
+	xfree (itsets[i].spec);
+
+      itsets[i].spec = xmalloc (16);
+      xsnprintf (itsets[i].spec, 16, "c%d", counter++);
+
+      itsets[i].changed = 1;
+    }
+}
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index e9752b0..debf47a 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4341,6 +4341,19 @@ linux_fetch_registers (struct regcache *regcache, int regno)
   int use_regsets;
   int all = 0;
 
+  if (target_is_async_p ())
+    {
+      /* Only for test.  */
+
+      extern struct notif notif_itset;
+      extern void itset_update_itsets (void);
+
+      itset_update_itsets ();
+      gdb_queue_notif_enque (&notif_itset, &notif_queue);
+      async_file_mark ();
+    }
+
+
   if (regno == -1)
     {
       if (the_low_target.fetch_register != NULL)
diff --git a/gdb/gdbserver/notif.c b/gdb/gdbserver/notif.c
index 838f391..a2fadab 100644
--- a/gdb/gdbserver/notif.c
+++ b/gdb/gdbserver/notif.c
@@ -20,10 +20,12 @@
 #include "notif.h"
 
 extern struct notif notif_stop;
+extern struct notif notif_itset;
 
 static struct notif *notif_packets [] =
 {
   &notif_stop,
+  &notif_itset,
   NULL,
 };
 
diff --git a/gdb/gdbserver/notif.h b/gdb/gdbserver/notif.h
index ac62eab..ce16930 100644
--- a/gdb/gdbserver/notif.h
+++ b/gdb/gdbserver/notif.h
@@ -42,7 +42,7 @@ struct vstop_notif
   struct target_waitstatus status;
 };
 
-enum notif_type { NOTIF_STOP };
+enum notif_type { NOTIF_STOP, NOTIF_ITSET };
 
 /* A notification to GDB.  */
 
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 0b756cd..2a4a117 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3412,6 +3412,9 @@ handle_target_event (int err, gdb_client_data client_data)
 		    new_notif = (struct notif_reply *) vstop_notif;
 		  }
 		  break;
+		case NOTIF_ITSET:
+		  new_notif = xmalloc (sizeof (struct notif_reply));
+		  break;
 		default:
 		  error ("Unknown notification type");
 		}
diff --git a/gdb/itset.c b/gdb/itset.c
index 573f332..b610f7f 100644
--- a/gdb/itset.c
+++ b/gdb/itset.c
@@ -2482,3 +2482,69 @@ Use the 'source' command in another debug session to restore them."),
 	       &save_cmdlist);
   set_cmd_completer (c, filename_completer);
 }
+
+/* Remote notification on ITSET.  */
+#include "remote-notif.h"
+#include "remote.h"
+
+struct itset_reply
+{
+  struct notif_reply base;
+};
+
+static void
+remote_notif_parse_itset (struct notif *self, char *buf, void *data)
+{
+  /* parse the name and spec of itset from BUF.  */
+  char *p = NULL;
+  char *next_p = NULL;
+
+  for (p = buf; p != NULL && *p; p = next_p)
+    {
+      next_p = strchr (p, ';');
+
+      if (next_p != NULL)
+	{
+	  char *spec = strchr (p, ':');
+
+	  if (spec == NULL)
+	    error (_("name:spec is expected"));
+
+	  next_p[0] =0;
+	  spec[0] = 0;
+	  spec++;
+	  next_p++;
+
+	  named_itset_create (p, spec);
+	}
+    }
+}
+
+static void
+remote_notif_ack_itset (struct notif *self, char *buf, void *data)
+{
+  struct notif_reply *reply = (struct notif_reply *) data;
+
+  /* acknowledge */
+  putpkt ((char *) self->ack_command);
+}
+
+static struct notif_reply *
+remote_notif_alloc_reply_itset (void)
+{
+  struct notif_reply *reply = xmalloc (sizeof (struct itset_reply));
+
+  reply->dtr = NULL;
+
+  return reply;
+}
+
+struct notif notif_packet_itset =
+{
+  "ITSET", "vITSET",
+  remote_notif_parse_itset,
+  remote_notif_ack_itset,
+  remote_notif_alloc_reply_itset,
+  NULL, NULL,
+};
+
diff --git a/gdb/remote-notif.c b/gdb/remote-notif.c
index 5b1d532..c8c7758 100644
--- a/gdb/remote-notif.c
+++ b/gdb/remote-notif.c
@@ -27,11 +27,14 @@
 
 extern struct remote_state *get_remote_state (void);
 
+extern struct notif notif_packet_itset;
+
 /* Supported notifications.  */
 
 static struct notif *notifs[] =
 {
   &notif_packet_stop,
+  &notif_packet_itset,
 };
 
 /* Parse the BUF for the expected notification NP, and send packet to
-- 
1.7.7.6

Re: [PATCH 1/4] create named itset.

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Thu, 30 Aug 2012 10:40:22 +0800
> 
> +When a new named itset is defined, which has already been defined before,
> +@value{GDBN} will query whether to overwrite the existing named itset
> +or dicard the new definition.
      ^^^^^^
"discard"

> +@item set resolve-duplicate overwrite
> +When a new named itset is defined, which has already been defined before,
> +@value{GDBN} will overwrite the existing named itset with the new
> +definition.

I wonder whether we need 2 almost identical descriptions, instead of 1
description with 2 possible values.

Otherwise OK.

Thanks.

Re: [PATCH 2/4] save itsets into file

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Thu, 30 Aug 2012 10:40:23 +0800
> 
> This patch adds a command 'save itsets' to a file, which can be used
> later by command 'source'.  New command 'save itsets' is similar to
> command 'save breakpoints'.

Thanks.

> +@kindex save itsets
> +@cindex save itsets to a file for future sessions

Please remove this @cindex, it is redundant given the @kindex before
it.

Alternatively, modify the @cindex test, like this:

 @cindex itset, saving to a file

> +@item save itsets
> +This command saves all current itset definitions into a file
> +@file{@var{filename}} suitable for use in a later debugging session.

What "@var{filename}"?  There's no such parameter on the @item line.

> +  pathname = tilde_expand (filename);
> +  cleanup = make_cleanup (xfree, pathname);
> +  fp = gdb_fopen (pathname, "w");

GNU coding standards discourage using "pathname" for file names.

> +  c = add_cmd ("itsets", no_class, itset_save_command, _("\
> +Save the definitions of named itsets as a script.\n\
> +This includes named itset\n\

This is not a complete sentence, and it is unclear what is its value
here.  I would remove it.

Thanks.

Re: [PATCH 3/4] observer on create/modify/delete itsets

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Thu, 30 Aug 2012 10:40:24 +0800
> 
> +@item =itset-created,name=@var{name},spec=@var{spec}
> +@item =itset-modified,name=@var{name},spec=@var{spec}
> +Reports that a named itset is created or modified respecitvely. The name
                                                     ^^^^^^^^^^^^
> +of the itset is @var{name}, and the specificaiton of the itset is
                                       ^^^^^^^^^^^^^
"respectively" and "specification".

> +@deftypefun void itset_created (const char *@var{name}, const char *@var{spec})
> +called after a named itset @var{name} is created, and its specification is @var{spec}.
                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

"whose specification is @var{spec}"

> +@end deftypefun
> +
> +@deftypefun void itset_deleted (const char *@var{name})
> +called after a named itset @var{name} is deleted.  If @var{name} is equal to
> +@code{NULL}, all named itsets are deleted.
> +@end deftypefun
>  
> +@deftypefun void itset_modified (const char *@var{name}, const char *@var{spec})
> +called after a named itset @var{name} is modified, and its specification is @var{spec}.
>  @end deftypefun

Each one of "called" above should be "Called", with a capital C.

OK with those changes.

Re: [PATCH 4/4] remote notification on itset

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> diff --git a/gdb/gdbserver/itset.c b/gdb/gdbserver/itset.c
Yao> new file mode 100644
Yao> index 0000000..8c55497
Yao> --- /dev/null
Yao> +++ b/gdb/gdbserver/itset.c
Yao> @@ -0,0 +1,62 @@
Yao> +/* Builtin ITSET.  */
Yao> +#include "notif.h"

Needs a copyright header.

Tom

Re: [PATCH WIP 0/4] GDB enhancement on ITSET

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> This patch set includes various GDB enhancement related to ITSET.  We
Yao> used to plan to post them after ITSET is ready, but we want users to
Yao> have a chance to have a try these new enhancements (even they have to
Yao> apply various patches first), so we post them.

I appreciate the work you did writing up this series and posting it.
Publishing a git repository would make sharing quite easy though...

I read through this series and it all looked pretty reasonable to me.

Yao>   [PATCH 0/3] Basic ITSET
Yao>   http://sourceware.org/ml/gdb-patches/2012-05/msg01079.html

I guess this is still blocked on some other work?

Tom

Re: [PATCH WIP 0/4] GDB enhancement on ITSET

[Yao Qi]

On 09/13/2012 01:53 AM, Tom Tromey wrote:
> I appreciate the work you did writing up this series and posting it.
> Publishing a git repository would make sharing quite easy though...
>

That is a good idea, and I am setting it up on github.  The tree will be 
ready in two or three days.

> I read through this series and it all looked pretty reasonable to me.

Thanks.

> Yao>   [PATCH 0/3] Basic ITSET
> Yao>http://sourceware.org/ml/gdb-patches/2012-05/msg01079.html
>
> I guess this is still blocked on some other work?

 From the patch/diff's point of view, the answer is no.  Patches 'Basic 
ITSET' should be applied cleanly on mainline, as they only add new 
files, and 'Basic ITSET' is only about creating/deleting itset.  This 
part is quite isolated, and is not blocked by other work.

In order to go to the final destination of ITSET, we still have some 
steps in my mind to go, (Pedro may/must know more)

   1) patches on 'run all-stop mode on top of non-stop target'.
Current patches are broken on 'software-single-step' ports, and we may 
need a per-thread software-single-step breakpoints (IIRC).
   2) patches to replace 'current thread' and 'current inferior' with
'current itset'.
   3) Finalize itset spec/syntax.

 From this point of view, 'ITSET enhancement' and 'Basic ITSET' is 
blocked by these three steps.

I am thinking how to parallelize these steps, because looks the first 
two steps are complicated, and the latter steps are relatively easier.
   Can we define a minimum set of ITSET spec/syntax first, in which we 
are sure that the spec/syntax will be unchanged?  For example, the 
minimum spec can be only for cores, like c1, c2, and c3.  Then, 'Basic 
ITSET' and 'ITSET enhancement' patches can go in.  In parallel, we can 
continue three steps above.  At the same time, we (or I) can start to 
investigate using ITSET in target side (in gdbserver).

In short, we can start from a minimum set of ITSET sepc and don't 
associate it with execution control, so that the starting point is 
relatively easy, and we can extend ITSET work gradually.  Thoughts?

-- 
Yao