[PATCH 4/5] Linux: Skip thread_db thread event reporting if PTRACE_EVENT_CLONE is supported
Simon Marchi
simon.marchi@ericsson.com
Tue Dec 16 21:24:00 GMT 2014
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 <palves@redhat.com>
>
> 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 <palves@redhat.com>
>
> * 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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4079 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20141216/b9647608/attachment.p7s>
More information about the Gdb-patches
mailing list