[RFC] MI notification on register changes

Yao Qi yao@codesourcery.com
Tue Oct 23 07:24:00 GMT 2012


Hi,
User can type command in console 'set $reg = foo' to modify register.
The modification may not really happen in registers, because register
is already saved in frame, and modification really goes to memory (frame).
The new MI notification this patch adds is about 'register change' from
user's point of view, instead of from machine's point of view.

Usually, MI frontend has a 'register view' to show the contents of
registers.  When users modify registers on a certain frame, this
notification is useful to tell MI frontend to refresh its 'register
view'.

This patch also extends the usage of field 'regnum' of 'struct value'.
Originally, this field is used to store the register number if content
is from register.  With this patch, this field also store the register
number if it "presents" a register.  For example, on non-innermost
frame, the value "reg" may be from memory instead of register.

   set $reg = foo

In order to emit 'register changed' notification, we have to know
whether a value presents a register or not.  I am not very sure about
the change here, so comments or advices are appreciated.

gdb/doc:

2012-10-23  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (GDB/MI Async Records): Add '=register-changed'.
	* observer.texi (GDB Observers): Add observer
	'register_changed'.
gdb:

2012-10-23  Yao Qi  <yao@codesourcery.com>

	* mi/mi-cmd-var.c (mi_cmd_var_assign): Set
	mi_suppress_notification.register_changed to 1 and restore it
	on exit.
	* mi/mi-interp.c: Include "regcache.h".
	Declare mi_register_changed.
	(mi_interpreter_init): Install mi_register_changed to observer
	'register_changed'.
	(mi_register_changed): New.
	* mi/mi-main.h (struct mi_suppress_notification)
	<register_changed>: New.

	* value.c (struct value) <regnum>: Update comments.
	* eval.c (evaluate_subexp_standard):

	* valops.c (value_assign): Get frame info of 'toval' earlier.
	(value_assign): Call observer_notify_register_changed if
	'toval' is related to a register.

	* NEWS: Mention it.

gdb/testsuite/

2012-10-23  Yao Qi  <yao@codesourcery.com>

	* gdb.mi/mi-reg-changed.c: New.
	* gdb.mi/mi-reg-changed.exp: New.
	* gdb.mi/mi-cli.exp: Update tests for new MI notification.
---
 gdb/NEWS                                |    2 +
 gdb/doc/gdb.texinfo                     |    5 +
 gdb/doc/observer.texi                   |    5 +
 gdb/eval.c                              |   12 +++-
 gdb/mi/mi-cmd-var.c                     |   12 ++-
 gdb/mi/mi-interp.c                      |   49 ++++++++++++
 gdb/mi/mi-main.h                        |    2 +
 gdb/testsuite/gdb.mi/mi-cli.exp         |    6 +-
 gdb/testsuite/gdb.mi/mi-reg-changed.c   |   30 +++++++
 gdb/testsuite/gdb.mi/mi-reg-changed.exp |  129 +++++++++++++++++++++++++++++++
 gdb/valops.c                            |   12 ++-
 gdb/value.c                             |    4 +-
 12 files changed, 254 insertions(+), 14 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-reg-changed.c
 create mode 100644 gdb/testsuite/gdb.mi/mi-reg-changed.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 485c21b..4660f99 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -68,6 +68,8 @@ py [command]
      "=tracepoint-downloaded".
   ** Passcount of tracepoint changes are now notified using new async record
      "=tracepoint-modified".
+  ** Register changes are now notified using new async record
+     "=register-changed".
 
 *** Changes in GDB 7.5
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 64cda47..b4b5c5f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -27714,6 +27714,11 @@ 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 =register-changed,name=@var{name},value=@var{value},group-id=@var{id},frame=@var{frame}
+Reports that register @var{name} on frame @var{frame} was changed to
+@var{value} in an inferior.  The @var{id} is the identifier of the
+thread group corresponding to the affected inferior.
 @end table
 
 @node GDB/MI Frame Information
diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index e2033a6..bb16307 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -98,6 +98,11 @@ inferior has stopped.
 The target's register contents have changed.
 @end deftypefun
 
+@deftypefun void register_changed (int @var{reg_num}, struct frame_info *@var{frame}, ptid_t @var{inferior_ptid}, const gdb_byte *@var{buf})
+The content of register @var{reg_num} in frame @var{frame} inferior
+@var{inferior_ptid} has changed to the new content pointed by @var{buf}.
+@end deftypefun
+
 @deftypefun void executable_changed (void)
 The executable being debugged by GDB has changed: The user decided
 to debug a different program, or the program he was debugging has
