darkkirb/old-cross-binutils
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

PR threads/18127 - threads spawned by infcall end up stuck in "running" state

Browse source 
Refs:
 https://sourceware.org/ml/gdb/2015-03/msg00024.html
 https://sourceware.org/ml/gdb/2015-06/msg00005.html

On GNU/Linux, if an infcall spawns a thread, that thread ends up with
stuck running state.  This happens because:

 - when linux-nat.c detects a new thread, it marks them as running,
   and does not report anything to the core.

 - we skip finish_thread_state when the thread that is running the
   infcall stops.

As result, that new thread ends up with stuck "running" state, even
though it really is stopped.

On Windows, _all_ threads end up stuck in running state, not just the
one that was spawned.  That happens because when a new thread is
detected, unlike linux-nat.c, windows-nat.c reports
TARGET_WAITKIND_SPURIOUS to infrun.  It's the fact that that event
does not cause a user-visible stop that triggers the problem.  When
the target is re-resumed, we call set_running with a wildcard ptid,
which marks all thread as running.  That set_running is not suppressed
because the (leader) thread being resumed does not have in_infcall
set.  Later, when the infcall finally finishes successfully, nothing
marks all threads back to stopped.

We can trigger the same problem on all targets by having a thread
other than the one that is running the infcall report a breakpoint hit
to infrun, and then have that breakpoint not cause a stop.  That's
what the included test does.

The fix is to stop GDB from suppressing the set_running calls while
doing an infcall, and then set the threads back to stopped when the
call finishes, iff they were originally stopped before the infcall
started.  (Note the MI *running/*stopped event suppression isn't
affected.)

Tested on x86_64 GNU/Linux.

gdb/ChangeLog:
2015-06-29  Pedro Alves  <palves@redhat.com>

	PR threads/18127
	* infcall.c (run_inferior_call): On infcall success, if the thread
	was marked stopped before, reset it back to stopped.
	* infrun.c (resume): Don't suppress the set_running calls when
	doing an infcall.
	(normal_stop): Only discard the finish_thread_state cleanup if the
	infcall succeeded.

gdb/testsuite/ChangeLog:
2015-06-29  Pedro Alves  <palves@redhat.com>

	PR threads/18127
	* gdb.threads/hand-call-new-thread.c: New file.
	* gdb.threads/hand-call-new-thread.c: New file.
This commit is contained in:
Pedro Alves 2015-06-29 16:07:57 +01:00
parent 2880b51c25
commit 28bf096c62
6 changed files with 148 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
gdb/ChangeLog
View file

@ -1,3 +1,13 @@
2015-06-29 Pedro Alves <palves@redhat.com>
PR threads/18127
* infcall.c (run_inferior_call): On infcall success, if the thread
was marked stopped before, reset it back to stopped.
* infrun.c (resume): Don't suppress the set_running calls when
doing an infcall.
(normal_stop): Only discard the finish_thread_state cleanup if the
infcall succeeded.
2015-06-29 Pierre Langlois <pierre.langlois@arm.com>
* MAINTAINERS (Write After Approval): Update my email address.

21
gdb/infcall.c
View file

@ -387,6 +387,7 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
int saved_in_infcall = call_thread->control.in_infcall;
ptid_t call_thread_ptid = call_thread->ptid;
int saved_sync_execution = sync_execution;
int was_running = call_thread->state == THREAD_RUNNING;
/* Infcalls run synchronously, in the foreground. */
if (target_can_async_p ())
@ -433,6 +434,26 @@ run_inferior_call (struct thread_info *call_thread, CORE_ADDR real_pc)
CALL_THREAD as it could be invalid if its thread has exited. */
call_thread = find_thread_ptid (call_thread_ptid);
/* If the infcall does NOT succeed, normal_stop will have already
finished the thread states. However, on success, normal_stop
defers here, so that we can set back the thread states to what
they were before the call. Note that we must also finish the
state of new threads that might have spawned while the call was
running. The main cases to handle are:
- "(gdb) print foo ()", or any other command that evaluates an
expression at the prompt. (The thread was marked stopped before.)
- "(gdb) break foo if return_false()" or similar cases where we
do an infcall while handling an event (while the thread is still
marked running). In this example, whether the condition
evaluates true and thus we'll present a user-visible stop is
decided elsewhere. */
if (!was_running
&& ptid_equal (call_thread_ptid, inferior_ptid)
&& stop_stack_dummy == STOP_STACK_DUMMY)
finish_thread_state (user_visible_resume_ptid (0));
enable_watchpoints_after_interactive_call_stop ();
/* Call breakpoint_auto_delete on the current contents of the bpstat

31
gdb/infrun.c
View file

@ -2264,11 +2264,8 @@ resume (enum gdb_signal sig)
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 (user_step), 1);
user/frontend's point of view, threads were set running. */
set_running (user_visible_resume_ptid (user_step), 1);
discard_cleanups (old_cleanups);
return;
}
@ -2346,10 +2343,8 @@ resume (enum gdb_signal sig)
/* 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);
running. */
set_running (resume_ptid, 1);
/* Maybe resume a single thread after all. */
if ((step || thread_has_single_step_breakpoints_set (tp))
@ -6664,15 +6659,15 @@ 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, 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)
/* Let the user/frontend see the threads as stopped, but defer to
call_function_by_hand if the thread finished an infcall
successfully. We may be e.g., evaluating a breakpoint condition.
In that case, the thread had state THREAD_RUNNING before the
infcall, and shall remain marked running, all without informing
the user/frontend about state transition changes. */
if (target_has_execution
&& inferior_thread ()->control.in_infcall
&& stop_stack_dummy == STOP_STACK_DUMMY)
discard_cleanups (old_chain);
else
do_cleanups (old_chain);

