diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 06d23e03ad..df6cc085b2 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,14 @@ +2014-05-29 Pedro Alves + + PR PR15693 + * infrun.c (resume): Determine how much to resume depending on + whether the caller wanted a step, not whether we can hardware step + the target. Mark all threads that we intend to run as running, + unless we're calling an inferior function. + (normal_stop): If the thread is running an infcall, don't finish + thread state. + * target.c (target_resume): Don't mark threads as running here. + 2014-05-28 Joel Brobecker * serial.c (_initialize_serial): Remove support for diff --git a/gdb/infrun.c b/gdb/infrun.c index ec78148a06..2068a2ae5b 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1780,6 +1780,7 @@ resume (int step, enum gdb_signal sig) CORE_ADDR pc = regcache_read_pc (regcache); struct address_space *aspace = get_regcache_aspace (regcache); ptid_t resume_ptid; + int hw_step = step; QUIT; @@ -1799,7 +1800,7 @@ resume (int step, enum gdb_signal sig) if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: resume : clear step\n"); - step = 0; + hw_step = 0; } if (debug_infrun) @@ -1844,7 +1845,7 @@ a command like `return' or `jump' to continue execution.")); step software breakpoint. */ if (use_displaced_stepping (gdbarch) && (tp->control.trap_expected - || (step && gdbarch_software_single_step_p (gdbarch))) + || (hw_step && gdbarch_software_single_step_p (gdbarch))) && sig == GDB_SIGNAL_0 && !current_inferior ()->waiting_for_vfork_done) { @@ -1854,11 +1855,14 @@ a command like `return' or `jump' to continue execution.")); { /* Got placed in displaced stepping queue. Will be resumed later when all the currently queued displaced stepping - requests finish. The thread is not executing at this point, - and the call to set_executing will be made later. But we - need to call set_running here, since from frontend point of view, - the thread is running. */ - set_running (inferior_ptid, 1); + requests finish. The thread is not executing at this + point, and the call to set_executing will be made later. + But we need to call set_running here, since from the + user/frontend's point of view, threads were set running. + Unless we're calling an inferior function, as in that + case we pretend the inferior doesn't run at all. */ + if (!tp->control.in_infcall) + set_running (user_visible_resume_ptid (step), 1); discard_cleanups (old_cleanups); return; } @@ -1868,8 +1872,8 @@ a command like `return' or `jump' to continue execution.")); pc = regcache_read_pc (get_thread_regcache (inferior_ptid)); displaced = get_displaced_stepping_state (ptid_get_pid (inferior_ptid)); - step = gdbarch_displaced_step_hw_singlestep (gdbarch, - displaced->step_closure); + hw_step = gdbarch_displaced_step_hw_singlestep (gdbarch, + displaced->step_closure); } /* Do we need to do it the hard way, w/temp breakpoints? */ @@ -1933,6 +1937,14 @@ a command like `return' or `jump' to continue execution.")); by applying increasingly restricting conditions. */ resume_ptid = user_visible_resume_ptid (step); + /* Even if RESUME_PTID is a wildcard, and we end up resuming less + (e.g., we might need to step over a breakpoint), from the + user/frontend's point of view, all threads in RESUME_PTID are now + running. Unless we're calling an inferior function, as in that + case pretend we inferior doesn't run at all. */ + if (!tp->control.in_infcall) + set_running (resume_ptid, 1); + /* Maybe resume a single thread after all. */ if ((step || singlestep_breakpoints_inserted_p) && tp->control.trap_expected) @@ -6188,8 +6200,18 @@ normal_stop (void) if (has_stack_frames () && !stop_stack_dummy) set_current_sal_from_frame (get_current_frame ()); - /* Let the user/frontend see the threads as stopped. */ - do_cleanups (old_chain); + /* Let the user/frontend see the threads as stopped, but do nothing + if the thread was running an infcall. We may be e.g., evaluating + a breakpoint condition. In that case, the thread had state + THREAD_RUNNING before the infcall, and shall remain set to + running, all without informing the user/frontend about state + transition changes. If this is actually a call command, then the + thread was originally already stopped, so there's no state to + finish either. */ + if (target_has_execution && inferior_thread ()->control.in_infcall) + discard_cleanups (old_chain); + else + do_cleanups (old_chain); /* Look up the hook_stop and run it (CLI internally handles problem of stop_command's pre-hook not existing). */ diff --git a/gdb/target.c b/gdb/target.c index cf86e73268..6a1a2ffb42 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -2150,8 +2150,9 @@ target_resume (ptid_t ptid, int step, enum gdb_signal signal) gdb_signal_to_name (signal)); registers_changed_ptid (ptid); + /* We only set the internal executing state here. The user/frontend + running state is set at a higher level. */ set_executing (ptid, 1); - set_running (ptid, 1); clear_inline_frame_state (ptid); } diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 4d9c597926..797a313b59 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2014-05-29 Pedro Alves + Hui Zhu + + PR PR15693 + * gdb.mi/mi-condbreak-call-thr-state-mt.c: New file. + * gdb.mi/mi-condbreak-call-thr-state-st.c: New file. + * gdb.mi/mi-condbreak-call-thr-state.c: New file. + * gdb.mi/mi-condbreak-call-thr-state.exp: New file. + 2014-05-28 Joel Brobecker * config/monitor.exp (gdb_target_monitor): Replace use of diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-call-thr-state-mt.c b/gdb/testsuite/gdb.mi/mi-condbreak-call-thr-state-mt.c new file mode 100644 index 0000000000..e9ca92d6cf --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-condbreak-call-thr-state-mt.c @@ -0,0 +1,61 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright (C) 2014 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 . */ + +/* This is the multi-threaded driver for the real test. */ + +#include +#include + +extern int test (void); + +#define NTHREADS 5 +pthread_barrier_t barrier; + +void * +thread_func (void *arg) +{ + pthread_barrier_wait (&barrier); + + while (1) + sleep (1); +} + +void +create_thread (void) +{ + pthread_t tid; + + if (pthread_create (&tid, NULL, thread_func, NULL)) + { + perror ("pthread_create"); + exit (1); + } +} + +int +main (void) +{ + int i; + + pthread_barrier_init (&barrier, NULL, NTHREADS + 1); + + for (i = 0; i < NTHREADS; i++) + create_thread (); + pthread_barrier_wait (&barrier); + + return test (); +} diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-call-thr-state-st.c b/gdb/testsuite/gdb.mi/mi-condbreak-call-thr-state-st.c new file mode 100644 index 0000000000..c90a7b0ae9 --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-condbreak-call-thr-state-st.c @@ -0,0 +1,26 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright (C) 2014 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 . */ + +/* This is single-threaded driver for the real test. */ + +extern int test (void); + +int +main (void) +{ + return test (); +} diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-call-thr-state.c b/gdb/testsuite/gdb.mi/mi-condbreak-call-thr-state.c new file mode 100644 index 0000000000..75d56014bf --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-condbreak-call-thr-state.c @@ -0,0 +1,33 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright (C) 2013-2014 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 . */ + +int +return_false (void) +{ + return 0; +} + +int +test (void) +{ + int a = 0; + + while (a < 10) + a++; /* set breakpoint here */ + + return 0; /* set end breakpoint here */ +} diff --git a/gdb/testsuite/gdb.mi/mi-condbreak-call-thr-state.exp b/gdb/testsuite/gdb.mi/mi-condbreak-call-thr-state.exp new file mode 100644 index 0000000000..5b4d91051f --- /dev/null +++ b/gdb/testsuite/gdb.mi/mi-condbreak-call-thr-state.exp @@ -0,0 +1,116 @@ +# Copyright (C) 2013-2014 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 PR15693. A breakpoint with a condition that +# calls a function that evaluates false would result in a spurious +# *running event sent to the frontend each time the breakpoint is hit +# (and the target re-resumed). Like: +# +# -exec-continue +# ^running +# *running,thread-id="all" +# (gdb) +# *running,thread-id="all" +# *running,thread-id="all" +# *running,thread-id="all" +# *running,thread-id="all" +# *running,thread-id="all" +# ... + +load_lib mi-support.exp +set MIFLAGS "-i=mi" + +# Run either the multi-threaded or the single-threaded variant of the +# test, as determined by VARIANT. +proc test { variant } { + global gdb_test_file_name + global testfile srcdir subdir srcfile srcfile2 binfile + global mi_gdb_prompt async + + with_test_prefix "$variant" { + gdb_exit + if [mi_gdb_start] { + continue + } + + set options {debug} + if {$variant == "mt" } { + lappend options "pthreads" + } + + # Don't use standard_testfile as we need a different binary + # for each variant. + set testfile $gdb_test_file_name-$variant + set binfile [standard_output_file ${testfile}] + set srcfile $testfile.c + set srcfile2 $gdb_test_file_name.c + + if {[build_executable "failed to prepare" \ + $testfile \ + "${srcfile} ${srcfile2}" \ + $options] == -1} { + return -1 + } + + mi_delete_breakpoints + mi_gdb_reinitialize_dir $srcdir/$subdir + mi_gdb_reinitialize_dir $srcdir/$subdir + mi_gdb_load ${binfile} + + mi_runto test + + # Leave the breakpoint at 'test' set, on purpose. The next + # resume shall emit a single '*running,thread-id="all"', even + # if GDB needs to step over a breakpoint (that is, even if GDB + # needs to run only one thread for a little bit). + + set bp_location [gdb_get_line_number "set breakpoint here" $srcfile2] + set bp_location_end [gdb_get_line_number "set end breakpoint here" $srcfile2] + + mi_gdb_test "-break-insert -c return_false() $srcfile2:$bp_location" ".*" \ + "insert conditional breakpoint" + + mi_gdb_test "-break-insert $srcfile2:$bp_location_end" ".*" \ + "insert end breakpoint" + + set msg "no spurious *running notifications" + send_gdb "-exec-continue\n" + gdb_expect { + -re "\\*running.*\\*running.*\\*stopped" { + fail $msg + } + -re "\\^running\r\n\\*running,thread-id=\"all\"\r\n${mi_gdb_prompt}.*\\*stopped" { + pass $msg + } + timeout { + fail "$msg (timeout)" + } + } + + # In sync mode, there's an extra prompt after *stopped. Consume it. + if {!$async} { + gdb_expect { + -re "$mi_gdb_prompt" { + } + } + } + } +} + +# Single-threaded. +test "st" + +# Multi-threaded. +test "mt"