From: Simon Marchi <simon.marchi@ericsson.com>
To: Pedro Alves <palves@redhat.com>, <gdb-patches@sourceware.org>
Subject: Re: [PATCH 4/5] Linux: Skip thread_db thread event reporting if PTRACE_EVENT_CLONE is supported
Date: Tue, 16 Dec 2014 21:24:00 -0000 [thread overview]
Message-ID: <5490A306.3060002@ericsson.com> (raw)
In-Reply-To: <1418748834-27545-5-git-send-email-palves@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 11824 bytes --]
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
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4079 bytes --]
next prev parent reply other threads:[~2014-12-16 21:24 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-16 16:54 [PATCH 0/5] GNU/Linux, fix attach races/problems Pedro Alves
2014-12-16 16:54 ` [PATCH 3/5] libthread_db: Skip attaching to terminated and joined threads Pedro Alves
2014-12-16 16:54 ` [PATCH 4/5] Linux: Skip thread_db thread event reporting if PTRACE_EVENT_CLONE is supported Pedro Alves
2014-12-16 21:24 ` Simon Marchi [this message]
2014-12-17 13:04 ` Pedro Alves
2014-12-16 16:54 ` [PATCH 2/5] Linux: on attach, attach to lwps listed under /proc/$pid/task/ Pedro Alves
2014-12-16 20:52 ` Simon Marchi
2014-12-17 13:35 ` Pedro Alves
2014-12-16 16:54 ` [PATCH 1/5] libthread_db: debug output should go to gdb_stdlog Pedro Alves
2014-12-17 8:02 ` Yao Qi
2014-12-17 13:45 ` Pedro Alves
2014-12-17 14:09 ` Yao Qi
2014-12-16 17:35 ` [PATCH 5/5] Test attaching to a program that constantly spawns short-lived threads Pedro Alves
2014-12-17 11:10 ` Yao Qi
2014-12-18 0:02 ` Pedro Alves
2015-01-05 19:02 ` Breazeal, Don
2015-01-07 16:17 ` [PATCH] skip "attach" tests when testing against stub-like targets (was: Re: [PATCH 5/5] Test attaching to a program that constantly spawns short-lived threads) Pedro Alves
2015-01-09 11:24 ` [PATCH] skip "attach" tests when testing against stub-like targets Pedro Alves
2015-01-12 4:43 ` [regression/native-gdbserver][buildbot] Python testscases get staled (was: Re: [PATCH] skip "attach" tests when testing against stub-like targets) Sergio Durigan Junior
2015-01-12 11:15 ` [regression/native-gdbserver][buildbot] Python testscases get staled Pedro Alves
2015-01-12 16:55 ` Sergio Durigan Junior
2015-01-12 17:01 ` Pedro Alves
2015-01-12 17:13 ` [PATCH] gdb.python/py-prompt.exp: restore GDBFLAGS Pedro Alves
2015-01-09 12:03 ` [PATCH 0/5] GNU/Linux, fix attach races/problems Pedro Alves
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5490A306.3060002@ericsson.com \
--to=simon.marchi@ericsson.com \
--cc=gdb-patches@sourceware.org \
--cc=palves@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).