diff --git a/gdb/eval.c b/gdb/eval.c
index 26e0cc8..b099499 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -939,7 +939,17 @@ evaluate_subexp_standard (struct type *expect_type,
 	if (val == NULL)
 	  error (_("Value of register %s not available."), name);
 	else
-	  return val;
+	  {
+	    /* Store the register number if it is not a user register.
+	       We don't have an easy way to get the number of cooked/frame
+	       register for a given user register, because on some arch,
+	       a 64-bit user register is composed by two 32-bit frame
+	       registers.  */
+	    if (regno < (gdbarch_num_regs (exp->gdbarch)
+			 + gdbarch_num_pseudo_regs (exp->gdbarch)))
+	      VALUE_REGNUM (val) = regno;
+	    return val;
+	  }
       }
     case OP_BOOL:
       (*pos) += 2;
diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index dc47bc1..41cd889 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -618,6 +618,7 @@ mi_cmd_var_assign (char *command, char **argv, int argc)
   struct varobj *var;
   char *expression, *val;
   struct cleanup *cleanup;
+  int *p;
 
   if (argc != 2)
     error (_("-var-assign: Usage: NAME EXPRESSION."));
@@ -630,11 +631,14 @@ mi_cmd_var_assign (char *command, char **argv, int argc)
 
   expression = xstrdup (argv[1]);
 
-  /* MI command '-var-assign' may write memory, so suppress memory
-     changed notification if it does.  */
-  cleanup
-    = make_cleanup_restore_integer (&mi_suppress_notification.memory);
+  /* MI command '-var-assign' may write memory or register, so suppress
+     memory or register changed notification if it does.  */
+  p = &mi_suppress_notification.memory;
+  make_cleanup_restore_integer (p);
   mi_suppress_notification.memory = 1;
