On 2014-12-16 11:53 AM, Pedro Alves wrote: > [A test I wrote stumbled on a libthread_db issue related to thread > event breakpoints. See glibc PR17705: > [nptl_db: stale thread create/death events if debugger detaches] > https://sourceware.org/bugzilla/show_bug.cgi?id=17705 > > This patch avoids that whole issue by making GDB stop using thread > event breakpoints in the first place, which is good for other reasons > as well, anyway.] > > Before PTRACE_EVENT_CLONE (Linux 2.6), the only way to learn about new > threads in the inferior (to attach to them) or to learn about thread > exit was to coordinate with the inferior's glibc/runtime, using > libthread_db. That works by putting a breakpoint at a magic address > which is called when a new thread is spawned, or when a thread is > about to exit. When that breakpoint is hit, all threads are stopped, > and then GDB coordinates with libthread_db to read data structures out > of the inferior to learn about what happened. That is libthread_db's TD_CREATE event? Could you point out where that is done (stopping all the threads)? From the previous discussion with you, I was thinking that those breakpoints did not affect execution. I don't find any code in linux-thread-db.c that would do such a thing. > Then the breakpoint is > single-stepped, and then all threads are re-resumed. This isn't very > efficient (stops all threads) and is more fragile (inferior's thread > list in memory may be corrupt; libthread_db bugs, etc.) than ideal. > > When the kernel supports PTRACE_EVENT_CLONE (which we already make use > of), there's really no need to use libthread_db's event reporting > mechanism to learn about new LWPs. And if the kernel supports that, > then we learn about LWP exits through regular WIFEXITED wait statuses, > so no need for the death event breakpoint either. > > GDBserver has been likewise skipping the thread_db events for a long > while: > https://sourceware.org/ml/gdb-patches/2007-10/msg00547.html > > There's one user-visible difference: we'll no longer print about > threads being created and exiting while the program is running, like: > > [Thread 0x7ffff7dbb700 (LWP 30670) exited] > [New Thread 0x7ffff7db3700 (LWP 30671)] > [Thread 0x7ffff7dd3700 (LWP 30667) exited] > [New Thread 0x7ffff7dab700 (LWP 30672)] > [Thread 0x7ffff7db3700 (LWP 30671) exited] > [Thread 0x7ffff7dcb700 (LWP 30668) exited] > > This is exactly the same behavior as when debugging against remote > targets / gdbserver. I actually think that's a good thing (and as > such have listed this in the local/remote parity wiki page a while > ago), as the printing slows down the inferior. It's also a > distraction to keep bothering the user about short-lived threads that > she won't be able to interact with anyway. Instead, the user (and > frontend) will be informed about new threads that currently exist in > the program when the program next stops: Is this a consequence of the change of algorithm, or did you actively changed the behavior? From what I understand, gdb still attaches to the new thread as soon as it spawns (when it receives the PTRACE_EVENT_CLONE event), so it could print the notice when the event happens. Not that I mind, but I just want to understand. > (gdb) c > ... > * ctrl-c * > [New Thread 0x7ffff7963700 (LWP 7797)] > [New Thread 0x7ffff796b700 (LWP 7796)] > > Program received signal SIGINT, Interrupt. > [Switching to Thread 0x7ffff796b700 (LWP 7796)] > clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:81 > 81 testq %rax,%rax > (gdb) info threads > > A couple of tests had assumptions on GDB thread numbers that no longer > hold. > > Tested on x86_64 Fedora 20. > > gdb/ > 2014-12-16 Pedro Alves > > Skip enabling event reporting if the kernel supports > PTRACE_EVENT_CLONE. > * linux-thread-db.c: Include "nat/linux-ptrace.h". > (thread_db_use_events): New function. > (try_thread_db_load_1): Check thread_db_use_events before enabling > event reporting. > (update_thread_state): New function. > (attach_thread): Use it. Check thread_db_use_events before > enabling event reporting. > (thread_db_detach): Check thread_db_use_events before disabling > event reporting. > (find_new_threads_callback): Check thread_db_use_events before > enabling event reporting. Update the thread's state if not using > libthread_db events. > > gdb/testsuite/ > 2014-12-16 Pedro Alves > > * gdb.threads/fork-thread-pending.exp: Switch to the main thread > instead of to thread 2. > * gdb.threads/signal-command-multiple-signals-pending.c (main): > Add barrier around each pthread_create call instead of around all > calls. > * gdb.threads/signal-command-multiple-signals-pending.exp (test): > Set a break on thread_function and have the child threads hit it > one at at a time. > --- > gdb/linux-thread-db.c | 39 ++++++++++++++++++---- > gdb/testsuite/gdb.threads/fork-thread-pending.exp | 2 +- > .../signal-command-multiple-signals-pending.c | 11 +++--- > .../signal-command-multiple-signals-pending.exp | 7 ++++ > 4 files changed, 47 insertions(+), 12 deletions(-) > > diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c > index 4b26984..35181f0 100644 > --- a/gdb/linux-thread-db.c > +++ b/gdb/linux-thread-db.c > @@ -38,6 +38,7 @@ > #include "observer.h" > #include "linux-nat.h" > #include "nat/linux-procfs.h" > +#include "nat/linux-ptrace.h" > #include "nat/linux-osdata.h" > #include "auto-load.h" > #include "cli/cli-utils.h" > @@ -77,6 +78,16 @@ static char *libthread_db_search_path; > by the "set auto-load libthread-db" command. */ > static int auto_load_thread_db = 1; > > +/* Returns true if we need to use thread_db thread create/death event > + breakpoints to learn about threads. */ > + > +static int > +thread_db_use_events (void) > +{ > + /* Not necessary if the kernel supports clone events. */ > + return !linux_supports_traceclone (); > +} > + > /* "show" command for the auto_load_thread_db configuration variable. */ > > static void > @@ -832,7 +843,7 @@ try_thread_db_load_1 (struct thread_db_info *info) > push_target (&thread_db_ops); > > /* Enable event reporting, but not when debugging a core file. */ > - if (target_has_execution) > + if (target_has_execution && thread_db_use_events ()) > enable_thread_event_reporting (); > > return 1; > @@ -1260,6 +1271,17 @@ thread_db_inferior_created (struct target_ops *target, int from_tty) > check_for_thread_db (); > } > > +/* Update the thread's state (what's displayed in "info threads"), > + from libthread_db thread state information. */ > + > +static void > +update_thread_state (struct private_thread_info *private, > + const td_thrinfo_t *ti_p) > +{ > + private->dying = (ti_p->ti_state == TD_THR_UNKNOWN > + || ti_p->ti_state == TD_THR_ZOMBIE); > +} > + > /* Attach to a new thread. This function is called when we receive a > TD_CREATE event or when we iterate over all threads and find one > that wasn't already in our list. Returns true on success. */ > @@ -1341,8 +1363,7 @@ attach_thread (ptid_t ptid, const td_thrhandle_t *th_p, > gdb_assert (ti_p->ti_tid != 0); > private->th = *th_p; > private->tid = ti_p->ti_tid; > - if (ti_p->ti_state == TD_THR_UNKNOWN || ti_p->ti_state == TD_THR_ZOMBIE) > - private->dying = 1; > + update_thread_state (private, ti_p); > > /* Add the thread to GDB's thread list. */ > if (tp == NULL) > @@ -1354,7 +1375,7 @@ attach_thread (ptid_t ptid, const td_thrhandle_t *th_p, > > /* Enable thread event reporting for this thread, except when > debugging a core file. */ > - if (target_has_execution) > + if (target_has_execution && thread_db_use_events ()) > { > err = info->td_thr_event_enable_p (th_p, 1); > if (err != TD_OK) > @@ -1393,7 +1414,7 @@ thread_db_detach (struct target_ops *ops, const char *args, int from_tty) > > if (info) > { > - if (target_has_execution) > + if (target_has_execution && thread_db_use_events ()) > { > disable_thread_event_reporting (info); > > @@ -1629,7 +1650,7 @@ find_new_threads_callback (const td_thrhandle_t *th_p, void *data) > need this glibc bug workaround. */ > info->need_stale_parent_threads_check = 0; > > - if (target_has_execution) > + if (target_has_execution && thread_db_use_events ()) > { > err = info->td_thr_event_enable_p (th_p, 1); > if (err != TD_OK) > @@ -1666,6 +1687,12 @@ find_new_threads_callback (const td_thrhandle_t *th_p, void *data) > iteration: thread_db_find_new_threads_2 will retry. */ > return 1; > } > + else if (target_has_execution && !thread_db_use_events ()) > + { > + /* Need to update this if not using the libthread_db events > + (particularly, the TD_DEATH event). */ > + update_thread_state (tp->private, &ti); > + } > > return 0; > } > diff --git a/gdb/testsuite/gdb.threads/fork-thread-pending.exp b/gdb/testsuite/gdb.threads/fork-thread-pending.exp > index 57e45c9..357cb9e 100644 > --- a/gdb/testsuite/gdb.threads/fork-thread-pending.exp > +++ b/gdb/testsuite/gdb.threads/fork-thread-pending.exp > @@ -46,7 +46,7 @@ gdb_test "continue" "Catchpoint.*" "1, get to the fork event" > > gdb_test "info threads" " Thread .* Thread .* Thread .* Thread .*" "1, multiple threads found" > > -gdb_test "thread 2" ".*" "1, switched away from event thread" > +gdb_test "thread 1" ".*" "1, switched away from event thread" > > gdb_test "continue" "Not resuming.*" "1, refused to resume" > > diff --git a/gdb/testsuite/gdb.threads/signal-command-multiple-signals-pending.c b/gdb/testsuite/gdb.threads/signal-command-multiple-signals-pending.c > index 2fc5f53..761bef1 100644 > --- a/gdb/testsuite/gdb.threads/signal-command-multiple-signals-pending.c > +++ b/gdb/testsuite/gdb.threads/signal-command-multiple-signals-pending.c > @@ -76,12 +76,13 @@ main (void) > signal (SIGUSR1, handler_sigusr1); > signal (SIGUSR2, handler_sigusr2); > > - pthread_barrier_init (&barrier, NULL, 3); > - > for (i = 0; i < 2; i++) > - pthread_create (&child_thread[i], NULL, thread_function, NULL); > - > - pthread_barrier_wait (&barrier); > + { > + pthread_barrier_init (&barrier, NULL, 2); > + pthread_create (&child_thread[i], NULL, thread_function, NULL); > + pthread_barrier_wait (&barrier); > + pthread_barrier_destroy (&barrier); > + } > > all_threads_started (); > > diff --git a/gdb/testsuite/gdb.threads/signal-command-multiple-signals-pending.exp b/gdb/testsuite/gdb.threads/signal-command-multiple-signals-pending.exp > index b5ec00a..f574c57 100644 > --- a/gdb/testsuite/gdb.threads/signal-command-multiple-signals-pending.exp > +++ b/gdb/testsuite/gdb.threads/signal-command-multiple-signals-pending.exp > @@ -46,6 +46,13 @@ proc test { schedlock } { > gdb_test "handle SIGUSR2 stop print pass" > > gdb_test "break all_threads_started" "Breakpoint .* at .*$srcfile.*" > + > + # Create threads one at a time, to insure stable thread > + # numbers between runs and targets. > + gdb_test "break thread_function" "Breakpoint .* at .*$srcfile.*" > + gdb_test "continue" "thread_function.*" "thread 2 created" > + gdb_test "continue" "thread_function.*" "thread 3 created" > + > gdb_test "continue" "all_threads_started.*" > > # Using schedlock, let the main thread queue a signal for each >