This is the mail archive of the
gdb-cvs@sourceware.org
mailing list for the GDB project.
[binutils-gdb] Bail out of processing stop if hook-stop resumes target / changes context
- From: Pedro Alves <palves at sourceware dot org>
- To: gdb-cvs at sourceware dot org
- Date: 14 Sep 2015 14:49:52 -0000
- Subject: [binutils-gdb] Bail out of processing stop if hook-stop resumes target / changes context
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=4c2f2a792a5971fcc7fe6511725eaf50d19d302b
commit 4c2f2a792a5971fcc7fe6511725eaf50d19d302b
Author: Pedro Alves <palves@redhat.com>
Date: Mon Sep 14 15:45:14 2015 +0100
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.
Diff:
---
gdb/ChangeLog | 13 ++
gdb/infrun.c | 148 ++++++++++++++++--
gdb/infrun.h | 11 +-
gdb/testsuite/ChangeLog | 9 ++
gdb/testsuite/gdb.base/hook-stop-continue.c | 42 ------
gdb/testsuite/gdb.base/hook-stop-continue.exp | 52 -------
gdb/testsuite/gdb.base/hook-stop-frame.exp | 48 ------
.../gdb.base/{hook-stop-frame.c => hook-stop.c} | 6 +
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
similarity index 95%
rename from gdb/testsuite/gdb.base/hook-stop-frame.c
rename to 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