+  p = &mi_suppress_notification.register_changed;
+  cleanup = make_cleanup_restore_integer (p);
+  mi_suppress_notification.register_changed = 1;
 
   if (!varobj_set_value (var, expression))
     error (_("-var-assign: Could not assign "
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index ec14f03..86b0e5b 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -36,6 +36,7 @@
 #include "solist.h"
 #include "gdb.h"
 #include "objfiles.h"
+#include "regcache.h"
 
 /* These are the interpreter setup, etc. functions for the MI
    interpreter.  */
@@ -81,6 +82,8 @@ static void mi_tracepoint_downloaded (struct bp_location *loc);
 static void mi_command_param_changed (const char *param, const char *value);
 static void mi_memory_changed (struct inferior *inf, CORE_ADDR memaddr,
 			       ssize_t len, const bfd_byte *myaddr);
+static void mi_register_changed (int regnum, struct frame_info *frame,
+				 ptid_t ptid, const gdb_byte *buf);
 
 static int report_initial_inferior (struct inferior *inf, void *closure);
 
@@ -146,6 +149,7 @@ mi_interpreter_init (struct interp *interp, int top_level)
       observer_attach_tracepoint_downloaded (mi_tracepoint_downloaded);
       observer_attach_command_param_changed (mi_command_param_changed);
       observer_attach_memory_changed (mi_memory_changed);
+      observer_attach_register_changed (mi_register_changed);
 
       /* The initial inferior is created before this function is
 	 called, so we need to report it explicitly.  Use iteration in
@@ -924,6 +928,51 @@ mi_memory_changed (struct inferior *inferior, CORE_ADDR memaddr,
   gdb_flush (mi->event_channel);
 }
 
+/* Emit notification about the register change.  */
+
+static void
+mi_register_changed (int regnum, struct frame_info *frame, ptid_t ptid,
+		     const gdb_byte *buf)
+{
+  if (!mi_suppress_notification.register_changed)
+    {
+      struct mi_interp *mi = top_level_interpreter_data ();
+      struct ui_out *mi_uiout
+	= interp_ui_out (top_level_interpreter ());
+      struct gdbarch *gdbarch = get_frame_arch (frame);
+      int regsize = register_size (gdbarch, regnum);
+      ULONGEST val = extract_unsigned_integer (buf, regsize,
+					       gdbarch_byte_order (gdbarch));
+      struct thread_info *ti = find_thread_ptid (ptid);
+      struct inferior *inf = find_inferior_pid (ptid_get_pid (ptid));
+      struct ui_out *saved_uiout;
+
+      gdb_assert (ti);
+      gdb_assert (inf);
+
+      target_terminal_ours ();
+
+      fprintf_unfiltered (mi->event_channel, "register-changed");
+
+      ui_out_redirect (mi_uiout, mi->event_channel);
+
+      ui_out_field_string (mi_uiout, "name",
+			   gdbarch_register_name (gdbarch, regnum));
+      ui_out_field_fmt (mi_uiout, "value", "0x%s", phex_nz (val, regsize));
+      ui_out_field_int (mi_uiout, "group-id", inf->num);
+      ui_out_field_int (mi_uiout, "thread-id", ti->num);
+
+      saved_uiout = current_uiout;
+      current_uiout = mi_uiout;
+      print_stack_frame (frame, 0, SRC_AND_LOC);
+      current_uiout = saved_uiout;
+
+      ui_out_redirect (mi_uiout, NULL);
+
+      gdb_flush (mi->event_channel);
+    }
+}
+
 static int
 report_initial_inferior (struct inferior *inf, void *closure)
 {
diff --git a/gdb/mi/mi-main.h b/gdb/mi/mi-main.h
index a815fbe..0f4b064 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;
+  /* Register changed notification suppressed?  */
+  int register_changed;
 };
 extern struct mi_suppress_notification mi_suppress_notification;
 
diff --git a/gdb/testsuite/gdb.mi/mi-cli.exp b/gdb/testsuite/gdb.mi/mi-cli.exp
index bab2373..ad0cdb8 100644
--- a/gdb/testsuite/gdb.mi/mi-cli.exp
+++ b/gdb/testsuite/gdb.mi/mi-cli.exp
@@ -188,12 +188,8 @@ mi_gdb_test "-interpreter-exec console \"help set args\"" \
   {\~"Set argument list to give program being debugged when it is started\.\\nFollow this command with any number of args, to be passed to the program\.".*\^done} \
   "-interpreter-exec console \"help set args\""
 
-# NOTE: cagney/2003-02-03: Not yet.
-# mi_gdb_test "-interpreter-exec console \"set \$pc=0x0\"" \
-#   {.*=target-changed.*\^done} \
-#   "-interpreter-exec console \"set \$pc=0x0\""
 mi_gdb_test "888-interpreter-exec console \"set \$pc=0x0\"" \
-  {888\^done} \
+  {.*=register-changed,name=\".*\",value=\"0x0\",group-id=\".*\",frame=.*\^done} \
   "-interpreter-exec console \"set \$pc=0x0\""
 
 #mi_gdb_test "-interpreter-exec console \"\"" \
diff --git a/gdb/testsuite/gdb.mi/mi-reg-changed.c b/gdb/testsuite/gdb.mi/mi-reg-changed.c
new file mode 100644
index 0000000..3d6950b
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-reg-changed.c
@@ -0,0 +1,30 @@
+/* This test is part of GDB, the GNU debugger.
+
+   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/>.  */
+
+static int
+middle (int x)
+{
+  x = 2 * x;
+  x++;
+  return x;
+}
+
+int
+main (int argc, char **argv)
+{
+  return middle (argc);
+}
diff --git a/gdb/testsuite/gdb.mi/mi-reg-changed.exp b/gdb/testsuite/gdb.mi/mi-reg-changed.exp
new file mode 100644
index 0000000..bd4037a
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-reg-changed.exp
@@ -0,0 +1,129 @@
+# 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 .c
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+     untested mi-reg-changed.exp
+     return -1
+}
+
+proc test_register_change { } { with_test_prefix "simple" {
+    if [mi_gdb_start] {
+	return
+    }
+    mi_run_to_main
+
+    global hex
+    global decimal
+    global expect_out
+
+    set reg ""
+    # Extract the name of a register.
+    mi_gdb_test "-data-list-register-names" \
+	"\\\^done,register-names=\\\[\"(\[a-zA-Z0-9\]+)\".*\\\].*" \
+	"-data-list-register-names"
+    set reg $expect_out(3,string)
+
+    if { $reg == "" } {
+	fail "get the register name"
+	return
+    }
+
+    set reg_val ""
+    if ![mi_gdb_test "p/x \$${reg}" ".*($hex).*\\^done" "print register ${reg}"] {
+	set reg_val $expect_out(3,string)
+    }
+    if { $reg_val == "" } {
+	fail "get the content of register ${reg}"
+	return
+    }
+
+    mi_gdb_test "set \$${reg} = ${reg_val}" \
+	".*=register-changed,name=\"${reg}\",value=\"${reg_val}\",group-id=\"${decimal}\",thread-id=\"${decimal}\",frame={addr=.*,func=\"main\",.*}.*\\^done" \
+	"change reg ${reg}"
+
+    mi_gdb_test "-var-create fp_var * \$${reg}" \
+	"\\^done,name=\"fp_var\",numchild=\"0\",value=\".*\",type=\".*\",has_more=\"0\"" \
+	"create objvar for reg ${reg}"
+    # GDB should not emit MI notification for register change requested
+    # from MI frontend.
+    mi_gdb_test "-var-assign fp_var ${reg_val}" \
+	"\\^done,value=\".*\"" "change reg ${reg} thru. varobj"
+
+    mi_gdb_exit
+}}
+
+
+proc test_register_on_frame { } { with_test_prefix "frame" {
+    if [mi_gdb_start] {
+	return
+    }
+    mi_run_to_main
+
+    global decimal
+    global hex
+    global expect_out
+
+    set reg ""
+
+    mi_gdb_test "-data-list-register-names" \
+	"\\\^done,register-names=\\\[\"(\[a-zA-Z0-9\]+)\".*\\\].*" \
+	"-data-list-register-names"
+    set reg $expect_out(3,string)
+
+    if { $reg == "" } {
+	fail "get the register name"
+	return
+    }
+
+    mi_gdb_test "-break-insert -t middle" \
+	"\\^done,bkpt=\{number=\"2\",type=\"breakpoint\",disp=\"del\".*" \
+	"-break-insert -t middle"
+    mi_execute_to "exec-continue" "breakpoint-hit" "middle" ".*" ".*" \
+	".*" { "" "disp=\"del\"" } "-exec-continue to middle"
+
+    set reg_val ""
+
+    if ![mi_gdb_test "p/x \$${reg}" ".*($hex).*\\^done" "print register ${reg}"] {
+	    set reg_val $expect_out(3,string)
+	}
+
+    mi_gdb_test "set \$${reg} = ${reg_val}" \
+	".*=register-changed,name=\"${reg}\",value=\"${reg_val}\",group-id=\"${decimal}\",thread-id=\"${decimal}\",frame={addr=.*,func=\"middle\",.*}.*\\^done" \
+	"change reg ${reg}"
+
+    mi_gdb_test "up" ".*" "up"
+
+    set reg_val ""
+    if ![mi_gdb_test "p/x \$${reg}" ".*($hex).*\\^done" "print register ${reg}"] {
+	set reg_val $expect_out(3,string)
+    }
+
+    mi_gdb_test "set \$${reg} = ${reg_val}" \
+	".*=register-changed,name=\"${reg}\",value=\"${reg_val}\",group-id=\"${decimal}\",thread-id=\"${decimal}\",frame={addr=.*,func=\"main\",.*}.*\\^done" \
+	    "change reg ${reg} in upper frame"
+
+    mi_gdb_exit
+}}
+
+test_register_change
+
+test_register_on_frame
+
+return 0
diff --git a/gdb/valops.c b/gdb/valops.c
index 502fb0d..256d29d 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1217,6 +1217,7 @@ value_assign (struct value *toval, struct value *fromval)
   struct type *type;
   struct value *val;
   struct frame_id old_frame;
+  struct frame_info *frame;
 
   if (!deprecated_value_modifiable (toval))
     error (_("Left operand of assignment is not a modifiable lvalue."));
@@ -1241,6 +1242,9 @@ value_assign (struct value *toval, struct value *fromval)
      and then restore the new frame afterwards.  */
   old_frame = get_frame_id (deprecated_safe_get_selected_frame ());
 
+  /* Figure out which frame this is in currently.  */
+  frame = frame_find_by_id (VALUE_FRAME_ID (toval));
+
   switch (VALUE_LVAL (toval))
     {
     case lval_internalvar:
@@ -1305,12 +1309,9 @@ value_assign (struct value *toval, struct value *fromval)
 
     case lval_register:
       {
-	struct frame_info *frame;
 	struct gdbarch *gdbarch;
 	int value_reg;
 
-	/* Figure out which frame this is in currently.  */
-	frame = frame_find_by_id (VALUE_FRAME_ID (toval));
 	value_reg = VALUE_REGNUM (toval);
 
 	if (!frame)
@@ -1462,6 +1463,11 @@ value_assign (struct value *toval, struct value *fromval)
       set_value_pointed_to_offset (val, value_pointed_to_offset (fromval));
     }
 
+  if (VALUE_REGNUM (toval) >= 0)
+    observer_notify_register_changed (VALUE_REGNUM (toval), frame,
+				      inferior_ptid,
+				      value_contents (val));
+
   return val;
 }
 
diff --git a/gdb/value.c b/gdb/value.c
index 3feb1ca..7720cd1 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -318,7 +318,9 @@ struct value
      taken off this list.  */
   struct value *next;
 
-  /* Register number if the value is from a register.  */
+  /* Register number if the value presents a register.  The contents may
+     be from a register or memory, which is determined by field
+     'LVAL'.  */
   short regnum;
 
   /* Actual contents of the value.  Target byte-order.  NULL or not
-- 
1.7.7.6



More information about the Gdb-patches mailing list