Re: [PATCH][gdb/threads] Fix lin_thread_get_thread_signals for glibc 2.28

Luis Machado via Gdb-patches
2021-02-05T11:54:31Z

On 2/5/21 8:15 AM, Tom de Vries wrote:
> Hi,
>
> When running test-case gdb.threads/create-fail.exp on openSUSE Factory
> (with glibc version 2.32) I run into:

I'd mention Ubuntu 20.04 as well.

> ...
> (gdb) continue
> Continuing.
> [New Thread 0x7ffff7c83700 (LWP 626354)]
> [New Thread 0x7ffff7482700 (LWP 626355)]
> [Thread 0x7ffff7c83700 (LWP 626354) exited]
> [New Thread 0x7ffff6c81700 (LWP 626356)]
> [Thread 0x7ffff7482700 (LWP 626355) exited]
> [New Thread 0x7ffff6480700 (LWP 626357)]
> [Thread 0x7ffff6c81700 (LWP 626356) exited]
> [New Thread 0x7ffff5c7f700 (LWP 626358)]
> [Thread 0x7ffff6480700 (LWP 626357) exited]
> pthread_create: 22: Invalid argument
>
> Thread 6 "create-fail" received signal SIG32, Real-time event 32.
> [Switching to Thread 0x7ffff5c7f700 (LWP 626358)]
> 0x00007ffff7d87695 in clone () from /lib64/libc.so.6
> (gdb) FAIL: gdb.threads/create-fail.exp: iteration 1: run till end
> ...
> The problem is that glibc-internal signal SIGCANCEL is not recognized by gdb.
>
> There's code in check_thread_signals that is supposed to take care of that,
> but it's not working because this code in lin_thread_get_thread_signals has
> stopped working:
> ...
> /* NPTL reserves the first two RT signals, but does not provide any
> way for the debugger to query the signal numbers - fortunately
> they don't change. */
> sigaddset (set, __SIGRTMIN);
> sigaddset (set, __SIGRTMIN + 1);
> ...
>
> Since glibc commit d2dc5467c6 "Filter out NPTL internal signals (BZ #22391)"
> (first released as part of glibc 2.28), a sigaddset with a glibc-internal
> signal has no other effect than setting errno to EINVALID.
>
> Fix this by eliminating the usage of sigset_t in check_thread_signals and
> lin_thread_get_thread_signals.
>
> Tested on x86_64-linux.

Also validated on aarch64-linux/Ubuntu 20.04 and Ubuntu 18.04.

>
> Any comments?
>
> Thanks,
> - Tom
>
> [gdb/threads] Fix lin_thread_get_thread_signals for glibc 2.28
>
> gdb/ChangeLog:
>
> 2021-02-05 Tom de Vries <tdevries@suse.de>
>
> PR threads/26228
> * linux-nat.c (lin_thread_get_thread_signals): Remove.
> (lin_thread_signals): New static var.
> (lin_thread_get_thread_signal_num, lin_thread_get_thread_signal):
> New function.
> * linux-nat.h (lin_thread_get_thread_signals): Remove.
> (lin_thread_get_thread_signal_num, lin_thread_get_thread_signal):
> Declare.
> * linux-thread-db.c (check_thread_signals): Use
> lin_thread_get_thread_signal_num and lin_thread_get_thread_signal.
>
> ---
> gdb/linux-nat.c | 26 +++++++++++++++++---------
> gdb/linux-nat.h | 7 +++++--
> gdb/linux-thread-db.c | 21 +++++----------------
> 3 files changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index 10419dc7bb5..42dcd77ac45 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -4411,16 +4411,24 @@ Enables printf debugging output."),
> the GNU/Linux Threads library and therefore doesn't really belong
> here. */
>
> -/* Return the set of signals used by the threads library in *SET. */
> +/* NPTL reserves the first two RT signals, but does not provide any
> + way for the debugger to query the signal numbers - fortunately
> + they don't change. */
> +static int lin_thread_signals[] = { __SIGRTMIN, __SIGRTMIN + 1 };
>
> -void
> -lin_thread_get_thread_signals (sigset_t *set)
> +/* See linux-nat.h. */
> +
> +unsigned int
> +lin_thread_get_thread_signal_num (void)
> {
> - sigemptyset (set);
> + return sizeof (lin_thread_signals) / sizeof (lin_thread_signals[0]);
> +}
>
> - /* NPTL reserves the first two RT signals, but does not provide any
> - way for the debugger to query the signal numbers - fortunately
> - they don't change. */
> - sigaddset (set, __SIGRTMIN);
> - sigaddset (set, __SIGRTMIN + 1);
> +/* See linux-nat.h. */
> +
> +int
> +lin_thread_get_thread_signal (unsigned int i)
> +{
> + gdb_assert (i < lin_thread_get_thread_signal_num ());
> + return lin_thread_signals[i];
> }
> diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
> index a5547f282ad..ff4d753422d 100644
> --- a/gdb/linux-nat.h
> +++ b/gdb/linux-nat.h
> @@ -304,8 +304,11 @@ void check_for_thread_db (void);
> true on success, false if the process isn't using libpthread. */
> extern int thread_db_notice_clone (ptid_t parent, ptid_t child);
>
> -/* Return the set of signals used by the threads library. */
> -extern void lin_thread_get_thread_signals (sigset_t *mask);
> +/* Return the number of signals used by the threads library. */
> +extern unsigned int lin_thread_get_thread_signal_num (void);
> +
> +/* Return the i-th signal used by the threads library. */
> +extern int lin_thread_get_thread_signal (unsigned int i);
>
> /* Find process PID's pending signal set from /proc/pid/status. */
> void linux_proc_pending_signals (int pid, sigset_t *pending,
> diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
> index dce4bd23c1b..4dab64ac344 100644
> --- a/gdb/linux-thread-db.c
> +++ b/gdb/linux-thread-db.c
> @@ -161,8 +161,6 @@ static thread_db_target the_thread_db_target;
> /* Non-zero if we have determined the signals used by the threads
> library. */
> static int thread_signals;
> -static sigset_t thread_stop_set;
> -static sigset_t thread_print_set;
>
> struct thread_db_info
> {
> @@ -1225,23 +1223,14 @@ check_thread_signals (void)
> {
> if (!thread_signals)
> {
> - sigset_t mask;
> int i;
>
> - lin_thread_get_thread_signals (&mask);
> - sigemptyset (&thread_stop_set);
> - sigemptyset (&thread_print_set);
> -
> - for (i = 1; i < NSIG; i++)
> + for (i = 0; i < lin_thread_get_thread_signal_num (); i++)
> {
> - if (sigismember (&mask, i))
> - {
> - if (signal_stop_update (gdb_signal_from_host (i), 0))
> - sigaddset (&thread_stop_set, i);
> - if (signal_print_update (gdb_signal_from_host (i), 0))
> - sigaddset (&thread_print_set, i);
> - thread_signals = 1;
> - }
> + int sig = lin_thread_get_thread_signal (i);
> + signal_stop_update (gdb_signal_from_host (sig), 0);
> + signal_print_update (gdb_signal_from_host (sig), 0);
> + thread_signals = 1;
> }
> }
> }
>