diff options
author | Pedro Alves <palves@redhat.com> | 2015-09-14 15:45:14 +0100 |
---|---|---|
committer | Pedro Alves <palves@redhat.com> | 2015-09-14 15:45:14 +0100 |
commit | 4c2f2a792a5971fcc7fe6511725eaf50d19d302b (patch) | |
tree | 1bcd3950962a71534108444d1a97e2d77de378f6 | |
parent | 919e6dbe9b61a27e8f7f89121ba182907df461a3 (diff) | |
download | fsf-binutils-gdb-4c2f2a792a5971fcc7fe6511725eaf50d19d302b.zip fsf-binutils-gdb-4c2f2a792a5971fcc7fe6511725eaf50d19d302b.tar.gz fsf-binutils-gdb-4c2f2a792a5971fcc7fe6511725eaf50d19d302b.tar.bz2 |
Bail out of processing stop if hook-stop resumes target / changes context
This patch, relative to a tree with
https://sourceware.org/ml/gdb-patches/2015-08/msg00295.html, fixes
issues/crashes that trigger if something unexpected happens during a
hook-stop.
E.g., if the inferior disappears while running the hook-stop, we hit
failed assertions:
(gdb) define hook-stop
Type commands for definition of "hook-stop".
End with a line saying just "end".
>kill
>end
(gdb) si
Kill the program being debugged? (y or n) [answered Y; input not from terminal]
/home/pedro/gdb/mygit/build/../src/gdb/thread.c:88: internal-error: inferior_thread: Assertion `tp' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)
I noticed that if a hook-stop issues a synchronous execution command,
we print the same stop event twice:
(gdb) define hook-stop
Type commands for definition of "hook-stop".
End with a line saying just "end".
>si
>end
(gdb) si
0x000000000040074a 42 args[i] = 1; /* Init value. */ <<<<<<< once
0x000000000040074a 42 args[i] = 1; /* Init value. */ <<<<<<< twice
(gdb)
In MI:
*stopped,reason="end-stepping-range",frame={addr="0x000000000040074a",func="main",args=[],file="threads.c",fullname="/home/pedro/gdb/tests/threads.c",line="42"},thread-id="1",stopped-threads="all",core="0"
*stopped,reason="end-stepping-range",frame={addr="0x000000000040074a",func="main",args=[],file="threads.c",fullname="/home/pedro/gdb/tests/threads.c",line="42"},thread-id="1",stopped-threads="all",core="0"
(gdb)
The fix has GDB stop processing the event if the context changed. I
don't expect people to be doing crazy things from the hook-stop.
E.g., it gives me headaches to try to come up a proper behavior for
handling a thread change from a hook-stop... (E.g., imagine the
hook-stop does thread N; step, with scheduler-locing on). I think the
most important bit here is preventing crashes.
The patch adds a new hook-stop.exp test that covers the above and also
merges in the old hook-stop-continue.exp and hook-stop-frame.exp into
the same framework.
gdb/ChangeLog:
2015-09-14 Pedro Alves <palves@redhat.com>
* infrun.c (current_stop_id): New global.
(get_stop_id, new_stop_id): New functions.
(fetch_inferior_event): Handle normal_stop proceeding the target.
(struct stop_context): New.
(save_stop_context, release_stop_context_cleanup)
(stop_context_changed): New functions.
(normal_stop): Return true if the hook-stop changes the stop
context.
* infrun.h (get_stop_id): Declare.
(normal_stop): Now returns int. Add documentation.
gdb/testsuite/ChangeLog:
2015-09-14 Pedro Alves <palves@redhat.com>
* gdb.base/hook-stop-continue.c: Delete.
* gdb.base/hook-stop-continue.exp: Delete.
* gdb.base/hook-stop-frame.c: Delete.
* gdb.base/hook-stop-frame.exp: Delete.
* gdb.base/hook-stop.c: New file.
* gdb.base/hook-stop.exp: New file.
-rw-r--r-- | gdb/ChangeLog | 13 | ||||
-rw-r--r-- | gdb/infrun.c | 148 | ||||
-rw-r--r-- | gdb/infrun.h | 11 | ||||
-rw-r--r-- | gdb/testsuite/ChangeLog | 9 | ||||
-rw-r--r-- | gdb/testsuite/gdb.base/hook-stop-continue.c | 42 | ||||
-rw-r--r-- | gdb/testsuite/gdb.base/hook-stop-continue.exp | 52 | ||||
-rw-r--r-- | gdb/testsuite/gdb.base/hook-stop-frame.exp | 48 | ||||
-rw-r--r-- | gdb/testsuite/gdb.base/hook-stop.c (renamed from gdb/testsuite/gdb.base/hook-stop-frame.c) | 6 | ||||
-rw-r--r-- | gdb/testsuite/gdb.base/hook-stop.exp | 168 |
9 files changed, 341 insertions, 156 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 3ca1bf1..0f9efce 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,16 @@ +2015-09-14 Pedro Alves <palves@redhat.com> + + * infrun.c (current_stop_id): New global. + (get_stop_id, new_stop_id): New functions. + (fetch_inferior_event): Handle normal_stop proceeding the target. + (struct stop_context): New. + (save_stop_context, release_stop_context_cleanup) + (stop_context_changed): New functions. + (normal_stop): Return true if the hook-stop changes the stop + context. + * infrun.h (get_stop_id): Declare. + (normal_stop): Now returns int. Add documentation. + 2015-09-14 Pierre-Marie de Rodat <derodat@adacore.com> * ada-lang.c (ada_value_ptr_subscript): Update the heading diff --git a/gdb/infrun.c b/gdb/infrun.c index 84890b4..8175fb1 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -2702,6 +2702,33 @@ resume (enum gdb_signal sig) /* Proceeding. */ +/* See infrun.h. */ + +/* Counter that tracks number of user visible stops. This can be used + to tell whether a command has proceeded the inferior past the + current location. This allows e.g., inferior function calls in + breakpoint commands to not interrupt the command list. When the + call finishes successfully, the inferior is standing at the same + breakpoint as if nothing happened (and so we don't call + normal_stop). */ +static ULONGEST current_stop_id; + +/* See infrun.h. */ + +ULONGEST +get_stop_id (void) +{ + return current_stop_id; +} + +/* Called when we report a user visible stop. */ + +static void +new_stop_id (void) +{ + current_stop_id++; +} + /* Clear out all variables saying what to do when inferior is continued. First do this, then set the ones you want, then call `proceed'. */ @@ -3854,12 +3881,17 @@ fetch_inferior_event (void *client_data) if (should_notify_stop) { + int proceeded = 0; + /* We may not find an inferior if this was a process exit. */ if (inf == NULL || inf->control.stop_soon == NO_STOP_QUIETLY) - normal_stop (); + proceeded = normal_stop (); - inferior_event_handler (INF_EXEC_COMPLETE, NULL); - cmd_done = 1; + if (!proceeded) + { + inferior_event_handler (INF_EXEC_COMPLETE, NULL); + cmd_done = 1; + } } } } @@ -7829,15 +7861,83 @@ maybe_remove_breakpoints (void) } } -/* Here to return control to GDB when the inferior stops for real. - Print appropriate messages, remove breakpoints, give terminal our modes. +/* The execution context that just caused a normal stop. */ + +struct stop_context +{ + /* The stop ID. */ + ULONGEST stop_id; - STOP_PRINT_FRAME nonzero means print the executing frame - (pc, function, args, file, line number and line text). - BREAKPOINTS_FAILED nonzero means stop was due to error - attempting to insert breakpoints. */ + /* The event PTID. */ -void + ptid_t ptid; + + /* If stopp for a thread event, this is the thread that caused the + stop. */ + struct thread_info *thread; + + /* The inferior that caused the stop. */ + int inf_num; +}; + +/* Returns a new stop context. If stopped for a thread event, this + takes a strong reference to the thread. */ + +static struct stop_context * +save_stop_context (void) +{ + struct stop_context *sc = xmalloc (sizeof (struct stop_context)); + + sc->stop_id = get_stop_id (); + sc->ptid = inferior_ptid; + sc->inf_num = current_inferior ()->num; + + if (!ptid_equal (inferior_ptid, null_ptid)) + { + /* Take a strong reference so that the thread can't be deleted + yet. */ + sc->thread = inferior_thread (); + sc->thread->refcount++; + } + else + sc->thread = NULL; + + return sc; +} + +/* Release a stop context previously created with save_stop_context. + Releases the strong reference to the thread as well. */ + +static void +release_stop_context_cleanup (void *arg) +{ + struct stop_context *sc = arg; + + if (sc->thread != NULL) + sc->thread->refcount--; + xfree (sc); +} + +/* Return true if the current context no longer matches the saved stop + context. */ + +static int +stop_context_changed (struct stop_context *prev) +{ + if (!ptid_equal (prev->ptid, inferior_ptid)) + return 1; + if (prev->inf_num != current_inferior ()->num) + return 1; + if (prev->thread != NULL && prev->thread->state != THREAD_STOPPED) + return 1; + if (get_stop_id () != prev->stop_id) + return 1; + return 0; +} + +/* See infrun.h. */ + +int normal_stop (void) { struct target_waitstatus last; @@ -7847,6 +7947,8 @@ normal_stop (void) get_last_target_status (&last_ptid, &last); + new_stop_id (); + /* If an exception is thrown from this point on, make sure to propagate GDB's knowledge of the executing state to the frontend/user running state. A QUIT is an easy exception to see @@ -7966,9 +8068,27 @@ normal_stop (void) /* Look up the hook_stop and run it (CLI internally handles problem of stop_command's pre-hook not existing). */ - if (stop_command) - catch_errors (hook_stop_stub, stop_command, - "Error while running hook_stop:\n", RETURN_MASK_ALL); + if (stop_command != NULL) + { + struct stop_context *saved_context = save_stop_context (); + struct cleanup *old_chain + = make_cleanup (release_stop_context_cleanup, saved_context); + + catch_errors (hook_stop_stub, stop_command, + "Error while running hook_stop:\n", RETURN_MASK_ALL); + + /* If the stop hook resumes the target, then there's no point in + trying to notify about the previous stop; its context is + gone. Likewise if the command switches thread or inferior -- + the observers would print a stop for the wrong + thread/inferior. */ + if (stop_context_changed (saved_context)) + { + do_cleanups (old_chain); + return 1; + } + do_cleanups (old_chain); + } /* Notify observers about the stop. This is where the interpreters print the stop event. */ @@ -7993,6 +8113,8 @@ normal_stop (void) longer needed. Keeping those around slows down things linearly. Note that this never removes the current inferior. */ prune_inferiors (); + + return 0; } static int diff --git a/gdb/infrun.h b/gdb/infrun.h index bf2c416..ab27538 100644 --- a/gdb/infrun.h +++ b/gdb/infrun.h @@ -62,6 +62,11 @@ extern int non_stop; starting an inferior. */ extern int disable_randomization; +/* Returns a unique identifier for the current stop. This can be used + to tell whether a command has proceeded the inferior past the + current location. */ +extern ULONGEST get_stop_id (void); + /* Reverse execution. */ enum exec_direction_kind { @@ -100,7 +105,11 @@ extern ptid_t user_visible_resume_ptid (int step); extern void wait_for_inferior (void); -extern void normal_stop (void); +/* Return control to GDB when the inferior stops for real. Print + appropriate messages, remove breakpoints, give terminal our modes, + and run the stop hook. Returns true if the stop hook proceeded the + target, false otherwise. */ +extern int normal_stop (void); extern void get_last_target_status (ptid_t *ptid, struct target_waitstatus *status); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 17b264d..d0bf156 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2015-09-14 Pedro Alves <palves@redhat.com> + + * gdb.base/hook-stop-continue.c: Delete. + * gdb.base/hook-stop-continue.exp: Delete. + * gdb.base/hook-stop-frame.c: Delete. + * gdb.base/hook-stop-frame.exp: Delete. + * gdb.base/hook-stop.c: New file. + * gdb.base/hook-stop.exp: New file. + 2015-09-14 Pierre-Marie de Rodat <derodat@adacore.com> * gdb.ada/access_to_packed_array.exp: New testcase. diff --git a/gdb/testsuite/gdb.base/hook-stop-continue.c b/gdb/testsuite/gdb.base/hook-stop-continue.c deleted file mode 100644 index 47090fc..0000000 --- a/gdb/testsuite/gdb.base/hook-stop-continue.c +++ /dev/null @@ -1,42 +0,0 @@ -/* This testcase is part of GDB, the GNU debugger. - - Copyright 2008-2015 Free Software Foundation, Inc. - - This program is free software; you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 3 of the License, or - (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with this program. If not, see <http://www.gnu.org/licenses/>. */ - -int -funbreak (int i) -{ - i = i * 2; /* set breakpoint here */ - i = i + 10; - return i; -} - -int -func (int i) -{ - return i * 2; -} - -int -main (int argc, char **argv, char **envp) -{ - func (1); - func (2); - func (3); - func (4); - funbreak (5); - - return 0; -} diff --git a/gdb/testsuite/gdb.base/hook-stop-continue.exp b/gdb/testsuite/gdb.base/hook-stop-continue.exp deleted file mode 100644 index 06c2fe6..0000000 --- a/gdb/testsuite/gdb.base/hook-stop-continue.exp +++ /dev/null @@ -1,52 +0,0 @@ -# Copyright 2008-2015 Free Software Foundation, Inc. - -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see <http://www.gnu.org/licenses/>. - -standard_testfile .c - -if { [prepare_for_testing ${testfile}.exp "${testfile}" "${testfile}.c" {debug nowarnings}] } { - return -1 -} - -if ![runto_main] then { - perror "Couldn't run to main" -} - -set bp_location [gdb_get_line_number "set breakpoint here"] - -gdb_test "break $bp_location" \ - "Breakpoint.*at.* file .*$srcfile, line $bp_location\\." \ - "breakpoint line number" - -gdb_test "print \$do_continue = 1" "1" - -send_gdb "define hook-stop\n" -gdb_expect { - -re "Type commands for definition of \"hook-stop\".\r\nEnd with a line saying just \"end\".\r\n>$"\ - {send_gdb "if \$do_continue\nset \$do_continue = 0\ncontinue\nend\nend\n" - gdb_expect { - -re "$gdb_prompt $"\ - {pass "define hook-stop command"} - timeout {fail "(timeout) define hook-stop command"} - } - } - -re "$gdb_prompt $"\ - {fail "define hook-stop command"} - timeout {fail "(timeout) define hook-stop command"} -} - -gdb_test "next" "Breakpoint.*funbreak \\(i=5\\) at .*:$bp_location\r\n$bp_location.*set breakpoint here \\*/" \ - "next triggering hook-stop" - -gdb_test "next" "i = i \\+ 10;" "next no hook-stop" diff --git a/gdb/testsuite/gdb.base/hook-stop-frame.exp b/gdb/testsuite/gdb.base/hook-stop-frame.exp deleted file mode 100644 index 5b443ff..0000000 --- a/gdb/testsuite/gdb.base/hook-stop-frame.exp +++ /dev/null @@ -1,48 +0,0 @@ -# Copyright 2009-2015 Free Software Foundation, Inc. - -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see <http://www.gnu.org/licenses/>. - -standard_testfile - -if { [prepare_for_testing ${testfile}.exp "${testfile}" $srcfile {debug nowarnings}] } { - return -1 -} - -if ![runto_main] then { - perror "Couldn't run to main" -} - -set bp_location [gdb_get_line_number "set breakpoint here"] - -gdb_test "break func" \ - "Breakpoint.*at.* file .*$srcfile.*\\." \ - "breakpoint line number" - -set test "define hook-stop command" -gdb_test_multiple "define hook-stop" "$test" { - -re "Type commands for definition of \"hook-stop\".\r\nEnd with a line saying just \"end\".\r\n>$" { - gdb_test "echo \"Hello.\"\nend" "" "$test" - } -} - -set test "hook-stop runs before frame print" -gdb_test_multiple "continue" "$test" { - -re "\"Hello\\.\"\r\nBreakpo.*func.*set breakpoint here.*${gdb_prompt} $" { - pass $test - } - - -re "Breakpo.*func.*set breakpoint here.*\"Hello\\.\".*${gdb_prompt} $" { - fail $test - } -} diff --git a/gdb/testsuite/gdb.base/hook-stop-frame.c b/gdb/testsuite/gdb.base/hook-stop.c index 8dbb8f1..4152f28 100644 --- a/gdb/testsuite/gdb.base/hook-stop-frame.c +++ b/gdb/testsuite/gdb.base/hook-stop.c @@ -21,11 +21,17 @@ func (void) int a; a = 1; /* set breakpoint here */ + a = 2; } int main (int argc, char **argv) { + int i; + + i = 1; + i = 2; + i = 3; func (); return 0; diff --git a/gdb/testsuite/gdb.base/hook-stop.exp b/gdb/testsuite/gdb.base/hook-stop.exp new file mode 100644 index 0000000..cdee7db --- /dev/null +++ b/gdb/testsuite/gdb.base/hook-stop.exp @@ -0,0 +1,168 @@ +# Copyright 2009-2015 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +standard_testfile + +if { [build_executable ${testfile}.exp "${testfile}" $srcfile {debug nowarnings}] } { + return -1 +} + +# Define the hook-stop that runs COMMANDS. + +proc define_hook_stop {commands} { + set test "define hook-stop command" + gdb_test_multiple "define hook-stop" "$test" { + -re "Type commands for definition of \"hook-stop\".\r\nEnd with a line saying just \"end\".\r\n>$" { + gdb_test "$commands\nend" "" "$test" + } + } +} + +# Restart GDB, run to main, set a breakpoint, and define a hook-stop +# that runs COMMANDS. If running to main fails, this returns to the +# caller's caller directly. + +proc setup {commands} { + global srcfile binfile + + clean_restart $binfile + + if ![runto_main] then { + fail "Can't run to main" + return -code return + } + + gdb_test "break func" \ + "Breakpoint.*at.* file .*$srcfile.*\\." \ + "breakpoint line number" + + define_hook_stop $commands +} + +# Check that the hook-stop runs before the frame is printed. + +proc hook_stop_before_frame {} { + with_test_prefix "hook-stop runs before frame print" { + global gdb_prompt + + setup "echo \"Hello.\"" + + set test "run hook-stop" + gdb_test_multiple "continue" "$test" { + -re "\"Hello\\.\"\r\nBreakpo.*func.*set breakpoint here.*${gdb_prompt} $" { + pass $test + } + + -re "Breakpo.*func.*set breakpoint here.*\"Hello\\.\".*${gdb_prompt} $" { + fail $test + } + } + } +} + +# Check that GDB gracefully handles the case of the inferior dying +# while running the hook-stop. + +proc hook_stop_kill {} { + with_test_prefix "hook-stop kills inferior" { + global gdb_prompt + + setup "kill" + + gdb_test_no_output "set confirm off" + + set test "run hook-stop" + gdb_test_multiple "continue" "$test" { + -re "Continuing.\r\n${gdb_prompt} $" { + pass $test + } + } + + gdb_test "info threads" "No threads.*" + } +} + +# Check that GDB gracefully handles the case of the hook-stop +# continuing the inferior in the foreground. + +proc hook_stop_continue_fg {} { + with_test_prefix "hook-stop runs continue" { + global gdb_prompt + + setup "if \$do_continue\nset \$do_continue = 0\ncontinue\nend" + + gdb_test "print \$do_continue = 1" " = 1" + + gdb_test "next" "Breakpoint.*func \\(\\) at .*set breakpoint here \\*/" \ + "next triggering hook-stop" + + gdb_test "next" "a = 2;" "next no hook-stop" + } +} + +# Check that GDB gracefully handles the case of the hook-stop +# continuing the inferior in the background. + +proc hook_stop_continue_bg {} { + with_test_prefix "hook-stop runs continue&" { + global gdb_prompt + + setup "if \$do_continue\nset \$do_continue = 0\ncontinue&\nend" + + gdb_test "print \$do_continue = 1" " = 1" + + set test "run hook-stop" + gdb_test_multiple "continue" "$test" { + -re "Continuing.\r\n.*${gdb_prompt} " { + pass $test + } + } + + set test "inferior exits normally" + gdb_test_multiple "" "$test" { + -re "exited normally" { + pass $test + } + } + gdb_test "info threads" "No threads.*" + } +} + +# Check that GDB gracefully handles the case of the hook-stop running +# "next". GDB used to print the stop event twice. + +proc hook_stop_next {} { + with_test_prefix "hook-stop runs next" { + global gdb_prompt + + setup "next" + + set test "run hook-stop" + gdb_test_multiple "continue" "$test" { + -re "a = 2.*a = 2${gdb_prompt} $" { + fail $test + } + -re "a = 2.*${gdb_prompt} $" { + pass $test + } + } + } +} + +hook_stop_before_frame +hook_stop_kill +hook_stop_continue_fg +hook_stop_continue_bg +hook_stop_next |