Re: [PATCH 1/2] new memory-changed MI notification.

Yao Qi
2012-10-09T07:44:05Z

On 09/29/2012 01:17 AM, Tom Tromey wrote:
> Usually I think it would be preferable to have a flag correspond to a
> notification and not a command; but this would not work so well if a
> command needed to suppress two different messages. (Though if that
> happens then maybe we should have a slightly different approach based on
> bitmasks.)
>

I agree with you that one flag should correspond to a notification. I
revised my patch a little bit to get rid of suppression flag
'var_assign'.

> Yao> +invalidate_bp_value_on_memory_change (struct inferior *inferior, CORE_ADDR addr,
>
> This line is too long now.

The length of this line is 80, so my editor doesn't remind me to
shorten it. Anyway, this line is shortened in the new patch. What do
you think?

--
Yao

gdb:

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

* breakpoint.c (invalidate_bp_value_on_memory_change): Add one
more parameter 'inferior'.
* corefile.c (write_memory_with_notification): Caller update.

* mi/mi-cmd-var.c: Include "mi-main.h".
(mi_cmd_var_assign): Set mi_suppress_notification.data_write_memory
to 1 and restore it later.
* mi/mi-cmds.c (mi_cmd mi_cmds): Update for "data-write-memory"
and "data-write-memory-bytes.
* mi/mi-interp.c: Include objfiles.h.
(mi_interpreter_init): Call observer_attach_memory_changed.
(mi_memory_changed): New.
* mi/mi-main.h (struct mi_suppress_notification) <memory>:
New field.

* NEWS: Mention new MI notification "memory-changed".

gdb/doc:

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

* observer.texi (GDB Observers): Update observer
'memory_changed'.

* gdb.texinfo (GDB/MI Async Records): Document for
"memory-changed" notification.

gdb/testsuite:

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

* gdb.mi/mi-memory-changed.exp: New.
---
gdb/NEWS | 2 +
gdb/breakpoint.c | 3 +-
gdb/corefile.c | 2 +-
gdb/doc/gdb.texinfo | 7 +++
gdb/doc/observer.texi | 4 +-
gdb/mi/mi-cmd-var.c | 10 ++++
gdb/mi/mi-cmds.c | 6 ++-
gdb/mi/mi-interp.c | 45 +++++++++++++++
gdb/mi/mi-main.h | 2 +
gdb/testsuite/gdb.mi/mi-memory-changed.exp | 81 ++++++++++++++++++++++++++++
10 files changed, 156 insertions(+), 6 deletions(-)
create mode 100644 gdb/testsuite/gdb.mi/mi-memory-changed.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index abd0932..798f050 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -57,6 +57,8 @@ py [command]
using new async records "=tsv-created" and "=tsv-deleted".
** The start and stop of process record are now notified using new
async record "=record-started" and "=record-stopped".
+ ** Memory changes are now notified using new async record
+ "=memory-changed".

*** Changes in GDB 7.5

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 4e7c145..8eeeacf 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14718,7 +14718,8 @@ show_breakpoint_cmd (char *args, int from_tty)
GDB itself. */

static void
-invalidate_bp_value_on_memory_change (CORE_ADDR addr, ssize_t len,
+invalidate_bp_value_on_memory_change (struct inferior *inferior,
+ CORE_ADDR addr, ssize_t len,
const bfd_byte *data)
{
struct breakpoint *bp;
diff --git a/gdb/corefile.c b/gdb/corefile.c
index ac8eff5..a2ed24f 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -369,7 +369,7 @@ write_memory_with_notification (CORE_ADDR memaddr, const bfd_byte *myaddr,
ssize_t len)
{
write_memory (memaddr, myaddr, len);
- observer_notify_memory_changed (memaddr, len, myaddr);
+ observer_notify_memory_changed (current_inferior (), memaddr, len, myaddr);
}

/* Store VALUE at ADDR in the inferior as a LEN-byte unsigned
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5fcbada..19d0868 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -27670,6 +27670,13 @@ changed to @var{value}. In the multi-word @code{set} command,
the @var{param} is the whole parameter list to @code{set} command.
For example, In command @code{set check type on}, @var{param}
is @code{check type} and @var{value} is @code{on}.
+
+@item =memory-changed,thread-group=@var{id},addr=@var{addr},len=@var{len}[,type="code"]
+Reports that bytes from @var{addr} to @var{data} + @var{len} were
+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.
@end table

@node GDB/MI Frame Information
diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index 3fdc90a..106475b 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -230,9 +230,9 @@ The inferior @var{inf} has been removed from the list of inferiors.
This method is called immediately before freeing @var{inf}.
@end deftypefun

-@deftypefun void memory_changed (CORE_ADDR @var{addr}, ssize_t @var{len}, const bfd_byte *@var{data})
+@deftypefun void memory_changed (struct inferior *@var{inferior}, CORE_ADDR @var{addr}, ssize_t @var{len}, const bfd_byte *@var{data})
Bytes from @var{data} to @var{data} + @var{len} have been written
-to the current inferior at @var{addr}.
+to the @var{inferior} at @var{addr}.
@end deftypefun

@deftypefun void before_prompt (const char *@var{current_prompt})
diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index 5d7081f..dc47bc1 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -21,6 +21,7 @@

#include "defs.h"
#include "mi-cmds.h"
+#include "mi-main.h"
#include "ui-out.h"
#include "mi-out.h"
#include "varobj.h"
@@ -616,6 +617,7 @@ mi_cmd_var_assign (char *command, char **argv, int argc)
struct ui_out *uiout = current_uiout;
struct varobj *var;
char *expression, *val;
+ struct cleanup *cleanup;

if (argc != 2)
error (_("-var-assign: Usage: NAME EXPRESSION."));
@@ -628,6 +630,12 @@ 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_suppress_notification.memory = 1;
+
if (!varobj_set_value (var, expression))
error (_("-var-assign: Could not assign "
"expression to variable object"));
@@ -635,6 +643,8 @@ mi_cmd_var_assign (char *command, char **argv, int argc)
val = varobj_get_value (var);
ui_out_field_string (uiout, "value", val);
xfree (val);
+
+ do_cleanups (cleanup);
}

/* Type used for parameters passing to mi_cmd_var_update_iter. */
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 2ed1905..572625f 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -75,8 +75,10 @@ static struct mi_cmd mi_cmds[] =
DEF_MI_CMD_MI ("data-list-register-values", mi_cmd_data_list_register_values),
DEF_MI_CMD_MI ("data-read-memory", mi_cmd_data_read_memory),
DEF_MI_CMD_MI ("data-read-memory-bytes", mi_cmd_data_read_memory_bytes),
- DEF_MI_CMD_MI ("data-write-memory", mi_cmd_data_write_memory),
- DEF_MI_CMD_MI ("data-write-memory-bytes", mi_cmd_data_write_memory_bytes),
+ DEF_MI_CMD_MI_1 ("data-write-memory", mi_cmd_data_write_memory,
+ &mi_suppress_notification.memory),
+ DEF_MI_CMD_MI_1 ("data-write-memory-bytes", mi_cmd_data_write_memory_bytes,
+ &mi_suppress_notification.memory),
DEF_MI_CMD_MI ("data-write-register-values",
mi_cmd_data_write_register_values),
DEF_MI_CMD_MI ("enable-timings", mi_cmd_enable_timings),
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index d383958..d3c3d81 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -35,6 +35,7 @@
#include "gdbthread.h"
#include "solist.h"
#include "gdb.h"
+#include "objfiles.h"

/* These are the interpreter setup, etc. functions for the MI
interpreter. */
@@ -76,6 +77,8 @@ static void mi_breakpoint_created (struct breakpoint *b);
static void mi_breakpoint_deleted (struct breakpoint *b);
static void mi_breakpoint_modified (struct breakpoint *b);
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 int report_initial_inferior (struct inferior *inf, void *closure);

@@ -138,6 +141,7 @@ mi_interpreter_init (struct interp *interp, int top_level)
observer_attach_breakpoint_deleted (mi_breakpoint_deleted);
observer_attach_breakpoint_modified (mi_breakpoint_modified);
observer_attach_command_param_changed (mi_command_param_changed);
+ observer_attach_memory_changed (mi_memory_changed);

/* The initial inferior is created before this function is
called, so we need to report it explicitly. Use iteration in
@@ -840,6 +844,47 @@ mi_command_param_changed (const char *param, const char *value)
gdb_flush (mi->event_channel);
}

+/* Emit notification about the target memory change. */
+
+static void
+mi_memory_changed (struct inferior *inferior, CORE_ADDR memaddr,
+ ssize_t len, const bfd_byte *myaddr)
+{
+ struct mi_interp *mi = top_level_interpreter_data ();
+ struct ui_out *mi_uiout = interp_ui_out (top_level_interpreter ());
+ struct obj_section *sec;
+
+ if (mi_suppress_notification.memory)
+ return;
+
+ target_terminal_ours ();
+
+ fprintf_unfiltered (mi->event_channel,
+ "memory-changed");
+
+ ui_out_redirect (mi_uiout, mi->event_channel);
+
+ ui_out_field_fmt (mi_uiout, "thread-group", "i%d", inferior->num);
+ ui_out_field_core_addr (mi_uiout, "addr", target_gdbarch, memaddr);
+ ui_out_field_fmt (mi_uiout, "len", "0x%zx", len);
+
+ /* Append 'type=code' into notification if MEMADDR falls in the range of
+ sections contain code. */
+ sec = find_pc_section (memaddr);
+ if (sec != NULL && sec->objfile != NULL)
+ {
+ flagword flags = bfd_get_section_flags (sec->objfile->obfd,
+ sec->the_bfd_section);
+
+ if (flags & SEC_CODE)
+ ui_out_field_string (mi_uiout, "type", "code");
+ }
+
+ 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 aad7eeb..a815fbe 100644
--- a/gdb/mi/mi-main.h
+++ b/gdb/mi/mi-main.h
@@ -41,6 +41,8 @@ struct mi_suppress_notification
int cmd_param_changed;
/* Traceframe changed notification suppressed? */
int traceframe;
+ /* Memory changed notification suppressed? */
+ int memory;
};
extern struct mi_suppress_notification mi_suppress_notification;

diff --git a/gdb/testsuite/gdb.mi/mi-memory-changed.exp b/gdb/testsuite/gdb.mi/mi-memory-changed.exp
new file mode 100644
index 0000000..f290992
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-memory-changed.exp
@@ -0,0 +1,81 @@
+# 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/>.
+
+standard_testfile basics.c
+if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
+ executable {debug nowarnings}] != "" } {
+ untested mi-record-changed.exp
+ return -1
+}
+
+load_lib mi-support.exp
+
+if [mi_gdb_start] {
+ return
+}
+mi_gdb_reinitialize_dir $srcdir/$subdir
+mi_gdb_load ${binfile}
+
+mi_gdb_test "-break-insert -t ${srcfile}:[gdb_get_line_number "C = A + B"]" \
+ "\\^done,bkpt=\{number=\"1\".*" \
+ "insert breakpoint"
+mi_run_cmd
+mi_expect_stop "breakpoint-hit" "callee4" "" ".*" ".*" {"" "disp=\"del\""} \
+ "continue to callee4"
+
+mi_gdb_test "set var C = 4" \
+ ".*=memory-changed,thread-group=\"i${decimal}\".addr=\"${hex}\",len=\"${hex}\".*\\^done" \
+ "set var C = 4"
+
+# Write memory through MI commands shouldn't trigger MI notification.
+mi_gdb_test "-var-create var_c * C" \
+ "\\^done,name=\"var_c\",numchild=\"0\",value=\"4\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \
+ "create objvar for C"
+
+mi_gdb_test "-var-assign var_c 5" \
+ "-var-assign var_c 5\r\n\\^done,value=\"5\"" \
+ "change C thru. varobj"
+
+mi_gdb_test "-data-write-memory-bytes &C \"00\"" \
+ {\^done} \
+ "change C thru. -data-write-memory-bytes"
+
+# Modify code section also triggers MI notification.
+
+# Get the instruction content of function main and its address.
+set main_addr ""
+set main_insn ""
+set test "get address of main"
+send_gdb "x/x main\n"
+gdb_expect {
+ -re ".*(${hex}) :.*(${hex}).*$mi_gdb_prompt$" {
+ set main_addr $expect_out(1,string)
+ set main_insn $expect_out(2,string)
+ pass $test
+ }
+ -re ".*$mi_gdb_prompt$" {
+ fail $test
+ return
+ }
+ timeout {
+ fail "$test (timeout)"
+ return
+ }
+}
+
+mi_gdb_test "set var *(unsigned int *) ${main_addr} = ${main_insn}" \
+ ".*=memory-changed,thread-group=\"i${decimal}\".addr=\"${main_addr}\",len=\"0x4\",type=\"code\".*\\^done"
+mi_gdb_exit
+return 0
--
1.7.7.6