* [RFA] Fix hand called function when another thread has hit a bp.
@ 2008-12-02 3:01 Doug Evans
2008-12-02 3:48 ` Doug Evans
0 siblings, 1 reply; 14+ messages in thread
From: Doug Evans @ 2008-12-02 3:01 UTC (permalink / raw)
To: gdb-patches
Hi. In http://sourceware.org/ml/gdb-patches/2008-11/msg00531.html
I wrote:
> I wrote a testcase that calls functions in multiple threads (with
> scheduler-locking on) by setting a breakpoint on the function being
> called. I think there's a bug in scheduler-locking support as when I
> make the second call in the second thread, gdb makes the first thread
> step over the breakpoint and then resume, and control returns to
> call_function_by_hand with inferior_ptid changed to the first thread.
Here's a patch.
This is separate from my dummy-frames cleanup patch.
Ok to check in?
2008-12-01 Doug Evans <dje@google.com>
* infrun.c (prepare_to_proceed): Document. Assert !non_stop.
If scheduler-locking is enabled, we're not going to be singlestepping
any other previously stopped thread.
* gdb.threads/hand-call-in-threads.exp: New file.
* gdb.threads/hand-call-in-threads.c: New file.
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.342
diff -u -p -r1.342 infrun.c
--- infrun.c 22 Nov 2008 04:41:45 -0000 1.342
+++ infrun.c 2 Dec 2008 02:10:59 -0000
@@ -1240,13 +1240,21 @@ clear_proceed_status (void)
}
}
-/* This should be suitable for any targets that support threads. */
+/* Check the current thread against the thread that reported the most recent
+ event. If a step-over is required return TRUE and set the current thread
+ to the old thread. Otherwise return FALSE.
+
+ This should be suitable for any targets that support threads. */
static int
prepare_to_proceed (int step)
{
ptid_t wait_ptid;
struct target_waitstatus wait_status;
+ int schedlock_enabled;
+
+ /* With non-stop mode on, threads are always handled individually. */
+ gdb_assert (! non_stop);
/* Get the last target status returned by target_wait(). */
get_last_target_status (&wait_ptid, &wait_status);
@@ -1258,9 +1266,15 @@ prepare_to_proceed (int step)
return 0;
}
+ schedlock_enabled = (scheduler_mode == schedlock_on
+ || (scheduler_mode == schedlock_step
+ && step));
+
/* Switched over from WAIT_PID. */
if (!ptid_equal (wait_ptid, minus_one_ptid)
- && !ptid_equal (inferior_ptid, wait_ptid))
+ && !ptid_equal (inferior_ptid, wait_ptid)
+ /* Don't single step WAIT_PID if scheduler locking is on. */
+ && !schedlock_enabled)
{
struct regcache *regcache = get_thread_regcache (wait_ptid);
Index: testsuite/gdb.threads/hand-call-in-threads.c
===================================================================
RCS file: testsuite/gdb.threads/hand-call-in-threads.c
diff -N testsuite/gdb.threads/hand-call-in-threads.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.threads/hand-call-in-threads.c 2 Dec 2008 02:54:49 -0000
@@ -0,0 +1,134 @@
+/* Test case for hand function calls in multi-threaded program.
+
+ Copyright 2008 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <time.h>
+#include <unistd.h>
+
+/* NOTE: Also defined in hand-call-in-threads.exp. */
+#define NR_THREADS 4
+
+int thread_count;
+
+pthread_mutex_t thread_count_mutex;
+
+pthread_cond_t thread_count_condvar;
+
+void
+incr_thread_count (void)
+{
+ pthread_mutex_lock (&thread_count_mutex);
+ ++thread_count;
+ if (thread_count == NR_THREADS)
+ pthread_cond_signal (&thread_count_condvar);
+ pthread_mutex_unlock (&thread_count_mutex);
+}
+
+void
+cond_wait (pthread_cond_t *cond, pthread_mutex_t *mut)
+{
+ pthread_mutex_lock (mut);
+ pthread_cond_wait (cond, mut);
+ pthread_mutex_unlock (mut);
+}
+
+void
+noreturn (void)
+{
+ pthread_mutex_t mut;
+ pthread_cond_t cond;
+
+ pthread_mutex_init (&mut, NULL);
+ pthread_cond_init (&cond, NULL);
+
+ /* Wait for a condition that will never be signaled, so we effectively
+ block the thread here. */
+ cond_wait (&cond, &mut);
+}
+
+void *
+forever_pthread (void *unused)
+{
+ incr_thread_count ();
+ noreturn ();
+}
+
+void
+hand_call (void)
+{
+}
+
+/* Wait until all threads are running. */
+
+void
+wait_all_threads_running (void)
+{
+ int timeout = 60; /* seconds */
+ int i;
+
+ for (i = 0; i < timeout; ++i)
+ {
+ pthread_mutex_lock (&thread_count_mutex);
+ if (thread_count == NR_THREADS)
+ {
+ pthread_mutex_unlock (&thread_count_mutex);
+ return;
+ }
+ pthread_cond_wait (&thread_count_condvar, &thread_count_mutex);
+ if (thread_count == NR_THREADS)
+ {
+ pthread_mutex_unlock (&thread_count_mutex);
+ return;
+ }
+ pthread_mutex_unlock (&thread_count_mutex);
+ sleep (1);
+ }
+
+ printf ("timeout waiting for all threads to start\n");
+ abort ();
+}
+
+/* Called when all threads are running.
+ Easy place for a breakpoint. */
+
+void
+all_threads_running (void)
+{
+}
+
+int
+main (void)
+{
+ pthread_t forever[NR_THREADS];
+ int i;
+
+ pthread_mutex_init (&thread_count_mutex, NULL);
+ pthread_cond_init (&thread_count_condvar, NULL);
+
+ for (i = 0; i < NR_THREADS; ++i)
+ pthread_create (&forever[i], NULL, forever_pthread, NULL);
+
+ wait_all_threads_running ();
+ all_threads_running ();
+
+ return 0;
+}
+
Index: testsuite/gdb.threads/hand-call-in-threads.exp
===================================================================
RCS file: testsuite/gdb.threads/hand-call-in-threads.exp
diff -N testsuite/gdb.threads/hand-call-in-threads.exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.threads/hand-call-in-threads.exp 2 Dec 2008 02:54:49 -0000
@@ -0,0 +1,140 @@
+# Copyright (C) 2004, 2007, 2008 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/>.
+
+# Test making hand function calls in multiple threads.
+
+# NOTE: Also defined in hand-call-in-threads.c.
+set NR_THREADS 4
+
+if $tracelevel then {
+ strace $tracelevel
+}
+
+set testfile "hand-call-in-threads"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "incdir=${objdir}"]] != "" } {
+ return -1
+}
+
+# Some targets can't do function calls, so don't even bother with this
+# test.
+if [target_info exists gdb,cannot_call_functions] {
+ setup_xfail "*-*-*" 2416
+ fail "This target can not call functions"
+ continue
+}
+
+proc get_dummy_frame_number { } {
+ global gdb_prompt
+
+ send_gdb "bt\n"
+ gdb_expect {
+ -re "#(\[0-9\]*) *<function called from gdb>.*$gdb_prompt $"
+ {
+ return $expect_out(1,string)
+ }
+ -re "$gdb_prompt $"
+ {
+ return ""
+ }
+ timeout
+ {
+ return ""
+ }
+ }
+ return ""
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if { ![runto_main] } {
+ fail "Can't run to main"
+ return 0
+}
+
+gdb_test "break all_threads_running" \
+ "Breakpoint 2 at .*: file .*${srcfile}, line .*" \
+ "breakpoint on all_threads_running"
+
+gdb_test "break hand_call" \
+ "Breakpoint 3 at .*: file .*${srcfile}, line .*" \
+ "breakpoint on hand_call"
+
+# Run the program and make sure GDB reports that we stopped after
+# hitting breakpoint 2 in all_threads_running().
+
+gdb_test "continue" \
+ ".*Breakpoint 2, all_threads_running ().*" \
+ "run to all_threads_running"
+
+# Before we start making hand function calls, turn on scheduler locking.
+
+gdb_test "set scheduler-locking on" "" "enable scheduler locking"
+gdb_test "show scheduler-locking" ".* locking scheduler .* is \"on\"." "show scheduler locking"
+
+# Now hand-call a function in each thread, having the function
+# stop without returning.
+
+# Add one for the main thread.
+set total_nr_threads [expr $NR_THREADS + 1]
+
+# Thread numbering in gdb is origin-1, so begin numbering at 1.
+for { set i 1 } { $i <= $total_nr_threads } { incr i } {
+ set thread_nr $i
+ gdb_test "thread $thread_nr" "" "prepare to make hand call, thread $thread_nr"
+ gdb_test "call hand_call()" "Breakpoint 3, .*" "hand call, thread $thread_nr"
+}
+
+# Now have each hand-called function return.
+
+# Turn confirmation off for the "return" command.
+gdb_test "set confirm off" ""
+
+clear_xfail "*-*-*"
+
+for { set i 1 } { $i <= $total_nr_threads } { incr i } {
+ set thread_nr $i
+ gdb_test "thread $thread_nr" "" "prepare to discard hand call, thread $thread_nr"
+ set frame_number [get_dummy_frame_number]
+ if { "$frame_number" == "" } {
+ fail "dummy stack frame number, thread $thread_nr"
+ setup_xfail "*-*-*"
+ # Need something.
+ set frame_number 0
+ } else {
+ pass "dummy stack frame number, thread $thread_nr"
+ }
+ # Pop the dummy frame.
+ gdb_test "frame $frame_number" "" "setting frame, thread $thread_nr"
+ gdb_test "return" "" "discard hand call, thread $thread_nr"
+ # In case getting the dummy frame number failed, re-enable for next iter.
+ clear_xfail "*-*-*"
+}
+
+# Make sure all dummy frames got popped.
+
+gdb_test_multiple "maint print dummy-frames" "all dummies popped" {
+ -re ".*stack=.*$gdb_prompt $" {
+ fail "all dummies popped"
+ }
+ -re ".*$gdb_prompt $" {
+ pass "all dummies popped"
+ }
+}
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFA] Fix hand called function when another thread has hit a bp. 2008-12-02 3:01 [RFA] Fix hand called function when another thread has hit a bp Doug Evans @ 2008-12-02 3:48 ` Doug Evans 2008-12-02 11:41 ` Doug Evans 0 siblings, 1 reply; 14+ messages in thread From: Doug Evans @ 2008-12-02 3:48 UTC (permalink / raw) To: gdb-patches On Mon, Dec 1, 2008 at 7:00 PM, Doug Evans <dje@google.com> wrote: > Hi. In http://sourceware.org/ml/gdb-patches/2008-11/msg00531.html > I wrote: > >> I wrote a testcase that calls functions in multiple threads (with >> scheduler-locking on) by setting a breakpoint on the function being >> called. I think there's a bug in scheduler-locking support as when I >> make the second call in the second thread, gdb makes the first thread >> step over the breakpoint and then resume, and control returns to >> call_function_by_hand with inferior_ptid changed to the first thread. > > Here's a patch. > This is separate from my dummy-frames cleanup patch. > > Ok to check in? Except of course nothing is ever this easy. :-( My testcase isn't complete: set scheduler-locking off continue Adding the above to the end of the testcase reveals the fact that this patch causes gdb to lose track of the fact that it needs to single step over the breakpoint at all_threads_running in the main thread, and resuming execution causes the breakpoint to be hit again. Global state, gotta love it. I'm assuming non-stop mode doesn't have this problem. Can we record in struct thread_info (or some such) the last stop reason and before resuming with !scheduler-locking iterate over all threads, single stepping them as necessary? Is there a better solution? > 2008-12-01 Doug Evans <dje@google.com> > > * infrun.c (prepare_to_proceed): Document. Assert !non_stop. > If scheduler-locking is enabled, we're not going to be singlestepping > any other previously stopped thread. > > * gdb.threads/hand-call-in-threads.exp: New file. > * gdb.threads/hand-call-in-threads.c: New file. > > Index: infrun.c > =================================================================== > RCS file: /cvs/src/src/gdb/infrun.c,v > retrieving revision 1.342 > diff -u -p -r1.342 infrun.c > --- infrun.c 22 Nov 2008 04:41:45 -0000 1.342 > +++ infrun.c 2 Dec 2008 02:10:59 -0000 > @@ -1240,13 +1240,21 @@ clear_proceed_status (void) > } > } > > -/* This should be suitable for any targets that support threads. */ > +/* Check the current thread against the thread that reported the most recent > + event. If a step-over is required return TRUE and set the current thread > + to the old thread. Otherwise return FALSE. > + > + This should be suitable for any targets that support threads. */ > > static int > prepare_to_proceed (int step) > { > ptid_t wait_ptid; > struct target_waitstatus wait_status; > + int schedlock_enabled; > + > + /* With non-stop mode on, threads are always handled individually. */ > + gdb_assert (! non_stop); > > /* Get the last target status returned by target_wait(). */ > get_last_target_status (&wait_ptid, &wait_status); > @@ -1258,9 +1266,15 @@ prepare_to_proceed (int step) > return 0; > } > > + schedlock_enabled = (scheduler_mode == schedlock_on > + || (scheduler_mode == schedlock_step > + && step)); > + > /* Switched over from WAIT_PID. */ > if (!ptid_equal (wait_ptid, minus_one_ptid) > - && !ptid_equal (inferior_ptid, wait_ptid)) > + && !ptid_equal (inferior_ptid, wait_ptid) > + /* Don't single step WAIT_PID if scheduler locking is on. */ > + && !schedlock_enabled) > { > struct regcache *regcache = get_thread_regcache (wait_ptid); > > Index: testsuite/gdb.threads/hand-call-in-threads.c > =================================================================== > RCS file: testsuite/gdb.threads/hand-call-in-threads.c > diff -N testsuite/gdb.threads/hand-call-in-threads.c > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ testsuite/gdb.threads/hand-call-in-threads.c 2 Dec 2008 02:54:49 -0000 > @@ -0,0 +1,134 @@ > +/* Test case for hand function calls in multi-threaded program. > + > + Copyright 2008 Free Software Foundation, Inc. > + > + This file is part of GDB. > + > + 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/>. */ > + > +#include <pthread.h> > +#include <stdio.h> > +#include <stdlib.h> > +#include <time.h> > +#include <unistd.h> > + > +/* NOTE: Also defined in hand-call-in-threads.exp. */ > +#define NR_THREADS 4 > + > +int thread_count; > + > +pthread_mutex_t thread_count_mutex; > + > +pthread_cond_t thread_count_condvar; > + > +void > +incr_thread_count (void) > +{ > + pthread_mutex_lock (&thread_count_mutex); > + ++thread_count; > + if (thread_count == NR_THREADS) > + pthread_cond_signal (&thread_count_condvar); > + pthread_mutex_unlock (&thread_count_mutex); > +} > + > +void > +cond_wait (pthread_cond_t *cond, pthread_mutex_t *mut) > +{ > + pthread_mutex_lock (mut); > + pthread_cond_wait (cond, mut); > + pthread_mutex_unlock (mut); > +} > + > +void > +noreturn (void) > +{ > + pthread_mutex_t mut; > + pthread_cond_t cond; > + > + pthread_mutex_init (&mut, NULL); > + pthread_cond_init (&cond, NULL); > + > + /* Wait for a condition that will never be signaled, so we effectively > + block the thread here. */ > + cond_wait (&cond, &mut); > +} > + > +void * > +forever_pthread (void *unused) > +{ > + incr_thread_count (); > + noreturn (); > +} > + > +void > +hand_call (void) > +{ > +} > + > +/* Wait until all threads are running. */ > + > +void > +wait_all_threads_running (void) > +{ > + int timeout = 60; /* seconds */ > + int i; > + > + for (i = 0; i < timeout; ++i) > + { > + pthread_mutex_lock (&thread_count_mutex); > + if (thread_count == NR_THREADS) > + { > + pthread_mutex_unlock (&thread_count_mutex); > + return; > + } > + pthread_cond_wait (&thread_count_condvar, &thread_count_mutex); > + if (thread_count == NR_THREADS) > + { > + pthread_mutex_unlock (&thread_count_mutex); > + return; > + } > + pthread_mutex_unlock (&thread_count_mutex); > + sleep (1); > + } > + > + printf ("timeout waiting for all threads to start\n"); > + abort (); > +} > + > +/* Called when all threads are running. > + Easy place for a breakpoint. */ > + > +void > +all_threads_running (void) > +{ > +} > + > +int > +main (void) > +{ > + pthread_t forever[NR_THREADS]; > + int i; > + > + pthread_mutex_init (&thread_count_mutex, NULL); > + pthread_cond_init (&thread_count_condvar, NULL); > + > + for (i = 0; i < NR_THREADS; ++i) > + pthread_create (&forever[i], NULL, forever_pthread, NULL); > + > + wait_all_threads_running (); > + all_threads_running (); > + > + return 0; > +} > + > Index: testsuite/gdb.threads/hand-call-in-threads.exp > =================================================================== > RCS file: testsuite/gdb.threads/hand-call-in-threads.exp > diff -N testsuite/gdb.threads/hand-call-in-threads.exp > --- /dev/null 1 Jan 1970 00:00:00 -0000 > +++ testsuite/gdb.threads/hand-call-in-threads.exp 2 Dec 2008 02:54:49 -0000 > @@ -0,0 +1,140 @@ > +# Copyright (C) 2004, 2007, 2008 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/>. > + > +# Test making hand function calls in multiple threads. > + > +# NOTE: Also defined in hand-call-in-threads.c. > +set NR_THREADS 4 > + > +if $tracelevel then { > + strace $tracelevel > +} > + > +set testfile "hand-call-in-threads" > +set srcfile ${testfile}.c > +set binfile ${objdir}/${subdir}/${testfile} > + > +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "incdir=${objdir}"]] != "" } { > + return -1 > +} > + > +# Some targets can't do function calls, so don't even bother with this > +# test. > +if [target_info exists gdb,cannot_call_functions] { > + setup_xfail "*-*-*" 2416 > + fail "This target can not call functions" > + continue > +} > + > +proc get_dummy_frame_number { } { > + global gdb_prompt > + > + send_gdb "bt\n" > + gdb_expect { > + -re "#(\[0-9\]*) *<function called from gdb>.*$gdb_prompt $" > + { > + return $expect_out(1,string) > + } > + -re "$gdb_prompt $" > + { > + return "" > + } > + timeout > + { > + return "" > + } > + } > + return "" > +} > + > +gdb_exit > +gdb_start > +gdb_reinitialize_dir $srcdir/$subdir > +gdb_load ${binfile} > + > +if { ![runto_main] } { > + fail "Can't run to main" > + return 0 > +} > + > +gdb_test "break all_threads_running" \ > + "Breakpoint 2 at .*: file .*${srcfile}, line .*" \ > + "breakpoint on all_threads_running" > + > +gdb_test "break hand_call" \ > + "Breakpoint 3 at .*: file .*${srcfile}, line .*" \ > + "breakpoint on hand_call" > + > +# Run the program and make sure GDB reports that we stopped after > +# hitting breakpoint 2 in all_threads_running(). > + > +gdb_test "continue" \ > + ".*Breakpoint 2, all_threads_running ().*" \ > + "run to all_threads_running" > + > +# Before we start making hand function calls, turn on scheduler locking. > + > +gdb_test "set scheduler-locking on" "" "enable scheduler locking" > +gdb_test "show scheduler-locking" ".* locking scheduler .* is \"on\"." "show scheduler locking" > + > +# Now hand-call a function in each thread, having the function > +# stop without returning. > + > +# Add one for the main thread. > +set total_nr_threads [expr $NR_THREADS + 1] > + > +# Thread numbering in gdb is origin-1, so begin numbering at 1. > +for { set i 1 } { $i <= $total_nr_threads } { incr i } { > + set thread_nr $i > + gdb_test "thread $thread_nr" "" "prepare to make hand call, thread $thread_nr" > + gdb_test "call hand_call()" "Breakpoint 3, .*" "hand call, thread $thread_nr" > +} > + > +# Now have each hand-called function return. > + > +# Turn confirmation off for the "return" command. > +gdb_test "set confirm off" "" > + > +clear_xfail "*-*-*" > + > +for { set i 1 } { $i <= $total_nr_threads } { incr i } { > + set thread_nr $i > + gdb_test "thread $thread_nr" "" "prepare to discard hand call, thread $thread_nr" > + set frame_number [get_dummy_frame_number] > + if { "$frame_number" == "" } { > + fail "dummy stack frame number, thread $thread_nr" > + setup_xfail "*-*-*" > + # Need something. > + set frame_number 0 > + } else { > + pass "dummy stack frame number, thread $thread_nr" > + } > + # Pop the dummy frame. > + gdb_test "frame $frame_number" "" "setting frame, thread $thread_nr" > + gdb_test "return" "" "discard hand call, thread $thread_nr" > + # In case getting the dummy frame number failed, re-enable for next iter. > + clear_xfail "*-*-*" > +} > + > +# Make sure all dummy frames got popped. > + > +gdb_test_multiple "maint print dummy-frames" "all dummies popped" { > + -re ".*stack=.*$gdb_prompt $" { > + fail "all dummies popped" > + } > + -re ".*$gdb_prompt $" { > + pass "all dummies popped" > + } > +} > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Fix hand called function when another thread has hit a bp. 2008-12-02 3:48 ` Doug Evans @ 2008-12-02 11:41 ` Doug Evans 2008-12-14 22:00 ` Doug Evans 0 siblings, 1 reply; 14+ messages in thread From: Doug Evans @ 2008-12-02 11:41 UTC (permalink / raw) To: gdb-patches [-- Attachment #1: Type: text/plain, Size: 3507 bytes --] On Mon, Dec 1, 2008 at 7:46 PM, Doug Evans <dje@google.com> wrote: > On Mon, Dec 1, 2008 at 7:00 PM, Doug Evans <dje@google.com> wrote: >> Hi. In http://sourceware.org/ml/gdb-patches/2008-11/msg00531.html >> I wrote: >> >>> I wrote a testcase that calls functions in multiple threads (with >>> scheduler-locking on) by setting a breakpoint on the function being >>> called. I think there's a bug in scheduler-locking support as when I >>> make the second call in the second thread, gdb makes the first thread >>> step over the breakpoint and then resume, and control returns to >>> call_function_by_hand with inferior_ptid changed to the first thread. >> >> Here's a patch. >> This is separate from my dummy-frames cleanup patch. >> >> Ok to check in? > > Except of course nothing is ever this easy. :-( > > My testcase isn't complete: > > set scheduler-locking off > continue > > Adding the above to the end of the testcase reveals the fact that this > patch causes gdb to lose track of the fact that it needs to single > step over the breakpoint at all_threads_running in the main thread, > and resuming execution causes the breakpoint to be hit again. Global > state, gotta love it. > > I'm assuming non-stop mode doesn't have this problem. > Can we record in struct thread_info (or some such) the last stop > reason and before resuming with !scheduler-locking iterate over all > threads, single stepping them as necessary? Is there a better > solution? This patch fixes the expanded hand-call-in-threads.exp testcase (which is part of the patch). In the process I discovered a bigger problem - gdb doesn't handle resuming after more than one thread is stopped at a breakpoint. This can happen if the user runs several threads in turn with scheduler-locking on, and then turns scheduler-locking off and resumes the program. I wrote another testcase, multi-bp-in-threads.exp, to expose this issue. Fixing this appears to be a much harder problem, and I think I'd like to declare partial victory with this patch ... Comments? I have some questions. Is it sufficient to test "thread->stop_signal == TRAP && breakpoint_here_p (pc)" to check if a thread is stopped at a breakpoint? It seems to be. Another thing this revised patch does is stop SIGTRAP from being passed to the current thread to be run if it was received in the last thread that ran. The comments say this is how gdb behaved before stop_signal was recorded for each thread. It seems odd though. If an asynchronous signal comes in and it is masked in some threads, if the kernel chooses a thread in which it's not blocked, should gdb be changing the thread that receives the signal? This patch takes a minimalist approach and only changes the behaviour of SIGTRAP. 2008-12-02 Doug Evans <dje@google.com> * infrun.c (prepare_to_proceed_callback): New function. (prepare_to_proceed): Document. Assert !non_stop. Add debugging printf. If scheduler-locking is enabled, no other thread need to be singlestepped. Otherwise scan all threads for whether they're stopped at a breakpoint instead of just the last thread that ran. (proceed): Add FIXME. Don't pass on TARGET_SIGNAL_TRAP from the last thread that ran to the current thread to run. * gdb.threads/hand-call-in-threads.exp: New file. * gdb.threads/hand-call-in-threads.c: New file. * gdb.threads/multi-bp-in-threads.exp: New file. * gdb.threads/multi-bp-in-threads.c: New file. [-- Attachment #2: gdb-081202-schedlock-2.patch.txt --] [-- Type: text/plain, Size: 21773 bytes --] 2008-12-02 Doug Evans <dje@google.com> * infrun.c (prepare_to_proceed_callback): New function. (prepare_to_proceed): Document. Assert !non_stop. Add debugging printf. If scheduler-locking is enabled, no other thread need to be singlestepped. Otherwise scan all threads for whether they're stopped at a breakpoint instead of just the last thread that ran. (proceed): Add FIXME. Don't pass on TARGET_SIGNAL_TRAP from the last thread that ran to the current thread to run. * gdb.threads/hand-call-in-threads.exp: New file. * gdb.threads/hand-call-in-threads.c: New file. * gdb.threads/multi-bp-in-threads.exp: New file. * gdb.threads/multi-bp-in-threads.c: New file. Index: infrun.c =================================================================== RCS file: /cvs/src/src/gdb/infrun.c,v retrieving revision 1.344 diff -u -p -r1.344 infrun.c --- infrun.c 2 Dec 2008 09:52:31 -0000 1.344 +++ infrun.c 2 Dec 2008 10:36:16 -0000 @@ -1240,46 +1240,113 @@ clear_proceed_status (void) } } -/* This should be suitable for any targets that support threads. */ +/* Callback for prepare_to_proceed. + Return non-zero if thread TP is not the current thread and is stopped + at a breakpoint. */ + +static int +prepare_to_proceed_callback (struct thread_info *tp, void *data) +{ + struct regcache *regcache; + + if (tp->stop_signal != TARGET_SIGNAL_TRAP) + return 0; + + if (ptid_equal (tp->ptid, inferior_ptid)) + return 0; + + /* Defer reading registers as long as possible. + We're iterating over all threads. */ + regcache = get_thread_regcache (tp->ptid); + + if (breakpoint_here_p (regcache_read_pc (regcache))) + return 1; + + return 0; +} + +/* Check whether any threads other than the current thread need to step over + a breakpoint. + If a step-over is required return TRUE and set the current thread + to the old thread. Otherwise return FALSE. + + This should be suitable for any targets that support threads. */ static int prepare_to_proceed (int step) { ptid_t wait_ptid; struct target_waitstatus wait_status; + int need_to_step_other_thread; - /* Get the last target status returned by target_wait(). */ - get_last_target_status (&wait_ptid, &wait_status); + if (debug_infrun) + fprintf_unfiltered (gdb_stdlog, + "infrun: prepare_to_proceed (step=%d), inferior_ptid %s\n", + step, target_pid_to_str (inferior_ptid)); - /* Make sure we were stopped at a breakpoint. */ - if (wait_status.kind != TARGET_WAITKIND_STOPPED - || wait_status.value.sig != TARGET_SIGNAL_TRAP) - { - return 0; - } + /* With non-stop mode on, threads are always handled individually. */ + gdb_assert (! non_stop); + + /* We won't be running any other threads of scheduler locking is enabled. */ + if (scheduler_mode == schedlock_on + || (scheduler_mode == schedlock_step + && step)) + return 0; + + need_to_step_other_thread = 0; + + /* ??? We call get_last_target_status here and see if it + is different than inferior_ptid and if it stopped at a breakpoint. + If it is it saves us iterating over the thread list. + But how often is it a win? */ + get_last_target_status (&wait_ptid, &wait_status); - /* Switched over from WAIT_PID. */ if (!ptid_equal (wait_ptid, minus_one_ptid) - && !ptid_equal (inferior_ptid, wait_ptid)) + && !ptid_equal (wait_ptid, inferior_ptid) + && wait_status.kind == TARGET_WAITKIND_STOPPED + && wait_status.value.sig == TARGET_SIGNAL_TRAP) { struct regcache *regcache = get_thread_regcache (wait_ptid); if (breakpoint_here_p (regcache_read_pc (regcache))) { - /* If stepping, remember current thread to switch back to. */ - if (step) - deferred_step_ptid = inferior_ptid; - - /* Switch back to WAIT_PID thread. */ - switch_to_thread (wait_ptid); - - /* We return 1 to indicate that there is a breakpoint here, - so we need to step over it before continuing to avoid - hitting it straight away. */ - return 1; + need_to_step_other_thread = 1; } } + if (! need_to_step_other_thread) + { + /* Bad luck, need to search all threads. */ + struct thread_info *tp; + + tp = iterate_over_threads (prepare_to_proceed_callback, NULL); + if (tp != NULL) + { + need_to_step_other_thread = 1; + wait_ptid = tp->ptid; + } + } + + if (need_to_step_other_thread) + { + if (debug_infrun) + fprintf_unfiltered (gdb_stdlog, + "infrun: prepare_to_proceed: need to step other thread first: %s\n", + target_pid_to_str (wait_ptid)); + + /* If stepping, remember current thread to switch back to. */ + if (step) + deferred_step_ptid = inferior_ptid; + + /* Switch back to WAIT_PID thread. */ + switch_to_thread (wait_ptid); + + /* We return 1 to indicate that there is a breakpoint here, + so we need to step over it before continuing to avoid + hitting it straight away. */ + return 1; + } + return 0; } @@ -1357,7 +1424,16 @@ proceed (CORE_ADDR addr, enum target_sig prepare_to_proceed checks the current thread against the thread that reported the most recent event. If a step-over is required it returns TRUE and sets the current thread to - the old thread. */ + the old thread. + + FIXME: This doesn't handle the case of resuming with multiple threads + all stopped at breakpoints. When scheduler-locking is off, we need to + singlestep all threads that are currently stopped at a breakpoint, + not just the last one. This situation can happen if the user runs + multiple threads in turn with scheduler-locking on, and then turns + scheduler-locking off and resumes the program. + See testcase gdb.threads/multi-bp-in-threads.exp. */ + if (prepare_to_proceed (step)) oneproc = 1; } @@ -1388,7 +1464,10 @@ proceed (CORE_ADDR addr, enum target_sig /* Pass the last stop signal to the thread we're resuming, irrespective of whether the current thread is the thread that got the last event or not. This was historically GDB's - behaviour before keeping a stop_signal per thread. */ + behaviour before keeping a stop_signal per thread. + However, we don't pass SIGTRAP - the breakpoint signal. + ??? There are other signals like SIGSEGV which we shouldn't + pass either. */ struct thread_info *last_thread; ptid_t last_ptid; @@ -1400,7 +1479,8 @@ proceed (CORE_ADDR addr, enum target_sig && !ptid_equal (last_ptid, minus_one_ptid)) { last_thread = find_thread_pid (last_ptid); - if (last_thread) + if (last_thread + && last_thread->stop_signal != TARGET_SIGNAL_TRAP) { tp->stop_signal = last_thread->stop_signal; last_thread->stop_signal = TARGET_SIGNAL_0; Index: testsuite/gdb.threads/hand-call-in-threads.c =================================================================== RCS file: testsuite/gdb.threads/hand-call-in-threads.c diff -N testsuite/gdb.threads/hand-call-in-threads.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ testsuite/gdb.threads/hand-call-in-threads.c 2 Dec 2008 10:36:21 -0000 @@ -0,0 +1,121 @@ +/* Test case for hand function calls in multi-threaded program. + + Copyright 2008 Free Software Foundation, Inc. + + This file is part of GDB. + + 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/>. */ + +#include <pthread.h> +#include <stdio.h> +#include <stdlib.h> +#include <time.h> +#include <unistd.h> + +/* NOTE: Also defined in hand-call-in-threads.exp. */ +#define NR_THREADS 4 + +int thread_count; + +pthread_mutex_t thread_count_mutex; + +pthread_cond_t thread_count_condvar; + +void +incr_thread_count (void) +{ + pthread_mutex_lock (&thread_count_mutex); + ++thread_count; + if (thread_count == NR_THREADS) + pthread_cond_signal (&thread_count_condvar); + pthread_mutex_unlock (&thread_count_mutex); +} + +void +cond_wait (pthread_cond_t *cond, pthread_mutex_t *mut) +{ + pthread_mutex_lock (mut); + pthread_cond_wait (cond, mut); + pthread_mutex_unlock (mut); +} + +void +noreturn (void) +{ + pthread_mutex_t mut; + pthread_cond_t cond; + + pthread_mutex_init (&mut, NULL); + pthread_cond_init (&cond, NULL); + + /* Wait for a condition that will never be signaled, so we effectively + block the thread here. */ + cond_wait (&cond, &mut); +} + +void * +forever_pthread (void *unused) +{ + incr_thread_count (); + noreturn (); +} + +void +hand_call (void) +{ +} + +/* Wait until all threads are running. */ + +void +wait_all_threads_running (void) +{ + pthread_mutex_lock (&thread_count_mutex); + pthread_cond_wait (&thread_count_condvar, &thread_count_mutex); + if (thread_count == NR_THREADS) + { + pthread_mutex_unlock (&thread_count_mutex); + return; + } + pthread_mutex_unlock (&thread_count_mutex); + printf ("failed waiting for all threads to start\n"); + abort (); +} + +/* Called when all threads are running. + Easy place for a breakpoint. */ + +void +all_threads_running (void) +{ +} + +int +main (void) +{ + pthread_t forever[NR_THREADS]; + int i; + + pthread_mutex_init (&thread_count_mutex, NULL); + pthread_cond_init (&thread_count_condvar, NULL); + + for (i = 0; i < NR_THREADS; ++i) + pthread_create (&forever[i], NULL, forever_pthread, NULL); + + wait_all_threads_running (); + all_threads_running (); + + return 0; +} + Index: testsuite/gdb.threads/hand-call-in-threads.exp =================================================================== RCS file: testsuite/gdb.threads/hand-call-in-threads.exp diff -N testsuite/gdb.threads/hand-call-in-threads.exp --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ testsuite/gdb.threads/hand-call-in-threads.exp 2 Dec 2008 10:36:21 -0000 @@ -0,0 +1,153 @@ +# Copyright (C) 2004, 2007, 2008 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/>. + +# Test making hand function calls in multiple threads. + +# NOTE: Also defined in hand-call-in-threads.c. +set NR_THREADS 4 + +if $tracelevel then { + strace $tracelevel +} + +set testfile "hand-call-in-threads" +set srcfile ${testfile}.c +set binfile ${objdir}/${subdir}/${testfile} + +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "incdir=${objdir}"]] != "" } { + return -1 +} + +# Some targets can't do function calls, so don't even bother with this +# test. +if [target_info exists gdb,cannot_call_functions] { + setup_xfail "*-*-*" 2416 + fail "This target can not call functions" + continue +} + +proc get_dummy_frame_number { } { + global gdb_prompt + + send_gdb "bt\n" + gdb_expect { + -re "#(\[0-9\]*) *<function called from gdb>.*$gdb_prompt $" + { + return $expect_out(1,string) + } + -re "$gdb_prompt $" + { + return "" + } + timeout + { + return "" + } + } + return "" +} + +gdb_exit +gdb_start +gdb_reinitialize_dir $srcdir/$subdir +gdb_load ${binfile} + +if { ![runto_main] } { + fail "Can't run to main" + return 0 +} + +gdb_test "break all_threads_running" \ + "Breakpoint 2 at .*: file .*${srcfile}, line .*" \ + "breakpoint on all_threads_running" + +gdb_test "break hand_call" \ + "Breakpoint 3 at .*: file .*${srcfile}, line .*" \ + "breakpoint on hand_call" + +# Run the program and make sure GDB reports that we stopped after +# hitting breakpoint 2 in all_threads_running(). + +gdb_test "continue" \ + ".*Breakpoint 2, all_threads_running ().*" \ + "run to all_threads_running" + +# Before we start making hand function calls, turn on scheduler locking. + +gdb_test "set scheduler-locking on" "" "enable scheduler locking" +gdb_test "show scheduler-locking" ".* locking scheduler .* is \"on\"." "show scheduler locking on" + +# Now hand-call a function in each thread, having the function +# stop without returning. + +# Add one for the main thread. +set total_nr_threads [expr $NR_THREADS + 1] + +# Thread numbering in gdb is origin-1, so begin numbering at 1. +for { set i 1 } { $i <= $total_nr_threads } { incr i } { + set thread_nr $i + gdb_test "thread $thread_nr" "" "prepare to make hand call, thread $thread_nr" + gdb_test "call hand_call()" "Breakpoint 3, .*" "hand call, thread $thread_nr" +} + +# Now have each hand-called function return. + +# Turn confirmation off for the "return" command. +gdb_test "set confirm off" "" + +clear_xfail "*-*-*" + +for { set i 1 } { $i <= $total_nr_threads } { incr i } { + set thread_nr $i + gdb_test "thread $thread_nr" "" "prepare to discard hand call, thread $thread_nr" + set frame_number [get_dummy_frame_number] + if { "$frame_number" == "" } { + fail "dummy stack frame number, thread $thread_nr" + setup_xfail "*-*-*" + # Need something. + set frame_number 0 + } else { + pass "dummy stack frame number, thread $thread_nr" + } + # Pop the dummy frame. + gdb_test "frame $frame_number" "" "setting frame, thread $thread_nr" + gdb_test "return" "" "discard hand call, thread $thread_nr" + # In case getting the dummy frame number failed, re-enable for next iter. + clear_xfail "*-*-*" +} + +# Make sure all dummy frames got popped. + +gdb_test_multiple "maint print dummy-frames" "all dummies popped" { + -re ".*stack=.*$gdb_prompt $" { + fail "all dummies popped" + } + -re ".*$gdb_prompt $" { + pass "all dummies popped" + } +} + +# Before we resume the full program, turn of scheduler locking. +gdb_test "set scheduler-locking off" "" "disable scheduler locking" +gdb_test "show scheduler-locking" ".* locking scheduler .* is \"off\"." "show scheduler locking off" + +#gdb_test "set debug infrun 1" "" "enable debug infrun" + +# Continue one last time, the program should exit normally. + +gdb_test "continue" "Program exited normally." \ + "continue to program exit" + +return 0 Index: testsuite/gdb.threads/multi-bp-in-threads.c =================================================================== RCS file: testsuite/gdb.threads/multi-bp-in-threads.c diff -N testsuite/gdb.threads/multi-bp-in-threads.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ testsuite/gdb.threads/multi-bp-in-threads.c 2 Dec 2008 10:36:21 -0000 @@ -0,0 +1,116 @@ +/* Test case for hand function calls in multi-threaded program. + + Copyright 2008 Free Software Foundation, Inc. + + This file is part of GDB. + + 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/>. */ + +#include <pthread.h> +#include <stdio.h> +#include <stdlib.h> +#include <time.h> +#include <unistd.h> + +/* NOTE: Also defined in multi-bp-in-threads.exp. */ +#define NR_THREADS 4 + +int thread_count; + +pthread_mutex_t thread_count_mutex; + +pthread_cond_t thread_count_condvar; + +/* Set by multi-bp-in-threads.exp when ready to advance each thread + to their breakpoint. */ +int advance_threads = 0; + +void +incr_thread_count (void) +{ + pthread_mutex_lock (&thread_count_mutex); + ++thread_count; + if (thread_count == NR_THREADS) + pthread_cond_signal (&thread_count_condvar); + pthread_mutex_unlock (&thread_count_mutex); +} + +/* Easy place to set a breakpoint. */ + +void +thread_breakpoint (void) +{ +} + +void * +thread_entry (void *unused) +{ + const struct timespec ts = { 0, 10000000 }; /* 0.01 sec */ + + incr_thread_count (); + + while (! advance_threads) + nanosleep (&ts, NULL); + + thread_breakpoint (); + + return NULL; +} + +/* Wait until all threads are running. */ + +void +wait_all_threads_running (void) +{ + pthread_mutex_lock (&thread_count_mutex); + pthread_cond_wait (&thread_count_condvar, &thread_count_mutex); + if (thread_count == NR_THREADS) + { + pthread_mutex_unlock (&thread_count_mutex); + return; + } + pthread_mutex_unlock (&thread_count_mutex); + printf ("failed waiting for all threads to start\n"); + abort (); +} + +/* Called when all threads are running. + Easy place for a breakpoint. */ + +void +all_threads_running (void) +{ +} + +int +main (void) +{ + pthread_t forever[NR_THREADS]; + int i; + + pthread_mutex_init (&thread_count_mutex, NULL); + pthread_cond_init (&thread_count_condvar, NULL); + + for (i = 0; i < NR_THREADS; ++i) + pthread_create (&forever[i], NULL, thread_entry, NULL); + + wait_all_threads_running (); + all_threads_running (); + + for (i = 0; i < NR_THREADS; ++i) + pthread_join (forever[i], NULL); + + return 0; +} + Index: testsuite/gdb.threads/multi-bp-in-threads.exp =================================================================== RCS file: testsuite/gdb.threads/multi-bp-in-threads.exp diff -N testsuite/gdb.threads/multi-bp-in-threads.exp --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ testsuite/gdb.threads/multi-bp-in-threads.exp 2 Dec 2008 10:36:21 -0000 @@ -0,0 +1,93 @@ +# Copyright (C) 2004, 2007, 2008 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/>. + +# Test resuming all threads after multiple threads have each hit a breakpoint. + +# NOTE: Also defined in multi-bp-in-threads.c. +set NR_THREADS 4 + +if $tracelevel then { + strace $tracelevel +} + +set testfile "multi-bp-in-threads" +set srcfile ${testfile}.c +set binfile ${objdir}/${subdir}/${testfile} + +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "incdir=${objdir}"]] != "" } { + return -1 +} + +gdb_exit +gdb_start +gdb_reinitialize_dir $srcdir/$subdir +gdb_load ${binfile} + +if { ![runto_main] } { + fail "Can't run to main" + return 0 +} + +gdb_test "break all_threads_running" \ + "Breakpoint 2 at .*: file .*${srcfile}, line .*" \ + "breakpoint on all_threads_running" + +gdb_test "break thread_breakpoint" \ + "Breakpoint 3 at .*: file .*${srcfile}, line .*" \ + "breakpoint on thread_breakpoint" + +# Run the program and make sure GDB reports that we stopped after +# hitting breakpoint 2 in all_threads_running(). + +gdb_test "continue" \ + ".*Breakpoint 2, all_threads_running ().*" \ + "run to all_threads_running" + +# Now run each thread, one at a time, with scheduler-locking on, +# each hitting a breakpoint + +gdb_test "set scheduler-locking on" "" "enable scheduler locking" +gdb_test "show scheduler-locking" ".* locking scheduler .* is \"on\"." "show scheduler locking on" + +gdb_test "set advance_threads = 1" "" + +set total_nr_threads [expr $NR_THREADS + 1] + +for { set i 0 } { $i < $NR_THREADS } { incr i } { + # The main thread is thread 1, the pthread_create'd threads are 2,3,... + set thread_nr [expr $i + 2] + gdb_test "thread $thread_nr" "" "prepare to advance thread $thread_nr" + gdb_test "continue" "Breakpoint 3, .*" "advance thread $thread_nr" +} + +# Each thread is now stopped at a breakpoint. +# Now resume the entire program, GDB should properly step each thread +# passed the breakpoint. + +# Turn off scheduler locking so all threads run. +gdb_test "set scheduler-locking off" "" "disable scheduler locking" +gdb_test "show scheduler-locking" ".* locking scheduler .* is \"off\"." "show scheduler locking off" + +#gdb_test "set debug infrun 1" "" "enable debug infrun" + +# Continue one last time, the program should exit normally. +# FIXME: It currently doesn't because GDB doesn't handle more than one thread +# (other than the current thread) stopped at a breakpoint. + +setup_xfail "*-*-*" +gdb_test "continue" "Program exited normally." \ + "continue to program exit" + +return 0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Fix hand called function when another thread has hit a bp. 2008-12-02 11:41 ` Doug Evans @ 2008-12-14 22:00 ` Doug Evans 2008-12-14 22:14 ` Ulrich Weigand 0 siblings, 1 reply; 14+ messages in thread From: Doug Evans @ 2008-12-14 22:00 UTC (permalink / raw) To: gdb-patches Ping. On Tue, Dec 2, 2008 at 3:40 AM, Doug Evans <dje@google.com> wrote: > On Mon, Dec 1, 2008 at 7:46 PM, Doug Evans <dje@google.com> wrote: >> On Mon, Dec 1, 2008 at 7:00 PM, Doug Evans <dje@google.com> wrote: >>> Hi. In http://sourceware.org/ml/gdb-patches/2008-11/msg00531.html >>> I wrote: >>> >>>> I wrote a testcase that calls functions in multiple threads (with >>>> scheduler-locking on) by setting a breakpoint on the function being >>>> called. I think there's a bug in scheduler-locking support as when I >>>> make the second call in the second thread, gdb makes the first thread >>>> step over the breakpoint and then resume, and control returns to >>>> call_function_by_hand with inferior_ptid changed to the first thread. >>> >>> Here's a patch. >>> This is separate from my dummy-frames cleanup patch. >>> >>> Ok to check in? >> >> Except of course nothing is ever this easy. :-( >> >> My testcase isn't complete: >> >> set scheduler-locking off >> continue >> >> Adding the above to the end of the testcase reveals the fact that this >> patch causes gdb to lose track of the fact that it needs to single >> step over the breakpoint at all_threads_running in the main thread, >> and resuming execution causes the breakpoint to be hit again. Global >> state, gotta love it. >> >> I'm assuming non-stop mode doesn't have this problem. >> Can we record in struct thread_info (or some such) the last stop >> reason and before resuming with !scheduler-locking iterate over all >> threads, single stepping them as necessary? Is there a better >> solution? > > This patch fixes the expanded hand-call-in-threads.exp testcase (which > is part of the patch). In the process I discovered a bigger problem - > gdb doesn't handle resuming after more than one thread is stopped at a > breakpoint. This can happen if the user runs several threads in turn > with scheduler-locking on, and then turns scheduler-locking off and > resumes the program. I wrote another testcase, > multi-bp-in-threads.exp, to expose this issue. Fixing this appears to > be a much harder problem, and I think I'd like to declare partial > victory with this patch ... > > Comments? > > I have some questions. Is it sufficient to test "thread->stop_signal > == TRAP && breakpoint_here_p (pc)" to check if a thread is stopped at > a breakpoint? It seems to be. > > Another thing this revised patch does is stop SIGTRAP from being > passed to the current thread to be run if it was received in the last > thread that ran. The comments say this is how gdb behaved before > stop_signal was recorded for each thread. It seems odd though. If an > asynchronous signal comes in and it is masked in some threads, if the > kernel chooses a thread in which it's not blocked, should gdb be > changing the thread that receives the signal? > This patch takes a minimalist approach and only changes the behaviour > of SIGTRAP. > > 2008-12-02 Doug Evans <dje@google.com> > > * infrun.c (prepare_to_proceed_callback): New function. > (prepare_to_proceed): Document. Assert !non_stop. Add debugging > printf. If scheduler-locking is enabled, no other thread need to > be singlestepped. Otherwise scan all threads for whether they're > stopped at a breakpoint instead of just the last thread that ran. > (proceed): Add FIXME. Don't pass on TARGET_SIGNAL_TRAP from the > last thread that ran to the current thread to run. > > * gdb.threads/hand-call-in-threads.exp: New file. > * gdb.threads/hand-call-in-threads.c: New file. > * gdb.threads/multi-bp-in-threads.exp: New file. > * gdb.threads/multi-bp-in-threads.c: New file. > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Fix hand called function when another thread has hit a bp. 2008-12-14 22:00 ` Doug Evans @ 2008-12-14 22:14 ` Ulrich Weigand 2008-12-15 22:07 ` Doug Evans 0 siblings, 1 reply; 14+ messages in thread From: Ulrich Weigand @ 2008-12-14 22:14 UTC (permalink / raw) To: Doug Evans; +Cc: gdb-patches Doug Evans wrote: > >> Adding the above to the end of the testcase reveals the fact that this > >> patch causes gdb to lose track of the fact that it needs to single > >> step over the breakpoint at all_threads_running in the main thread, > >> and resuming execution causes the breakpoint to be hit again. Global > >> state, gotta love it. > >> > >> I'm assuming non-stop mode doesn't have this problem. > >> Can we record in struct thread_info (or some such) the last stop > >> reason and before resuming with !scheduler-locking iterate over all > >> threads, single stepping them as necessary? Is there a better > >> solution? > > > > This patch fixes the expanded hand-call-in-threads.exp testcase (which > > is part of the patch). In the process I discovered a bigger problem - > > gdb doesn't handle resuming after more than one thread is stopped at a > > breakpoint. This can happen if the user runs several threads in turn > > with scheduler-locking on, and then turns scheduler-locking off and > > resumes the program. I wrote another testcase, > > multi-bp-in-threads.exp, to expose this issue. Fixing this appears to > > be a much harder problem, and I think I'd like to declare partial > > victory with this patch ... I'm not sure this is the right approach -- how can we be sure those breakpoints in other threads have already been reported to the user? The original code specifically steps only over the one breakpoint that was reported when GDB last stopped. If other threads hit breakpoints simultaneously, we *want* them to trigger again, so that they can be properly reported to the user ... Why does this not work for your test case? Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Fix hand called function when another thread has hit a bp. 2008-12-14 22:14 ` Ulrich Weigand @ 2008-12-15 22:07 ` Doug Evans 2008-12-15 22:50 ` Ulrich Weigand 0 siblings, 1 reply; 14+ messages in thread From: Doug Evans @ 2008-12-15 22:07 UTC (permalink / raw) To: Ulrich Weigand; +Cc: gdb-patches On Sun, Dec 14, 2008 at 2:13 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote: > Doug Evans wrote: > >> >> Adding the above to the end of the testcase reveals the fact that this >> >> patch causes gdb to lose track of the fact that it needs to single >> >> step over the breakpoint at all_threads_running in the main thread, >> >> and resuming execution causes the breakpoint to be hit again. Global >> >> state, gotta love it. >> >> >> >> I'm assuming non-stop mode doesn't have this problem. >> >> Can we record in struct thread_info (or some such) the last stop >> >> reason and before resuming with !scheduler-locking iterate over all >> >> threads, single stepping them as necessary? Is there a better >> >> solution? >> > >> > This patch fixes the expanded hand-call-in-threads.exp testcase (which >> > is part of the patch). In the process I discovered a bigger problem - >> > gdb doesn't handle resuming after more than one thread is stopped at a >> > breakpoint. This can happen if the user runs several threads in turn >> > with scheduler-locking on, and then turns scheduler-locking off and >> > resumes the program. I wrote another testcase, >> > multi-bp-in-threads.exp, to expose this issue. Fixing this appears to >> > be a much harder problem, and I think I'd like to declare partial >> > victory with this patch ... > > I'm not sure this is the right approach -- how can we be sure those > breakpoints in other threads have already been reported to the user? Hmm, do we need a flag that says which signals have been reported? > The original code specifically steps only over the one breakpoint that > was reported when GDB last stopped. If other threads hit breakpoints > simultaneously, we *want* them to trigger again, so that they can be > properly reported to the user ... > > Why does this not work for your test case? Hmm, I think we need to distinguish which testcase we're talking about. hand-call-in-threads.exp: What it does: - start up N threads and stop - set scheduler-locking on - hand-call a function in N threads, each of which hits a breakpoint - manually pop pending dummy frames - set scheduler-locking off - continue What it tests: - verify a previously hit breakpoint in a different thread is NOT singlestepped when resuming with scheduler-locking on - verify manually popping all pending dummy frames works - verify that resuming after manually popping all pending dummy frames works (i.e. a previously hit and reported breakpoint is not re-reported) multi-bp-in-threads.exp: What it does: - start up N threads and stop - set scheduler-locking on - hand-call a function in N threads, each of which hits a breakpoint - set scheduler-locking off - continue What it tests: - see what happens ... it's not clear to me what _should_ happen here Perhaps first we should agree on what the right behavior is. For hand-call-in-threads.exp I think the testcase is correct as is. For multi-bp-in-threads.exp, one approach is to just define the problem away and say that GDB's current behavior is the correct behavior. :) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Fix hand called function when another thread has hit a bp. 2008-12-15 22:07 ` Doug Evans @ 2008-12-15 22:50 ` Ulrich Weigand 2008-12-15 23:15 ` Doug Evans 0 siblings, 1 reply; 14+ messages in thread From: Ulrich Weigand @ 2008-12-15 22:50 UTC (permalink / raw) To: Doug Evans; +Cc: gdb-patches Doug Evans wrote: > Hmm, I think we need to distinguish which testcase we're talking about. > > hand-call-in-threads.exp: > What it does: > - start up N threads and stop > - set scheduler-locking on > - hand-call a function in N threads, each of which hits a breakpoint > - manually pop pending dummy frames > - set scheduler-locking off > - continue > What it tests: > - verify a previously hit breakpoint in a different thread is NOT > singlestepped when resuming with scheduler-locking on > - verify manually popping all pending dummy frames works > - verify that resuming after manually popping all pending dummy > frames works (i.e. a previously hit and reported breakpoint is not > re-reported) > > multi-bp-in-threads.exp: > What it does: > - start up N threads and stop > - set scheduler-locking on > - hand-call a function in N threads, each of which hits a breakpoint > - set scheduler-locking off > - continue > What it tests: > - see what happens ... it's not clear to me what _should_ happen here > > Perhaps first we should agree on what the right behavior is. > For hand-call-in-threads.exp I think the testcase is correct as is. > For multi-bp-in-threads.exp, one approach is to just define the > problem away and say that GDB's current behavior is the correct > behavior. :) Ah, OK. It seems to be those problems are all triggered by scheduler locking; this is a feature that doesn't seem to be handled cleanly by the existing "prepare_to_proceed" mechanism. Generally, I think what *should* happen is pretty straightforward: each instance of a breakpoint hit should be reported exactly once. And in fact in the absence of scheduler locking, this is what the current code should (I hope!) achieve. In the case of the multi-bp-in-threads test case (without scheduler locking), we'd hit the breakpoint on all_threads_running (thread 1) first. If the user then switch to some (any) other thread, and issues the "continue" command, the prepare_to_proceed logic would switch back to thread 1, single-step once across the breakpoint without reporting it, and then continue all threads. Some thread X would then hit the breakpoint on thread_breakpoint. Again, if the user switches to any other thread and issues continue, GDB would switch back to thread X, single-step once, and then continue all threads. The breakpoint on thread_breakpoint would then hit in some other thread etc. All in all, every breakpoint would be hit and reported exactly once. Similarly, in the case of the hand-call-in-threads test case without scheduler locking, we hit the breakpoint on all_threads_running in thread 1 first. If the user switches to some other thread X and issues an inferior function call, GDB would switch back to thread 1, single- step over the breakpoint, and then continue all threads -- this will allow the inferior call to proceed to breakpoint on hand_call in thread X. If the user then switches to yet another thread Y and issues the inferior call there, GDB will switch back and single-step over the breakpoint in thread X, before continuing all threads allowing the breakpoint on hand_call in Y to hit, etc. This works without requiring per-thread state because at any point there is only ever one thread in the breakpoint-reported-and-needs- to-be-skipped state, and this state is handled/resolved as the very first action in any case, no matter what action the user performs. The problem arises when scheduler locking is switched on. Actually, I think there are really two problems. First of all, after we've switched back and single-stepped over an already-hit breakpoint via the prepare_to_proceed logic, we'll continue only a single thread if scheduler-locking is on -- and that is the wrong thread. The prepare_to_proceed logic only explicitly switches *back* to the user-selected thread if the user was *stepping* (that's the deferred_step_ptid logic). For scheduler-locking, we should probably switch back always ... The second problem is more a problem of definition: even if the first problem above were fixed, we've have to single-step the other thread at least once to get over the breakpoint. This would seem to violate the definition of scheduler locking if interpreted absolutely strictly. Now you could argue that as the user should never be aware of that single step, it doesn't really matter. If we really wanted to avoid that completely, I don't see any other way but adding a new per-thread flag that says "we've just stopped at and reported a breakpoint". Whenever we finally switch back and start executing that thread, and we're still on that breakpoint, we need to perform the single-step at this point. Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Fix hand called function when another thread has hit a bp. 2008-12-15 22:50 ` Ulrich Weigand @ 2008-12-15 23:15 ` Doug Evans 2008-12-17 19:14 ` Ulrich Weigand 0 siblings, 1 reply; 14+ messages in thread From: Doug Evans @ 2008-12-15 23:15 UTC (permalink / raw) To: Ulrich Weigand; +Cc: gdb-patches On Mon, Dec 15, 2008 at 2:49 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote: > Doug Evans wrote: > >> Hmm, I think we need to distinguish which testcase we're talking about. >> >> hand-call-in-threads.exp: >> What it does: >> - start up N threads and stop >> - set scheduler-locking on >> - hand-call a function in N threads, each of which hits a breakpoint >> - manually pop pending dummy frames >> - set scheduler-locking off >> - continue >> What it tests: >> - verify a previously hit breakpoint in a different thread is NOT >> singlestepped when resuming with scheduler-locking on >> - verify manually popping all pending dummy frames works >> - verify that resuming after manually popping all pending dummy >> frames works (i.e. a previously hit and reported breakpoint is not >> re-reported) >> >> multi-bp-in-threads.exp: >> What it does: >> - start up N threads and stop >> - set scheduler-locking on >> - hand-call a function in N threads, each of which hits a breakpoint >> - set scheduler-locking off >> - continue >> What it tests: >> - see what happens ... it's not clear to me what _should_ happen here >> >> Perhaps first we should agree on what the right behavior is. >> For hand-call-in-threads.exp I think the testcase is correct as is. >> For multi-bp-in-threads.exp, one approach is to just define the >> problem away and say that GDB's current behavior is the correct >> behavior. :) > > Ah, OK. It seems to be those problems are all triggered by > scheduler locking; this is a feature that doesn't seem to be > handled cleanly by the existing "prepare_to_proceed" mechanism. > > Generally, I think what *should* happen is pretty straightforward: > each instance of a breakpoint hit should be reported exactly once. > > And in fact in the absence of scheduler locking, this is what the > current code should (I hope!) achieve. In the case of the > multi-bp-in-threads test case (without scheduler locking), we'd > hit the breakpoint on all_threads_running (thread 1) first. > > If the user then switch to some (any) other thread, and issues the > "continue" command, the prepare_to_proceed logic would switch back > to thread 1, single-step once across the breakpoint without reporting > it, and then continue all threads. > > Some thread X would then hit the breakpoint on thread_breakpoint. > Again, if the user switches to any other thread and issues continue, > GDB would switch back to thread X, single-step once, and then continue > all threads. The breakpoint on thread_breakpoint would then hit in > some other thread etc. > > All in all, every breakpoint would be hit and reported exactly once. > > > Similarly, in the case of the hand-call-in-threads test case without > scheduler locking, we hit the breakpoint on all_threads_running in > thread 1 first. If the user switches to some other thread X and issues > an inferior function call, GDB would switch back to thread 1, single- > step over the breakpoint, and then continue all threads -- this will > allow the inferior call to proceed to breakpoint on hand_call in > thread X. > > If the user then switches to yet another thread Y and issues the > inferior call there, GDB will switch back and single-step over the > breakpoint in thread X, before continuing all threads allowing the > breakpoint on hand_call in Y to hit, etc. > > This works without requiring per-thread state because at any point > there is only ever one thread in the breakpoint-reported-and-needs- > to-be-skipped state, and this state is handled/resolved as the very > first action in any case, no matter what action the user performs. > > > The problem arises when scheduler locking is switched on. Actually, > I think there are really two problems. First of all, after we've > switched back and single-stepped over an already-hit breakpoint via > the prepare_to_proceed logic, we'll continue only a single thread > if scheduler-locking is on -- and that is the wrong thread. The > prepare_to_proceed logic only explicitly switches *back* to the > user-selected thread if the user was *stepping* (that's the > deferred_step_ptid logic). For scheduler-locking, we should probably > switch back always ... If scheduler locking is on, why is there any switching at all? If scheduler-locking is on and I switch threads I'd want gdb to defer single-stepping the other thread over its breakpoint until the point when I make that other thread runnable. Also, I think removing the notion of one previously stopped thread and generalizing it to not caring, i.e. checking the status of every stopped thread before resuming will simplify things and fix a few bugs along the way. IOW, make deferred_ptid go away. > > The second problem is more a problem of definition: even if the > first problem above were fixed, we've have to single-step the other > thread at least once to get over the breakpoint. This would seem > to violate the definition of scheduler locking if interpreted > absolutely strictly. Now you could argue that as the user should > never be aware of that single step, it doesn't really matter. I'm not sure how we necessarily have a violation of the definition of scheduler locking. > If we really wanted to avoid that completely, I don't see any other > way but adding a new per-thread flag that says "we've just stopped > at and reported a breakpoint". Whenever we finally switch back and > start executing that thread, and we're still on that breakpoint, > we need to perform the single-step at this point. I agree with wanting to singlestep the thread at the point it's made runnable, and not before. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Fix hand called function when another thread has hit a bp. 2008-12-15 23:15 ` Doug Evans @ 2008-12-17 19:14 ` Ulrich Weigand 2009-02-24 10:42 ` Doug Evans 0 siblings, 1 reply; 14+ messages in thread From: Ulrich Weigand @ 2008-12-17 19:14 UTC (permalink / raw) To: Doug Evans; +Cc: gdb-patches Doug Evans wrote: > > The problem arises when scheduler locking is switched on. Actually, > > I think there are really two problems. First of all, after we've > > switched back and single-stepped over an already-hit breakpoint via > > the prepare_to_proceed logic, we'll continue only a single thread > > if scheduler-locking is on -- and that is the wrong thread. The > > prepare_to_proceed logic only explicitly switches *back* to the > > user-selected thread if the user was *stepping* (that's the > > deferred_step_ptid logic). For scheduler-locking, we should probably > > switch back always ... > > If scheduler locking is on, why is there any switching at all? If > scheduler-locking is on and I switch threads I'd want gdb to defer > single-stepping the other thread over its breakpoint until the point > when I make that other thread runnable. > > Also, I think removing the notion of one previously stopped thread and > generalizing it to not caring, i.e. checking the status of every > stopped thread before resuming will simplify things and fix a few bugs > along the way. IOW, make deferred_ptid go away. That may indeed be the best solution. The simplest implementation might be to simply remember in a per-thread flag the fact that the last time this thread stopped, we reported a breakpoint at stop_pc (which would have to be made per-thread as well, but we'd already decided this should happen anyway). This information could then be consulted the next time the thread is made runnable again. > > The second problem is more a problem of definition: even if the > > first problem above were fixed, we've have to single-step the other > > thread at least once to get over the breakpoint. This would seem > > to violate the definition of scheduler locking if interpreted > > absolutely strictly. Now you could argue that as the user should > > never be aware of that single step, it doesn't really matter. > > I'm not sure how we necessarily have a violation of the definition of > scheduler locking. This is just saying the same you said in other words: "If scheduler- locking is on and I switch threads I'd want gdb to defer single- stepping the other thread over its breakpoint until the point when I make that other thread runnable." I.e. "definition of scheduler locking" meaning: no other thread but the one selected by the user runs, ever. Today, this is not true, in the case of single-stepping over a breakpoint in another thread. Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Fix hand called function when another thread has hit a bp. 2008-12-17 19:14 ` Ulrich Weigand @ 2009-02-24 10:42 ` Doug Evans 2009-03-13 17:06 ` Doug Evans 2009-03-30 18:48 ` Ulrich Weigand 0 siblings, 2 replies; 14+ messages in thread From: Doug Evans @ 2009-02-24 10:42 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches [-- Attachment #1: Type: text/plain, Size: 3185 bytes --] On Wed, Dec 17, 2008 at 11:14 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote: > Doug Evans wrote: > >> > The problem arises when scheduler locking is switched on. Actually, >> > I think there are really two problems. First of all, after we've >> > switched back and single-stepped over an already-hit breakpoint via >> > the prepare_to_proceed logic, we'll continue only a single thread >> > if scheduler-locking is on -- and that is the wrong thread. The >> > prepare_to_proceed logic only explicitly switches *back* to the >> > user-selected thread if the user was *stepping* (that's the >> > deferred_step_ptid logic). For scheduler-locking, we should probably >> > switch back always ... >> >> If scheduler locking is on, why is there any switching at all? If >> scheduler-locking is on and I switch threads I'd want gdb to defer >> single-stepping the other thread over its breakpoint until the point >> when I make that other thread runnable. >> >> Also, I think removing the notion of one previously stopped thread and >> generalizing it to not caring, i.e. checking the status of every >> stopped thread before resuming will simplify things and fix a few bugs >> along the way. IOW, make deferred_ptid go away. > > That may indeed be the best solution. The simplest implementation > might be to simply remember in a per-thread flag the fact that the > last time this thread stopped, we reported a breakpoint at stop_pc > (which would have to be made per-thread as well, but we'd already > decided this should happen anyway). > > This information could then be consulted the next time the thread > is made runnable again. > >> > The second problem is more a problem of definition: even if the >> > first problem above were fixed, we've have to single-step the other >> > thread at least once to get over the breakpoint. This would seem >> > to violate the definition of scheduler locking if interpreted >> > absolutely strictly. Now you could argue that as the user should >> > never be aware of that single step, it doesn't really matter. >> >> I'm not sure how we necessarily have a violation of the definition of >> scheduler locking. > > This is just saying the same you said in other words: "If scheduler- > locking is on and I switch threads I'd want gdb to defer single- > stepping the other thread over its breakpoint until the point when > I make that other thread runnable." > > I.e. "definition of scheduler locking" meaning: no other thread but > the one selected by the user runs, ever. Today, this is not true, > in the case of single-stepping over a breakpoint in another thread. Hi. Here's an updated version of the patch. Handling the restart after several threads are all stopped at a breakpoint (via scheduler-locking = on), is left for a later patch (it's happens more rarely). Ok to check in? 2009-02-23 Doug Evans <dje@google.com> * infrun.c (prepare_to_proceed): Document. Assert !non_stop. If scheduler-locking is enabled, we're not going to be singlestepping any other previously stopped thread. * gdb.threads/hand-call-in-threads.exp: New file. * gdb.threads/hand-call-in-threads.c: New file. [-- Attachment #2: gdb-090223-schedlock-4.patch.txt --] [-- Type: text/plain, Size: 10722 bytes --] 2009-02-23 Doug Evans <dje@google.com> * infrun.c (prepare_to_proceed): Document. Assert !non_stop. If scheduler-locking is enabled, we're not going to be singlestepping any other previously stopped thread. * gdb.threads/hand-call-in-threads.exp: New file. * gdb.threads/hand-call-in-threads.c: New file. Index: infrun.c =================================================================== RCS file: /cvs/src/src/gdb/infrun.c,v retrieving revision 1.359 diff -u -p -u -p -r1.359 infrun.c --- infrun.c 21 Feb 2009 16:14:48 -0000 1.359 +++ infrun.c 24 Feb 2009 00:37:03 -0000 @@ -1239,13 +1239,21 @@ clear_proceed_status (void) } } -/* This should be suitable for any targets that support threads. */ +/* Check the current thread against the thread that reported the most recent + event. If a step-over is required return TRUE and set the current thread + to the old thread. Otherwise return FALSE. + + This should be suitable for any targets that support threads. */ static int prepare_to_proceed (int step) { ptid_t wait_ptid; struct target_waitstatus wait_status; + int schedlock_enabled; + + /* With non-stop mode on, threads are always handled individually. */ + gdb_assert (! non_stop); /* Get the last target status returned by target_wait(). */ get_last_target_status (&wait_ptid, &wait_status); @@ -1257,9 +1265,15 @@ prepare_to_proceed (int step) return 0; } + schedlock_enabled = (scheduler_mode == schedlock_on + || (scheduler_mode == schedlock_step + && step)); + /* Switched over from WAIT_PID. */ if (!ptid_equal (wait_ptid, minus_one_ptid) - && !ptid_equal (inferior_ptid, wait_ptid)) + && !ptid_equal (inferior_ptid, wait_ptid) + /* Don't single step WAIT_PID if scheduler locking is on. */ + && !schedlock_enabled) { struct regcache *regcache = get_thread_regcache (wait_ptid); Index: testsuite/gdb.threads/hand-call-in-threads.c =================================================================== RCS file: testsuite/gdb.threads/hand-call-in-threads.c diff -N testsuite/gdb.threads/hand-call-in-threads.c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ testsuite/gdb.threads/hand-call-in-threads.c 24 Feb 2009 00:37:04 -0000 @@ -0,0 +1,127 @@ +/* Test case for hand function calls in multi-threaded program. + + Copyright 2008 Free Software Foundation, Inc. + + This file is part of GDB. + + 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/>. */ + +#include <pthread.h> +#include <stdio.h> +#include <stdlib.h> +#include <time.h> +#include <unistd.h> + +#ifndef NR_THREADS +#define NR_THREADS 4 +#endif + +int thread_count; + +pthread_mutex_t thread_count_mutex; + +pthread_cond_t thread_count_condvar; + +void +incr_thread_count (void) +{ + pthread_mutex_lock (&thread_count_mutex); + ++thread_count; + if (thread_count == NR_THREADS) + pthread_cond_signal (&thread_count_condvar); + pthread_mutex_unlock (&thread_count_mutex); +} + +void +cond_wait (pthread_cond_t *cond, pthread_mutex_t *mut) +{ + pthread_mutex_lock (mut); + pthread_cond_wait (cond, mut); + pthread_mutex_unlock (mut); +} + +void +noreturn (void) +{ + pthread_mutex_t mut; + pthread_cond_t cond; + + pthread_mutex_init (&mut, NULL); + pthread_cond_init (&cond, NULL); + + /* Wait for a condition that will never be signaled, so we effectively + block the thread here. */ + cond_wait (&cond, &mut); +} + +void * +forever_pthread (void *unused) +{ + incr_thread_count (); + noreturn (); +} + +void +hand_call (void) +{ +} + +/* Wait until all threads are running. */ + +void +wait_all_threads_running (void) +{ + pthread_mutex_lock (&thread_count_mutex); + if (thread_count == NR_THREADS) + { + pthread_mutex_unlock (&thread_count_mutex); + return; + } + pthread_cond_wait (&thread_count_condvar, &thread_count_mutex); + if (thread_count == NR_THREADS) + { + pthread_mutex_unlock (&thread_count_mutex); + return; + } + pthread_mutex_unlock (&thread_count_mutex); + printf ("failed waiting for all threads to start\n"); + abort (); +} + +/* Called when all threads are running. + Easy place for a breakpoint. */ + +void +all_threads_running (void) +{ +} + +int +main (void) +{ + pthread_t forever[NR_THREADS]; + int i; + + pthread_mutex_init (&thread_count_mutex, NULL); + pthread_cond_init (&thread_count_condvar, NULL); + + for (i = 0; i < NR_THREADS; ++i) + pthread_create (&forever[i], NULL, forever_pthread, NULL); + + wait_all_threads_running (); + all_threads_running (); + + return 0; +} + Index: testsuite/gdb.threads/hand-call-in-threads.exp =================================================================== RCS file: testsuite/gdb.threads/hand-call-in-threads.exp diff -N testsuite/gdb.threads/hand-call-in-threads.exp --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ testsuite/gdb.threads/hand-call-in-threads.exp 24 Feb 2009 00:37:04 -0000 @@ -0,0 +1,160 @@ +# Copyright (C) 2004, 2007, 2008 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/>. + +# Test making hand function calls in multiple threads. + +set NR_THREADS 4 + +if $tracelevel then { + strace $tracelevel +} + +set testfile "hand-call-in-threads" +set srcfile ${testfile}.c +set binfile ${objdir}/${subdir}/${testfile} + +if {[gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "incdir=${objdir}" "additional_flags=-DNR_THREADS=$NR_THREADS"]] != "" } { + return -1 +} + +# Some targets can't do function calls, so don't even bother with this +# test. +if [target_info exists gdb,cannot_call_functions] { + setup_xfail "*-*-*" 2416 + fail "This target can not call functions" + continue +} + +proc get_dummy_frame_number { } { + global gdb_prompt + + send_gdb "bt\n" + gdb_expect { + -re "#(\[0-9\]*) *<function called from gdb>.*$gdb_prompt $" + { + return $expect_out(1,string) + } + -re "$gdb_prompt $" + { + return "" + } + timeout + { + return "" + } + } + return "" +} + +gdb_exit +gdb_start +gdb_reinitialize_dir $srcdir/$subdir +gdb_load ${binfile} + +if { ![runto_main] } { + fail "Can't run to main" + return 0 +} + +gdb_test "break all_threads_running" \ + "Breakpoint 2 at .*: file .*${srcfile}, line .*" \ + "breakpoint on all_threads_running" + +gdb_test "break hand_call" \ + "Breakpoint 3 at .*: file .*${srcfile}, line .*" \ + "breakpoint on hand_call" + +# Run the program and make sure GDB reports that we stopped after +# hitting breakpoint 2 in all_threads_running(). + +gdb_test "continue" \ + ".*Breakpoint 2, all_threads_running ().*" \ + "run to all_threads_running" + +# Before we start making hand function calls, turn on scheduler locking. + +gdb_test "set scheduler-locking on" "" "enable scheduler locking" +gdb_test "show scheduler-locking" ".* locking scheduler .* is \"on\"." "show scheduler locking on" + +# Now hand-call a function in each thread, having the function +# stop without returning. + +# Add one for the main thread. +set total_nr_threads [expr $NR_THREADS + 1] + +# Thread numbering in gdb is origin-1, so begin numbering at 1. +for { set i 1 } { $i <= $total_nr_threads } { incr i } { + set thread_nr $i + gdb_test "thread $thread_nr" "" "prepare to make hand call, thread $thread_nr" + gdb_test "call hand_call()" "Breakpoint 3, .*" "hand call, thread $thread_nr" +} + +# Now have each hand-called function return. + +# Turn confirmation off for the "return" command. +gdb_test "set confirm off" "" + +clear_xfail "*-*-*" + +for { set i 1 } { $i <= $total_nr_threads } { incr i } { + set thread_nr $i + gdb_test "thread $thread_nr" "" "prepare to discard hand call, thread $thread_nr" + set frame_number [get_dummy_frame_number] + if { "$frame_number" == "" } { + fail "dummy stack frame number, thread $thread_nr" + setup_xfail "*-*-*" + # Need something. + set frame_number 0 + } else { + pass "dummy stack frame number, thread $thread_nr" + } + # Pop the dummy frame. + gdb_test "frame $frame_number" "" "setting frame, thread $thread_nr" + gdb_test "return" "" "discard hand call, thread $thread_nr" + # In case getting the dummy frame number failed, re-enable for next iter. + clear_xfail "*-*-*" +} + +# Make sure all dummy frames got popped. + +gdb_test_multiple "maint print dummy-frames" "all dummies popped" { + -re ".*stack=.*$gdb_prompt $" { + fail "all dummies popped" + } + -re ".*$gdb_prompt $" { + pass "all dummies popped" + } +} + +# Before we resume the full program, turn of scheduler locking. +gdb_test "set scheduler-locking off" "" "disable scheduler locking" +gdb_test "show scheduler-locking" ".* locking scheduler .* is \"off\"." "show scheduler locking off" + +# Continue one last time, the program should exit normally. +# +# ??? This currently doesn't work because gdb doesn't know how to singlestep +# over reported breakpoints that weren't in the last thread to run. +# Fixing this first requires defining what the correct behaviour is. +# Commented out until then. +# +# Manually set the thread back to the first thread: the program is still at +# the all_threads_running breakpoint, which wasn't the last thread to run, +# and gdb doesn't know how to singlestep over reported breakpoints that +# weren't in the last thread to run. +#gdb_test "thread 1" "" "set thread to 1, prepare to resume" +# +#gdb_continue_to_end "hand-call-in-threads" + +return 0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Fix hand called function when another thread has hit a bp. 2009-02-24 10:42 ` Doug Evans @ 2009-03-13 17:06 ` Doug Evans 2009-03-29 13:36 ` Doug Evans 2009-03-30 18:48 ` Ulrich Weigand 1 sibling, 1 reply; 14+ messages in thread From: Doug Evans @ 2009-03-13 17:06 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches Ping. On Mon, Feb 23, 2009 at 6:32 PM, Doug Evans <dje@google.com> wrote: > On Wed, Dec 17, 2008 at 11:14 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote: >> Doug Evans wrote: >> >>> > The problem arises when scheduler locking is switched on. Actually, >>> > I think there are really two problems. First of all, after we've >>> > switched back and single-stepped over an already-hit breakpoint via >>> > the prepare_to_proceed logic, we'll continue only a single thread >>> > if scheduler-locking is on -- and that is the wrong thread. The >>> > prepare_to_proceed logic only explicitly switches *back* to the >>> > user-selected thread if the user was *stepping* (that's the >>> > deferred_step_ptid logic). For scheduler-locking, we should probably >>> > switch back always ... >>> >>> If scheduler locking is on, why is there any switching at all? If >>> scheduler-locking is on and I switch threads I'd want gdb to defer >>> single-stepping the other thread over its breakpoint until the point >>> when I make that other thread runnable. >>> >>> Also, I think removing the notion of one previously stopped thread and >>> generalizing it to not caring, i.e. checking the status of every >>> stopped thread before resuming will simplify things and fix a few bugs >>> along the way. IOW, make deferred_ptid go away. >> >> That may indeed be the best solution. The simplest implementation >> might be to simply remember in a per-thread flag the fact that the >> last time this thread stopped, we reported a breakpoint at stop_pc >> (which would have to be made per-thread as well, but we'd already >> decided this should happen anyway). >> >> This information could then be consulted the next time the thread >> is made runnable again. >> >>> > The second problem is more a problem of definition: even if the >>> > first problem above were fixed, we've have to single-step the other >>> > thread at least once to get over the breakpoint. This would seem >>> > to violate the definition of scheduler locking if interpreted >>> > absolutely strictly. Now you could argue that as the user should >>> > never be aware of that single step, it doesn't really matter. >>> >>> I'm not sure how we necessarily have a violation of the definition of >>> scheduler locking. >> >> This is just saying the same you said in other words: "If scheduler- >> locking is on and I switch threads I'd want gdb to defer single- >> stepping the other thread over its breakpoint until the point when >> I make that other thread runnable." >> >> I.e. "definition of scheduler locking" meaning: no other thread but >> the one selected by the user runs, ever. Today, this is not true, >> in the case of single-stepping over a breakpoint in another thread. > > Hi. Here's an updated version of the patch. > Handling the restart after several threads are all stopped at a > breakpoint (via scheduler-locking = on), is left for a later patch > (it's happens more rarely). > > Ok to check in? > > 2009-02-23 Doug Evans <dje@google.com> > > * infrun.c (prepare_to_proceed): Document. Assert !non_stop. > If scheduler-locking is enabled, we're not going to be singlestepping > any other previously stopped thread. > > * gdb.threads/hand-call-in-threads.exp: New file. > * gdb.threads/hand-call-in-threads.c: New file. > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Fix hand called function when another thread has hit a bp. 2009-03-13 17:06 ` Doug Evans @ 2009-03-29 13:36 ` Doug Evans 0 siblings, 0 replies; 14+ messages in thread From: Doug Evans @ 2009-03-29 13:36 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches Ping. On Fri, Mar 13, 2009 at 10:03 AM, Doug Evans <dje@google.com> wrote: > Ping. > > On Mon, Feb 23, 2009 at 6:32 PM, Doug Evans <dje@google.com> wrote: >> On Wed, Dec 17, 2008 at 11:14 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote: >>> Doug Evans wrote: >>> >>>> > The problem arises when scheduler locking is switched on. Actually, >>>> > I think there are really two problems. First of all, after we've >>>> > switched back and single-stepped over an already-hit breakpoint via >>>> > the prepare_to_proceed logic, we'll continue only a single thread >>>> > if scheduler-locking is on -- and that is the wrong thread. The >>>> > prepare_to_proceed logic only explicitly switches *back* to the >>>> > user-selected thread if the user was *stepping* (that's the >>>> > deferred_step_ptid logic). For scheduler-locking, we should probably >>>> > switch back always ... >>>> >>>> If scheduler locking is on, why is there any switching at all? If >>>> scheduler-locking is on and I switch threads I'd want gdb to defer >>>> single-stepping the other thread over its breakpoint until the point >>>> when I make that other thread runnable. >>>> >>>> Also, I think removing the notion of one previously stopped thread and >>>> generalizing it to not caring, i.e. checking the status of every >>>> stopped thread before resuming will simplify things and fix a few bugs >>>> along the way. IOW, make deferred_ptid go away. >>> >>> That may indeed be the best solution. The simplest implementation >>> might be to simply remember in a per-thread flag the fact that the >>> last time this thread stopped, we reported a breakpoint at stop_pc >>> (which would have to be made per-thread as well, but we'd already >>> decided this should happen anyway). >>> >>> This information could then be consulted the next time the thread >>> is made runnable again. >>> >>>> > The second problem is more a problem of definition: even if the >>>> > first problem above were fixed, we've have to single-step the other >>>> > thread at least once to get over the breakpoint. This would seem >>>> > to violate the definition of scheduler locking if interpreted >>>> > absolutely strictly. Now you could argue that as the user should >>>> > never be aware of that single step, it doesn't really matter. >>>> >>>> I'm not sure how we necessarily have a violation of the definition of >>>> scheduler locking. >>> >>> This is just saying the same you said in other words: "If scheduler- >>> locking is on and I switch threads I'd want gdb to defer single- >>> stepping the other thread over its breakpoint until the point when >>> I make that other thread runnable." >>> >>> I.e. "definition of scheduler locking" meaning: no other thread but >>> the one selected by the user runs, ever. Today, this is not true, >>> in the case of single-stepping over a breakpoint in another thread. >> >> Hi. Here's an updated version of the patch. >> Handling the restart after several threads are all stopped at a >> breakpoint (via scheduler-locking = on), is left for a later patch >> (it's happens more rarely). >> >> Ok to check in? >> >> 2009-02-23 Doug Evans <dje@google.com> >> >> * infrun.c (prepare_to_proceed): Document. Assert !non_stop. >> If scheduler-locking is enabled, we're not going to be singlestepping >> any other previously stopped thread. >> >> * gdb.threads/hand-call-in-threads.exp: New file. >> * gdb.threads/hand-call-in-threads.c: New file. >> > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Fix hand called function when another thread has hit a bp. 2009-02-24 10:42 ` Doug Evans 2009-03-13 17:06 ` Doug Evans @ 2009-03-30 18:48 ` Ulrich Weigand 2009-04-03 23:25 ` Doug Evans 1 sibling, 1 reply; 14+ messages in thread From: Ulrich Weigand @ 2009-03-30 18:48 UTC (permalink / raw) To: Doug Evans; +Cc: gdb-patches Doug Evans wrote: > Hi. Here's an updated version of the patch. Sorry for the late reply on this! > Handling the restart after several threads are all stopped at a > breakpoint (via scheduler-locking = on), is left for a later patch > (it's happens more rarely). This patch, as far as I can see, just replaces one incorrect behaviour with a different incorrect behaviour, right? That is to say, in the scenario where we have - set scheduler-locking on - stop on BP in thread A - manually switch to thread B - continue execution the behaviour today is: - GDB will switch back to A and single-step - (correctly) bypass the already-hit breakpoint in A - (incorrectly) continue execution thread A i.e. the incorrect behaviour is that thread A is continued, and not thread B. With your patch, the behaviour is: - GDB (correctly) continues execution of thread B - but the next time thread A is run, GDB will (incorrectly) report a second time the same breakpoint the user already saw Did I miss anything here? In any case, I guess I agree that the "new" type of incorrect behaviour is probably less bad that what we have today, so your patch does seem to be a step forward. Will you be working on a follow-on patch to fix the new incorrect behaviour? >+for { set i 1 } { $i <= $total_nr_threads } { incr i } { >+ set thread_nr $i >+ gdb_test "thread $thread_nr" "" "prepare to discard hand call, thread $thread_nr" >+ set frame_number [get_dummy_frame_number] >+ if { "$frame_number" == "" } { >+ fail "dummy stack frame number, thread $thread_nr" >+ setup_xfail "*-*-*" >+ # Need something. >+ set frame_number 0 Why do we need this xfail here? >+# Continue one last time, the program should exit normally. >+# >+# ??? This currently doesn't work because gdb doesn't know how to singlestep >+# over reported breakpoints that weren't in the last thread to run. >+# Fixing this first requires defining what the correct behaviour is. >+# Commented out until then. >+# >+# Manually set the thread back to the first thread: the program is still at >+# the all_threads_running breakpoint, which wasn't the last thread to run, >+# and gdb doesn't know how to singlestep over reported breakpoints that >+# weren't in the last thread to run. >+#gdb_test "thread 1" "" "set thread to 1, prepare to resume" >+# >+#gdb_continue_to_end "hand-call-in-threads" Should the "thread 1" really be here? It seems to me this was just an unsuccessful attempt to work-around the bug ... Otherwise, the patch is OK. Thanks, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA] Fix hand called function when another thread has hit a bp. 2009-03-30 18:48 ` Ulrich Weigand @ 2009-04-03 23:25 ` Doug Evans 0 siblings, 0 replies; 14+ messages in thread From: Doug Evans @ 2009-04-03 23:25 UTC (permalink / raw) To: Ulrich Weigand; +Cc: gdb-patches On Mon, Mar 30, 2009 at 11:40 AM, Ulrich Weigand <uweigand@de.ibm.com> wrote: > Doug Evans wrote: > >> Hi. Here's an updated version of the patch. > > Sorry for the late reply on this! No worries. >> Handling the restart after several threads are all stopped at a >> breakpoint (via scheduler-locking = on), is left for a later patch >> (it's happens more rarely). > > This patch, as far as I can see, just replaces one incorrect > behaviour with a different incorrect behaviour, right? One could look at it that way. I look at it as taking one step of a two step fix. :-) > That is to say, in the scenario where we have > > - set scheduler-locking on > - stop on BP in thread A > - manually switch to thread B > - continue execution > > the behaviour today is: > > - GDB will switch back to A and single-step > - (correctly) bypass the already-hit breakpoint in A > - (incorrectly) continue execution thread A > > i.e. the incorrect behaviour is that thread A is continued, > and not thread B. > > With your patch, the behaviour is: > > - GDB (correctly) continues execution of thread B > - but the next time thread A is run, GDB will (incorrectly) > report a second time the same breakpoint the user already saw > > Did I miss anything here? > > In any case, I guess I agree that the "new" type of incorrect > behaviour is probably less bad that what we have today, so > your patch does seem to be a step forward. > > Will you be working on a follow-on patch to fix the new > incorrect behaviour? That's the plan. It involves recording the stop reason for every thread, and appropriately applying it when resuming. > >>+for { set i 1 } { $i <= $total_nr_threads } { incr i } { >>+ set thread_nr $i >>+ gdb_test "thread $thread_nr" "" "prepare to discard hand call, thread $thread_nr" >>+ set frame_number [get_dummy_frame_number] >>+ if { "$frame_number" == "" } { >>+ fail "dummy stack frame number, thread $thread_nr" >>+ setup_xfail "*-*-*" >>+ # Need something. >>+ set frame_number 0 > > Why do we need this xfail here? It's not needed. I can remove it. >>+# Continue one last time, the program should exit normally. >>+# >>+# ??? This currently doesn't work because gdb doesn't know how to singlestep >>+# over reported breakpoints that weren't in the last thread to run. >>+# Fixing this first requires defining what the correct behaviour is. >>+# Commented out until then. >>+# >>+# Manually set the thread back to the first thread: the program is still at >>+# the all_threads_running breakpoint, which wasn't the last thread to run, >>+# and gdb doesn't know how to singlestep over reported breakpoints that >>+# weren't in the last thread to run. >>+#gdb_test "thread 1" "" "set thread to 1, prepare to resume" >>+# >>+#gdb_continue_to_end "hand-call-in-threads" > > Should the "thread 1" really be here? It seems to me this was just an > unsuccessful attempt to work-around the bug ... It's there to document the bug (until it's fixed). > Otherwise, the patch is OK. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2009-04-03 23:25 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-12-02 3:01 [RFA] Fix hand called function when another thread has hit a bp Doug Evans 2008-12-02 3:48 ` Doug Evans 2008-12-02 11:41 ` Doug Evans 2008-12-14 22:00 ` Doug Evans 2008-12-14 22:14 ` Ulrich Weigand 2008-12-15 22:07 ` Doug Evans 2008-12-15 22:50 ` Ulrich Weigand 2008-12-15 23:15 ` Doug Evans 2008-12-17 19:14 ` Ulrich Weigand 2009-02-24 10:42 ` Doug Evans 2009-03-13 17:06 ` Doug Evans 2009-03-29 13:36 ` Doug Evans 2009-03-30 18:48 ` Ulrich Weigand 2009-04-03 23:25 ` Doug Evans
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox