diff --git a/gdb/ChangeLog b/gdb/ChangeLog index bbcbabc2a1a..315da655983 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,15 @@ +2014-01-08 Pedro Alves + + * remote.c (remote_add_thread): Add threads silently if starting + up. + (remote_notice_new_inferior): If in all-stop, and starting up, + don't call notice_new_inferior. + (get_current_thread): New function, factored out from ... + (add_current_inferior_and_thread): ... this. Adjust. + (remote_start_remote) : Fetch the thread list. If we + found any thread, then select the remote's current thread as GDB's + current thread too. + 2014-01-08 Joel Brobecker * NEWS: Create a new section for the next release branch. diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index 81042f2e99a..bf874a15ea4 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,21 @@ +2014-01-08 Pedro Alves + + * gdbthread.h (struct thread_info) : New field. + * server.c (visit_actioned_threads, handle_pending_status): New + function. + (handle_v_cont): Factor out parts to ... + (resume): ... this new function. If in all-stop, and a thread + being resumed has a pending status, report it without actually + resuming. + (myresume): Adjust to use the new 'resume' function. + (clear_pending_status_callback, set_pending_status_callback) + (find_status_pending_thread_callback): New functions. + (handle_status): Handle the case of multiple threads having + interesting statuses to report. Report threads' real last signal + instead of always reporting GDB_SIGNAL_TRAP. Look for a thread + with an interesting thread to report the status for, instead of + always reporting the status of the first thread. + 2014-01-01 Joel Brobecker * gdbserver.c (gdbserver_version): Set copyright year to 2014. diff --git a/gdb/gdbserver/gdbthread.h b/gdb/gdbserver/gdbthread.h index 507d9fecba9..a19056670c0 100644 --- a/gdb/gdbserver/gdbthread.h +++ b/gdb/gdbserver/gdbthread.h @@ -36,6 +36,9 @@ struct thread_info /* The last wait status reported for this thread. */ struct target_waitstatus last_status; + /* True if LAST_STATUS hasn't been reported to GDB yet. */ + int status_pending_p; + /* Given `while-stepping', a thread may be collecting data for more than one tracepoint simultaneously. E.g.: diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index 6edce81e259..5e8007511d2 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -2016,6 +2016,63 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) } static void gdb_wants_all_threads_stopped (void); +static void resume (struct thread_resume *actions, size_t n); + +/* Call CALLBACK for any thread to which ACTIONS applies to. Returns + true if CALLBACK returns true. Returns false if no matching thread + is found or CALLBACK results false. */ + +static int +visit_actioned_threads (const struct thread_resume *actions, + size_t num_actions, + int (*callback) (const struct thread_resume *, + struct thread_info *)) +{ + struct inferior_list_entry *entry; + + for (entry = all_threads.head; entry != NULL; entry = entry->next) + { + size_t i; + + for (i = 0; i < num_actions; i++) + { + const struct thread_resume *action = &actions[i]; + + if (ptid_equal (action->thread, minus_one_ptid) + || ptid_equal (action->thread, entry->id) + || ((ptid_get_pid (action->thread) + == ptid_get_pid (entry->id)) + && ptid_get_lwp (action->thread) == -1)) + { + struct thread_info *thread = (struct thread_info *) entry; + + if ((*callback) (action, thread)) + return 1; + } + } + } + + return 0; +} + +/* Callback for visit_actioned_threads. If the thread has a pending + status to report, report it now. */ + +static int +handle_pending_status (const struct thread_resume *resumption, + struct thread_info *thread) +{ + if (thread->status_pending_p) + { + thread->status_pending_p = 0; + + last_status = thread->last_status; + last_ptid = thread->entry.id; + prepare_resume_reply (own_buf, last_ptid, &last_status); + return 1; + } + return 0; +} /* Parse vCont packets. */ void @@ -2128,12 +2185,34 @@ handle_v_cont (char *own_buf) cont_thread = minus_one_ptid; set_desired_inferior (0); - if (!non_stop) - enable_async_io (); - - (*the_target->resume) (resume_info, n); - + resume (resume_info, n); free (resume_info); + return; + +err: + write_enn (own_buf); + free (resume_info); + return; +} + +/* Resume target with ACTIONS, an array of NUM_ACTIONS elements. */ + +static void +resume (struct thread_resume *actions, size_t num_actions) +{ + if (!non_stop) + { + /* Check if among the threads that GDB wants actioned, there's + one with a pending status to report. If so, skip actually + resuming/stopping and report the pending event + immediately. */ + if (visit_actioned_threads (actions, num_actions, handle_pending_status)) + return; + + enable_async_io (); + } + + (*the_target->resume) (actions, num_actions); if (non_stop) write_ok (own_buf); @@ -2157,12 +2236,6 @@ handle_v_cont (char *own_buf) || last_status.kind == TARGET_WAITKIND_SIGNALLED) mourn_inferior (find_process_pid (ptid_get_pid (last_ptid))); } - return; - -err: - write_enn (own_buf); - free (resume_info); - return; } /* Attach to a new program. Return 1 if successful, 0 if failure. */ @@ -2422,31 +2495,7 @@ myresume (char *own_buf, int step, int sig) n++; } - if (!non_stop) - enable_async_io (); - - (*the_target->resume) (resume_info, n); - - if (non_stop) - write_ok (own_buf); - else - { - last_ptid = mywait (minus_one_ptid, &last_status, 0, 1); - - if (last_status.kind != TARGET_WAITKIND_EXITED - && last_status.kind != TARGET_WAITKIND_SIGNALLED) - { - current_inferior->last_resume_kind = resume_stop; - current_inferior->last_status = last_status; - } - - prepare_resume_reply (own_buf, last_ptid, &last_status); - disable_async_io (); - - if (last_status.kind == TARGET_WAITKIND_EXITED - || last_status.kind == TARGET_WAITKIND_SIGNALLED) - mourn_inferior (find_process_pid (ptid_get_pid (last_ptid))); - } + resume (resume_info, n); } /* Callback for for_each_inferior. Make a new stop reply for each @@ -2536,6 +2585,48 @@ gdb_reattached_process (struct inferior_list_entry *entry) process->gdb_detached = 0; } +/* Callback for for_each_inferior. Clear the thread's pending status + flag. */ + +static void +clear_pending_status_callback (struct inferior_list_entry *entry) +{ + struct thread_info *thread = (struct thread_info *) entry; + + thread->status_pending_p = 0; +} + +/* Callback for for_each_inferior. If the thread is stopped with an + interesting event, mark it as having a pending event. */ + +static void +set_pending_status_callback (struct inferior_list_entry *entry) +{ + struct thread_info *thread = (struct thread_info *) entry; + + if (thread->last_status.kind != TARGET_WAITKIND_STOPPED + || (thread->last_status.value.sig != GDB_SIGNAL_0 + /* A breakpoint, watchpoint or finished step from a previous + GDB run isn't considered interesting for a new GDB run. + If we left those pending, the new GDB could consider them + random SIGTRAPs. This leaves out real async traps. We'd + have to peek into the (target-specific) siginfo to + distinguish those. */ + && thread->last_status.value.sig != GDB_SIGNAL_TRAP)) + thread->status_pending_p = 1; +} + +/* Callback for find_inferior. Return true if ENTRY (a thread) has a + pending status to report to GDB. */ + +static int +find_status_pending_thread_callback (struct inferior_list_entry *entry, void *data) +{ + struct thread_info *thread = (struct thread_info *) entry; + + return thread->status_pending_p; +} + /* Status handler for the '?' packet. */ static void @@ -2544,13 +2635,15 @@ handle_status (char *own_buf) /* GDB is connected, don't forward events to the target anymore. */ for_each_inferior (&all_processes, gdb_reattached_process); + discard_queued_stop_replies (-1); + for_each_inferior (&all_threads, clear_pending_status_callback); + /* In non-stop mode, we must send a stop reply for each stopped thread. In all-stop mode, just send one for the first stopped thread we find. */ if (non_stop) { - discard_queued_stop_replies (-1); find_inferior (&all_threads, queue_stop_reply_callback, NULL); /* The first is sent immediatly. OK is sent if there is no @@ -2560,18 +2653,53 @@ handle_status (char *own_buf) } else { + struct inferior_list_entry *thread = NULL; + pause_all (0); stabilize_threads (); gdb_wants_all_threads_stopped (); - if (all_threads.head) - { - struct target_waitstatus status; + /* We can only report one status, but we might be coming out of + non-stop -- if more than one thread is stopped with + interesting events, leave events for the threads we're not + reporting now pending. They'll be reported the next time the + threads are resumed. Start by marking all interesting events + as pending. */ + for_each_inferior (&all_threads, set_pending_status_callback); - status.kind = TARGET_WAITKIND_STOPPED; - status.value.sig = GDB_SIGNAL_TRAP; - prepare_resume_reply (own_buf, - all_threads.head->id, &status); + /* Prefer the last thread that reported an event to GDB (even if + that was a GDB_SIGNAL_TRAP). */ + if (last_status.kind != TARGET_WAITKIND_IGNORE + && last_status.kind != TARGET_WAITKIND_EXITED + && last_status.kind != TARGET_WAITKIND_SIGNALLED) + thread = find_inferior_id (&all_threads, last_ptid); + + /* If the last event thread is not found for some reason, look + for some other thread that might have an event to report. */ + if (thread == NULL) + thread = find_inferior (&all_threads, + find_status_pending_thread_callback, NULL); + + /* If we're still out of luck, simply pick the first thread in + the thread list. */ + if (thread == NULL) + thread = all_threads.head; + + if (thread != NULL) + { + struct thread_info *tp = (struct thread_info *) thread; + + /* We're reporting this event, so it's no longer + pending. */ + tp->status_pending_p = 0; + + /* GDB assumes the current thread is the thread we're + reporting the status for. */ + general_thread = thread->id; + set_desired_inferior (1); + + gdb_assert (tp->last_status.kind != TARGET_WAITKIND_IGNORE); + prepare_resume_reply (own_buf, tp->entry.id, &tp->last_status); } else strcpy (own_buf, "W00"); diff --git a/gdb/remote.c b/gdb/remote.c index 8366c5ddc50..70946b17667 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -1561,7 +1561,18 @@ remote_add_inferior (int fake_pid_p, int pid, int attached) static void remote_add_thread (ptid_t ptid, int running) { - add_thread (ptid); + struct remote_state *rs = get_remote_state (); + + /* GDB historically didn't pull threads in the initial connection + setup. If the remote target doesn't even have a concept of + threads (e.g., a bare-metal target), even if internally we + consider that a single-threaded target, mentioning a new thread + might be confusing to the user. Be silent then, preserving the + age old behavior. */ + if (rs->starting_up) + add_thread_silent (ptid); + else + add_thread (ptid); set_executing (ptid, running); set_running (ptid, running); @@ -1639,9 +1650,15 @@ remote_notice_new_inferior (ptid_t currthread, int running) /* If we found a new inferior, let the common code do whatever it needs to with it (e.g., read shared libraries, insert - breakpoints). */ + breakpoints), unless we're just setting up an all-stop + connection. */ if (inf != NULL) - notice_new_inferior (currthread, running, 0); + { + struct remote_state *rs = get_remote_state (); + + if (non_stop || !rs->starting_up) + notice_new_inferior (currthread, running, 0); + } } } @@ -3309,6 +3326,28 @@ stop_reply_extract_thread (char *stop_reply) return null_ptid; } +/* Determine the remote side's current thread. If we have a stop + reply handy (in WAIT_STATUS), maybe it's a T stop reply with a + "thread" register we can extract the current thread from. If not, + ask the remote which is the current thread with qC. The former + method avoids a roundtrip. */ + +static ptid_t +get_current_thread (char *wait_status) +{ + ptid_t ptid; + + /* Note we don't use remote_parse_stop_reply as that makes use of + the target architecture, which we haven't yet fully determined at + this point. */ + if (wait_status != NULL) + ptid = stop_reply_extract_thread (wait_status); + if (ptid_equal (ptid, null_ptid)) + ptid = remote_current_thread (inferior_ptid); + + return ptid; +} + /* Query the remote target for which is the current thread/process, add it to our tables, and update INFERIOR_PTID. The caller is responsible for setting the state such that the remote end is ready @@ -3329,18 +3368,8 @@ add_current_inferior_and_thread (char *wait_status) inferior_ptid = null_ptid; - /* Now, if we have thread information, update inferior_ptid. First - if we have a stop reply handy, maybe it's a T stop reply with a - "thread" register we can extract the current thread from. If - not, ask the remote which is the current thread, with qC. The - former method avoids a roundtrip. Note we don't use - remote_parse_stop_reply as that makes use of the target - architecture, which we haven't yet fully determined at this - point. */ - if (wait_status != NULL) - ptid = stop_reply_extract_thread (wait_status); - if (ptid_equal (ptid, null_ptid)) - ptid = remote_current_thread (inferior_ptid); + /* Now, if we have thread information, update inferior_ptid. */ + ptid = get_current_thread (wait_status); if (!ptid_equal (ptid, null_ptid)) { @@ -3510,10 +3539,35 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p) strcpy (wait_status, rs->buf); } + /* Fetch thread list. */ + target_find_new_threads (); + /* Let the stub know that we want it to return the thread. */ set_continue_thread (minus_one_ptid); - add_current_inferior_and_thread (wait_status); + if (thread_count () == 0) + { + /* Target has no concept of threads at all. GDB treats + non-threaded target as single-threaded; add a main + thread. */ + add_current_inferior_and_thread (wait_status); + } + else + { + /* We have thread information; select the thread the target + says should be current. If we're reconnecting to a + multi-threaded program, this will ideally be the thread + that last reported an event before GDB disconnected. */ + inferior_ptid = get_current_thread (wait_status); + if (ptid_equal (inferior_ptid, null_ptid)) + { + /* Odd... The target was able to list threads, but not + tell us which thread was current (no "thread" + register in T stop reply?). Just pick the first + thread in the thread list then. */ + inferior_ptid = thread_list->ptid; + } + } /* init_wait_for_inferior should be called before get_offsets in order to manage `inserted' flag in bp loc in a correct state. diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 6f7aa377a21..9ba11922a54 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2014-01-08 Pedro Alves + + * gdb.threads/reconnect-signal.c: New file. + * gdb.threads/reconnect-signal.exp: New file. + 2014-01-07 Jan Kratochvil * gdb.base/source-dir.exp: New file. diff --git a/gdb/testsuite/gdb.threads/reconnect-signal.c b/gdb/testsuite/gdb.threads/reconnect-signal.c new file mode 100644 index 00000000000..498ecb21aab --- /dev/null +++ b/gdb/testsuite/gdb.threads/reconnect-signal.c @@ -0,0 +1,67 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 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 . */ + +#include +#include +#include + +static pthread_t thread_2; +sig_atomic_t unlocked; + +/* The test has three threads, and it's always thread 2 that gets the + signal, to avoid spurious passes in case the remote side happens to + always pick the first or the last thread in the list as the + current/status thread on reconnection. */ + +static void * +start2 (void *arg) +{ + unsigned int count; + + pthread_kill (thread_2, SIGUSR1); + + for (count = 1; !unlocked && count != 0; count++) + usleep (1); + return NULL; +} + +static void * +start (void *arg) +{ + pthread_t thread; + + pthread_create (&thread, NULL, start2, NULL); + pthread_join (thread, NULL); + return NULL; +} + +void +handle (int sig) +{ + unlocked = 1; +} + +int +main () +{ + signal (SIGUSR1, handle); + + pthread_create (&thread_2, NULL, start, NULL); + pthread_join (thread_2, NULL); + + return 0; +} diff --git a/gdb/testsuite/gdb.threads/reconnect-signal.exp b/gdb/testsuite/gdb.threads/reconnect-signal.exp new file mode 100644 index 00000000000..ccb4a4239a8 --- /dev/null +++ b/gdb/testsuite/gdb.threads/reconnect-signal.exp @@ -0,0 +1,84 @@ +# Copyright 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 . */ + +# Test that disconnecting and reconnecting doesn't lose signals. + +set gdbserver_reconnect_p 1 +if { [info proc gdb_reconnect] == "" } { + return 0 +} + +standard_testfile +set executable ${testfile} + +if { [gdb_compile_pthreads \ + "${srcdir}/${subdir}/${srcfile}" \ + "${binfile}" \ + executable {debug}] != "" } { + untested "Couldn't compile test program." + return -1 +} + +clean_restart $executable + +if ![runto_main] then { + fail "Can't run to main" + return 0 +} + +gdb_test "continue" "signal SIGUSR1.*" "continue to signal" + +# Check that it's thread 2 that is selected. +gdb_test "info threads" "\\* 2 .*" "thread 2 is selected" + +set msg "save \$pc after signal" +set saved_pc "" +gdb_test_multiple "print/x \$pc" $msg { + -re "\\\$$decimal = (\[^\r\n\]*)\r\n$gdb_prompt $" { + set saved_pc $expect_out(1,string) + pass $msg + } +} + +# Switch to the other thread. +gdb_test "thread 1" "thread 1.*" "switch to thread 1" + +# Force GDB to select thread 1 on the remote end as well. +gdb_test "print/x \$pc" + +gdb_test "disconnect" "Ending remote debugging\\." "disconnect after signal" + +set test "reconnect after signal" + +set res [gdb_reconnect] +if { [lindex $res 0] == 0 } { + pass $test +} else { + fail $test + return 0 +} + +# Check that thread 2 is re-selected. +gdb_test "info threads" "\\* 2 .*" "thread 2 is selected on reconnect" + +# Check that the program is still alive, and stopped in the same spot. +gdb_test "print/x \$pc" "\\\$$decimal = $saved_pc" "check \$pc after signal" + +# Check that we didn't lose the signal. +gdb_test "info program" "stopped with signal SIGUSR1,.*" + +# Nor does the program. +gdb_test "b handle" "Breakpoint .*" "set breakpoint in signal handler" +gdb_test "continue" "handle.*" "continue to signal handler"