* [RFA] Use observers to report stop events. @ 2008-04-11 20:05 Vladimir Prus 2008-04-24 13:47 ` Vladimir Prus 2008-04-29 8:03 ` Joel Brobecker 0 siblings, 2 replies; 14+ messages in thread From: Vladimir Prus @ 2008-04-11 20:05 UTC (permalink / raw) To: gdb-patches Presently, the *stopped async output, despite been documented as async, is actually output during processing of MI command. Naturally, this is not going to work in non-stop mode, since there may be more stops that there were MI commands. This patches makes the *stopped output be printed via observer. Presently, there's one regression in mi tests -- namely the mi-async.exp test -- which uses hardcoded tests for *stopped and so fails after update as predicated :-) I'll fix this up later; this is a local change and should not affect the review of the patch itself. Note that there are no current uses of the normal_stop observer, so this patch is as safe as it can get :-) - Volodya * defs.h (make_cleanup_restore_integer): New declaration. * utils.c (restore_integer_closure, restore_integer) (make_cleanup_restore_integer): New. * breakpoint.c (restore_always_inserted_mode): Remove. (update_breakpoints_after_exec): Use make_cleanup_restore_integer. * inferior.h (suppress_normal_stop_observer): New. * infcall.c (call_function_by_hand): Disable stop events when doing function calls. * infmcd.c (suppress_normal_stop_observer): New. (finish_command_continuation): Call normal_stop observer explicitly. (finish_command): Disable stop events inside proceed. * infrun.c (normal_stop): Don't call normal stop observer if suppressed of if multi-step is in progress. * interps.h (top_level_interpreter): New. * interps.c (top_level_interpreter): Rename to top_level_interpreter_ptr. (top_level_interpreter): New. * mi/mi-interp.c (mi_on_normal_stop): New. (mi_interpreter_init): Register mi_on_normal_stop. (mi_interpreter_exec_continuation): Remove. (mi_cmd_interpreter_exec): Don't register the above. * mi/mi-main.c (captured_mi_execute_command): Don't care about sync_execution. (mi_execute_async_cli_command): Don't install continuation. Don't print *stopped. (mi_exec_async_cli_cmd_continuation): Remove. [gdb/testsuite] * gdb.mi/mi-break.exp (test_ignore_count): Adjust stopped pattern. * gdb.mi/mi-syn-frame.exp: Use mi_expect_stop instead of direct testing of stopped. * gdb.mi/mi2-syn-frame.exp: Likewise. * lib/mi-support.exp (default_mi_gdb_start): Call detect_async. (async, detect_async): New. (mi_expect_stop, mi_continue_to_line): Adjust expectation depending on if we're running in sync or async mode. --- gdb/breakpoint.c | 10 +---- gdb/defs.h | 2 + gdb/infcall.c | 6 ++- gdb/infcmd.c | 16 +++++-- gdb/inferior.h | 3 + gdb/infrun.c | 3 +- gdb/interps.c | 17 ++++++-- gdb/interps.h | 2 + gdb/mi/mi-interp.c | 43 ++++++++++----------- gdb/mi/mi-main.c | 36 +---------------- gdb/testsuite/gdb.mi/mi-break.exp | 2 +- gdb/testsuite/gdb.mi/mi-syn-frame.exp | 6 +-- gdb/testsuite/gdb.mi/mi2-syn-frame.exp | 25 +++--------- gdb/testsuite/lib/mi-support.exp | 64 +++++++++++++++++++++++++++++--- gdb/utils.c | 27 +++++++++++++ 15 files changed, 157 insertions(+), 105 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index d220d00..537b9bf 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1448,12 +1448,6 @@ reattach_breakpoints (int pid) return 0; } -static void -restore_always_inserted_mode (void *p) -{ - always_inserted_mode = (uintptr_t) p; -} - void update_breakpoints_after_exec (void) { @@ -1469,9 +1463,7 @@ update_breakpoints_after_exec (void) /* The binary we used to debug is now gone, and we're updating breakpoints for the new binary. Until we're done, we should not try to insert breakpoints. */ - cleanup = make_cleanup (restore_always_inserted_mode, - (void *) (uintptr_t) always_inserted_mode); - always_inserted_mode = 0; + cleanup = make_cleanup_restore_integer (&always_inserted_mode, 0); ALL_BREAKPOINTS_SAFE (b, temp) { diff --git a/gdb/defs.h b/gdb/defs.h index 5c35051..a632840 100644 --- a/gdb/defs.h +++ b/gdb/defs.h @@ -345,6 +345,8 @@ extern struct cleanup *make_cleanup_close (int fd); extern struct cleanup *make_cleanup_bfd_close (bfd *abfd); +extern struct cleanup *make_cleanup_restore_integer (int *variable, int value); + extern struct cleanup *make_final_cleanup (make_cleanup_ftype *, void *); extern struct cleanup *make_my_cleanup (struct cleanup **, diff --git a/gdb/infcall.c b/gdb/infcall.c index 721b32d..d879b79 100644 --- a/gdb/infcall.c +++ b/gdb/infcall.c @@ -706,6 +706,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args) { struct cleanup *old_cleanups = make_cleanup (null_cleanup, 0); + struct cleanup *old_cleanups2; int saved_async = 0; /* If all error()s out of proceed ended up calling normal_stop @@ -718,8 +719,11 @@ call_function_by_hand (struct value *function, int nargs, struct value **args) if (target_can_async_p ()) saved_async = target_async_mask (0); - + + old_cleanups2 = make_cleanup_restore_integer + (&suppress_normal_stop_observer, 1); proceed (real_pc, TARGET_SIGNAL_0, 0); + do_cleanups (old_cleanups2); if (saved_async) target_async_mask (saved_async); diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 2823259..c787f95 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -202,6 +202,9 @@ int step_multi; in format described in environ.h. */ struct gdb_environ *inferior_environ; + +/* When set, normal_stop will not call the normal_stop observer. */ +int suppress_normal_stop_observer = 0; \f /* Accessor routines. */ @@ -1275,9 +1278,14 @@ finish_command_continuation (struct continuation_arg *arg, int error) if (TYPE_CODE (value_type) != TYPE_CODE_VOID) print_return_value (value_type); } + + /* We suppress normal call of normal_stop observer and do it here so that + that *stopped notification includes the return value. */ + observer_notify_normal_stop (stop_bpstat); } - delete_breakpoint (breakpoint); + suppress_normal_stop_observer = 0; + delete_breakpoint (breakpoint); } /* "finish": Set a temporary breakpoint at the place the selected @@ -1343,6 +1351,7 @@ finish_command (char *arg, int from_tty) } proceed_to_finish = 1; /* We want stop_registers, please... */ + make_cleanup_restore_integer (&suppress_normal_stop_observer, 1); proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0); arg1 = @@ -1358,10 +1367,7 @@ finish_command (char *arg, int from_tty) arg2->data.pointer = function; arg3->data.pointer = old_chain; add_continuation (finish_command_continuation, arg1); - - /* Do this only if not running asynchronously or if the target - cannot do async execution. Otherwise, complete this command when - the target actually stops, in fetch_inferior_event. */ + discard_cleanups (old_chain); if (!target_can_async_p ()) do_all_continuations (0); diff --git a/gdb/inferior.h b/gdb/inferior.h index 3aaaa26..630cc52 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -399,6 +399,9 @@ extern int debug_displaced; void displaced_step_dump_bytes (struct ui_file *file, const gdb_byte *buf, size_t len); + +/* When set, normal_stop will not call the normal_stop observer. */ +extern int suppress_normal_stop_observer; \f /* Possible values for gdbarch_call_dummy_location. */ #define ON_STACK 1 diff --git a/gdb/infrun.c b/gdb/infrun.c index 7f151f7..d6b78ea 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -3635,7 +3635,8 @@ Further execution is probably impossible.\n")); done: annotate_stopped (); - observer_notify_normal_stop (stop_bpstat); + if (!suppress_normal_stop_observer && !step_multi) + observer_notify_normal_stop (stop_bpstat); } static int diff --git a/gdb/interps.c b/gdb/interps.c index 9d47843..936fe89 100644 --- a/gdb/interps.c +++ b/gdb/interps.c @@ -81,7 +81,7 @@ void _initialize_interpreter (void); static struct interp *interp_list = NULL; static struct interp *current_interpreter = NULL; -static struct interp *top_level_interpreter = NULL; +static struct interp *top_level_interpreter_ptr = NULL; static int interpreter_initialized = 0; @@ -144,7 +144,7 @@ interp_set (struct interp *interp, int top_level) /* If we already have an interpreter, then trying to set top level interpreter is kinda pointless. */ gdb_assert (!top_level || !current_interpreter); - gdb_assert (!top_level || !top_level_interpreter); + gdb_assert (!top_level || !top_level_interpreter_ptr); if (current_interpreter != NULL) { @@ -165,7 +165,7 @@ interp_set (struct interp *interp, int top_level) current_interpreter = interp; if (top_level) - top_level_interpreter = interp; + top_level_interpreter_ptr = interp; /* We use interpreter_p for the "set interpreter" variable, so we need to make sure we have a malloc'ed copy for the set command to free. */ @@ -476,11 +476,18 @@ interpreter_completer (char *text, char *word) return matches; } +struct interp * +top_level_interpreter (void) +{ + gdb_assert (top_level_interpreter_ptr); + return top_level_interpreter_ptr; +} + extern void * top_level_interpreter_data (void) { - gdb_assert (top_level_interpreter); - return top_level_interpreter->data; + gdb_assert (top_level_interpreter_ptr); + return top_level_interpreter_ptr->data; } /* This just adds the "interpreter-exec" command. */ diff --git a/gdb/interps.h b/gdb/interps.h index e1030fa..5e91080 100644 --- a/gdb/interps.h +++ b/gdb/interps.h @@ -64,6 +64,8 @@ extern struct ui_out *interp_ui_out (struct interp *interp); extern int current_interp_named_p (const char *name); extern int current_interp_display_prompt_p (void); extern void current_interp_command_loop (void); +/* Returns the top-level interpreter. */ +extern struct interp *top_level_interpreter (); /* Returns opaque data associated with the top-level interpreter. */ extern void *top_level_interpreter_data (void); diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index ae9e07b..e874a56 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -65,6 +65,7 @@ static void mi1_command_loop (void); static void mi_insert_notify_hooks (void); static void mi_remove_notify_hooks (void); +static void mi_on_normal_stop (struct bpstats *bs); static void mi_new_thread (struct thread_info *t); @@ -88,7 +89,10 @@ mi_interpreter_init (int top_level) mi->event_channel = mi_console_file_new (raw_stdout, "=", 0); if (top_level) - observer_attach_new_thread (mi_new_thread); + { + observer_attach_new_thread (mi_new_thread); + observer_attach_normal_stop (mi_on_normal_stop); + } return mi; } @@ -167,26 +171,6 @@ mi_interpreter_prompt_p (void *data) return 0; } -static void -mi_interpreter_exec_continuation (struct continuation_arg *arg, int error) -{ - bpstat_do_actions (&stop_bpstat); - /* It's not clear what to do in the case of errror -- should we assume that - the target is stopped, or that it still runs? */ - if (!target_executing) - { - fputs_unfiltered ("*stopped", raw_stdout); - mi_out_put (uiout, raw_stdout); - fputs_unfiltered ("\n", raw_stdout); - fputs_unfiltered ("(gdb) \n", raw_stdout); - gdb_flush (raw_stdout); - } - else if (target_can_async_p ()) - { - add_continuation (mi_interpreter_exec_continuation, NULL); - } -} - enum mi_cmd_result mi_cmd_interpreter_exec (char *command, char **argv, int argc) { @@ -237,7 +221,6 @@ mi_cmd_interpreter_exec (char *command, char **argv, int argc) if (target_can_async_p () && target_executing) { fputs_unfiltered ("^running\n", raw_stdout); - add_continuation (mi_interpreter_exec_continuation, NULL); } if (mi_error_message != NULL) @@ -317,6 +300,22 @@ mi_new_thread (struct thread_info *t) gdb_flush (mi->event_channel); } +static void +mi_on_normal_stop (struct bpstats *bs) +{ + /* Since this can be called when CLI command is executing, + using cli interpreter, be sure to use MI uiout for output, + not the current one. */ + struct ui_out *uiout = interp_ui_out (top_level_interpreter ()); + struct mi_interp *mi = top_level_interpreter_data (); + + fputs_unfiltered ("*stopped", raw_stdout); + mi_out_put (uiout, raw_stdout); + mi_out_rewind (uiout); + fputs_unfiltered ("\n", raw_stdout); + gdb_flush (raw_stdout); +} + extern initialize_file_ftype _initialize_mi_interp; /* -Wmissing-prototypes */ void diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 177d0ec..deb2df9 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -102,9 +102,6 @@ static void mi_execute_cli_command (const char *cmd, int args_p, const char *args); static enum mi_cmd_result mi_execute_async_cli_command (char *mi, char *args, int from_tty); -static void mi_exec_async_cli_cmd_continuation (struct continuation_arg *arg, - int error); - static int register_changed_p (int regnum, struct regcache *, struct regcache *); static void get_register (int regnum, int format); @@ -1067,15 +1064,11 @@ captured_mi_execute_command (struct ui_out *uiout, void *data) fputs_unfiltered ("\n", raw_stdout); } else + /* The command does not want anything to be printed. In that + case, the command probably should not have written anything + to uiout, but in case it has written something, discard it. */ mi_out_rewind (uiout); } - else if (sync_execution) - { - /* Don't print the prompt. We are executing the target in - synchronous mode. */ - args->action = EXECUTE_COMMAND_SUPPRESS_PROMPT; - return; - } break; case CLI_COMMAND: @@ -1296,12 +1289,6 @@ mi_execute_async_cli_command (char *mi, char *args, int from_tty) fputs_unfiltered (current_token, raw_stdout); fputs_unfiltered ("^running\n", raw_stdout); - /* Ideally, we should be intalling continuation only when - the target is already running. However, this will break right now, - because continuation installed by the 'finish' command must be after - the continuation that prints *stopped. This issue will be - fixed soon. */ - add_continuation (mi_exec_async_cli_cmd_continuation, NULL); } execute_command ( /*ui */ run, 0 /*from_tty */ ); @@ -1317,31 +1304,14 @@ mi_execute_async_cli_command (char *mi, char *args, int from_tty) /* Do this before doing any printing. It would appear that some print code leaves garbage around in the buffer. */ do_cleanups (old_cleanups); - /* If the target was doing the operation synchronously we fake - the stopped message. */ - fputs_unfiltered ("*stopped", raw_stdout); - mi_out_put (uiout, raw_stdout); - mi_out_rewind (uiout); if (do_timings) print_diff_now (current_command_ts); - fputs_unfiltered ("\n", raw_stdout); return MI_CMD_QUIET; } return MI_CMD_DONE; } void -mi_exec_async_cli_cmd_continuation (struct continuation_arg *arg, int error) -{ - /* Assume 'error' means that target is stopped, too. */ - fputs_unfiltered ("*stopped", raw_stdout); - mi_out_put (uiout, raw_stdout); - fputs_unfiltered ("\n", raw_stdout); - fputs_unfiltered ("(gdb) \n", raw_stdout); - gdb_flush (raw_stdout); -} - -void mi_load_progress (const char *section_name, unsigned long sent_so_far, unsigned long total_section, diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp index c0f5132..e4fd5c9 100644 --- a/gdb/testsuite/gdb.mi/mi-break.exp +++ b/gdb/testsuite/gdb.mi/mi-break.exp @@ -163,7 +163,7 @@ proc test_ignore_count {} { mi_run_cmd gdb_expect { - -re ".*func=\"callme\".*args=\\\[\{name=\"i\",value=\"2\"\}\\\].*\r\n$mi_gdb_prompt$" { + -re ".*\\*stopped.*func=\"callme\".*args=\\\[\{name=\"i\",value=\"2\"\}\\\].*\r\n($mi_gdb_prompt)?$" { pass "run to breakpoint with ignore count" } -re ".*$mi_gdb_prompt$" { diff --git a/gdb/testsuite/gdb.mi/mi-syn-frame.exp b/gdb/testsuite/gdb.mi/mi-syn-frame.exp index ed89965..80d36c9 100644 --- a/gdb/testsuite/gdb.mi/mi-syn-frame.exp +++ b/gdb/testsuite/gdb.mi/mi-syn-frame.exp @@ -60,9 +60,7 @@ mi_gdb_test "403-exec-continue" \ "403\\^running" \ "testing exec continue" -# Presently, the *stopped notification for this case does not include -# any information. This can be considered a bug. -mi_gdb_test "" "\\*stopped" "finished exec continue" +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" mi_gdb_test "404-stack-list-frames 0 0" \ "404\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" \ @@ -91,7 +89,7 @@ mi_gdb_test "407-stack-list-frames" \ mi_gdb_test "408-exec-continue" "408\\^running" -mi_gdb_test "" ".*\\*stopped.*" "finished exec continue" +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" mi_gdb_test "409-stack-list-frames 0 0" \ "409\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" \ diff --git a/gdb/testsuite/gdb.mi/mi2-syn-frame.exp b/gdb/testsuite/gdb.mi/mi2-syn-frame.exp index 9c1daab..6e9792a 100644 --- a/gdb/testsuite/gdb.mi/mi2-syn-frame.exp +++ b/gdb/testsuite/gdb.mi/mi2-syn-frame.exp @@ -58,15 +58,11 @@ mi_gdb_test "402-stack-list-frames" "402\\^done,stack=\\\[frame=\{level=\"0\",ad # Continue back to main() # -send_gdb "403-exec-continue\n" -gdb_expect { - -re "403\\^running\[\r\n\]+${my_mi_gdb_prompt}.*\\\*stopped\[\r\n\]+${my_mi_gdb_prompt}$" { - pass "403-exec-continue" - } - timeout { - fail "403-exec-continue" - } -} +mi_gdb_test "403-exec-continue" \ + "403\\^running" \ + "testing exec continue" + +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" mi_gdb_test "404-stack-list-frames 0 0" \ "404\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" \ @@ -92,16 +88,9 @@ mi_gdb_test "407-stack-list-frames" \ "407\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"subroutine\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"handler\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"<signal handler called>\"\},.*frame=\{level=\"$decimal\",addr=\"$hex\",func=\"have_a_very_merry_interrupt\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]" \ "list stack frames" +mi_gdb_test "408-exec-continue" "408\\^running" -send_gdb "408-exec-continue\n" -gdb_expect { - -re "408\\^running\[\r\n\]+${my_mi_gdb_prompt}.*\\\*stopped\[\r\n\]+${my_mi_gdb_prompt}$" { - pass "408-exec-continue" - } - timeout { - fail "408-exec-continue" - } -} +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" mi_gdb_test "409-stack-list-frames 0 0" \ "409\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" \ diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp index 5926f16..0a6eb8a 100644 --- a/gdb/testsuite/lib/mi-support.exp +++ b/gdb/testsuite/lib/mi-support.exp @@ -227,6 +227,8 @@ proc default_mi_gdb_start { args } { } } + detect_async + return 0; } @@ -911,6 +913,30 @@ proc mi_step { test } { return [mi_step_to {.*} {.*} {.*} {.*} $test] } +set async "unknown" + +proc detect_async {} { + global async + global mi_gdb_prompt + + if { $async == "unknown" } { + send_gdb "maint show linux-async\n" + + gdb_expect { + -re ".*Controlling the GNU/Linux inferior in asynchronous mode is on...*$mi_gdb_prompt$" { + set async 1 + } + -re ".*$mi_gdb_prompt$" { + set async 0 + } + timeout { + set async 0 + } + } + } + return $async +} + # Wait for MI *stopped notification to appear. # The REASON, FUNC, ARGS, FILE and LINE are regular expressions # to match against whatever is output in *stopped. ARGS should @@ -933,6 +959,7 @@ proc mi_expect_stop { reason func args file line extra test } { global hex global decimal global fullname_syntax + global async set after_stopped "" set after_reason "" @@ -944,10 +971,28 @@ proc mi_expect_stop { reason func args file line extra test } { set after_stopped [lindex $extra 0] } + if {$async} { + set prompt_re "" + } else { + set prompt_re "$mi_gdb_prompt" + } + + if { $reason == "really-no-reason" } { + gdb_expect { + -re "\\*stopped\r\n$prompt_re$" { + pass "$test" + } + timeout { + fail "$test (unknown output after running)" + } + } + return + } + if { $reason == "exited-normally" } { gdb_expect { - -re "\\*stopped,reason=\"exited-normally\"\r\n$mi_gdb_prompt$" { + -re "\\*stopped,reason=\"exited-normally\"\r\n$prompt_re$" { pass "$test" } -re ".*$mi_gdb_prompt$" {fail "continue to end (2)"} @@ -970,17 +1015,17 @@ proc mi_expect_stop { reason func args file line extra test } { set r "reason=\"$reason\"," } - verbose -log "mi_expect_stop: expecting: .*\\*stopped,${r}${bn}${after_reason}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}$after_stopped\r\n$mi_gdb_prompt$" + verbose -log "mi_expect_stop: expecting: .*\\*stopped,${r}${bn}${after_reason}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}$after_stopped\r\n$prompt_re$" gdb_expect { - -re ".*\\*stopped,${r}${bn}${after_reason}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n$mi_gdb_prompt$" { + -re ".*\\*stopped,${r}${bn}${after_reason}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n$prompt_re$" { pass "$test" return $expect_out(2,string) } - -re ".*\\*stopped,${r}${bn}${after_reason}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\".*\",args=\[\\\[\{\].*\[\\\]\}\],file=\".*\",fullname=\"${fullname_syntax}.*\",line=\"\[0-9\]*\"\}.*\r\n$mi_gdb_prompt$" { + -re ".*\\*stopped,${r}${bn}${after_reason}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\".*\",args=\[\\\[\{\].*\[\\\]\}\],file=\".*\",fullname=\"${fullname_syntax}.*\",line=\"\[0-9\]*\"\}.*\r\n$prompt_re$" { fail "$test (stopped at wrong place)" return -1 } - -re ".*\r\n${mi_gdb_prompt}$" { + -re ".*\r\n$mi_gdb_prompt$" { fail "$test (unknown output after running)" return -1 } @@ -1337,9 +1382,16 @@ proc mi_continue_to_line {location test} { proc mi_get_stop_line {test} { global mi_gdb_prompt + global async + + if {$async} { + set prompt_re "" + } else { + set prompt_re "$mi_gdb_prompt" + } gdb_expect { - -re ".*line=\"(.*)\".*\r\n$mi_gdb_prompt$" { + -re ".*line=\"(.*)\".*\r\n$prompt_re$" { return $expect_out(1,string) } -re ".*$mi_gdb_prompt$" { diff --git a/gdb/utils.c b/gdb/utils.c index d9953a0..fa8e455 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -277,6 +277,33 @@ make_cleanup_free_section_addr_info (struct section_addr_info *addrs) return make_my_cleanup (&cleanup_chain, do_free_section_addr_info, addrs); } +struct restore_integer_closure +{ + int *variable; + int value; +}; + +static void +restore_integer (void *p) +{ + struct restore_integer_closure *closure = p; + *(closure->variable) = closure->value; + xfree (closure); +} + +/* Assign VALUE to *VARIABLE and arranges for the old value to + be restored via cleanup. */ +struct cleanup * +make_cleanup_restore_integer (int *variable, int value) +{ + struct restore_integer_closure *c = + xmalloc (sizeof (struct restore_integer_closure)); + struct cleanup *cleanup = make_cleanup (restore_integer, (void *) c); + c->variable = variable; + c->value = *variable; + *variable = value; + return cleanup; +} struct cleanup * make_my_cleanup (struct cleanup **pmy_chain, make_cleanup_ftype *function, -- 1.5.3.5 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Use observers to report stop events. 2008-04-11 20:05 [RFA] Use observers to report stop events Vladimir Prus @ 2008-04-24 13:47 ` Vladimir Prus 2008-04-29 8:03 ` Joel Brobecker 1 sibling, 0 replies; 14+ messages in thread From: Vladimir Prus @ 2008-04-24 13:47 UTC (permalink / raw) To: gdb-patches, Daniel Jacobowitz Vladimir Prus wrote: > > Presently, the *stopped async output, despite been documented as async, > is actually output during processing of MI command. Naturally, this is > not going to work in non-stop mode, since there may be more stops that > there were MI commands. This patches makes the *stopped output be printed > via observer. Presently, there's one regression in mi tests -- namely the > mi-async.exp test -- which uses hardcoded tests for *stopped and so > fails after update as predicated :-) I'll fix this up later; this is a local > change and should not affect the review of the patch itself. > > Note that there are no current uses of the normal_stop observer, so this > patch is as safe as it can get :-) Ping? > - Volodya > > * defs.h (make_cleanup_restore_integer): New declaration. > * utils.c (restore_integer_closure, restore_integer) > (make_cleanup_restore_integer): New. > * breakpoint.c (restore_always_inserted_mode): Remove. > (update_breakpoints_after_exec): Use make_cleanup_restore_integer. > > * inferior.h (suppress_normal_stop_observer): New. > * infcall.c (call_function_by_hand): Disable stop events when > doing function calls. > * infmcd.c (suppress_normal_stop_observer): New. > (finish_command_continuation): Call normal_stop observer > explicitly. > (finish_command): Disable stop events inside proceed. > * infrun.c (normal_stop): Don't call normal stop observer if > suppressed of if multi-step is in progress. > > * interps.h (top_level_interpreter): New. > * interps.c (top_level_interpreter): Rename to > top_level_interpreter_ptr. > (top_level_interpreter): New. > > * mi/mi-interp.c (mi_on_normal_stop): New. > (mi_interpreter_init): Register mi_on_normal_stop. > (mi_interpreter_exec_continuation): Remove. > (mi_cmd_interpreter_exec): Don't register the above. > * mi/mi-main.c (captured_mi_execute_command): Don't care > about sync_execution. > (mi_execute_async_cli_command): Don't install continuation. Don't > print *stopped. > (mi_exec_async_cli_cmd_continuation): Remove. > > [gdb/testsuite] > * gdb.mi/mi-break.exp (test_ignore_count): Adjust stopped pattern. > * gdb.mi/mi-syn-frame.exp: Use mi_expect_stop instead of direct > testing of stopped. > * gdb.mi/mi2-syn-frame.exp: Likewise. > * lib/mi-support.exp (default_mi_gdb_start): Call detect_async. > (async, detect_async): New. > (mi_expect_stop, mi_continue_to_line): Adjust expectation > depending on if we're running in sync or async mode. > --- > gdb/breakpoint.c | 10 +---- > gdb/defs.h | 2 + > gdb/infcall.c | 6 ++- > gdb/infcmd.c | 16 +++++-- > gdb/inferior.h | 3 + > gdb/infrun.c | 3 +- > gdb/interps.c | 17 ++++++-- > gdb/interps.h | 2 + > gdb/mi/mi-interp.c | 43 ++++++++++----------- > gdb/mi/mi-main.c | 36 +---------------- > gdb/testsuite/gdb.mi/mi-break.exp | 2 +- > gdb/testsuite/gdb.mi/mi-syn-frame.exp | 6 +-- > gdb/testsuite/gdb.mi/mi2-syn-frame.exp | 25 +++--------- > gdb/testsuite/lib/mi-support.exp | 64 +++++++++++++++++++++++++++++--- > gdb/utils.c | 27 +++++++++++++ > 15 files changed, 157 insertions(+), 105 deletions(-) > > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c > index d220d00..537b9bf 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -1448,12 +1448,6 @@ reattach_breakpoints (int pid) > return 0; > } > > -static void > -restore_always_inserted_mode (void *p) > -{ > - always_inserted_mode = (uintptr_t) p; > -} > - > void > update_breakpoints_after_exec (void) > { > @@ -1469,9 +1463,7 @@ update_breakpoints_after_exec (void) > /* The binary we used to debug is now gone, and we're updating > breakpoints for the new binary. Until we're done, we should not > try to insert breakpoints. */ > - cleanup = make_cleanup (restore_always_inserted_mode, > - (void *) (uintptr_t) always_inserted_mode); > - always_inserted_mode = 0; > + cleanup = make_cleanup_restore_integer (&always_inserted_mode, 0); > > ALL_BREAKPOINTS_SAFE (b, temp) > { > diff --git a/gdb/defs.h b/gdb/defs.h > index 5c35051..a632840 100644 > --- a/gdb/defs.h > +++ b/gdb/defs.h > @@ -345,6 +345,8 @@ extern struct cleanup *make_cleanup_close (int fd); > > extern struct cleanup *make_cleanup_bfd_close (bfd *abfd); > > +extern struct cleanup *make_cleanup_restore_integer (int *variable, int value); > + > extern struct cleanup *make_final_cleanup (make_cleanup_ftype *, void *); > > extern struct cleanup *make_my_cleanup (struct cleanup **, > diff --git a/gdb/infcall.c b/gdb/infcall.c > index 721b32d..d879b79 100644 > --- a/gdb/infcall.c > +++ b/gdb/infcall.c > @@ -706,6 +706,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args) > > { > struct cleanup *old_cleanups = make_cleanup (null_cleanup, 0); > + struct cleanup *old_cleanups2; > int saved_async = 0; > > /* If all error()s out of proceed ended up calling normal_stop > @@ -718,8 +719,11 @@ call_function_by_hand (struct value *function, int nargs, struct value > **args) > > if (target_can_async_p ()) > saved_async = target_async_mask (0); > - > + > + old_cleanups2 = make_cleanup_restore_integer > + (&suppress_normal_stop_observer, 1); > proceed (real_pc, TARGET_SIGNAL_0, 0); > + do_cleanups (old_cleanups2); > > if (saved_async) > target_async_mask (saved_async); > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index 2823259..c787f95 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -202,6 +202,9 @@ int step_multi; > in format described in environ.h. */ > > struct gdb_environ *inferior_environ; > + > +/* When set, normal_stop will not call the normal_stop observer. */ > +int suppress_normal_stop_observer = 0; > > /* Accessor routines. */ > > @@ -1275,9 +1278,14 @@ finish_command_continuation (struct continuation_arg *arg, int error) > if (TYPE_CODE (value_type) != TYPE_CODE_VOID) > print_return_value (value_type); > } > + > + /* We suppress normal call of normal_stop observer and do it here so that > + that *stopped notification includes the return value. */ > + observer_notify_normal_stop (stop_bpstat); > } > > - delete_breakpoint (breakpoint); > + suppress_normal_stop_observer = 0; > + delete_breakpoint (breakpoint); > } > > /* "finish": Set a temporary breakpoint at the place the selected > @@ -1343,6 +1351,7 @@ finish_command (char *arg, int from_tty) > } > > proceed_to_finish = 1; /* We want stop_registers, please... */ > + make_cleanup_restore_integer (&suppress_normal_stop_observer, 1); > proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0); > > arg1 = > @@ -1358,10 +1367,7 @@ finish_command (char *arg, int from_tty) > arg2->data.pointer = function; > arg3->data.pointer = old_chain; > add_continuation (finish_command_continuation, arg1); > - > - /* Do this only if not running asynchronously or if the target > - cannot do async execution. Otherwise, complete this command when > - the target actually stops, in fetch_inferior_event. */ > + > discard_cleanups (old_chain); > if (!target_can_async_p ()) > do_all_continuations (0); > diff --git a/gdb/inferior.h b/gdb/inferior.h > index 3aaaa26..630cc52 100644 > --- a/gdb/inferior.h > +++ b/gdb/inferior.h > @@ -399,6 +399,9 @@ extern int debug_displaced; > void displaced_step_dump_bytes (struct ui_file *file, > const gdb_byte *buf, size_t len); > > + > +/* When set, normal_stop will not call the normal_stop observer. */ > +extern int suppress_normal_stop_observer; > > /* Possible values for gdbarch_call_dummy_location. */ > #define ON_STACK 1 > diff --git a/gdb/infrun.c b/gdb/infrun.c > index 7f151f7..d6b78ea 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -3635,7 +3635,8 @@ Further execution is probably impossible.\n")); > > done: > annotate_stopped (); > - observer_notify_normal_stop (stop_bpstat); > + if (!suppress_normal_stop_observer && !step_multi) > + observer_notify_normal_stop (stop_bpstat); > } > > static int > diff --git a/gdb/interps.c b/gdb/interps.c > index 9d47843..936fe89 100644 > --- a/gdb/interps.c > +++ b/gdb/interps.c > @@ -81,7 +81,7 @@ void _initialize_interpreter (void); > > static struct interp *interp_list = NULL; > static struct interp *current_interpreter = NULL; > -static struct interp *top_level_interpreter = NULL; > +static struct interp *top_level_interpreter_ptr = NULL; > > static int interpreter_initialized = 0; > > @@ -144,7 +144,7 @@ interp_set (struct interp *interp, int top_level) > /* If we already have an interpreter, then trying to > set top level interpreter is kinda pointless. */ > gdb_assert (!top_level || !current_interpreter); > - gdb_assert (!top_level || !top_level_interpreter); > + gdb_assert (!top_level || !top_level_interpreter_ptr); > > if (current_interpreter != NULL) > { > @@ -165,7 +165,7 @@ interp_set (struct interp *interp, int top_level) > > current_interpreter = interp; > if (top_level) > - top_level_interpreter = interp; > + top_level_interpreter_ptr = interp; > > /* We use interpreter_p for the "set interpreter" variable, so we need > to make sure we have a malloc'ed copy for the set command to free. */ > @@ -476,11 +476,18 @@ interpreter_completer (char *text, char *word) > return matches; > } > > +struct interp * > +top_level_interpreter (void) > +{ > + gdb_assert (top_level_interpreter_ptr); > + return top_level_interpreter_ptr; > +} > + > extern void * > top_level_interpreter_data (void) > { > - gdb_assert (top_level_interpreter); > - return top_level_interpreter->data; > + gdb_assert (top_level_interpreter_ptr); > + return top_level_interpreter_ptr->data; > } > > /* This just adds the "interpreter-exec" command. */ > diff --git a/gdb/interps.h b/gdb/interps.h > index e1030fa..5e91080 100644 > --- a/gdb/interps.h > +++ b/gdb/interps.h > @@ -64,6 +64,8 @@ extern struct ui_out *interp_ui_out (struct interp *interp); > extern int current_interp_named_p (const char *name); > extern int current_interp_display_prompt_p (void); > extern void current_interp_command_loop (void); > +/* Returns the top-level interpreter. */ > +extern struct interp *top_level_interpreter (); > /* Returns opaque data associated with the top-level interpreter. */ > extern void *top_level_interpreter_data (void); > > diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c > index ae9e07b..e874a56 100644 > --- a/gdb/mi/mi-interp.c > +++ b/gdb/mi/mi-interp.c > @@ -65,6 +65,7 @@ static void mi1_command_loop (void); > > static void mi_insert_notify_hooks (void); > static void mi_remove_notify_hooks (void); > +static void mi_on_normal_stop (struct bpstats *bs); > > static void mi_new_thread (struct thread_info *t); > > @@ -88,7 +89,10 @@ mi_interpreter_init (int top_level) > mi->event_channel = mi_console_file_new (raw_stdout, "=", 0); > > if (top_level) > - observer_attach_new_thread (mi_new_thread); > + { > + observer_attach_new_thread (mi_new_thread); > + observer_attach_normal_stop (mi_on_normal_stop); > + } > > return mi; > } > @@ -167,26 +171,6 @@ mi_interpreter_prompt_p (void *data) > return 0; > } > > -static void > -mi_interpreter_exec_continuation (struct continuation_arg *arg, int error) > -{ > - bpstat_do_actions (&stop_bpstat); > - /* It's not clear what to do in the case of errror -- should we assume that > - the target is stopped, or that it still runs? */ > - if (!target_executing) > - { > - fputs_unfiltered ("*stopped", raw_stdout); > - mi_out_put (uiout, raw_stdout); > - fputs_unfiltered ("\n", raw_stdout); > - fputs_unfiltered ("(gdb) \n", raw_stdout); > - gdb_flush (raw_stdout); > - } > - else if (target_can_async_p ()) > - { > - add_continuation (mi_interpreter_exec_continuation, NULL); > - } > -} > - > enum mi_cmd_result > mi_cmd_interpreter_exec (char *command, char **argv, int argc) > { > @@ -237,7 +221,6 @@ mi_cmd_interpreter_exec (char *command, char **argv, int argc) > if (target_can_async_p () && target_executing) > { > fputs_unfiltered ("^running\n", raw_stdout); > - add_continuation (mi_interpreter_exec_continuation, NULL); > } > > if (mi_error_message != NULL) > @@ -317,6 +300,22 @@ mi_new_thread (struct thread_info *t) > gdb_flush (mi->event_channel); > } > > +static void > +mi_on_normal_stop (struct bpstats *bs) > +{ > + /* Since this can be called when CLI command is executing, > + using cli interpreter, be sure to use MI uiout for output, > + not the current one. */ > + struct ui_out *uiout = interp_ui_out (top_level_interpreter ()); > + struct mi_interp *mi = top_level_interpreter_data (); > + > + fputs_unfiltered ("*stopped", raw_stdout); > + mi_out_put (uiout, raw_stdout); > + mi_out_rewind (uiout); > + fputs_unfiltered ("\n", raw_stdout); > + gdb_flush (raw_stdout); > +} > + > extern initialize_file_ftype _initialize_mi_interp; /* -Wmissing-prototypes */ > > void > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c > index 177d0ec..deb2df9 100644 > --- a/gdb/mi/mi-main.c > +++ b/gdb/mi/mi-main.c > @@ -102,9 +102,6 @@ static void mi_execute_cli_command (const char *cmd, int args_p, > const char *args); > static enum mi_cmd_result mi_execute_async_cli_command (char *mi, char *args, int from_tty); > > -static void mi_exec_async_cli_cmd_continuation (struct continuation_arg *arg, > - int error); > - > static int register_changed_p (int regnum, struct regcache *, > struct regcache *); > static void get_register (int regnum, int format); > @@ -1067,15 +1064,11 @@ captured_mi_execute_command (struct ui_out *uiout, void *data) > fputs_unfiltered ("\n", raw_stdout); > } > else > + /* The command does not want anything to be printed. In that > + case, the command probably should not have written anything > + to uiout, but in case it has written something, discard it. */ > mi_out_rewind (uiout); > } > - else if (sync_execution) > - { > - /* Don't print the prompt. We are executing the target in > - synchronous mode. */ > - args->action = EXECUTE_COMMAND_SUPPRESS_PROMPT; > - return; > - } > break; > > case CLI_COMMAND: > @@ -1296,12 +1289,6 @@ mi_execute_async_cli_command (char *mi, char *args, int from_tty) > fputs_unfiltered (current_token, raw_stdout); > fputs_unfiltered ("^running\n", raw_stdout); > > - /* Ideally, we should be intalling continuation only when > - the target is already running. However, this will break right now, > - because continuation installed by the 'finish' command must be after > - the continuation that prints *stopped. This issue will be > - fixed soon. */ > - add_continuation (mi_exec_async_cli_cmd_continuation, NULL); > } > > execute_command ( /*ui */ run, 0 /*from_tty */ ); > @@ -1317,31 +1304,14 @@ mi_execute_async_cli_command (char *mi, char *args, int from_tty) > /* Do this before doing any printing. It would appear that some > print code leaves garbage around in the buffer. */ > do_cleanups (old_cleanups); > - /* If the target was doing the operation synchronously we fake > - the stopped message. */ > - fputs_unfiltered ("*stopped", raw_stdout); > - mi_out_put (uiout, raw_stdout); > - mi_out_rewind (uiout); > if (do_timings) > print_diff_now (current_command_ts); > - fputs_unfiltered ("\n", raw_stdout); > return MI_CMD_QUIET; > } > return MI_CMD_DONE; > } > > void > -mi_exec_async_cli_cmd_continuation (struct continuation_arg *arg, int error) > -{ > - /* Assume 'error' means that target is stopped, too. */ > - fputs_unfiltered ("*stopped", raw_stdout); > - mi_out_put (uiout, raw_stdout); > - fputs_unfiltered ("\n", raw_stdout); > - fputs_unfiltered ("(gdb) \n", raw_stdout); > - gdb_flush (raw_stdout); > -} > - > -void > mi_load_progress (const char *section_name, > unsigned long sent_so_far, > unsigned long total_section, > diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp > index c0f5132..e4fd5c9 100644 > --- a/gdb/testsuite/gdb.mi/mi-break.exp > +++ b/gdb/testsuite/gdb.mi/mi-break.exp > @@ -163,7 +163,7 @@ proc test_ignore_count {} { > mi_run_cmd > > gdb_expect { > - -re ".*func=\"callme\".*args=\\\[\{name=\"i\",value=\"2\"\}\\\].*\r\n$mi_gdb_prompt$" { > + -re > ".*\\*stopped.*func=\"callme\".*args=\\\[\{name=\"i\",value=\"2\"\}\\\].*\r\n($mi_gdb_prompt)?$" { > pass "run to breakpoint with ignore count" > } > -re ".*$mi_gdb_prompt$" { > diff --git a/gdb/testsuite/gdb.mi/mi-syn-frame.exp b/gdb/testsuite/gdb.mi/mi-syn-frame.exp > index ed89965..80d36c9 100644 > --- a/gdb/testsuite/gdb.mi/mi-syn-frame.exp > +++ b/gdb/testsuite/gdb.mi/mi-syn-frame.exp > @@ -60,9 +60,7 @@ mi_gdb_test "403-exec-continue" \ > "403\\^running" \ > "testing exec continue" > > -# Presently, the *stopped notification for this case does not include > -# any information. This can be considered a bug. > -mi_gdb_test "" "\\*stopped" "finished exec continue" > +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" > > mi_gdb_test "404-stack-list-frames 0 0" \ > "404\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" > \ > @@ -91,7 +89,7 @@ mi_gdb_test "407-stack-list-frames" \ > > mi_gdb_test "408-exec-continue" "408\\^running" > > -mi_gdb_test "" ".*\\*stopped.*" "finished exec continue" > +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" > > mi_gdb_test "409-stack-list-frames 0 0" \ > "409\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" > \ > diff --git a/gdb/testsuite/gdb.mi/mi2-syn-frame.exp b/gdb/testsuite/gdb.mi/mi2-syn-frame.exp > index 9c1daab..6e9792a 100644 > --- a/gdb/testsuite/gdb.mi/mi2-syn-frame.exp > +++ b/gdb/testsuite/gdb.mi/mi2-syn-frame.exp > @@ -58,15 +58,11 @@ mi_gdb_test "402-stack-list-frames" > "402\\^done,stack=\\\[frame=\{level=\"0\",ad > # Continue back to main() > # > > -send_gdb "403-exec-continue\n" > -gdb_expect { > - -re "403\\^running\[\r\n\]+${my_mi_gdb_prompt}.*\\\*stopped\[\r\n\]+${my_mi_gdb_prompt}$" { > - pass "403-exec-continue" > - } > - timeout { > - fail "403-exec-continue" > - } > -} > +mi_gdb_test "403-exec-continue" \ > + "403\\^running" \ > + "testing exec continue" > + > +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" > > mi_gdb_test "404-stack-list-frames 0 0" \ > "404\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" > \ > @@ -92,16 +88,9 @@ mi_gdb_test "407-stack-list-frames" \ > "407\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"subroutine\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"handler\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"<signal > handler > called>\"\},.*frame=\{level=\"$decimal\",addr=\"$hex\",func=\"have_a_very_merry_interrupt\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"<function > called from > gdb>\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]" > \ "list stack frames" > > +mi_gdb_test "408-exec-continue" "408\\^running" > > -send_gdb "408-exec-continue\n" > -gdb_expect { > - -re "408\\^running\[\r\n\]+${my_mi_gdb_prompt}.*\\\*stopped\[\r\n\]+${my_mi_gdb_prompt}$" { > - pass "408-exec-continue" > - } > - timeout { > - fail "408-exec-continue" > - } > -} > +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" > > mi_gdb_test "409-stack-list-frames 0 0" \ > "409\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" > \ > diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp > index 5926f16..0a6eb8a 100644 > --- a/gdb/testsuite/lib/mi-support.exp > +++ b/gdb/testsuite/lib/mi-support.exp > @@ -227,6 +227,8 @@ proc default_mi_gdb_start { args } { > } > } > > + detect_async > + > return 0; > } > > @@ -911,6 +913,30 @@ proc mi_step { test } { > return [mi_step_to {.*} {.*} {.*} {.*} $test] > } > > +set async "unknown" > + > +proc detect_async {} { > + global async > + global mi_gdb_prompt > + > + if { $async == "unknown" } { > + send_gdb "maint show linux-async\n" > + > + gdb_expect { > + -re ".*Controlling the GNU/Linux inferior in asynchronous mode is on...*$mi_gdb_prompt$" { > + set async 1 > + } > + -re ".*$mi_gdb_prompt$" { > + set async 0 > + } > + timeout { > + set async 0 > + } > + } > + } > + return $async > +} > + > # Wait for MI *stopped notification to appear. > # The REASON, FUNC, ARGS, FILE and LINE are regular expressions > # to match against whatever is output in *stopped. ARGS should > @@ -933,6 +959,7 @@ proc mi_expect_stop { reason func args file line extra test } { > global hex > global decimal > global fullname_syntax > + global async > > set after_stopped "" > set after_reason "" > @@ -944,10 +971,28 @@ proc mi_expect_stop { reason func args file line extra test } { > set after_stopped [lindex $extra 0] > } > > + if {$async} { > + set prompt_re "" > + } else { > + set prompt_re "$mi_gdb_prompt" > + } > + > + if { $reason == "really-no-reason" } { > + gdb_expect { > + -re "\\*stopped\r\n$prompt_re$" { > + pass "$test" > + } > + timeout { > + fail "$test (unknown output after running)" > + } > + } > + return > + } > + > if { $reason == "exited-normally" } { > > gdb_expect { > - -re "\\*stopped,reason=\"exited-normally\"\r\n$mi_gdb_prompt$" { > + -re "\\*stopped,reason=\"exited-normally\"\r\n$prompt_re$" { > pass "$test" > } > -re ".*$mi_gdb_prompt$" {fail "continue to end (2)"} > @@ -970,17 +1015,17 @@ proc mi_expect_stop { reason func args file line extra test } { > set r "reason=\"$reason\"," > } > > - verbose -log "mi_expect_stop: expecting: > .*\\*stopped ${r}${bn}${after_reason}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}$after_stopped\r\n$mi_gdb_prompt$" > + verbose -log "mi_expect_stop: expecting: > .*\\*stopped ${r}${bn}${after_reason}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}$after_stopped\r\n$prompt_re$" > gdb_expect { > - -re > ".*\\*stopped ${r}${bn}${after_reason}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n$mi_gdb_prompt$" > { > + -re > ".*\\*stopped ${r}${bn}${after_reason}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n$prompt_re$" > { > pass "$test" > return $expect_out(2,string) > } > - -re > ".*\\*stopped ${r}${bn}${after_reason}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\".*\",args=\[\\\[\{\].*\[\\\]\}\],file=\".*\",fullname=\"${fullname_syntax}.*\",line=\"\[0-9\]*\"\}.*\r\n$mi_gdb_prompt$" > { > + -re > ".*\\*stopped ${r}${bn}${after_reason}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\".*\",args=\[\\\[\{\].*\[\\\]\}\],file=\".*\",fullname=\"${fullname_syntax}.*\",line=\"\[0-9\]*\"\}.*\r\n$prompt_re$" > { > fail "$test (stopped at wrong place)" > return -1 > } > - -re ".*\r\n${mi_gdb_prompt}$" { > + -re ".*\r\n$mi_gdb_prompt$" { > fail "$test (unknown output after running)" > return -1 > } > @@ -1337,9 +1382,16 @@ proc mi_continue_to_line {location test} { > proc mi_get_stop_line {test} { > > global mi_gdb_prompt > + global async > + > + if {$async} { > + set prompt_re "" > + } else { > + set prompt_re "$mi_gdb_prompt" > + } > > gdb_expect { > - -re ".*line=\"(.*)\".*\r\n$mi_gdb_prompt$" { > + -re ".*line=\"(.*)\".*\r\n$prompt_re$" { > return $expect_out(1,string) > } > -re ".*$mi_gdb_prompt$" { > diff --git a/gdb/utils.c b/gdb/utils.c > index d9953a0..fa8e455 100644 > --- a/gdb/utils.c > +++ b/gdb/utils.c > @@ -277,6 +277,33 @@ make_cleanup_free_section_addr_info (struct section_addr_info *addrs) > return make_my_cleanup (&cleanup_chain, do_free_section_addr_info, addrs); > } > > +struct restore_integer_closure > +{ > + int *variable; > + int value; > +}; > + > +static void > +restore_integer (void *p) > +{ > + struct restore_integer_closure *closure = p; > + *(closure->variable) = closure->value; > + xfree (closure); > +} > + > +/* Assign VALUE to *VARIABLE and arranges for the old value to > + be restored via cleanup. */ > +struct cleanup * > +make_cleanup_restore_integer (int *variable, int value) > +{ > + struct restore_integer_closure *c = > + xmalloc (sizeof (struct restore_integer_closure)); > + struct cleanup *cleanup = make_cleanup (restore_integer, (void *) c); > + c->variable = variable; > + c->value = *variable; > + *variable = value; > + return cleanup; > +} > > struct cleanup * > make_my_cleanup (struct cleanup **pmy_chain, make_cleanup_ftype *function, ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Use observers to report stop events. 2008-04-11 20:05 [RFA] Use observers to report stop events Vladimir Prus 2008-04-24 13:47 ` Vladimir Prus @ 2008-04-29 8:03 ` Joel Brobecker 2008-04-29 18:30 ` Vladimir Prus 1 sibling, 1 reply; 14+ messages in thread From: Joel Brobecker @ 2008-04-29 8:03 UTC (permalink / raw) To: Vladimir Prus; +Cc: gdb-patches Volodya, > * defs.h (make_cleanup_restore_integer): New declaration. > * utils.c (restore_integer_closure, restore_integer) > (make_cleanup_restore_integer): New. > * breakpoint.c (restore_always_inserted_mode): Remove. > (update_breakpoints_after_exec): Use make_cleanup_restore_integer. > > * inferior.h (suppress_normal_stop_observer): New. > * infcall.c (call_function_by_hand): Disable stop events when > doing function calls. > * infmcd.c (suppress_normal_stop_observer): New. > (finish_command_continuation): Call normal_stop observer > explicitly. > (finish_command): Disable stop events inside proceed. > * infrun.c (normal_stop): Don't call normal stop observer if > suppressed of if multi-step is in progress. > > * interps.h (top_level_interpreter): New. > * interps.c (top_level_interpreter): Rename to > top_level_interpreter_ptr. > (top_level_interpreter): New. > > * mi/mi-interp.c (mi_on_normal_stop): New. > (mi_interpreter_init): Register mi_on_normal_stop. > (mi_interpreter_exec_continuation): Remove. > (mi_cmd_interpreter_exec): Don't register the above. > * mi/mi-main.c (captured_mi_execute_command): Don't care > about sync_execution. > (mi_execute_async_cli_command): Don't install continuation. Don't > print *stopped. > (mi_exec_async_cli_cmd_continuation): Remove. I tried to have a look at your patch, but I couldn't get into it within the short amount of time that I have today. What I did notice is that it contains several changes that could be made independent. For instance, the make_cleanup_restore_integer/restore_always_inserted_mode part could be introduced separately (honestly, this part looks a little scary as you will leak memory is someone cancels the cleanup - so far, I think the usual practice is to have one make_cleanup_bla_bla_bla that specially restores your variable). I need to document myself about the "*stopped async output" because I didn't quite get the idea of the patch. But if I had known that this patch had some MI-logic to it, I'd probably have stayed away from it. I seem to find more excitement in other parts of GDB... If no one else gets to it, I'll see I can find some time later in the week or next week to try again, but it would definitely help to see this patch broken down into smaller pieces. -- Joel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Use observers to report stop events. 2008-04-29 8:03 ` Joel Brobecker @ 2008-04-29 18:30 ` Vladimir Prus 2008-05-01 19:58 ` Daniel Jacobowitz 0 siblings, 1 reply; 14+ messages in thread From: Vladimir Prus @ 2008-04-29 18:30 UTC (permalink / raw) To: Joel Brobecker, gdb-patches [-- Attachment #1: Type: text/plain, Size: 3394 bytes --] On Tuesday 29 April 2008 07:14:11 you wrote: > Volodya, > > > * defs.h (make_cleanup_restore_integer): New declaration. > > * utils.c (restore_integer_closure, restore_integer) > > (make_cleanup_restore_integer): New. > > * breakpoint.c (restore_always_inserted_mode): Remove. > > (update_breakpoints_after_exec): Use make_cleanup_restore_integer. > > > > * inferior.h (suppress_normal_stop_observer): New. > > * infcall.c (call_function_by_hand): Disable stop events when > > doing function calls. > > * infmcd.c (suppress_normal_stop_observer): New. > > (finish_command_continuation): Call normal_stop observer > > explicitly. > > (finish_command): Disable stop events inside proceed. > > * infrun.c (normal_stop): Don't call normal stop observer if > > suppressed of if multi-step is in progress. > > > > * interps.h (top_level_interpreter): New. > > * interps.c (top_level_interpreter): Rename to > > top_level_interpreter_ptr. > > (top_level_interpreter): New. > > > > * mi/mi-interp.c (mi_on_normal_stop): New. > > (mi_interpreter_init): Register mi_on_normal_stop. > > (mi_interpreter_exec_continuation): Remove. > > (mi_cmd_interpreter_exec): Don't register the above. > > * mi/mi-main.c (captured_mi_execute_command): Don't care > > about sync_execution. > > (mi_execute_async_cli_command): Don't install continuation. Don't > > print *stopped. > > (mi_exec_async_cli_cmd_continuation): Remove. > > I tried to have a look at your patch, but I couldn't get into it > within the short amount of time that I have today. What I did notice > is that it contains several changes that could be made independent. > For instance, the make_cleanup_restore_integer/restore_always_inserted_mode > part could be introduced separately (honestly, this part looks a little > scary as you will leak memory is someone cancels the cleanup - so far, > I think the usual practice is to have one make_cleanup_bla_bla_bla > that specially restores your variable). > > I need to document myself about the "*stopped async output" because > I didn't quite get the idea of the patch. But if I had known that this > patch had some MI-logic to it, I'd probably have stayed away from it. > I seem to find more excitement in other parts of GDB... If no one else > gets to it, I'll see I can find some time later in the week or next week > to try again, but it would definitely help to see this patch broken down > into smaller pieces. Here are 3 independent bits. 1. Introduce the make_cleanup_restore_integer function. You're right that it can lead to bad results if one discards this cleanup, but then one should be careful with discarding cleanups anyway. 2. Modify the normal_stop observer not to fire in some cases. One case is when doing function call -- we don't announce the stop in CLI and for similar reason we don't have observer to be called. Also, for the benefit of next patch, we want the call to observer to be delayed until we print function return value, if we're doing finish. 3. The MI patch itself. Note that it does not seen an approval -- I can self-approve it. The point of the patch is to report target stop in MI immediately when a stop in some thread is detected by gdb. Currently, MI reports target stop only when it's done doing some command, which is incompatible with idea that target can stop many times per one MI command. Thanks, Volodya [-- Attachment #2: 1.diff --] [-- Type: text/x-diff, Size: 2840 bytes --] commit caf21ec5b9503db75f40d58de005d393da9c10af Author: Vladimir Prus <vladimir@codesourcery.com> Date: Tue Apr 29 21:31:59 2008 +0400 Introduce common cleanup for restoring integers. * defs.h (make_cleanup_restore_integer): New declaration. * utils.c (restore_integer_closure, restore_integer) (make_cleanup_restore_integer): New. * breakpoint.c (restore_always_inserted_mode): Remove. (update_breakpoints_after_exec): Use make_cleanup_restore_integer. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 4180ea1..af92d47 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1456,12 +1456,6 @@ reattach_breakpoints (int pid) return 0; } -static void -restore_always_inserted_mode (void *p) -{ - always_inserted_mode = (uintptr_t) p; -} - void update_breakpoints_after_exec (void) { @@ -1477,9 +1471,7 @@ update_breakpoints_after_exec (void) /* The binary we used to debug is now gone, and we're updating breakpoints for the new binary. Until we're done, we should not try to insert breakpoints. */ - cleanup = make_cleanup (restore_always_inserted_mode, - (void *) (uintptr_t) always_inserted_mode); - always_inserted_mode = 0; + cleanup = make_cleanup_restore_integer (&always_inserted_mode, 0); ALL_BREAKPOINTS_SAFE (b, temp) { diff --git a/gdb/defs.h b/gdb/defs.h index 0fa0e6c..7966967 100644 --- a/gdb/defs.h +++ b/gdb/defs.h @@ -345,6 +345,8 @@ extern struct cleanup *make_cleanup_close (int fd); extern struct cleanup *make_cleanup_bfd_close (bfd *abfd); +extern struct cleanup *make_cleanup_restore_integer (int *variable, int value); + extern struct cleanup *make_final_cleanup (make_cleanup_ftype *, void *); extern struct cleanup *make_my_cleanup (struct cleanup **, diff --git a/gdb/utils.c b/gdb/utils.c index d9953a0..fa8e455 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -277,6 +277,33 @@ make_cleanup_free_section_addr_info (struct section_addr_info *addrs) return make_my_cleanup (&cleanup_chain, do_free_section_addr_info, addrs); } +struct restore_integer_closure +{ + int *variable; + int value; +}; + +static void +restore_integer (void *p) +{ + struct restore_integer_closure *closure = p; + *(closure->variable) = closure->value; + xfree (closure); +} + +/* Assign VALUE to *VARIABLE and arranges for the old value to + be restored via cleanup. */ +struct cleanup * +make_cleanup_restore_integer (int *variable, int value) +{ + struct restore_integer_closure *c = + xmalloc (sizeof (struct restore_integer_closure)); + struct cleanup *cleanup = make_cleanup (restore_integer, (void *) c); + c->variable = variable; + c->value = *variable; + *variable = value; + return cleanup; +} struct cleanup * make_my_cleanup (struct cleanup **pmy_chain, make_cleanup_ftype *function, [-- Attachment #3: 3.diff --] [-- Type: text/x-diff, Size: 15686 bytes --] commit 951a6d036d8808086d11496dd46f7eba6f27fb98 Author: Vladimir Prus <vladimir@codesourcery.com> Date: Mon Mar 10 17:52:40 2008 +0300 Use observers to report stop events in MI. * mi/mi-interp.c (mi_on_normal_stop): New. (mi_interpreter_init): Register mi_on_normal_stop. (mi_interpreter_exec_continuation): Remove. (mi_cmd_interpreter_exec): Don't register the above. * mi/mi-main.c (captured_mi_execute_command): Don't care about sync_execution. (mi_execute_async_cli_command): Don't install continuation. Don't print *stopped. (mi_exec_async_cli_cmd_continuation): Remove. [gdb/testsuite] * gdb.mi/mi-break.exp (test_ignore_count): Adjust stopped pattern. * gdb.mi/mi-syn-frame.exp: Use mi_expect_stop instead of direct testing of stopped. * gdb.mi/mi2-syn-frame.exp: Likewise. * lib/mi-support.exp (default_mi_gdb_start): Call detect_async. (async, detect_async): New. (mi_expect_stop, mi_continue_to_line): Adjust expectation depending on if we're running in sync or async mode. diff --git a/gdb/infcmd.c b/gdb/infcmd.c index a0980c7..37ca2bb 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -1371,10 +1371,7 @@ finish_command (char *arg, int from_tty) arg2->data.pointer = function; arg3->data.pointer = old_chain; add_continuation (finish_command_continuation, arg1); - - /* Do this only if not running asynchronously or if the target - cannot do async execution. Otherwise, complete this command when - the target actually stops, in fetch_inferior_event. */ + discard_cleanups (old_chain); if (!target_can_async_p ()) do_all_continuations (0); diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index 8b0d909..583c288 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -65,6 +65,7 @@ static void mi1_command_loop (void); static void mi_insert_notify_hooks (void); static void mi_remove_notify_hooks (void); +static void mi_on_normal_stop (struct bpstats *bs); static void mi_new_thread (struct thread_info *t); static void mi_thread_exit (struct thread_info *t); @@ -92,6 +93,7 @@ mi_interpreter_init (int top_level) { observer_attach_new_thread (mi_new_thread); observer_attach_thread_exit (mi_thread_exit); + observer_attach_normal_stop (mi_on_normal_stop); } return mi; @@ -171,26 +173,6 @@ mi_interpreter_prompt_p (void *data) return 0; } -static void -mi_interpreter_exec_continuation (struct continuation_arg *arg, int error_p) -{ - bpstat_do_actions (&stop_bpstat); - /* It's not clear what to do in the case of errror -- should we assume that - the target is stopped, or that it still runs? */ - if (!target_executing) - { - fputs_unfiltered ("*stopped", raw_stdout); - mi_out_put (uiout, raw_stdout); - fputs_unfiltered ("\n", raw_stdout); - fputs_unfiltered ("(gdb) \n", raw_stdout); - gdb_flush (raw_stdout); - } - else if (target_can_async_p ()) - { - add_continuation (mi_interpreter_exec_continuation, NULL); - } -} - enum mi_cmd_result mi_cmd_interpreter_exec (char *command, char **argv, int argc) { @@ -241,7 +223,6 @@ mi_cmd_interpreter_exec (char *command, char **argv, int argc) if (target_can_async_p () && target_executing) { fputs_unfiltered ("^running\n", raw_stdout); - add_continuation (mi_interpreter_exec_continuation, NULL); } if (mi_error_message != NULL) @@ -325,12 +306,27 @@ static void mi_thread_exit (struct thread_info *t) { struct mi_interp *mi = top_level_interpreter_data (); - target_terminal_ours (); fprintf_unfiltered (mi->event_channel, "thread-exited,id=\"%d\"", t->num); gdb_flush (mi->event_channel); } +static void +mi_on_normal_stop (struct bpstats *bs) +{ + /* Since this can be called when CLI command is executing, + using cli interpreter, be sure to use MI uiout for output, + not the current one. */ + struct ui_out *uiout = interp_ui_out (top_level_interpreter ()); + struct mi_interp *mi = top_level_interpreter_data (); + + fputs_unfiltered ("*stopped", raw_stdout); + mi_out_put (uiout, raw_stdout); + mi_out_rewind (uiout); + fputs_unfiltered ("\n", raw_stdout); + gdb_flush (raw_stdout); +} + extern initialize_file_ftype _initialize_mi_interp; /* -Wmissing-prototypes */ void diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 9dce9b0..206b2c9 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -102,10 +102,6 @@ static void mi_execute_cli_command (const char *cmd, int args_p, const char *args); static enum mi_cmd_result mi_execute_async_cli_command (char *cli_command, char **argv, int argc); - -static void mi_exec_async_cli_cmd_continuation (struct continuation_arg *arg, - int error_p); - static int register_changed_p (int regnum, struct regcache *, struct regcache *); static void get_register (int regnum, int format); @@ -1085,15 +1081,11 @@ captured_mi_execute_command (struct ui_out *uiout, void *data) fputs_unfiltered ("\n", raw_stdout); } else + /* The command does not want anything to be printed. In that + case, the command probably should not have written anything + to uiout, but in case it has written something, discard it. */ mi_out_rewind (uiout); } - else if (sync_execution) - { - /* Don't print the prompt. We are executing the target in - synchronous mode. */ - args->action = EXECUTE_COMMAND_SUPPRESS_PROMPT; - return; - } break; case CLI_COMMAND: @@ -1309,12 +1301,6 @@ mi_execute_async_cli_command (char *cli_command, char **argv, int argc) fputs_unfiltered (current_token, raw_stdout); fputs_unfiltered ("^running\n", raw_stdout); - /* Ideally, we should be intalling continuation only when - the target is already running. However, this will break right now, - because continuation installed by the 'finish' command must be after - the continuation that prints *stopped. This issue will be - fixed soon. */ - add_continuation (mi_exec_async_cli_cmd_continuation, NULL); } execute_command ( /*ui */ run, 0 /*from_tty */ ); @@ -1330,31 +1316,14 @@ mi_execute_async_cli_command (char *cli_command, char **argv, int argc) /* Do this before doing any printing. It would appear that some print code leaves garbage around in the buffer. */ do_cleanups (old_cleanups); - /* If the target was doing the operation synchronously we fake - the stopped message. */ - fputs_unfiltered ("*stopped", raw_stdout); - mi_out_put (uiout, raw_stdout); - mi_out_rewind (uiout); if (do_timings) print_diff_now (current_command_ts); - fputs_unfiltered ("\n", raw_stdout); return MI_CMD_QUIET; } return MI_CMD_DONE; } void -mi_exec_async_cli_cmd_continuation (struct continuation_arg *arg, int error_p) -{ - /* Assume 'error' means that target is stopped, too. */ - fputs_unfiltered ("*stopped", raw_stdout); - mi_out_put (uiout, raw_stdout); - fputs_unfiltered ("\n", raw_stdout); - fputs_unfiltered ("(gdb) \n", raw_stdout); - gdb_flush (raw_stdout); -} - -void mi_load_progress (const char *section_name, unsigned long sent_so_far, unsigned long total_section, diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp index f2f5b03..2798569 100644 --- a/gdb/testsuite/gdb.mi/mi-break.exp +++ b/gdb/testsuite/gdb.mi/mi-break.exp @@ -159,7 +159,7 @@ proc test_ignore_count {} { mi_run_cmd gdb_expect { - -re ".*func=\"callme\".*args=\\\[\{name=\"i\",value=\"2\"\}\\\].*\r\n$mi_gdb_prompt$" { + -re ".*\\*stopped.*func=\"callme\".*args=\\\[\{name=\"i\",value=\"2\"\}\\\].*\r\n($mi_gdb_prompt)?$" { pass "run to breakpoint with ignore count" } -re ".*$mi_gdb_prompt$" { diff --git a/gdb/testsuite/gdb.mi/mi-syn-frame.exp b/gdb/testsuite/gdb.mi/mi-syn-frame.exp index 2f2ca02..208678b 100644 --- a/gdb/testsuite/gdb.mi/mi-syn-frame.exp +++ b/gdb/testsuite/gdb.mi/mi-syn-frame.exp @@ -58,9 +58,7 @@ mi_gdb_test "403-exec-continue" \ "403\\^running" \ "testing exec continue" -# Presently, the *stopped notification for this case does not include -# any information. This can be considered a bug. -mi_gdb_test "" "\\*stopped" "finished exec continue" +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" mi_gdb_test "404-stack-list-frames 0 0" \ "404\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" \ @@ -88,7 +86,7 @@ mi_gdb_test "407-stack-list-frames" \ mi_gdb_test "408-exec-continue" "408\\^running" -mi_gdb_test "" ".*\\*stopped.*" "finished exec continue" +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" mi_gdb_test "409-stack-list-frames 0 0" \ "409\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" \ diff --git a/gdb/testsuite/gdb.mi/mi2-syn-frame.exp b/gdb/testsuite/gdb.mi/mi2-syn-frame.exp index c447404..b69812f 100644 --- a/gdb/testsuite/gdb.mi/mi2-syn-frame.exp +++ b/gdb/testsuite/gdb.mi/mi2-syn-frame.exp @@ -56,15 +56,11 @@ mi_gdb_test "402-stack-list-frames" "402\\^done,stack=\\\[frame=\{level=\"0\",ad # Continue back to main() # -send_gdb "403-exec-continue\n" -gdb_expect { - -re "403\\^running\[\r\n\]+${my_mi_gdb_prompt}.*\\\*stopped\[\r\n\]+${my_mi_gdb_prompt}$" { - pass "403-exec-continue" - } - timeout { - fail "403-exec-continue" - } -} +mi_gdb_test "403-exec-continue" \ + "403\\^running" \ + "testing exec continue" + +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" mi_gdb_test "404-stack-list-frames 0 0" \ "404\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" \ @@ -89,16 +85,9 @@ mi_gdb_test "407-stack-list-frames" \ "407\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"subroutine\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"handler\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"<signal handler called>\"\},.*frame=\{level=\"$decimal\",addr=\"$hex\",func=\"have_a_very_merry_interrupt\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]" \ "list stack frames" +mi_gdb_test "408-exec-continue" "408\\^running" -send_gdb "408-exec-continue\n" -gdb_expect { - -re "408\\^running\[\r\n\]+${my_mi_gdb_prompt}.*\\\*stopped\[\r\n\]+${my_mi_gdb_prompt}$" { - pass "408-exec-continue" - } - timeout { - fail "408-exec-continue" - } -} +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" mi_gdb_test "409-stack-list-frames 0 0" \ "409\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" \ diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp index 1f4c3dd..0783936 100644 --- a/gdb/testsuite/lib/mi-support.exp +++ b/gdb/testsuite/lib/mi-support.exp @@ -227,6 +227,8 @@ proc default_mi_gdb_start { args } { } } + detect_async + return 0; } @@ -911,6 +913,30 @@ proc mi_step { test } { return [mi_step_to {.*} {.*} {.*} {.*} $test] } +set async "unknown" + +proc detect_async {} { + global async + global mi_gdb_prompt + + if { $async == "unknown" } { + send_gdb "maint show linux-async\n" + + gdb_expect { + -re ".*Controlling the GNU/Linux inferior in asynchronous mode is on...*$mi_gdb_prompt$" { + set async 1 + } + -re ".*$mi_gdb_prompt$" { + set async 0 + } + timeout { + set async 0 + } + } + } + return $async +} + # Wait for MI *stopped notification to appear. # The REASON, FUNC, ARGS, FILE and LINE are regular expressions # to match against whatever is output in *stopped. ARGS should @@ -933,6 +959,7 @@ proc mi_expect_stop { reason func args file line extra test } { global hex global decimal global fullname_syntax + global async set after_stopped "" set after_reason "" @@ -944,10 +971,28 @@ proc mi_expect_stop { reason func args file line extra test } { set after_stopped [lindex $extra 0] } + if {$async} { + set prompt_re "" + } else { + set prompt_re "$mi_gdb_prompt" + } + + if { $reason == "really-no-reason" } { + gdb_expect { + -re "\\*stopped\r\n$prompt_re$" { + pass "$test" + } + timeout { + fail "$test (unknown output after running)" + } + } + return + } + if { $reason == "exited-normally" } { gdb_expect { - -re "\\*stopped,reason=\"exited-normally\"\r\n$mi_gdb_prompt$" { + -re "\\*stopped,reason=\"exited-normally\"\r\n$prompt_re$" { pass "$test" } -re ".*$mi_gdb_prompt$" {fail "continue to end (2)"} @@ -973,17 +1018,17 @@ proc mi_expect_stop { reason func args file line extra test } { set a $after_reason - verbose -log "mi_expect_stop: expecting: .*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}$after_stopped\r\n$mi_gdb_prompt$" + verbose -log "mi_expect_stop: expecting: .*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}$after_stopped\r\n$prompt_re$" gdb_expect { - -re ".*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n$mi_gdb_prompt$" { + -re ".*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n$prompt_re$" { pass "$test" return $expect_out(2,string) } - -re ".*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\".*\",args=\[\\\[\{\].*\[\\\]\}\],file=\".*\",fullname=\"${fullname_syntax}.*\",line=\"\[0-9\]*\"\}.*\r\n$mi_gdb_prompt$" { + -re ".*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\".*\",args=\[\\\[\{\].*\[\\\]\}\],file=\".*\",fullname=\"${fullname_syntax}.*\",line=\"\[0-9\]*\"\}.*\r\n$prompt_re$" { fail "$test (stopped at wrong place)" return -1 } - -re ".*\r\n${mi_gdb_prompt}$" { + -re ".*\r\n$mi_gdb_prompt$" { fail "$test (unknown output after running)" return -1 } @@ -1388,9 +1433,16 @@ proc mi_continue_to_line {location test} { proc mi_get_stop_line {test} { global mi_gdb_prompt + global async + + if {$async} { + set prompt_re "" + } else { + set prompt_re "$mi_gdb_prompt" + } gdb_expect { - -re ".*line=\"(.*)\".*\r\n$mi_gdb_prompt$" { + -re ".*line=\"(.*)\".*\r\n$prompt_re$" { return $expect_out(1,string) } -re ".*$mi_gdb_prompt$" { [-- Attachment #4: 2.diff --] [-- Type: text/x-diff, Size: 3702 bytes --] commit 8f767c7ec7a933f9ac597b7f50420962025d6726 Author: Vladimir Prus <vladimir@codesourcery.com> Date: Tue Apr 29 21:40:51 2008 +0400 Suppress normal stop observer when it's problematic. * inferior.h (suppress_normal_stop_observer): New. * infcall.c (call_function_by_hand): Disable stop events when doing function calls. * infmcd.c (suppress_normal_stop_observer): New. (finish_command_continuation): Call normal_stop observer explicitly. (finish_command): Disable stop events inside proceed. * infrun.c (normal_stop): Don't call normal stop observer if suppressed of if multi-step is in progress. diff --git a/gdb/infcall.c b/gdb/infcall.c index ca4785e..d8257da 100644 --- a/gdb/infcall.c +++ b/gdb/infcall.c @@ -706,6 +706,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args) { struct cleanup *old_cleanups = make_cleanup (null_cleanup, 0); + struct cleanup *old_cleanups2; int saved_async = 0; /* If all error()s out of proceed ended up calling normal_stop @@ -718,8 +719,11 @@ call_function_by_hand (struct value *function, int nargs, struct value **args) if (target_can_async_p ()) saved_async = target_async_mask (0); - + + old_cleanups2 = make_cleanup_restore_integer + (&suppress_normal_stop_observer, 1); proceed (real_pc, TARGET_SIGNAL_0, 0); + do_cleanups (old_cleanups2); if (saved_async) target_async_mask (saved_async); diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 18aa544..a0980c7 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -205,6 +205,9 @@ int step_multi; in format described in environ.h. */ struct gdb_environ *inferior_environ; + +/* When set, normal_stop will not call the normal_stop observer. */ +int suppress_normal_stop_observer = 0; \f /* Accessor routines. */ @@ -1279,8 +1282,13 @@ finish_command_continuation (struct continuation_arg *arg, int error_p) if (TYPE_CODE (value_type) != TYPE_CODE_VOID) print_return_value (SYMBOL_TYPE (function), value_type); } + + /* We suppress normal call of normal_stop observer and do it here so that + that *stopped notification includes the return value. */ + observer_notify_normal_stop (stop_bpstat); } + suppress_normal_stop_observer = 0; delete_breakpoint (breakpoint); } @@ -1347,6 +1355,7 @@ finish_command (char *arg, int from_tty) } proceed_to_finish = 1; /* We want stop_registers, please... */ + make_cleanup_restore_integer (&suppress_normal_stop_observer, 1); proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0); arg1 = diff --git a/gdb/inferior.h b/gdb/inferior.h index 3aaaa26..630cc52 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -399,6 +399,9 @@ extern int debug_displaced; void displaced_step_dump_bytes (struct ui_file *file, const gdb_byte *buf, size_t len); + +/* When set, normal_stop will not call the normal_stop observer. */ +extern int suppress_normal_stop_observer; \f /* Possible values for gdbarch_call_dummy_location. */ #define ON_STACK 1 diff --git a/gdb/infrun.c b/gdb/infrun.c index 0e58749..b0ba57c 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -3627,7 +3627,8 @@ Further execution is probably impossible.\n")); done: annotate_stopped (); - observer_notify_normal_stop (stop_bpstat); + if (!suppress_normal_stop_observer && !step_multi) + observer_notify_normal_stop (stop_bpstat); /* Delete the breakpoint we stopped at, if it wants to be deleted. Delete any breakpoint that is to be deleted at the next stop. */ breakpoint_auto_delete (stop_bpstat); ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Use observers to report stop events. 2008-04-29 18:30 ` Vladimir Prus @ 2008-05-01 19:58 ` Daniel Jacobowitz 2008-05-01 20:11 ` Pedro Alves 2008-05-04 9:05 ` Vladimir Prus 0 siblings, 2 replies; 14+ messages in thread From: Daniel Jacobowitz @ 2008-05-01 19:58 UTC (permalink / raw) To: Vladimir Prus; +Cc: Joel Brobecker, gdb-patches On Tue, Apr 29, 2008 at 10:10:30PM +0400, Vladimir Prus wrote: > Here are 3 independent bits. > > 1. Introduce the make_cleanup_restore_integer function. You're right > that it can lead to bad results if one discards this cleanup, but then > one should be careful with discarding cleanups anyway. This patch looks fine except that ... > 2. Modify the normal_stop observer not to fire in some cases. One > case is when doing function call -- we don't announce the stop in CLI > and for similar reason we don't have observer to be called. Also, > for the benefit of next patch, we want the call to observer to > be delayed until we print function return value, if we're doing finish. ... unless I'm mistaken you have exactly the memory leak Joel warned about, since finish_command discards continuations. Am I correct that the cleanup for finish_command is never supposed to survive the function returning? It's run on error and discarded on normal return. So you could put the closure in a local variable, maybe. struct foo_closure my_closure = { &my_global, my_global }; make_cleanup (restore_integer, &my_closure); -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Use observers to report stop events. 2008-05-01 19:58 ` Daniel Jacobowitz @ 2008-05-01 20:11 ` Pedro Alves 2008-05-01 20:17 ` Vladimir Prus 2008-05-04 9:05 ` Vladimir Prus 1 sibling, 1 reply; 14+ messages in thread From: Pedro Alves @ 2008-05-01 20:11 UTC (permalink / raw) To: gdb-patches; +Cc: Daniel Jacobowitz, Vladimir Prus, Joel Brobecker A Thursday 01 May 2008 20:57:58, Daniel Jacobowitz wrote: > Am I correct that the cleanup for finish_command is never supposed to > survive the function returning? It's run on error and discarded on > normal return. So you could put the closure in a local variable, > maybe. > > struct foo_closure my_closure = { &my_global, my_global }; > make_cleanup (restore_integer, &my_closure); Humm, I notice that a cleanup is being passed around to finish_command_continuation, but it isn't being used inside -- and it shoud not be. With continuations per thread, and and non-stop, we can have more than one simultaneous finish command. The cleanup chains are not per-thread, so it is not safe to run cleanups like that. Either we have to use some other form of cleanup in this continuation, or revert to having exec_cleanups, but this time, per thread. -- Pedro Alves ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Use observers to report stop events. 2008-05-01 20:11 ` Pedro Alves @ 2008-05-01 20:17 ` Vladimir Prus 0 siblings, 0 replies; 14+ messages in thread From: Vladimir Prus @ 2008-05-01 20:17 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches, Daniel Jacobowitz, Joel Brobecker On Friday 02 May 2008 00:11:04 Pedro Alves wrote: > A Thursday 01 May 2008 20:57:58, Daniel Jacobowitz wrote: > > > Am I correct that the cleanup for finish_command is never supposed to > > survive the function returning? It's run on error and discarded on > > normal return. So you could put the closure in a local variable, > > maybe. > > > > struct foo_closure my_closure = { &my_global, my_global }; > > make_cleanup (restore_integer, &my_closure); > > Humm, I notice that a cleanup is being passed around to > finish_command_continuation, but it isn't being used inside -- and > it shoud not be. With continuations per thread, and and non-stop, we > can have more than one simultaneous finish command. The cleanup > chains are not per-thread, so it is not safe to run cleanups like that. > Either we have to use some other form of cleanup in this continuation, > or revert to having exec_cleanups, but this time, per thread. We just should not pass cleanup to continuation -- as it's not used and should not be. I've missed that bit in previous patches. - Volodya ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Use observers to report stop events. 2008-05-01 19:58 ` Daniel Jacobowitz 2008-05-01 20:11 ` Pedro Alves @ 2008-05-04 9:05 ` Vladimir Prus 2008-05-28 18:30 ` Vladimir Prus 2008-06-05 15:41 ` Daniel Jacobowitz 1 sibling, 2 replies; 14+ messages in thread From: Vladimir Prus @ 2008-05-04 9:05 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: Joel Brobecker, gdb-patches [-- Attachment #1: Type: text/plain, Size: 1625 bytes --] On Thursday 01 May 2008 23:57:58 Daniel Jacobowitz wrote: > On Tue, Apr 29, 2008 at 10:10:30PM +0400, Vladimir Prus wrote: > > Here are 3 independent bits. > > > > 1. Introduce the make_cleanup_restore_integer function. You're right > > that it can lead to bad results if one discards this cleanup, but then > > one should be careful with discarding cleanups anyway. > > This patch looks fine except that ... > > > 2. Modify the normal_stop observer not to fire in some cases. One > > case is when doing function call -- we don't announce the stop in CLI > > and for similar reason we don't have observer to be called. Also, > > for the benefit of next patch, we want the call to observer to > > be delayed until we print function return value, if we're doing finish. > > ... unless I'm mistaken you have exactly the memory leak Joel warned > about, since finish_command discards continuations. > > Am I correct that the cleanup for finish_command is never supposed to > survive the function returning? It's run on error and discarded on > normal return. So you could put the closure in a local variable, > maybe. > > struct foo_closure my_closure = { &my_global, my_global }; > make_cleanup (restore_integer, &my_closure); This is somewhat limiting. Instead, I've implemented a mechanism that allows a cleanup to well, "cleanup" its argument. I attach a patch for that. I also attach a patch stop the 'finish_command_continuation' from accessing a cleanup is has no business with. Are those two new patches, together with the previously posted one (changing stop_observer not to always fire) OK? - Volodya [-- Attachment #2: 0001-Introduce-common-cleanup-for-restoring-integers.patch --] [-- Type: text/x-diff, Size: 5933 bytes --] From d855d879f515a4ac4a177a45a2b08fea48064b21 Mon Sep 17 00:00:00 2001 From: Vladimir Prus <vladimir@codesourcery.com> Date: Tue, 29 Apr 2008 21:31:59 +0400 Subject: [RFA] Introduce common cleanup for restoring integers. To: gdb-patches@sources.redhat.com X-KMail-Transport: CodeSourcery X-KMail-Identity: 901867920 * defs.h (make_cleanup_restore_integer): New declaration. (struct cleanup): New field free_arg. (make_my_cleanup_2): New. * utils.c (restore_integer_closure, restore_integer) (make_cleanup_restore_integer): New. (make_my_cleanup): Initialize the free_arg field and renamed to make_my_cleanup_2. (do_my_cleanups): Call free_arg. (discard_cleanups): Call free_arg. * breakpoint.c (restore_always_inserted_mode): Remove. (update_breakpoints_after_exec): Use make_cleanup_restore_integer. --- gdb/breakpoint.c | 10 +--------- gdb/defs.h | 14 +++++++++++++- gdb/utils.c | 43 +++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 0648a4b..e5e5de0 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1452,12 +1452,6 @@ reattach_breakpoints (int pid) return 0; } -static void -restore_always_inserted_mode (void *p) -{ - always_inserted_mode = (uintptr_t) p; -} - void update_breakpoints_after_exec (void) { @@ -1473,9 +1467,7 @@ update_breakpoints_after_exec (void) /* The binary we used to debug is now gone, and we're updating breakpoints for the new binary. Until we're done, we should not try to insert breakpoints. */ - cleanup = make_cleanup (restore_always_inserted_mode, - (void *) (uintptr_t) always_inserted_mode); - always_inserted_mode = 0; + cleanup = make_cleanup_restore_integer (&always_inserted_mode, 0); ALL_BREAKPOINTS_SAFE (b, temp) { diff --git a/gdb/defs.h b/gdb/defs.h index 0fa0e6c..ce7e1b9 100644 --- a/gdb/defs.h +++ b/gdb/defs.h @@ -236,12 +236,18 @@ enum return_value_convention Use make_cleanup to add an element to the cleanup chain. Use do_cleanups to do all cleanup actions back to a given point in the chain. Use discard_cleanups to remove cleanups - from the chain back to a given point, not doing them. */ + from the chain back to a given point, not doing them. + + If the argument is pointer to allocated memory, then you need to + to additionally set the 'free_arg' member to a function that will + free that memory. This function will be called both when the cleanup + is executed and when it's discarded. */ struct cleanup { struct cleanup *next; void (*function) (void *); + void (*free_arg) (void *); void *arg; }; @@ -345,11 +351,17 @@ extern struct cleanup *make_cleanup_close (int fd); extern struct cleanup *make_cleanup_bfd_close (bfd *abfd); +extern struct cleanup *make_cleanup_restore_integer (int *variable, int value); + extern struct cleanup *make_final_cleanup (make_cleanup_ftype *, void *); extern struct cleanup *make_my_cleanup (struct cleanup **, make_cleanup_ftype *, void *); +extern struct cleanup *make_my_cleanup2 (struct cleanup **, + make_cleanup_ftype *, void *, + void (*free_arg) (void *)); + extern struct cleanup *save_cleanups (void); extern struct cleanup *save_final_cleanups (void); extern struct cleanup *save_my_cleanups (struct cleanup **); diff --git a/gdb/utils.c b/gdb/utils.c index d9953a0..9610bd0 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -277,10 +277,37 @@ make_cleanup_free_section_addr_info (struct section_addr_info *addrs) return make_my_cleanup (&cleanup_chain, do_free_section_addr_info, addrs); } +struct restore_integer_closure +{ + int *variable; + int value; +}; +static void +restore_integer (void *p) +{ + struct restore_integer_closure *closure = p; + *(closure->variable) = closure->value; +} + +/* Assign VALUE to *VARIABLE and arrange for the old value to + be restored via cleanup. */ struct cleanup * -make_my_cleanup (struct cleanup **pmy_chain, make_cleanup_ftype *function, - void *arg) +make_cleanup_restore_integer (int *variable, int value) +{ + struct restore_integer_closure *c = + xmalloc (sizeof (struct restore_integer_closure)); + struct cleanup *cleanup = make_cleanup (restore_integer, (void *) c); + c->variable = variable; + c->value = *variable; + *variable = value; + cleanup_chain->free_arg = xfree; + return cleanup; +} + +struct cleanup * +make_my_cleanup2 (struct cleanup **pmy_chain, make_cleanup_ftype *function, + void *arg, void (*free_arg) (void *)) { struct cleanup *new = (struct cleanup *) xmalloc (sizeof (struct cleanup)); @@ -288,12 +315,20 @@ make_my_cleanup (struct cleanup **pmy_chain, make_cleanup_ftype *function, new->next = *pmy_chain; new->function = function; + new->free_arg = free_arg; new->arg = arg; *pmy_chain = new; return old_chain; } +struct cleanup * +make_my_cleanup (struct cleanup **pmy_chain, make_cleanup_ftype *function, + void *arg) +{ + return make_my_cleanup2 (pmy_chain, function, arg, NULL); +} + /* Discard cleanups and do the actions they describe until we get back to the point OLD_CHAIN in the cleanup_chain. */ @@ -318,6 +353,8 @@ do_my_cleanups (struct cleanup **pmy_chain, { *pmy_chain = ptr->next; /* Do this first incase recursion */ (*ptr->function) (ptr->arg); + if (ptr->free_arg) + (*ptr->free_arg) (ptr->arg); xfree (ptr); } } @@ -345,6 +382,8 @@ discard_my_cleanups (struct cleanup **pmy_chain, while ((ptr = *pmy_chain) != old_chain) { *pmy_chain = ptr->next; + if (ptr->free_arg) + (*ptr->free_arg) (ptr->arg); xfree (ptr); } } -- 1.5.3.5 [-- Attachment #3: 0002-Remove-stale-code.patch --] [-- Type: text/x-diff, Size: 1658 bytes --] From 962b24aa709c684a616070453b7f9d31175a2d61 Mon Sep 17 00:00:00 2001 From: Vladimir Prus <vladimir@codesourcery.com> Date: Sun, 4 May 2008 12:09:45 +0400 Subject: [RFA] Remove stale code. To: gdb-patches@sources.redhat.com X-KMail-Transport: CodeSourcery X-KMail-Identity: 901867920 * infrun.c (finish_command): Don't pass cleanup to continuation. (finish_command_continuation): Don't grab cleanup from the passed data, as we don't use, and cannot, use it anyway. --- gdb/infcmd.c | 7 +------ 1 files changed, 1 insertions(+), 6 deletions(-) diff --git a/gdb/infcmd.c b/gdb/infcmd.c index abf1354..66c8a05 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -1263,7 +1263,6 @@ finish_command_continuation (struct continuation_arg *arg, int error_p) breakpoint = (struct breakpoint *) arg->data.pointer; function = (struct symbol *) arg->next->data.pointer; - cleanups = (struct cleanup *) arg->next->next->data.pointer; if (!error_p) { @@ -1354,14 +1353,10 @@ finish_command (char *arg, int from_tty) (struct continuation_arg *) xmalloc (sizeof (struct continuation_arg)); arg2 = (struct continuation_arg *) xmalloc (sizeof (struct continuation_arg)); - arg3 = - (struct continuation_arg *) xmalloc (sizeof (struct continuation_arg)); arg1->next = arg2; - arg2->next = arg3; - arg3->next = NULL; + arg2->next = NULL; arg1->data.pointer = breakpoint; arg2->data.pointer = function; - arg3->data.pointer = old_chain; add_continuation (finish_command_continuation, arg1); /* Do this only if not running asynchronously or if the target -- 1.5.3.5 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Use observers to report stop events. 2008-05-04 9:05 ` Vladimir Prus @ 2008-05-28 18:30 ` Vladimir Prus 2008-06-05 15:41 ` Daniel Jacobowitz 1 sibling, 0 replies; 14+ messages in thread From: Vladimir Prus @ 2008-05-28 18:30 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: Joel Brobecker, gdb-patches On Sunday 04 May 2008 12:25:54 Vladimir Prus wrote: > On Thursday 01 May 2008 23:57:58 Daniel Jacobowitz wrote: > > On Tue, Apr 29, 2008 at 10:10:30PM +0400, Vladimir Prus wrote: > > > Here are 3 independent bits. > > > > > > 1. Introduce the make_cleanup_restore_integer function. You're right > > > that it can lead to bad results if one discards this cleanup, but then > > > one should be careful with discarding cleanups anyway. > > > > This patch looks fine except that ... > > > > > 2. Modify the normal_stop observer not to fire in some cases. One > > > case is when doing function call -- we don't announce the stop in CLI > > > and for similar reason we don't have observer to be called. Also, > > > for the benefit of next patch, we want the call to observer to > > > be delayed until we print function return value, if we're doing finish. > > > > ... unless I'm mistaken you have exactly the memory leak Joel warned > > about, since finish_command discards continuations. > > > > Am I correct that the cleanup for finish_command is never supposed to > > survive the function returning? It's run on error and discarded on > > normal return. So you could put the closure in a local variable, > > maybe. > > > > struct foo_closure my_closure = { &my_global, my_global }; > > make_cleanup (restore_integer, &my_closure); > > This is somewhat limiting. Instead, I've implemented a mechanism that > allows a cleanup to well, "cleanup" its argument. I attach a patch > for that. > > I also attach a patch stop the 'finish_command_continuation' from > accessing a cleanup is has no business with. > > Are those two new patches, together with the previously posted one > (changing stop_observer not to always fire) OK? Ping? - Volodya ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Use observers to report stop events. 2008-05-04 9:05 ` Vladimir Prus 2008-05-28 18:30 ` Vladimir Prus @ 2008-06-05 15:41 ` Daniel Jacobowitz 2008-06-10 11:58 ` Vladimir Prus 1 sibling, 1 reply; 14+ messages in thread From: Daniel Jacobowitz @ 2008-06-05 15:41 UTC (permalink / raw) To: Vladimir Prus; +Cc: Joel Brobecker, gdb-patches On Sun, May 04, 2008 at 12:25:54PM +0400, Vladimir Prus wrote: > Are those two new patches, together with the previously posted one > (changing stop_observer not to always fire) OK? For this patch, if an error occurs during proceed (i.e. if the cleanup restoring suppress_normal_stop_observer is run), will the normal_stop observer ever be called? Is that a problem? > + If the argument is pointer to allocated memory, then you need to is a pointer > struct cleanup * > -make_my_cleanup (struct cleanup **pmy_chain, make_cleanup_ftype *function, > - void *arg) > +make_cleanup_restore_integer (int *variable, int value) > +{ > + struct restore_integer_closure *c = > + xmalloc (sizeof (struct restore_integer_closure)); > + struct cleanup *cleanup = make_cleanup (restore_integer, (void *) c); > + c->variable = variable; > + c->value = *variable; > + *variable = value; > + cleanup_chain->free_arg = xfree; > + return cleanup; > +} Could you use make_my_cleanup2 here to avoid poking around in cleanup_chain, please? Also, the only thing value is used for is to set *variable. I suggest not setting the variable in a function named "make_cleanup_restore_integer", which doesn't say anything about setting. So I would prefer this: old_cleanup = make_cleanup_restore_integer (some_var); some_var = 0; > * infrun.c (finish_command): Don't pass cleanup > to continuation. > (finish_command_continuation): Don't grab cleanup from > the passed data, as we don't use, and cannot, use it anyway. OK. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Use observers to report stop events. 2008-06-05 15:41 ` Daniel Jacobowitz @ 2008-06-10 11:58 ` Vladimir Prus 2008-06-10 13:21 ` Daniel Jacobowitz 0 siblings, 1 reply; 14+ messages in thread From: Vladimir Prus @ 2008-06-10 11:58 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: Joel Brobecker, gdb-patches [-- Attachment #1: Type: text/plain, Size: 2557 bytes --] On Thursday 05 June 2008 19:41:11 Daniel Jacobowitz wrote: > On Sun, May 04, 2008 at 12:25:54PM +0400, Vladimir Prus wrote: > > Are those two new patches, together with the previously posted one > > (changing stop_observer not to always fire) OK? > > For this patch, if an error occurs during proceed (i.e. if the > cleanup restoring suppress_normal_stop_observer is run), will the > normal_stop observer ever be called? Is that a problem? You mean this block: old_cleanups2 = make_cleanup_restore_integer (&suppress_normal_stop_observer, 1); proceed (real_pc, TARGET_SIGNAL_0, 0); do_cleanups (old_cleanups2); If proceed throws, before calling normal_stop, we'll get back to event loop, and run cleanup. We won't call the observer. It's an issue if we've printed "*running" and thrown after after. However, it's the issue we have now, as well -- we print ^running even before calling proceed, and if something later throws, we'll never print *stopped. Possible solutions are: - Require that frontend refresh thread state on ^error - Emit *stopped if exception is thrown (this requires checking that the target is actually stopped, if exception is thrown). > > + If the argument is pointer to allocated memory, then you need to > > is a pointer > > > struct cleanup * > > -make_my_cleanup (struct cleanup **pmy_chain, make_cleanup_ftype *function, > > - void *arg) > > +make_cleanup_restore_integer (int *variable, int value) > > +{ > > + struct restore_integer_closure *c = > > + xmalloc (sizeof (struct restore_integer_closure)); > > + struct cleanup *cleanup = make_cleanup (restore_integer, (void *) c); > > + c->variable = variable; > > + c->value = *variable; > > + *variable = value; > > + cleanup_chain->free_arg = xfree; > > + return cleanup; > > +} > > Could you use make_my_cleanup2 here to avoid poking around in > cleanup_chain, please? Done. > > Also, the only thing value is used for is to set *variable. I > suggest not setting the variable in a function named > "make_cleanup_restore_integer", which doesn't say anything about > setting. So I would prefer this: > > old_cleanup = make_cleanup_restore_integer (some_var); > some_var = 0; Done. > > > * infrun.c (finish_command): Don't pass cleanup > > to continuation. > > (finish_command_continuation): Don't grab cleanup from > > the passed data, as we don't use, and cannot, use it anyway. > > OK. Thanks, checked in. Since there were some adjustment, I attach the final versions of the patches as checked in. - Volodya [-- Attachment #2: 0001-Introduce-common-cleanup-for-restoring-integers.patch --] [-- Type: text/x-diff, Size: 5835 bytes --] From d354ec6250185a3083636b349acebb89faafe359 Mon Sep 17 00:00:00 2001 From: Vladimir Prus <vladimir@codesourcery.com> Date: Tue, 29 Apr 2008 21:31:59 +0400 Subject: [RFA] Introduce common cleanup for restoring integers. To: gdb-patches@sources.redhat.com X-KMail-Transport: CodeSourcery X-KMail-Identity: 901867920 * defs.h (make_cleanup_restore_integer): New declaration. (struct cleanup): New field free_arg. (make_my_cleanup_2): New. * utils.c (restore_integer_closure, restore_integer) (make_cleanup_restore_integer): New. (make_my_cleanup): Initialize the free_arg field and renamed to make_my_cleanup_2. (do_my_cleanups): Call free_arg. (discard_cleanups): Call free_arg. * breakpoint.c (restore_always_inserted_mode): Remove. (update_breakpoints_after_exec): Use make_cleanup_restore_integer. --- gdb/breakpoint.c | 9 +-------- gdb/defs.h | 14 +++++++++++++- gdb/utils.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 54 insertions(+), 11 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 051b753..dd7bf12 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1435,12 +1435,6 @@ reattach_breakpoints (int pid) return 0; } -static void -restore_always_inserted_mode (void *p) -{ - always_inserted_mode = (uintptr_t) p; -} - void update_breakpoints_after_exec (void) { @@ -1456,8 +1450,7 @@ update_breakpoints_after_exec (void) /* The binary we used to debug is now gone, and we're updating breakpoints for the new binary. Until we're done, we should not try to insert breakpoints. */ - cleanup = make_cleanup (restore_always_inserted_mode, - (void *) (uintptr_t) always_inserted_mode); + cleanup = make_cleanup_restore_integer (&always_inserted_mode); always_inserted_mode = 0; ALL_BREAKPOINTS_SAFE (b, temp) diff --git a/gdb/defs.h b/gdb/defs.h index 9069156..ca3fad8 100644 --- a/gdb/defs.h +++ b/gdb/defs.h @@ -230,12 +230,18 @@ enum return_value_convention Use make_cleanup to add an element to the cleanup chain. Use do_cleanups to do all cleanup actions back to a given point in the chain. Use discard_cleanups to remove cleanups - from the chain back to a given point, not doing them. */ + from the chain back to a given point, not doing them. + + If the argument is pointer to allocated memory, then you need to + to additionally set the 'free_arg' member to a function that will + free that memory. This function will be called both when the cleanup + is executed and when it's discarded. */ struct cleanup { struct cleanup *next; void (*function) (void *); + void (*free_arg) (void *); void *arg; }; @@ -339,11 +345,17 @@ extern struct cleanup *make_cleanup_close (int fd); extern struct cleanup *make_cleanup_bfd_close (bfd *abfd); +extern struct cleanup *make_cleanup_restore_integer (int *variable); + extern struct cleanup *make_final_cleanup (make_cleanup_ftype *, void *); extern struct cleanup *make_my_cleanup (struct cleanup **, make_cleanup_ftype *, void *); +extern struct cleanup *make_my_cleanup2 (struct cleanup **, + make_cleanup_ftype *, void *, + void (*free_arg) (void *)); + extern struct cleanup *save_cleanups (void); extern struct cleanup *save_final_cleanups (void); extern struct cleanup *save_my_cleanups (struct cleanup **); diff --git a/gdb/utils.c b/gdb/utils.c index 76ea6b1..cdea5a6 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -277,10 +277,36 @@ make_cleanup_free_section_addr_info (struct section_addr_info *addrs) return make_my_cleanup (&cleanup_chain, do_free_section_addr_info, addrs); } +struct restore_integer_closure +{ + int *variable; + int value; +}; + +static void +restore_integer (void *p) +{ + struct restore_integer_closure *closure = p; + *(closure->variable) = closure->value; +} +/* Remember the current value of *VARIABLE and make it restored when the cleanup + is run. */ struct cleanup * -make_my_cleanup (struct cleanup **pmy_chain, make_cleanup_ftype *function, - void *arg) +make_cleanup_restore_integer (int *variable) +{ + struct restore_integer_closure *c = + xmalloc (sizeof (struct restore_integer_closure)); + c->variable = variable; + c->value = *variable; + + return make_my_cleanup2 (&cleanup_chain, restore_integer, (void *)c, + xfree); +} + +struct cleanup * +make_my_cleanup2 (struct cleanup **pmy_chain, make_cleanup_ftype *function, + void *arg, void (*free_arg) (void *)) { struct cleanup *new = (struct cleanup *) xmalloc (sizeof (struct cleanup)); @@ -288,12 +314,20 @@ make_my_cleanup (struct cleanup **pmy_chain, make_cleanup_ftype *function, new->next = *pmy_chain; new->function = function; + new->free_arg = free_arg; new->arg = arg; *pmy_chain = new; return old_chain; } +struct cleanup * +make_my_cleanup (struct cleanup **pmy_chain, make_cleanup_ftype *function, + void *arg) +{ + return make_my_cleanup2 (pmy_chain, function, arg, NULL); +} + /* Discard cleanups and do the actions they describe until we get back to the point OLD_CHAIN in the cleanup_chain. */ @@ -318,6 +352,8 @@ do_my_cleanups (struct cleanup **pmy_chain, { *pmy_chain = ptr->next; /* Do this first incase recursion */ (*ptr->function) (ptr->arg); + if (ptr->free_arg) + (*ptr->free_arg) (ptr->arg); xfree (ptr); } } @@ -345,6 +381,8 @@ discard_my_cleanups (struct cleanup **pmy_chain, while ((ptr = *pmy_chain) != old_chain) { *pmy_chain = ptr->next; + if (ptr->free_arg) + (*ptr->free_arg) (ptr->arg); xfree (ptr); } } -- 1.5.3.5 [-- Attachment #3: 0003-Suppress-normal-stop-observer-when-it-s-problematic.patch --] [-- Type: text/x-diff, Size: 4195 bytes --] From 85eda1b65c61be67f9098f001a5f52f70f44c33f Mon Sep 17 00:00:00 2001 From: Vladimir Prus <vladimir@codesourcery.com> Date: Mon, 9 Jun 2008 21:14:33 +0400 Subject: [RFA] Suppress normal stop observer when it's problematic. To: gdb-patches@sources.redhat.com X-KMail-Transport: CodeSourcery X-KMail-Identity: 901867920 * inferior.h (suppress_normal_stop_observer): New. * infcall.c (call_function_by_hand): Disable stop events when doing function calls. * infmcd.c (suppress_normal_stop_observer): New. (finish_command_continuation): Call normal_stop observer explicitly. (finish_command): Disable stop events inside proceed. * infrun.c (normal_stop): Don't call normal stop observer if suppressed of if multi-step is in progress. --- gdb/infcall.c | 7 ++++++- gdb/infcmd.c | 10 ++++++++++ gdb/inferior.h | 3 +++ gdb/infrun.c | 3 ++- 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/gdb/infcall.c b/gdb/infcall.c index c065b59..ded3211 100644 --- a/gdb/infcall.c +++ b/gdb/infcall.c @@ -706,6 +706,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args) { struct cleanup *old_cleanups = make_cleanup (null_cleanup, 0); + struct cleanup *old_cleanups2; int saved_async = 0; /* If all error()s out of proceed ended up calling normal_stop @@ -718,8 +719,12 @@ call_function_by_hand (struct value *function, int nargs, struct value **args) if (target_can_async_p ()) saved_async = target_async_mask (0); - + + old_cleanups2 = make_cleanup_restore_integer + (&suppress_normal_stop_observer); + suppress_normal_stop_observer = 1; proceed (real_pc, TARGET_SIGNAL_0, 0); + do_cleanups (old_cleanups2); if (saved_async) target_async_mask (saved_async); diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 30858f1..a844b7d 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -206,6 +206,9 @@ int step_multi; in format described in environ.h. */ struct gdb_environ *inferior_environ; + +/* When set, normal_stop will not call the normal_stop observer. */ +int suppress_normal_stop_observer = 0; \f /* Accessor routines. */ @@ -1294,8 +1297,13 @@ finish_command_continuation (struct continuation_arg *arg, int error_p) if (TYPE_CODE (value_type) != TYPE_CODE_VOID) print_return_value (SYMBOL_TYPE (function), value_type); } + + /* We suppress normal call of normal_stop observer and do it here so that + that *stopped notification includes the return value. */ + observer_notify_normal_stop (stop_bpstat); } + suppress_normal_stop_observer = 0; delete_breakpoint (breakpoint); } @@ -1362,6 +1370,8 @@ finish_command (char *arg, int from_tty) } proceed_to_finish = 1; /* We want stop_registers, please... */ + make_cleanup_restore_integer (&suppress_normal_stop_observer); + suppress_normal_stop_observer = 1; proceed ((CORE_ADDR) -1, TARGET_SIGNAL_DEFAULT, 0); arg1 = diff --git a/gdb/inferior.h b/gdb/inferior.h index 1dd152a..7f85507 100644 --- a/gdb/inferior.h +++ b/gdb/inferior.h @@ -391,6 +391,9 @@ extern int debug_displaced; void displaced_step_dump_bytes (struct ui_file *file, const gdb_byte *buf, size_t len); + +/* When set, normal_stop will not call the normal_stop observer. */ +extern int suppress_normal_stop_observer; \f /* Possible values for gdbarch_call_dummy_location. */ #define ON_STACK 1 diff --git a/gdb/infrun.c b/gdb/infrun.c index 2960acb..1e10ecc 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -3772,7 +3772,8 @@ Further execution is probably impossible.\n")); done: annotate_stopped (); - observer_notify_normal_stop (stop_bpstat); + if (!suppress_normal_stop_observer && !step_multi) + observer_notify_normal_stop (stop_bpstat); /* Delete the breakpoint we stopped at, if it wants to be deleted. Delete any breakpoint that is to be deleted at the next stop. */ breakpoint_auto_delete (stop_bpstat); -- 1.5.3.5 [-- Attachment #4: 0002-Remove-stale-code.patch --] [-- Type: text/x-diff, Size: 1658 bytes --] From 36e76ca870e36e406b1c301b28d6e95f514b0974 Mon Sep 17 00:00:00 2001 From: Vladimir Prus <vladimir@codesourcery.com> Date: Sun, 4 May 2008 12:09:45 +0400 Subject: [RFA] Remove stale code. To: gdb-patches@sources.redhat.com X-KMail-Transport: CodeSourcery X-KMail-Identity: 901867920 * infrun.c (finish_command): Don't pass cleanup to continuation. (finish_command_continuation): Don't grab cleanup from the passed data, as we don't use, and cannot, use it anyway. --- gdb/infcmd.c | 7 +------ 1 files changed, 1 insertions(+), 6 deletions(-) diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 2397c30..30858f1 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -1278,7 +1278,6 @@ finish_command_continuation (struct continuation_arg *arg, int error_p) breakpoint = (struct breakpoint *) arg->data.pointer; function = (struct symbol *) arg->next->data.pointer; - cleanups = (struct cleanup *) arg->next->next->data.pointer; if (!error_p) { @@ -1369,14 +1368,10 @@ finish_command (char *arg, int from_tty) (struct continuation_arg *) xmalloc (sizeof (struct continuation_arg)); arg2 = (struct continuation_arg *) xmalloc (sizeof (struct continuation_arg)); - arg3 = - (struct continuation_arg *) xmalloc (sizeof (struct continuation_arg)); arg1->next = arg2; - arg2->next = arg3; - arg3->next = NULL; + arg2->next = NULL; arg1->data.pointer = breakpoint; arg2->data.pointer = function; - arg3->data.pointer = old_chain; add_continuation (finish_command_continuation, arg1); /* Do this only if not running asynchronously or if the target -- 1.5.3.5 [-- Attachment #5: 0004-Use-observers-to-report-stop-events-in-MI.patch --] [-- Type: text/x-diff, Size: 16774 bytes --] From 8b8a8557c8b1c34395053105a420fbc252c4fb61 Mon Sep 17 00:00:00 2001 From: Vladimir Prus <vladimir@codesourcery.com> Date: Mon, 9 Jun 2008 21:18:28 +0400 Subject: [RFA] Use observers to report stop events in MI. To: gdb-patches@sources.redhat.com X-KMail-Transport: CodeSourcery X-KMail-Identity: 901867920 * mi/mi-interp.c (mi_on_normal_stop): New. (mi_interpreter_init): Register mi_on_normal_stop. (mi_interpreter_exec_continuation): Remove. (mi_cmd_interpreter_exec): Don't register the above. * mi/mi-main.c (captured_mi_execute_command): Don't care about sync_execution. (mi_execute_async_cli_command): Don't install continuation. Don't print *stopped. (mi_exec_async_cli_cmd_continuation): Remove. [gdb/testsuite] * gdb.mi/mi-break.exp (test_ignore_count): Adjust stopped pattern. * gdb.mi/mi-syn-frame.exp: Use mi_expect_stop instead of direct testing of stopped. * gdb.mi/mi2-syn-frame.exp: Likewise. * lib/mi-support.exp (default_mi_gdb_start): Call detect_async. (async, detect_async): New. (mi_expect_stop, mi_continue_to_line): Adjust expectation depending on if we're running in sync or async mode. --- gdb/infcmd.c | 5 +-- gdb/mi/mi-interp.c | 40 +++++++++----------- gdb/mi/mi-main.c | 37 +----------------- gdb/testsuite/gdb.mi/mi-break.exp | 2 +- gdb/testsuite/gdb.mi/mi-syn-frame.exp | 6 +-- gdb/testsuite/gdb.mi/mi2-syn-frame.exp | 25 +++--------- gdb/testsuite/lib/mi-support.exp | 64 +++++++++++++++++++++++++++++--- 7 files changed, 90 insertions(+), 89 deletions(-) diff --git a/gdb/infcmd.c b/gdb/infcmd.c index a844b7d..01e1ebe 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -1383,10 +1383,7 @@ finish_command (char *arg, int from_tty) arg1->data.pointer = breakpoint; arg2->data.pointer = function; add_continuation (finish_command_continuation, arg1); - - /* Do this only if not running asynchronously or if the target - cannot do async execution. Otherwise, complete this command when - the target actually stops, in fetch_inferior_event. */ + discard_cleanups (old_chain); if (!target_can_async_p ()) do_all_continuations (0); diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c index 8b0d909..583c288 100644 --- a/gdb/mi/mi-interp.c +++ b/gdb/mi/mi-interp.c @@ -65,6 +65,7 @@ static void mi1_command_loop (void); static void mi_insert_notify_hooks (void); static void mi_remove_notify_hooks (void); +static void mi_on_normal_stop (struct bpstats *bs); static void mi_new_thread (struct thread_info *t); static void mi_thread_exit (struct thread_info *t); @@ -92,6 +93,7 @@ mi_interpreter_init (int top_level) { observer_attach_new_thread (mi_new_thread); observer_attach_thread_exit (mi_thread_exit); + observer_attach_normal_stop (mi_on_normal_stop); } return mi; @@ -171,26 +173,6 @@ mi_interpreter_prompt_p (void *data) return 0; } -static void -mi_interpreter_exec_continuation (struct continuation_arg *arg, int error_p) -{ - bpstat_do_actions (&stop_bpstat); - /* It's not clear what to do in the case of errror -- should we assume that - the target is stopped, or that it still runs? */ - if (!target_executing) - { - fputs_unfiltered ("*stopped", raw_stdout); - mi_out_put (uiout, raw_stdout); - fputs_unfiltered ("\n", raw_stdout); - fputs_unfiltered ("(gdb) \n", raw_stdout); - gdb_flush (raw_stdout); - } - else if (target_can_async_p ()) - { - add_continuation (mi_interpreter_exec_continuation, NULL); - } -} - enum mi_cmd_result mi_cmd_interpreter_exec (char *command, char **argv, int argc) { @@ -241,7 +223,6 @@ mi_cmd_interpreter_exec (char *command, char **argv, int argc) if (target_can_async_p () && target_executing) { fputs_unfiltered ("^running\n", raw_stdout); - add_continuation (mi_interpreter_exec_continuation, NULL); } if (mi_error_message != NULL) @@ -325,12 +306,27 @@ static void mi_thread_exit (struct thread_info *t) { struct mi_interp *mi = top_level_interpreter_data (); - target_terminal_ours (); fprintf_unfiltered (mi->event_channel, "thread-exited,id=\"%d\"", t->num); gdb_flush (mi->event_channel); } +static void +mi_on_normal_stop (struct bpstats *bs) +{ + /* Since this can be called when CLI command is executing, + using cli interpreter, be sure to use MI uiout for output, + not the current one. */ + struct ui_out *uiout = interp_ui_out (top_level_interpreter ()); + struct mi_interp *mi = top_level_interpreter_data (); + + fputs_unfiltered ("*stopped", raw_stdout); + mi_out_put (uiout, raw_stdout); + mi_out_rewind (uiout); + fputs_unfiltered ("\n", raw_stdout); + gdb_flush (raw_stdout); +} + extern initialize_file_ftype _initialize_mi_interp; /* -Wmissing-prototypes */ void diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c index 4ae509e..6dc7609 100644 --- a/gdb/mi/mi-main.c +++ b/gdb/mi/mi-main.c @@ -103,10 +103,6 @@ static void mi_execute_cli_command (const char *cmd, int args_p, const char *args); static enum mi_cmd_result mi_execute_async_cli_command (char *cli_command, char **argv, int argc); - -static void mi_exec_async_cli_cmd_continuation (struct continuation_arg *arg, - int error_p); - static int register_changed_p (int regnum, struct regcache *, struct regcache *); static void get_register (int regnum, int format); @@ -1087,15 +1083,11 @@ captured_mi_execute_command (struct ui_out *uiout, void *data) fputs_unfiltered ("\n", raw_stdout); } else + /* The command does not want anything to be printed. In that + case, the command probably should not have written anything + to uiout, but in case it has written something, discard it. */ mi_out_rewind (uiout); } - else if (sync_execution) - { - /* Don't print the prompt. We are executing the target in - synchronous mode. */ - args->action = EXECUTE_COMMAND_SUPPRESS_PROMPT; - return; - } break; case CLI_COMMAND: @@ -1311,12 +1303,6 @@ mi_execute_async_cli_command (char *cli_command, char **argv, int argc) fputs_unfiltered (current_token, raw_stdout); fputs_unfiltered ("^running\n", raw_stdout); - /* Ideally, we should be intalling continuation only when - the target is already running. However, this will break right now, - because continuation installed by the 'finish' command must be after - the continuation that prints *stopped. This issue will be - fixed soon. */ - add_continuation (mi_exec_async_cli_cmd_continuation, NULL); } execute_command ( /*ui */ run, 0 /*from_tty */ ); @@ -1332,31 +1318,14 @@ mi_execute_async_cli_command (char *cli_command, char **argv, int argc) /* Do this before doing any printing. It would appear that some print code leaves garbage around in the buffer. */ do_cleanups (old_cleanups); - /* If the target was doing the operation synchronously we fake - the stopped message. */ - fputs_unfiltered ("*stopped", raw_stdout); - mi_out_put (uiout, raw_stdout); - mi_out_rewind (uiout); if (do_timings) print_diff_now (current_command_ts); - fputs_unfiltered ("\n", raw_stdout); return MI_CMD_QUIET; } return MI_CMD_DONE; } void -mi_exec_async_cli_cmd_continuation (struct continuation_arg *arg, int error_p) -{ - /* Assume 'error' means that target is stopped, too. */ - fputs_unfiltered ("*stopped", raw_stdout); - mi_out_put (uiout, raw_stdout); - fputs_unfiltered ("\n", raw_stdout); - fputs_unfiltered ("(gdb) \n", raw_stdout); - gdb_flush (raw_stdout); -} - -void mi_load_progress (const char *section_name, unsigned long sent_so_far, unsigned long total_section, diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp index f2f5b03..2798569 100644 --- a/gdb/testsuite/gdb.mi/mi-break.exp +++ b/gdb/testsuite/gdb.mi/mi-break.exp @@ -159,7 +159,7 @@ proc test_ignore_count {} { mi_run_cmd gdb_expect { - -re ".*func=\"callme\".*args=\\\[\{name=\"i\",value=\"2\"\}\\\].*\r\n$mi_gdb_prompt$" { + -re ".*\\*stopped.*func=\"callme\".*args=\\\[\{name=\"i\",value=\"2\"\}\\\].*\r\n($mi_gdb_prompt)?$" { pass "run to breakpoint with ignore count" } -re ".*$mi_gdb_prompt$" { diff --git a/gdb/testsuite/gdb.mi/mi-syn-frame.exp b/gdb/testsuite/gdb.mi/mi-syn-frame.exp index 2f2ca02..208678b 100644 --- a/gdb/testsuite/gdb.mi/mi-syn-frame.exp +++ b/gdb/testsuite/gdb.mi/mi-syn-frame.exp @@ -58,9 +58,7 @@ mi_gdb_test "403-exec-continue" \ "403\\^running" \ "testing exec continue" -# Presently, the *stopped notification for this case does not include -# any information. This can be considered a bug. -mi_gdb_test "" "\\*stopped" "finished exec continue" +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" mi_gdb_test "404-stack-list-frames 0 0" \ "404\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" \ @@ -88,7 +86,7 @@ mi_gdb_test "407-stack-list-frames" \ mi_gdb_test "408-exec-continue" "408\\^running" -mi_gdb_test "" ".*\\*stopped.*" "finished exec continue" +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" mi_gdb_test "409-stack-list-frames 0 0" \ "409\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" \ diff --git a/gdb/testsuite/gdb.mi/mi2-syn-frame.exp b/gdb/testsuite/gdb.mi/mi2-syn-frame.exp index c447404..b69812f 100644 --- a/gdb/testsuite/gdb.mi/mi2-syn-frame.exp +++ b/gdb/testsuite/gdb.mi/mi2-syn-frame.exp @@ -56,15 +56,11 @@ mi_gdb_test "402-stack-list-frames" "402\\^done,stack=\\\[frame=\{level=\"0\",ad # Continue back to main() # -send_gdb "403-exec-continue\n" -gdb_expect { - -re "403\\^running\[\r\n\]+${my_mi_gdb_prompt}.*\\\*stopped\[\r\n\]+${my_mi_gdb_prompt}$" { - pass "403-exec-continue" - } - timeout { - fail "403-exec-continue" - } -} +mi_gdb_test "403-exec-continue" \ + "403\\^running" \ + "testing exec continue" + +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" mi_gdb_test "404-stack-list-frames 0 0" \ "404\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" \ @@ -89,16 +85,9 @@ mi_gdb_test "407-stack-list-frames" \ "407\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"subroutine\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"handler\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"2\",addr=\"$hex\",func=\"<signal handler called>\"\},.*frame=\{level=\"$decimal\",addr=\"$hex\",func=\"have_a_very_merry_interrupt\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"<function called from gdb>\"\},frame=\{level=\"$decimal\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",line=\"$decimal\"\}.*\\\]" \ "list stack frames" +mi_gdb_test "408-exec-continue" "408\\^running" -send_gdb "408-exec-continue\n" -gdb_expect { - -re "408\\^running\[\r\n\]+${my_mi_gdb_prompt}.*\\\*stopped\[\r\n\]+${my_mi_gdb_prompt}$" { - pass "408-exec-continue" - } - timeout { - fail "408-exec-continue" - } -} +mi_expect_stop "really-no-reason" "" "" "" "" "" "finished exec continue" mi_gdb_test "409-stack-list-frames 0 0" \ "409\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"main\",file=\".*mi-syn-frame.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$decimal\"\}.*\\\]" \ diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp index 6a52c88..23f3f07 100644 --- a/gdb/testsuite/lib/mi-support.exp +++ b/gdb/testsuite/lib/mi-support.exp @@ -227,6 +227,8 @@ proc default_mi_gdb_start { args } { } } + detect_async + return 0; } @@ -911,6 +913,30 @@ proc mi_step { test } { return [mi_step_to {.*} {.*} {.*} {.*} $test] } +set async "unknown" + +proc detect_async {} { + global async + global mi_gdb_prompt + + if { $async == "unknown" } { + send_gdb "maint show linux-async\n" + + gdb_expect { + -re ".*Controlling the GNU/Linux inferior in asynchronous mode is on...*$mi_gdb_prompt$" { + set async 1 + } + -re ".*$mi_gdb_prompt$" { + set async 0 + } + timeout { + set async 0 + } + } + } + return $async +} + # Wait for MI *stopped notification to appear. # The REASON, FUNC, ARGS, FILE and LINE are regular expressions # to match against whatever is output in *stopped. ARGS should @@ -933,6 +959,7 @@ proc mi_expect_stop { reason func args file line extra test } { global hex global decimal global fullname_syntax + global async set after_stopped "" set after_reason "" @@ -944,10 +971,28 @@ proc mi_expect_stop { reason func args file line extra test } { set after_stopped [lindex $extra 0] } + if {$async} { + set prompt_re "" + } else { + set prompt_re "$mi_gdb_prompt" + } + + if { $reason == "really-no-reason" } { + gdb_expect { + -re "\\*stopped\r\n$prompt_re$" { + pass "$test" + } + timeout { + fail "$test (unknown output after running)" + } + } + return + } + if { $reason == "exited-normally" } { gdb_expect { - -re "\\*stopped,reason=\"exited-normally\"\r\n$mi_gdb_prompt$" { + -re "\\*stopped,reason=\"exited-normally\"\r\n$prompt_re$" { pass "$test" } -re ".*$mi_gdb_prompt$" {fail "continue to end (2)"} @@ -973,17 +1018,17 @@ proc mi_expect_stop { reason func args file line extra test } { set a $after_reason - verbose -log "mi_expect_stop: expecting: .*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}$after_stopped\r\n$mi_gdb_prompt$" + verbose -log "mi_expect_stop: expecting: .*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"$line\"\}$after_stopped\r\n$prompt_re$" gdb_expect { - -re ".*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n$mi_gdb_prompt$" { + -re ".*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\"$func\",args=$args,file=\".*$file\",fullname=\"${fullname_syntax}$file\",line=\"($line)\"\}$after_stopped\r\n$prompt_re$" { pass "$test" return $expect_out(2,string) } - -re ".*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\".*\",args=\[\\\[\{\].*\[\\\]\}\],file=\".*\",fullname=\"${fullname_syntax}.*\",line=\"\[0-9\]*\"\}.*\r\n$mi_gdb_prompt$" { + -re ".*\\*stopped,${r}${a}${bn}thread-id=\"$decimal\",frame=\{addr=\"$hex\",func=\".*\",args=\[\\\[\{\].*\[\\\]\}\],file=\".*\",fullname=\"${fullname_syntax}.*\",line=\"\[0-9\]*\"\}.*\r\n$prompt_re$" { fail "$test (stopped at wrong place)" return -1 } - -re ".*\r\n${mi_gdb_prompt}$" { + -re ".*\r\n$mi_gdb_prompt$" { fail "$test (unknown output after running)" return -1 } @@ -1388,9 +1433,16 @@ proc mi_continue_to_line {location test} { proc mi_get_stop_line {test} { global mi_gdb_prompt + global async + + if {$async} { + set prompt_re "" + } else { + set prompt_re "$mi_gdb_prompt" + } gdb_expect { - -re ".*line=\"(.*)\".*\r\n$mi_gdb_prompt$" { + -re ".*line=\"(.*)\".*\r\n$prompt_re$" { return $expect_out(1,string) } -re ".*$mi_gdb_prompt$" { -- 1.5.3.5 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Use observers to report stop events. 2008-06-10 11:58 ` Vladimir Prus @ 2008-06-10 13:21 ` Daniel Jacobowitz 2008-06-10 13:25 ` Vladimir Prus 0 siblings, 1 reply; 14+ messages in thread From: Daniel Jacobowitz @ 2008-06-10 13:21 UTC (permalink / raw) To: Vladimir Prus; +Cc: Joel Brobecker, gdb-patches On Tue, Jun 10, 2008 at 01:36:23PM +0400, Vladimir Prus wrote: > If proceed throws, before calling normal_stop, we'll get back to event loop, > and run cleanup. We won't call the observer. It's an issue if we've printed > "*running" and thrown after after. However, it's the issue we have now, as > well -- we print ^running even before calling proceed, and if something later > throws, we'll never print *stopped. Possible solutions are: > - Require that frontend refresh thread state on ^error > - Emit *stopped if exception is thrown (this requires checking that the > target is actually stopped, if exception is thrown). Or just don't print *running until we're actually running. For now it's fine as you have it. Thanks. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Use observers to report stop events. 2008-06-10 13:21 ` Daniel Jacobowitz @ 2008-06-10 13:25 ` Vladimir Prus 2008-06-10 13:51 ` Daniel Jacobowitz 0 siblings, 1 reply; 14+ messages in thread From: Vladimir Prus @ 2008-06-10 13:25 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: Joel Brobecker, gdb-patches On Tuesday 10 June 2008 16:43:13 Daniel Jacobowitz wrote: > On Tue, Jun 10, 2008 at 01:36:23PM +0400, Vladimir Prus wrote: > > If proceed throws, before calling normal_stop, we'll get back to event loop, > > and run cleanup. We won't call the observer. It's an issue if we've printed > > "*running" and thrown after after. However, it's the issue we have now, as > > well -- we print ^running even before calling proceed, and if something later > > throws, we'll never print *stopped. Possible solutions are: > > - Require that frontend refresh thread state on ^error > > - Emit *stopped if exception is thrown (this requires checking that the > > target is actually stopped, if exception is thrown). > > Or just don't print *running until we're actually running. We emit *running only when we're actually running -- after resuming target. The danger, as I understand it, is that if exception is thrown after we've resumed target, but before normal_stop called an observer, we'll have *running not paired with *stopped, so the frontend will think the target is still running. - Volodya ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Use observers to report stop events. 2008-06-10 13:25 ` Vladimir Prus @ 2008-06-10 13:51 ` Daniel Jacobowitz 0 siblings, 0 replies; 14+ messages in thread From: Daniel Jacobowitz @ 2008-06-10 13:51 UTC (permalink / raw) To: Vladimir Prus; +Cc: Joel Brobecker, gdb-patches On Tue, Jun 10, 2008 at 04:57:29PM +0400, Vladimir Prus wrote: > We emit *running only when we're actually running -- after resuming > target. The danger, as I understand it, is that if exception is thrown > after we've resumed target, but before normal_stop called an observer, > we'll have *running not paired with *stopped, so the frontend will think > the target is still running. Er, right. Sorry, I got turned around in there somewhere. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2008-06-10 13:21 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-04-11 20:05 [RFA] Use observers to report stop events Vladimir Prus 2008-04-24 13:47 ` Vladimir Prus 2008-04-29 8:03 ` Joel Brobecker 2008-04-29 18:30 ` Vladimir Prus 2008-05-01 19:58 ` Daniel Jacobowitz 2008-05-01 20:11 ` Pedro Alves 2008-05-01 20:17 ` Vladimir Prus 2008-05-04 9:05 ` Vladimir Prus 2008-05-28 18:30 ` Vladimir Prus 2008-06-05 15:41 ` Daniel Jacobowitz 2008-06-10 11:58 ` Vladimir Prus 2008-06-10 13:21 ` Daniel Jacobowitz 2008-06-10 13:25 ` Vladimir Prus 2008-06-10 13:51 ` Daniel Jacobowitz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox