public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH][gdb/threads] Fix lin_thread_get_thread_signals for glibc 2.28
@ 2021-02-05 11:15 Tom de Vries
  2021-02-05 11:54 ` Luis Machado
  0 siblings, 1 reply; 3+ messages in thread
From: Tom de Vries @ 2021-02-05 11:15 UTC (permalink / raw)
  To: gdb-patches

Hi,

When running test-case gdb.threads/create-fail.exp on openSUSE Factory
(with glibc version 2.32) I run into:
...
(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.

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;
 	}
     }
 }

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][gdb/threads] Fix lin_thread_get_thread_signals for glibc 2.28
  2021-02-05 11:15 [PATCH][gdb/threads] Fix lin_thread_get_thread_signals for glibc 2.28 Tom de Vries
@ 2021-02-05 11:54 ` Luis Machado
  2021-02-12 19:11   ` Tom de Vries
  0 siblings, 1 reply; 3+ messages in thread
From: Luis Machado @ 2021-02-05 11:54 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches

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;
>   	}
>       }
>   }
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][gdb/threads] Fix lin_thread_get_thread_signals for glibc 2.28
  2021-02-05 11:54 ` Luis Machado
@ 2021-02-12 19:11   ` Tom de Vries
  0 siblings, 0 replies; 3+ messages in thread
From: Tom de Vries @ 2021-02-12 19:11 UTC (permalink / raw)
  To: Luis Machado, gdb-patches

On 2/5/21 12:54 PM, Luis Machado wrote:
> 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.
> 

Thanks.  Added mention of that, and committed.

- Tom

>>
>> 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;
>>       }
>>       }
>>   }
>>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-02-12 19:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-05 11:15 [PATCH][gdb/threads] Fix lin_thread_get_thread_signals for glibc 2.28 Tom de Vries
2021-02-05 11:54 ` Luis Machado
2021-02-12 19:11   ` Tom de Vries

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).