diff --git a/gdb/ChangeLog b/gdb/ChangeLog index ce37aa79f6..6f4182d125 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,26 @@ +2017-03-08 Pedro Alves + + PR gdb/18360 + * infrun.c (start_step_over, do_target_resume, resume) + (restart_threads): Assert we're not resuming a thread that is + meant to be stopped. + (infrun_thread_stop_requested_callback): Delete. + (infrun_thread_stop_requested): If the thread is internally + stopped, queue a pending stop event and clear the thread's + inline-frame state. + (handle_stop_requested): New function. + (handle_syscall_event, handle_inferior_event_1): Use + handle_stop_requested. + (handle_stop_requested): New function. + (handle_signal_stop): Set the thread's stop_signal here instead of + at caller. + (finish_step_over): Clear step over info unconditionally. + (handle_signal_stop): If the user had interrupted the event + thread, consider the stop a random signal. + (handle_signal_stop) : Don't restart threads here. + (stop_waiting): Don't clear step-over info here. + 2017-03-08 Pedro Alves PR 21206 diff --git a/gdb/infrun.c b/gdb/infrun.c index 1e5e9f14a6..c8c2d6e0b2 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -2083,6 +2083,8 @@ start_step_over (void) step_over_what step_what; int must_be_in_line; + gdb_assert (!tp->stop_requested); + next = thread_step_over_chain_next (tp); /* If this inferior already has a displaced step in process, @@ -2331,6 +2333,8 @@ do_target_resume (ptid_t resume_ptid, int step, enum gdb_signal sig) { struct thread_info *tp = inferior_thread (); + gdb_assert (!tp->stop_requested); + /* Install inferior's terminal modes. */ target_terminal_inferior (); @@ -2395,6 +2399,7 @@ resume (enum gdb_signal sig) single-step). */ int step; + gdb_assert (!tp->stop_requested); gdb_assert (!thread_is_in_step_over_chain (tp)); QUIT; @@ -3276,65 +3281,6 @@ static void keep_going (struct execution_control_state *ecs); static void process_event_stop_test (struct execution_control_state *ecs); static int switch_back_to_stepped_thread (struct execution_control_state *ecs); -/* Callback for iterate over threads. If the thread is stopped, but - the user/frontend doesn't know about that yet, go through - normal_stop, as if the thread had just stopped now. ARG points at - a ptid. If PTID is MINUS_ONE_PTID, applies to all threads. If - ptid_is_pid(PTID) is true, applies to all threads of the process - pointed at by PTID. Otherwise, apply only to the thread pointed by - PTID. */ - -static int -infrun_thread_stop_requested_callback (struct thread_info *info, void *arg) -{ - ptid_t ptid = * (ptid_t *) arg; - - if ((ptid_equal (info->ptid, ptid) - || ptid_equal (minus_one_ptid, ptid) - || (ptid_is_pid (ptid) - && ptid_get_pid (ptid) == ptid_get_pid (info->ptid))) - && is_running (info->ptid) - && !is_executing (info->ptid)) - { - struct cleanup *old_chain; - struct execution_control_state ecss; - struct execution_control_state *ecs = &ecss; - - memset (ecs, 0, sizeof (*ecs)); - - old_chain = make_cleanup_restore_current_thread (); - - overlay_cache_invalid = 1; - /* Flush target cache before starting to handle each event. - Target was running and cache could be stale. This is just a - heuristic. Running threads may modify target memory, but we - don't get any event. */ - target_dcache_invalidate (); - - /* Go through handle_inferior_event/normal_stop, so we always - have consistent output as if the stop event had been - reported. */ - ecs->ptid = info->ptid; - ecs->event_thread = info; - ecs->ws.kind = TARGET_WAITKIND_STOPPED; - ecs->ws.value.sig = GDB_SIGNAL_0; - - handle_inferior_event (ecs); - - if (!ecs->wait_some_more) - { - /* Cancel any running execution command. */ - thread_cancel_execution_command (info); - - normal_stop (); - } - - do_cleanups (old_chain); - } - - return 0; -} - /* This function is attached as a "thread_stop_requested" observer. Cleanup local state that assumed the PTID was to be resumed, and report the stop to the frontend. */ @@ -3344,17 +3290,51 @@ infrun_thread_stop_requested (ptid_t ptid) { struct thread_info *tp; - /* PTID was requested to stop. Remove matching threads from the - step-over queue, so we don't try to resume them - automatically. */ + /* PTID was requested to stop. If the thread was already stopped, + but the user/frontend doesn't know about that yet (e.g., the + thread had been temporarily paused for some step-over), set up + for reporting the stop now. */ ALL_NON_EXITED_THREADS (tp) if (ptid_match (tp->ptid, ptid)) { + if (tp->state != THREAD_RUNNING) + continue; + if (tp->executing) + continue; + + /* Remove matching threads from the step-over queue, so + start_step_over doesn't try to resume them + automatically. */ if (thread_is_in_step_over_chain (tp)) thread_step_over_chain_remove (tp); - } - iterate_over_threads (infrun_thread_stop_requested_callback, &ptid); + /* If the thread is stopped, but the user/frontend doesn't + know about that yet, queue a pending event, as if the + thread had just stopped now. Unless the thread already had + a pending event. */ + if (!tp->suspend.waitstatus_pending_p) + { + tp->suspend.waitstatus_pending_p = 1; + tp->suspend.waitstatus.kind = TARGET_WAITKIND_STOPPED; + tp->suspend.waitstatus.value.sig = GDB_SIGNAL_0; + } + + /* Clear the inline-frame state, since we're re-processing the + stop. */ + clear_inline_frame_state (tp->ptid); + + /* If this thread was paused because some other thread was + doing an inline-step over, let that finish first. Once + that happens, we'll restart all threads and consume pending + stop events then. */ + if (step_over_info_valid_p ()) + continue; + + /* Otherwise we can process the (new) pending event now. Set + it so this pending event is considered by + do_target_wait. */ + tp->resumed = 1; + } } static void @@ -4248,6 +4228,23 @@ stepped_in_from (struct frame_info *frame, struct frame_id step_frame_id) return 0; } +/* If the event thread has the stop requested flag set, pretend it + stopped for a GDB_SIGNAL_0 (i.e., as if it stopped due to + target_stop). */ + +static bool +handle_stop_requested (struct execution_control_state *ecs) +{ + if (ecs->event_thread->stop_requested) + { + ecs->ws.kind = TARGET_WAITKIND_STOPPED; + ecs->ws.value.sig = GDB_SIGNAL_0; + handle_signal_stop (ecs); + return true; + } + return false; +} + /* Auxiliary function that handles syscall entry/return events. It returns 1 if the inferior should keep going (and GDB should ignore the event), or 0 if the event deserves to be @@ -4277,6 +4274,9 @@ handle_syscall_event (struct execution_control_state *ecs) = bpstat_stop_status (get_regcache_aspace (regcache), stop_pc, ecs->ptid, &ecs->ws); + if (handle_stop_requested (ecs)) + return 0; + if (bpstat_causes_stop (ecs->event_thread->control.stop_bpstat)) { /* Catchpoint hit. */ @@ -4284,6 +4284,9 @@ handle_syscall_event (struct execution_control_state *ecs) } } + if (handle_stop_requested (ecs)) + return 0; + /* If no catchpoint triggered for this, then keep going. */ keep_going (ecs); return 1; @@ -4974,6 +4977,9 @@ handle_inferior_event_1 (struct execution_control_state *ecs) = bpstat_stop_status (get_regcache_aspace (regcache), stop_pc, ecs->ptid, &ecs->ws); + if (handle_stop_requested (ecs)) + return; + if (bpstat_causes_stop (ecs->event_thread->control.stop_bpstat)) { /* A catchpoint triggered. */ @@ -5028,6 +5034,8 @@ handle_inferior_event_1 (struct execution_control_state *ecs) case TARGET_WAITKIND_SPURIOUS: if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_SPURIOUS\n"); + if (handle_stop_requested (ecs)) + return; if (!ptid_equal (ecs->ptid, inferior_ptid)) context_switch (ecs->ptid); resume (GDB_SIGNAL_0); @@ -5037,6 +5045,8 @@ handle_inferior_event_1 (struct execution_control_state *ecs) case TARGET_WAITKIND_THREAD_CREATED: if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_THREAD_CREATED\n"); + if (handle_stop_requested (ecs)) + return; if (!ptid_equal (ecs->ptid, inferior_ptid)) context_switch (ecs->ptid); if (!switch_back_to_stepped_thread (ecs)) @@ -5221,6 +5231,9 @@ Cannot fill $_exitsignal with the correct signal number.\n")); = bpstat_stop_status (get_regcache_aspace (get_current_regcache ()), stop_pc, ecs->ptid, &ecs->ws); + if (handle_stop_requested (ecs)) + return; + /* If no catchpoint triggered for this, then keep going. Note that we're interested in knowing the bpstat actually causes a stop, not just if it may explain the signal. Software @@ -5295,6 +5308,10 @@ Cannot fill $_exitsignal with the correct signal number.\n")); current_inferior ()->waiting_for_vfork_done = 0; current_inferior ()->pspace->breakpoints_not_allowed = 0; + + if (handle_stop_requested (ecs)) + return; + /* This also takes care of reinserting breakpoints in the previously locked inferior. */ keep_going (ecs); @@ -5331,6 +5348,9 @@ Cannot fill $_exitsignal with the correct signal number.\n")); xfree (ecs->ws.value.execd_pathname); ecs->ws.value.execd_pathname = NULL; + if (handle_stop_requested (ecs)) + return; + /* If no catchpoint triggered for this, then keep going. */ if (!bpstat_causes_stop (ecs->event_thread->control.stop_bpstat)) { @@ -5368,7 +5388,6 @@ Cannot fill $_exitsignal with the correct signal number.\n")); case TARGET_WAITKIND_STOPPED: if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: TARGET_WAITKIND_STOPPED\n"); - ecs->event_thread->suspend.stop_signal = ecs->ws.value.sig; handle_signal_stop (ecs); return; @@ -5385,6 +5404,10 @@ Cannot fill $_exitsignal with the correct signal number.\n")); delete_just_stopped_threads_single_step_breakpoints (); stop_pc = regcache_read_pc (get_thread_regcache (inferior_ptid)); + + if (handle_stop_requested (ecs)) + return; + observer_notify_no_history (); stop_waiting (ecs); return; @@ -5474,6 +5497,8 @@ restart_threads (struct thread_info *event_thread) continue; } + gdb_assert (!tp->stop_requested); + /* If some thread needs to start a step-over at this point, it should still be in the step-over queue, and thus skipped above. */ @@ -5543,8 +5568,7 @@ finish_step_over (struct execution_control_state *ecs) back an event. */ gdb_assert (ecs->event_thread->control.trap_expected); - if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP) - clear_step_over_info (); + clear_step_over_info (); } if (!target_is_non_stop_p ()) @@ -5656,6 +5680,8 @@ handle_signal_stop (struct execution_control_state *ecs) gdb_assert (ecs->ws.kind == TARGET_WAITKIND_STOPPED); + ecs->event_thread->suspend.stop_signal = ecs->ws.value.sig; + /* Do we need to clean up the state of a thread that has completed a displaced single-step? (Doing so usually affects the PC, so do it here, before we set stop_pc.) */ @@ -6045,6 +6071,15 @@ handle_signal_stop (struct execution_control_state *ecs) if (random_signal) random_signal = !stopped_by_watchpoint; + /* Always stop if the user explicitly requested this thread to + remain stopped. */ + if (ecs->event_thread->stop_requested) + { + random_signal = 1; + if (debug_infrun) + fprintf_unfiltered (gdb_stdlog, "infrun: user-requested stop\n"); + } + /* For the program's own signals, act according to the signal handling tables. */ @@ -6091,8 +6126,6 @@ handle_signal_stop (struct execution_control_state *ecs) && ecs->event_thread->control.trap_expected && ecs->event_thread->control.step_resume_breakpoint == NULL) { - int was_in_line; - /* We were just starting a new sequence, attempting to single-step off of a breakpoint and expecting a SIGTRAP. Instead this signal arrives. This signal will take us out @@ -6108,34 +6141,11 @@ handle_signal_stop (struct execution_control_state *ecs) "infrun: signal arrived while stepping over " "breakpoint\n"); - was_in_line = step_over_info_valid_p (); - clear_step_over_info (); insert_hp_step_resume_breakpoint_at_frame (frame); ecs->event_thread->step_after_step_resume_breakpoint = 1; /* Reset trap_expected to ensure breakpoints are re-inserted. */ ecs->event_thread->control.trap_expected = 0; - if (target_is_non_stop_p ()) - { - /* Either "set non-stop" is "on", or the target is - always in non-stop mode. In this case, we have a bit - more work to do. Resume the current thread, and if - we had paused all threads, restart them while the - signal handler runs. */ - keep_going (ecs); - - if (was_in_line) - { - restart_threads (ecs->event_thread); - } - else if (debug_infrun) - { - fprintf_unfiltered (gdb_stdlog, - "infrun: no need to restart threads\n"); - } - return; - } - /* If we were nexting/stepping some other thread, switch to it, so that we don't continue it, losing control. */ if (!switch_back_to_stepped_thread (ecs)) @@ -7676,8 +7686,6 @@ stop_waiting (struct execution_control_state *ecs) if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: stop_waiting\n"); - clear_step_over_info (); - /* Let callers know we don't want to wait for the inferior anymore. */ ecs->wait_some_more = 0; diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 4575e2784f..499377bd73 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2017-03-08 Pedro Alves + + PR gdb/18360 + * gdb.threads/interrupt-while-step-over.c: New file. + * gdb.threads/interrupt-while-step-over.exp: New file. + 2017-03-08 Pedro Alves * gdb.arch/amd64-entry-value-param-dwarf5.exp: Use with_test_prefix. diff --git a/gdb/testsuite/gdb.threads/interrupt-while-step-over.c b/gdb/testsuite/gdb.threads/interrupt-while-step-over.c new file mode 100644 index 0000000000..0a62e34e4a --- /dev/null +++ b/gdb/testsuite/gdb.threads/interrupt-while-step-over.c @@ -0,0 +1,75 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2016-2017 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 . */ + +#include +#include +#include + +#define NUM_THREADS 20 +const int num_threads = NUM_THREADS; + +static pthread_t child_thread[NUM_THREADS]; + +static pthread_barrier_t threads_started_barrier; + +volatile int always_zero; +volatile unsigned int dummy; + +static void +infinite_loop (void) +{ + while (1) + { + dummy++; /* set breakpoint here */ + } +} + +static void * +child_function (void *arg) +{ + pthread_barrier_wait (&threads_started_barrier); + + infinite_loop (); +} + +void +all_started (void) +{ +} + +int +main (void) +{ + int res; + int i; + + alarm (300); + + pthread_barrier_init (&threads_started_barrier, NULL, NUM_THREADS + 1); + + for (i = 0; i < NUM_THREADS; i++) + { + res = pthread_create (&child_thread[i], NULL, child_function, NULL); + } + + /* Wait until all threads have been scheduled. */ + pthread_barrier_wait (&threads_started_barrier); + + all_started (); + + infinite_loop (); +} diff --git a/gdb/testsuite/gdb.threads/interrupt-while-step-over.exp b/gdb/testsuite/gdb.threads/interrupt-while-step-over.exp new file mode 100644 index 0000000000..68d25704a9 --- /dev/null +++ b/gdb/testsuite/gdb.threads/interrupt-while-step-over.exp @@ -0,0 +1,204 @@ +# Copyright (C) 2016-2017 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 . + +# Regression test for PR gdb/18360. Test that "interrupt -a" while +# some thread is stepping over a breakpoint behaves as expected. + +standard_testfile + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile \ + {debug pthreads}] == -1} { + return -1 +} + +if {![runto_main]} { + fail "can't run to main" + return -1 +} + +# Read the number of threads out of the inferior. +set NUM_THREADS [get_integer_valueof "num_threads" -1] + +# Account for the main thread. +incr NUM_THREADS + +# Run command and wait for the prompt, without end anchor. + +proc gdb_test_no_anchor {cmd} { + global gdb_prompt + + gdb_test_multiple $cmd $cmd { + -re "$gdb_prompt " { + pass $cmd + } + -re "infrun:" { + exp_continue + } + } +} + +# Enable/disable debugging. + +proc enable_debug {enable} { + + # Comment out to debug problems with the test. + return + + gdb_test_no_anchor "set debug infrun $enable" + gdb_test_no_anchor "set debug displaced $enable" +} + +# If RESULT is not zero, make the caller return RESULT. + +proc return_if_nonzero { result } { + if {$result != 0} { + return -code return $result + } +} + +# Do one iteration of "c -a& -> interrupt -a". Return zero on sucess, +# and non-zero if some test fails. + +proc test_one_iteration {} { + global gdb_prompt + global NUM_THREADS + global decimal + + set saw_continuing 0 + set test "continue -a &" + return_if_nonzero [gdb_test_multiple $test $test { + -re "Continuing.\r\n" { + set saw_continuing 1 + exp_continue + } + -re "$gdb_prompt " { + if ![gdb_assert $saw_continuing $test] { + return 1 + } + } + -re "infrun:" { + exp_continue + } + }] + + set running_count 0 + set test "all threads are running" + return_if_nonzero [gdb_test_multiple "info threads" $test { + -re "Thread \[^\r\n\]* \\(running\\)" { + incr running_count + exp_continue + } + -re "$gdb_prompt " { + if ![gdb_assert {$running_count == $NUM_THREADS} $test] { + return 1 + } + } + -re "infrun:" { + exp_continue + } + }] + + set test "interrupt -a" + return_if_nonzero [gdb_test_multiple $test $test { + -re "$gdb_prompt " { + pass $test + } + -re "infrun:" { + exp_continue + } + }] + + set stopped_count 0 + set test "wait for stops" + # Don't return on failure here, in order to let "info threads" put + # useful info in gdb.log. + gdb_test_multiple "" $test { + -re "Thread $decimal \[^\r\n\]*stopped" { + incr stopped_count + if {$stopped_count != $NUM_THREADS} { + exp_continue + } + } + -re "$gdb_prompt " { + gdb_assert {$stopped_count == $NUM_THREADS} $test + } + -re "infrun:" { + exp_continue + } + } + + # Check if all threads are seen as stopped with "info + # threads". It's a bit redundant with the test above, but + # it's useful to have this in the gdb.log if the above ever + # happens to fail. + set running_count 0 + set test "all threads are stopped" + return_if_nonzero [gdb_test_multiple "info threads" $test { + -re "Thread \[^\r\n\]* \\(running\\)" { + incr running_count + exp_continue + } + -re "$gdb_prompt " { + if ![gdb_assert {$running_count == 0} $test] { + return 1 + } + } + }] + + return 0 +} + +# The test driver proper. If DISPLACED is "on", turn on displaced +# stepping. If "off", turn it off. + +proc testdriver {displaced} { + global binfile + global GDBFLAGS + + save_vars { GDBFLAGS } { + append GDBFLAGS " -ex \"set non-stop on\"" + clean_restart $binfile + } + + gdb_test_no_output "set displaced-stepping $displaced" + + if ![runto all_started] { + fail "couldn't run to all_started" + return + } + set break_line [gdb_get_line_number "set breakpoint here"] + + gdb_test "break $break_line if always_zero" "Breakpoint .*" "set breakpoint" + + enable_debug 1 + + for {set iter 0} {$iter < 20} {incr iter} { + with_test_prefix "iter=$iter" { + # Return early if some test fails, to avoid cascading + # timeouts if something goes wrong. + if {[test_one_iteration] != 0} { + return + } + } + } +} + +foreach_with_prefix displaced-stepping {"on" "off"} { + if { ${displaced-stepping} != "off" && ![support_displaced_stepping] } { + continue + } + + testdriver ${displaced-stepping} +} diff --git a/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp b/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp index 200ab5bf54..144a13b70d 100644 --- a/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp +++ b/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp @@ -100,7 +100,6 @@ gdb_test_sequence $test $test { "need to step-over" "resume \\(step=1" "signal arrived while stepping over breakpoint" - "(restart threads|switching back to stepped thread)" "stepped to a different line" "callme" }