6
gdb/testsuite/ChangeLog
View file

@ -1,3 +1,9 @@
2015-06-29 Pedro Alves <palves@redhat.com>
PR threads/18127
* gdb.threads/hand-call-new-thread.c: New file.
* gdb.threads/hand-call-new-thread.c: New file.
2015-06-26 Keith Seitz <keiths@redhat.com>
Doug Evans <dje@google.com>

50
gdb/testsuite/gdb.threads/hand-call-new-thread.c Normal file
View file

@ -0,0 +1,50 @@
/* This testcase is part of GDB, the GNU debugger.
Copyright 2015 Free Software Foundation, Inc.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 3 of the License, or
(at your option) any later version.
This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include <unistd.h>
#include <stdlib.h>
#include <pthread.h>
#include <assert.h>
static int
foo (void)
{
usleep (1);
}
static void *
thread_function (void *arg)
{
while (1)
foo ();
}
void
new_thread (void)
{
pthread_t thread;
int res;
res = pthread_create (&thread, NULL, thread_function, NULL);
assert (res == 0);
}
int
main (int argc, char **argv)
{
return 0;
}

48
gdb/testsuite/gdb.threads/hand-call-new-thread.exp Normal file
View file

@ -0,0 +1,48 @@
# Copyright (C) 2015 Free Software Foundation, Inc.
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
# Ensure that new threads created while an infcall is ongoing are set
# to stopped state once the call finishes.
standard_testfile
if [prepare_for_testing "failed to prepare" $testfile $srcfile {debug pthreads}] {
return -1
}
if ![runto_main] {
continue
}
# Set a thread-specific breakpoint that the wrong thread trips on
# while running the infcall. Check that no thread ends up in stale
# "running" state once the call finishes.
gdb_test "b foo thread 1" "Breakpoint .*$srcfile.*"
for {set i 0} {$i < 3} {incr i} {
with_test_prefix "iter $i" {
gdb_test "p new_thread ()" " = void"
set message "no thread marked running"
gdb_test_multiple "info threads" $message {
-re "\\\(running\\\).*$gdb_prompt $" {
fail $message
}
-re "$gdb_prompt $" {
pass $message
}
}
}
}