[v6] Events when inferior is modified
Commit Message
Hi,
[Resending because the previous discussion petered out without achieving
signoff. The patch is identical to before except where line numbers have
changed as the mainline has moved on.]
This patch was originally submitted a few months ago; at that time it
was looked at by Phil Muldoon and others, and there were no outstanding
comments. I've updated it to take account of mainline changes, but
otherwise it is unchanged.
"This patch adds new observers, and corresponding Python events, for
various actions on an inferior: calling a function (by hand),
modifying registers or modifying memory.
A use case for these events is that by flagging these occurrences, it
can be noticed that the execution of the program is potentially
'dirty'. (Hence why the notification doesn't carry information about
what was modified; for my purposes it's enough to know that /a/
change has happened.)"
Nick
gdb/ChangeLog
2014-05-15 Nick Bull <nicholaspbull@gmail.com>
* NEWS: Mention new Python events.
* Makefile.in: add py-infcallevent.c / py-infcallevent.o.
* doc/observer.texi (inferior_call_pre, inferior_call_post)
(memory_changed, register_changed): New observers.
* infcall.c (call_function_by_hand): Notify observer before and
after inferior call.
* python/py-event.h (inferior_call_kind): New enum.
(emit_inferior_call_event): New prototype.
(emit_register_changed_event): New prototype.
(emit_memory_changed_event): New prototype.
* python/py-events.h (events_object): New registries
inferior_call, memory_changed and register_changed.
* python/py-evts.c (gdbpy_initialize_py_events): Add the
inferior_call, memory_changed and register_changed registries.
* python/py-infcallevent.c: New.
* python/py-inferior.c (python_on_inferior_call_pre)
(python_on_inferior_call_post, python_on_register_change)
(python_on_memory_change): New functions.
(gdbpy_initialize_inferior): Attach python handler to new
observers.
* python/python-internal.h:
(gdbpy_initialize_inferior_call_pre_event)
(gdbpy_initialize_inferior_call_post_event)
(gdbpy_initialize_register_changed_event)
(gdbpy_initialize_memory_changed_event): New prototypes.
* python/python.c (_initialize_python): Initialize new events.
* valops.c (value_assign): Notify register_changed observer.
gdb/doc/ChangeLog
2014-05-15 Nick Bull <nicholaspbull@gmail.com>
* python.texi (Events In Python): Document new events
InferiorCallPreEvent, InferiorCallPostEvent, MemoryChangedEvent
and RegisterChangedEvent.
gdb/testsuite/ChangeLog
2014-05-15 Nick Bull <nicholaspbull@gmail.com>
* gdb.python/py-events.py (inferior_fn_handler): New.
(register_changed_handler, memory_changed_handler): New.
(test_events.invoke): Register new handlers.
* gdb.python/py-events.exp: Add tests for inferior call,
memory_changed and register_changed events.
break;
Comments
This patch would be useful for rr too!
On 17/09/14 18:32, Nick Bull wrote:
> Hi,
>
> [Resending because the previous discussion petered out without achieving signoff. The patch is identical to before except where line numbers have changed as the mainline has moved on.]
>
> This patch was originally submitted a few months ago; at that time it
> was looked at by Phil Muldoon and others, and there were no outstanding
> comments. I've updated it to take account of mainline changes, but
> otherwise it is unchanged.
>
> "This patch adds new observers, and corresponding Python events, for
> various actions on an inferior: calling a function (by hand),
> modifying registers or modifying memory.
>
> A use case for these events is that by flagging these occurrences, it
> can be noticed that the execution of the program is potentially
> 'dirty'. (Hence why the notification doesn't carry information about
> what was modified; for my purposes it's enough to know that /a/
> change has happened.)"
>
> Nick
Can a global maintainer please look at this and give a sign-off? Just a
straight yes or no will do. I have already reviewed it and asked
several times for a sign-off. Nick has been very patient, and I think
at this point it is becoming somewhat annoying to pester for someone
to let Python patches through.
Cheers,
Phil
On 25/09/14 11:14, Phil Muldoon wrote:
> On 17/09/14 18:32, Nick Bull wrote:
>> Hi,
>>
>> [Resending because the previous discussion petered out without achieving signoff. The patch is identical to before except where line numbers have changed as the mainline has moved on.]
>>
>> This patch was originally submitted a few months ago; at that time it
>> was looked at by Phil Muldoon and others, and there were no outstanding
>> comments. I've updated it to take account of mainline changes, but
>> otherwise it is unchanged.
>>
>> "This patch adds new observers, and corresponding Python events, for
>> various actions on an inferior: calling a function (by hand),
>> modifying registers or modifying memory.
>>
>> A use case for these events is that by flagging these occurrences, it
>> can be noticed that the execution of the program is potentially
>> 'dirty'. (Hence why the notification doesn't carry information about
>> what was modified; for my purposes it's enough to know that /a/
>> change has happened.)"
>>
>> Nick
>
> Can a global maintainer please look at this and give a sign-off? Just a
> straight yes or no will do. I have already reviewed it and asked
> several times for a sign-off. Nick has been very patient, and I think
> at this point it is becoming somewhat annoying to pester for someone
> to let Python patches through.
>
> Cheers,
>
> Phil
>
Thanks Phil (and Robert for support). It would be really good to get it
in this time.
Nick
I see many new functions without intro comments. Please add an intro comment
to each new function, describing the contract of the function. If the function
is just implementing some hook, then have the function comment say that
"Inplements method foo for bar", instead of duplicating comments about
parameters and return types.
Please add comments to new enums (inferior_call_kind), and their values, etc. too.
> @@ -627,6 +628,8 @@ call_function_by_hand (struct value *function, int
> nargs, struct value **args)
> target_values_type = values_type;
> }
>
> + observer_notify_inferior_call_pre ();
> +
> /* Determine the location of the breakpoint (and possibly other
> stuff) that the called function will return to. The SPARC, for a
> function returning a structure or union, needs to make space for
> @@ -860,6 +863,8 @@ call_function_by_hand (struct value *function, int
> nargs, struct value **args)
> e = run_inferior_call (tp, real_pc);
> }
>
> + observer_notify_inferior_call_post ();
> +
> /* Rethrow an error if we got one trying to run the inferior. */
>
> if (e.reason < 0)
I see an issue with these events. On the call_pre case, the
caller can infer which thread the infcall was being run on
from the currently selected thread.
But, not so from the call_post. The inferior may have crashed,
exited, or stopped for some breakpoint in a different thread
at that point. So the user of that event doesn't really know
which function call finished.
GDB only does one function call at a time currently, but
that's subject to change. It'd be best to design with that
in mind already.
Also, the documentation leaves this issue unspecified:
> +@item events.inferior_call_post
> +Emits @code{gdb.InferiorCallPostEvent} which indicates that a function in
> +the inferior has returned.
I, as I reader, immediately wonder what happens if the function
doesn't get to return, but inferior exits, etc. E.g., "is the
event called in that scenario as well?".
> +# Test register changed event
> +gdb_test_no_output {set $old_sp = $sp}
> +gdb_test {set $sp = 0} ".*event type: register-changed.*
> +.*frame: .*
> +.*num: .*"
> +gdb_test {set $sp = 1} ".*event type: register-changed.*
> +.*frame: .*
> +.*num: .*"
> +gdb_test {set $sp = $old_sp} ".*event type: register-changed.*
> +.*frame: .*
> +.*num: .*"
Please write explicit \r\n instead. (or use e.g.,
gdb_test_sequence).
> +set test "continue to breakpoint 5"
> +gdb_test_multiple "continue" $test {
> + -re "event type: memory-changed" {
> + fail $test
> + }
> + -re ".*event type: continue.*
> +.*event type: stop.*
> +.*stop reason: breakpoint.*
> +.*all threads stopped.*$gdb_prompt $" {
> + pass $test
> + }
> +}
> +
> +gdb_test_no_output "delete 5"
Please avoid hard coding breakpoint numbers in tests.
You can instead extract the number with expect_out
when you create the breakpoint, and store it in a
tcl variable. Alternatively, use $bpnum.
> +gdb_test_multiple {up} $test {
> + -re "event type: register-changed" {
> + fail $test
> + }
Please always match $gdb_prompt $, otherwise you're
leaving the prompt in the expect buffer, which confuses
the next gdb_test_multiple/gdb_test call.
While at it, please make sure the test passes with "make check-read1".
> + -re "#1.*in first.*\r\n.*do_nothing.*\r\n$gdb_prompt $" {
> + pass $test
> + }
> +}
> + ** gdb.events.inferior_call_pre Function call is about to be made.
> + ** gdb.events.inferior_call_post Function call has just been made.
> + ** gdb.events.memory_changed A memory location has been altered.
> + ** gdb.events.register_changed A register has been altered.
Maybe it's just me, but I find this hard to read. I'd expect some kind
of separator between the event name and its description. Like e.g.,:
** gdb.events.register_changed - A register has been altered.
or:
** gdb.events.register_changed: A register has been altered.
or:
** gdb.events.register_changed
A register has been altered.
Thanks,
Pedro Alves
@@ -360,6 +360,7 @@ SUBDIR_PYTHON_OBS = \
py-framefilter.o \
py-function.o \
py-gdb-readline.o \
+ py-infcallevent.o \
py-inferior.o \
py-infthread.o \
py-lazy-string.o \
@@ -398,6 +399,7 @@ SUBDIR_PYTHON_SRCS = \
python/py-framefilter.c \
python/py-function.c \
python/py-gdb-readline.c \
+ python/py-infcallevent.c \
python/py-inferior.c \
python/py-infthread.c \
python/py-lazy-string.c \
@@ -2475,6 +2477,10 @@ py-gdb-readline.o: $(srcdir)/python/py-gdb-readline.c
$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-gdb-readline.c
$(POSTCOMPILE)
+py-infcallevent.o: $(srcdir)/python/py-infcallevent.c
+ $(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-infcallevent.c
+ $(POSTCOMPILE)
+
py-inferior.o: $(srcdir)/python/py-inferior.c
$(COMPILE) $(PYTHON_CFLAGS) $(srcdir)/python/py-inferior.c
$(POSTCOMPILE)
@@ -38,6 +38,16 @@ queue-signal signal-name-or-number
-D data-directory
This is an alias for the --data-directory option.
+* Python Scripting
+
+ New events which are triggered when GDB modifies the state of the
+ inferior.
+
+ ** gdb.events.inferior_call_pre Function call is about to be made.
+ ** gdb.events.inferior_call_post Function call has just been made.
+ ** gdb.events.memory_changed A memory location has been altered.
+ ** gdb.events.register_changed A register has been altered.
+
* GDB supports printing and modifying of variable length automatic arrays
as specified in ISO C99.
@@ -281,6 +281,18 @@ The trace state variable @var{tsv} is deleted. If
@var{tsv} is
The trace state value @var{tsv} is modified.
@end deftypefun
+@deftypefun void inferior_call_pre (void)
+An inferior function is about to be called.
+@end deftypefun
+
+@deftypefun void inferior_call_post (void)
+An inferior function has just been called.
+@end deftypefun
+
+@deftypefun void register_changed (struct frame_info *@var{frame},
short @var{regnum})
+A register in the inferior has been modified.
+@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.
@@ -2728,6 +2728,39 @@ A reference to the object file
(@code{gdb.Objfile}) which has been loaded.
@xref{Objfiles In Python}, for details of the @code{gdb.Objfile} object.
@end defvar
+@item events.inferior_call_pre
+Emits @code{gdb.InferiorCallPreEvent} which indicates that a function in
+the inferior is about to be called.
+
+@item events.inferior_call_post
+Emits @code{gdb.InferiorCallPostEvent} which indicates that a function in
+the inferior has returned.
+
+@item events.memory_changed
+Emits @code{gdb.MemoryChangedEvent} which indicates that the memory of the
+inferior has been modified by the @value{GDBN} user, for instance via a
+command like @w{@code{set *addr = value}}. The event has the following
+attributes:
+
+@defvar MemoryChangedEvent.address
+The start address of the changed region.
+@end defvar
+
+@defvar MemoryChangedEvent.length
+Length in bytes of the changed region.
+@end defvar
+
+@item events.register_changed
+Emits @code{gdb.RegisterChangedEvent} which indicates that a register
in the
+inferior has been modified by the @value{GDBN} user.
+
+@defvar RegisterChangedEvent.frame
+A gdb.Frame object representing the frame in which the register was
modified.
+@end defvar
+@defvar RegisterChangedEvent.regnum
+Denotes which register was modified.
+@end defvar
+
@end table
@node Threads In Python
@@ -36,6 +36,7 @@
#include "gdbthread.h"
#include "exceptions.h"
#include "event-top.h"
+#include "observer.h"
/* If we can't find a function's name from its address,
we print this instead. */
@@ -627,6 +628,8 @@ call_function_by_hand (struct value *function, int
nargs, struct value **args)
target_values_type = values_type;
}
+ observer_notify_inferior_call_pre ();
+
/* Determine the location of the breakpoint (and possibly other
stuff) that the called function will return to. The SPARC, for a
function returning a structure or union, needs to make space for
@@ -860,6 +863,8 @@ call_function_by_hand (struct value *function, int
nargs, struct value **args)
e = run_inferior_call (tp, real_pc);
}
+ observer_notify_inferior_call_post ();
+
/* Rethrow an error if we got one trying to run the inferior. */
if (e.reason < 0)
@@ -105,6 +105,16 @@ typedef struct
extern int emit_continue_event (ptid_t ptid);
extern int emit_exited_event (const LONGEST *exit_code, struct
inferior *inf);
+typedef enum
+{
+ inferior_call_pre,
+ inferior_call_post,
+} inferior_call_kind;
+
+extern int emit_inferior_call_event (inferior_call_kind kind);
+extern int emit_register_changed_event (struct frame_info *frame,
+ short regnum);
+extern int emit_memory_changed_event (CORE_ADDR addr, ssize_t len);
extern int evpy_emit_event (PyObject *event,
eventregistry_object *registry)
CPYCHECKER_STEALS_REFERENCE_TO_ARG (1);
@@ -46,6 +46,9 @@ typedef struct
eventregistry_object *cont;
eventregistry_object *exited;
eventregistry_object *new_objfile;
+ eventregistry_object *inferior_call;
+ eventregistry_object *memory_changed;
+ eventregistry_object *register_changed;
PyObject *module;
@@ -73,6 +73,18 @@ gdbpy_initialize_py_events (void)
if (add_new_registry (&gdb_py_events.exited, "exited") < 0)
return -1;
+ if (add_new_registry (&gdb_py_events.inferior_call,
+ "inferior_call") < 0)
+ return -1;
+
+ if (add_new_registry (&gdb_py_events.memory_changed,
+ "memory_changed") < 0)
+ return -1;
+
+ if (add_new_registry (&gdb_py_events.register_changed,
+ "register_changed") < 0)
+ return -1;
+
if (add_new_registry (&gdb_py_events.new_objfile, "new_objfile") < 0)
return -1;
@@ -118,6 +118,58 @@ python_on_resume (ptid_t ptid)
}
static void
+python_on_inferior_call_pre (void)
+{
+ struct cleanup *cleanup;
+
+ cleanup = ensure_python_env (target_gdbarch (), current_language);
+
+ if (emit_inferior_call_event (inferior_call_pre) < 0)
+ gdbpy_print_stack ();
+
+ do_cleanups (cleanup);
+}
+
+static void
+python_on_inferior_call_post (void)
+{
+ struct cleanup *cleanup;
+
+ cleanup = ensure_python_env (target_gdbarch (), current_language);
+
+ if (emit_inferior_call_event (inferior_call_post) < 0)
+ gdbpy_print_stack ();
+
+ do_cleanups (cleanup);
+}
+
+static void
+python_on_memory_change (struct inferior *inferior, CORE_ADDR addr,
ssize_t len, const bfd_byte *data)
+{
+ struct cleanup *cleanup;
+
+ cleanup = ensure_python_env (target_gdbarch (), current_language);
+
+ if (emit_memory_changed_event (addr, len) < 0)
+ gdbpy_print_stack ();
+
+ do_cleanups (cleanup);
+}
+
+static void
+python_on_register_change (struct frame_info *frame, short regnum)
+{
+ struct cleanup *cleanup;
+
+ cleanup = ensure_python_env (target_gdbarch (), current_language);
+
+ if (emit_register_changed_event (frame, regnum) < 0)
+ gdbpy_print_stack ();
+
+ do_cleanups (cleanup);
+}
+
+static void
python_inferior_exit (struct inferior *inf)
{
struct cleanup *cleanup;
@@ -794,6 +846,10 @@ gdbpy_initialize_inferior (void)
observer_attach_thread_exit (delete_thread_object);
observer_attach_normal_stop (python_on_normal_stop);
observer_attach_target_resumed (python_on_resume);
+ observer_attach_inferior_call_pre (python_on_inferior_call_pre);
+ observer_attach_inferior_call_post (python_on_inferior_call_post);
+ observer_attach_memory_changed (python_on_memory_change);
+ observer_attach_register_changed (python_on_register_change);
observer_attach_inferior_exit (python_inferior_exit);
observer_attach_new_objfile (python_new_objfile);
@@ -466,6 +466,14 @@ int gdbpy_initialize_breakpoint_event (void)
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
int gdbpy_initialize_continue_event (void)
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
+int gdbpy_initialize_inferior_call_pre_event (void)
+ CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
+int gdbpy_initialize_inferior_call_post_event (void)
+ CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
+int gdbpy_initialize_register_changed_event (void)
+ CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
+int gdbpy_initialize_memory_changed_event (void)
+ CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
int gdbpy_initialize_exited_event (void)
CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
int gdbpy_initialize_thread_event (void)
@@ -1754,6 +1754,10 @@ message == an error message without a stack will
be printed."),
|| gdbpy_initialize_signal_event () < 0
|| gdbpy_initialize_breakpoint_event () < 0
|| gdbpy_initialize_continue_event () < 0
+ || gdbpy_initialize_inferior_call_pre_event () < 0
+ || gdbpy_initialize_inferior_call_post_event () < 0
+ || gdbpy_initialize_register_changed_event () < 0
+ || gdbpy_initialize_memory_changed_event () < 0
|| gdbpy_initialize_exited_event () < 0
|| gdbpy_initialize_thread_event () < 0
|| gdbpy_initialize_new_objfile_event () < 0
b/gdb/testsuite/gdb.python/py-events.exp
@@ -79,6 +79,92 @@ all threads stopped"
delete_breakpoints
+# Test inferior call events
+gdb_test "call do_nothing()" ".*event type: pre-call.*
+.*event type: post-call.*"
+
+# Test register changed event
+gdb_test_no_output {set $old_sp = $sp}
+gdb_test {set $sp = 0} ".*event type: register-changed.*
+.*frame: .*
+.*num: .*"
+gdb_test {set $sp = 1} ".*event type: register-changed.*
+.*frame: .*
+.*num: .*"
+gdb_test {set $sp = $old_sp} ".*event type: register-changed.*
+.*frame: .*
+.*num: .*"
+
+# Test that no register_changed event is generated on "non-user"
+# modifications
+set test "up"
+gdb_test_multiple {up} $test {
+ -re "event type: register-changed" {
+ fail $test
+ }
+ -re "#1.*in first.*\r\n.*do_nothing.*\r\n$gdb_prompt $" {
+ pass $test
+ }
+}
+
+set test "down"
+gdb_test_multiple {down} $test {
+ -re "event type: register-changed" {
+ fail $test
+ }
+ -re "#0.*do_nothing.*at.*\r\n.*void do_nothing.*\r\n$gdb_prompt $" {
+ pass $test
+ }
+}
+
+set test "step"
+gdb_test_multiple {step} $test {
+ -re "event type: register-changed" {
+ fail $test
+ }
+ -re "first.*at.*\r\n.*28.*for.*\r\n$gdb_prompt $" {
+ pass $test
+ }
+}
+
+
+# Test memory changed event
+gdb_test_no_output {set $saved = *(int*) $sp}
+gdb_test {set *(int*) $sp = 0} ".*event type: memory-changed.*
+.*address: .*
+.*length: .*"
+gdb_test {set *(int*) $sp = $saved} ".*event type: memory-changed.*
+.*address: .*
+.*length: .*"
+
+# Test that no memory_changed event is generated on breakpoint
+# activity
+set test "break second"
+gdb_test_multiple "break second" $test {
+ -re "event type: memory-changed" {
+ fail $test
+ }
+ -re "Breakpoint 5 at .*\r\n$gdb_prompt $" {
+ pass $test
+ }
+
+}
+
+set test "continue to breakpoint 5"
+gdb_test_multiple "continue" $test {
+ -re "event type: memory-changed" {
+ fail $test
+ }
+ -re ".*event type: continue.*
+.*event type: stop.*
+.*stop reason: breakpoint.*
+.*all threads stopped.*$gdb_prompt $" {
+ pass $test
+ }
+}
+
+gdb_test_no_output "delete 5"
+
#test exited event.
gdb_test "continue" ".*event type: continue.*
.*event type: exit.*
b/gdb/testsuite/gdb.python/py-events.py
@@ -57,6 +57,28 @@ def new_objfile_handler (event):
print ("event type: new_objfile")
print ("new objfile name: %s" % (event.new_objfile.filename))
+def inferior_fn_handler (event):
+ if (isinstance (event, gdb.InferiorCallPreEvent)):
+ print ("event type: pre-call")
+ elif (isinstance (event, gdb.InferiorCallPostEvent)):
+ print ("event type: post-call")
+ else:
+ assert False
+
+def register_changed_handler (event):
+ assert (isinstance (event, gdb.RegisterChangedEvent))
+ print ("event type: register-changed")
+ assert (isinstance (event.frame, gdb.Frame))
+ print ("frame: %s" % (event.frame))
+ print ("num: %s" % (event.regnum))
+
+def memory_changed_handler (event):
+ assert (isinstance (event, gdb.MemoryChangedEvent))
+ print ("event type: memory-changed")
+ print ("address: %s" % (event.address))
+ print ("length: %s" % (event.length))
+
+
class test_events (gdb.Command):
"""Test events."""
@@ -68,6 +90,9 @@ class test_events (gdb.Command):
gdb.events.stop.connect (breakpoint_stop_handler)
gdb.events.exited.connect (exit_handler)
gdb.events.cont.connect (continue_handler)
+ gdb.events.inferior_call.connect(inferior_fn_handler)
+ gdb.events.memory_changed.connect(memory_changed_handler)
+ gdb.events.register_changed.connect(register_changed_handler)
print ("Event testers registered.")
test_events ()
@@ -1168,6 +1168,7 @@ value_assign (struct value *toval, struct value
*fromval)
}
}
+ observer_notify_register_changed (frame, value_reg);
if (deprecated_register_changed_hook)
deprecated_register_changed_hook (-1);