diff mbox series

[3/8] nptl: Remove always-disabled debugging support

Message ID 313425893d8419592a17bdad60ce2bf37d485b3f.1620650045.git.fweimer@redhat.com
State Committed
Delegated to: Adhemerval Zanella Netto
Headers show
Series nptl: Move pthread_detach and pthread_join into libc | expand

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Florian Weimer May 10, 2021, 12:37 p.m. UTC
This removes the DEBUGGING_P macro and the __pthread_debug variable.
The __find_in_stack_list function is now unused and deleted as well.
---
 nptl/pthreadP.h         | 26 +++++-----------------
 nptl/pthread_create.c   | 49 -----------------------------------------
 nptl/pthread_sigqueue.c |  5 -----
 3 files changed, 5 insertions(+), 75 deletions(-)

Comments

Adhemerval Zanella May 10, 2021, 1:47 p.m. UTC | #1
On 10/05/2021 09:37, Florian Weimer via Libc-alpha wrote:
> This removes the DEBUGGING_P macro and the __pthread_debug variable.
> The __find_in_stack_list function is now unused and deleted as well.

LGTM, thanks.  As a side note we might revise the INVALID_TD_P and 
INVALID_NOT_TERMINATED_TD_P in the future, I don't their usage are
entirely safe within glibc.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  nptl/pthreadP.h         | 26 +++++-----------------
>  nptl/pthread_create.c   | 49 -----------------------------------------
>  nptl/pthread_sigqueue.c |  5 -----
>  3 files changed, 5 insertions(+), 75 deletions(-)
> 
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index d9b97c814a..d9a6137bd3 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -243,23 +243,11 @@ libc_hidden_proto (__pthread_tpp_change_priority)
>  extern int __pthread_current_priority (void);
>  libc_hidden_proto (__pthread_current_priority)
>  
> -/* The library can run in debugging mode where it performs a lot more
> -   tests.  */
> -extern int __pthread_debug attribute_hidden;
> -/** For now disable debugging support.  */
> -#if 0
> -# define DEBUGGING_P __builtin_expect (__pthread_debug, 0)
> -# define INVALID_TD_P(pd) (DEBUGGING_P && __find_in_stack_list (pd) == NULL)
> -# define INVALID_NOT_TERMINATED_TD_P(pd) INVALID_TD_P (pd)
> -#else
> -# define DEBUGGING_P 0
> -/* Simplified test.  This will not catch all invalid descriptors but
> -   is better than nothing.  And if the test triggers the thread
> -   descriptor is guaranteed to be invalid.  */
> -# define INVALID_TD_P(pd) __builtin_expect ((pd)->tid <= 0, 0)
> -# define INVALID_NOT_TERMINATED_TD_P(pd) __builtin_expect ((pd)->tid < 0, 0)
> -#endif
> -
> +/* This will not catch all invalid descriptors but is better than
> +   nothing.  And if the test triggers the thread descriptor is
> +   guaranteed to be invalid.  */
> +#define INVALID_TD_P(pd) __builtin_expect ((pd)->tid <= 0, 0)
> +#define INVALID_NOT_TERMINATED_TD_P(pd) __builtin_expect ((pd)->tid < 0, 0)
>  
>  /* Cancellation test.  */
>  #define CANCELLATION_P(self) \
> @@ -322,10 +310,6 @@ __do_cancel (void)
>  
>  /* Internal prototypes.  */
>  
> -/* Thread list handling.  */
> -extern struct pthread *__find_in_stack_list (struct pthread *pd)
> -     attribute_hidden;
> -
>  /* Deallocate a thread's stack after optionally making sure the thread
>     descriptor is still valid.  */
>  extern void __free_tcb (struct pthread *pd) attribute_hidden;

Ok.

> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index 775287d0e4..d19456d48b 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -42,9 +42,6 @@
>  #include <stap-probe.h>
>  
>  
> -/* Nozero if debugging mode is enabled.  */
> -int __pthread_debug;
> -
>  /* Globally enabled events.  */
>  static td_thr_events_t __nptl_threads_events __attribute_used__;
>  
> @@ -210,46 +207,6 @@ static int create_thread (struct pthread *pd, const struct pthread_attr *attr,
>  
>  #include <createthread.c>
>  
> -
> -struct pthread *
> -__find_in_stack_list (struct pthread *pd)
> -{
> -  list_t *entry;
> -  struct pthread *result = NULL;
> -
> -  lll_lock (GL (dl_stack_cache_lock), LLL_PRIVATE);
> -
> -  list_for_each (entry, &GL (dl_stack_used))
> -    {
> -      struct pthread *curp;
> -
> -      curp = list_entry (entry, struct pthread, list);
> -      if (curp == pd)
> -	{
> -	  result = curp;
> -	  break;
> -	}
> -    }
> -
> -  if (result == NULL)
> -    list_for_each (entry, &GL (dl_stack_user))
> -      {
> -	struct pthread *curp;
> -
> -	curp = list_entry (entry, struct pthread, list);
> -	if (curp == pd)
> -	  {
> -	    result = curp;
> -	    break;
> -	  }
> -      }
> -
> -  lll_unlock (GL (dl_stack_cache_lock), LLL_PRIVATE);
> -
> -  return result;
> -}
> -
> -
>  /* Deallocate a thread's stack after optionally making sure the thread
>     descriptor is still valid.  */
>  void

Ok.

> @@ -259,12 +216,6 @@ __free_tcb (struct pthread *pd)
>    if (__builtin_expect (atomic_bit_test_set (&pd->cancelhandling,
>  					     TERMINATED_BIT) == 0, 1))
>      {
> -      /* Remove the descriptor from the list.  */
> -      if (DEBUGGING_P && __find_in_stack_list (pd) == NULL)
> -	/* Something is really wrong.  The descriptor for a still
> -	   running thread is gone.  */
> -	abort ();
> -
>        /* Free TPP data.  */
>        if (__glibc_unlikely (pd->tpp != NULL))
>  	{

Ok.

> diff --git a/nptl/pthread_sigqueue.c b/nptl/pthread_sigqueue.c
> index 64bacfe41b..3ffb595489 100644
> --- a/nptl/pthread_sigqueue.c
> +++ b/nptl/pthread_sigqueue.c
> @@ -31,11 +31,6 @@ pthread_sigqueue (pthread_t threadid, int signo, const union sigval value)
>  #ifdef __NR_rt_tgsigqueueinfo
>    struct pthread *pd = (struct pthread *) threadid;
>  
> -  /* Make sure the descriptor is valid.  */
> -  if (DEBUGGING_P && INVALID_TD_P (pd))
> -    /* Not a valid thread handle.  */
> -    return ESRCH;
> -
>    /* Force load of pd->tid into local variable or register.  Otherwise
>       if a thread exits between ESRCH test and tgkill, we might return
>       EINVAL, because pd->tid would be cleared by the kernel.  */
> 

Ok.
diff mbox series

Patch

diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index d9b97c814a..d9a6137bd3 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -243,23 +243,11 @@  libc_hidden_proto (__pthread_tpp_change_priority)
 extern int __pthread_current_priority (void);
 libc_hidden_proto (__pthread_current_priority)
 
-/* The library can run in debugging mode where it performs a lot more
-   tests.  */
-extern int __pthread_debug attribute_hidden;
-/** For now disable debugging support.  */
-#if 0
-# define DEBUGGING_P __builtin_expect (__pthread_debug, 0)
-# define INVALID_TD_P(pd) (DEBUGGING_P && __find_in_stack_list (pd) == NULL)
-# define INVALID_NOT_TERMINATED_TD_P(pd) INVALID_TD_P (pd)
-#else
-# define DEBUGGING_P 0
-/* Simplified test.  This will not catch all invalid descriptors but
-   is better than nothing.  And if the test triggers the thread
-   descriptor is guaranteed to be invalid.  */
-# define INVALID_TD_P(pd) __builtin_expect ((pd)->tid <= 0, 0)
-# define INVALID_NOT_TERMINATED_TD_P(pd) __builtin_expect ((pd)->tid < 0, 0)
-#endif
-
+/* This will not catch all invalid descriptors but is better than
+   nothing.  And if the test triggers the thread descriptor is
+   guaranteed to be invalid.  */
+#define INVALID_TD_P(pd) __builtin_expect ((pd)->tid <= 0, 0)
+#define INVALID_NOT_TERMINATED_TD_P(pd) __builtin_expect ((pd)->tid < 0, 0)
 
 /* Cancellation test.  */
 #define CANCELLATION_P(self) \
@@ -322,10 +310,6 @@  __do_cancel (void)
 
 /* Internal prototypes.  */
 
-/* Thread list handling.  */
-extern struct pthread *__find_in_stack_list (struct pthread *pd)
-     attribute_hidden;
-
 /* Deallocate a thread's stack after optionally making sure the thread
    descriptor is still valid.  */
 extern void __free_tcb (struct pthread *pd) attribute_hidden;
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 775287d0e4..d19456d48b 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -42,9 +42,6 @@ 
 #include <stap-probe.h>
 
 
-/* Nozero if debugging mode is enabled.  */
-int __pthread_debug;
-
 /* Globally enabled events.  */
 static td_thr_events_t __nptl_threads_events __attribute_used__;
 
@@ -210,46 +207,6 @@  static int create_thread (struct pthread *pd, const struct pthread_attr *attr,
 
 #include <createthread.c>
 
-
-struct pthread *
-__find_in_stack_list (struct pthread *pd)
-{
-  list_t *entry;
-  struct pthread *result = NULL;
-
-  lll_lock (GL (dl_stack_cache_lock), LLL_PRIVATE);
-
-  list_for_each (entry, &GL (dl_stack_used))
-    {
-      struct pthread *curp;
-
-      curp = list_entry (entry, struct pthread, list);
-      if (curp == pd)
-	{
-	  result = curp;
-	  break;
-	}
-    }
-
-  if (result == NULL)
-    list_for_each (entry, &GL (dl_stack_user))
-      {
-	struct pthread *curp;
-
-	curp = list_entry (entry, struct pthread, list);
-	if (curp == pd)
-	  {
-	    result = curp;
-	    break;
-	  }
-      }
-
-  lll_unlock (GL (dl_stack_cache_lock), LLL_PRIVATE);
-
-  return result;
-}
-
-
 /* Deallocate a thread's stack after optionally making sure the thread
    descriptor is still valid.  */
 void
@@ -259,12 +216,6 @@  __free_tcb (struct pthread *pd)
   if (__builtin_expect (atomic_bit_test_set (&pd->cancelhandling,
 					     TERMINATED_BIT) == 0, 1))
     {
-      /* Remove the descriptor from the list.  */
-      if (DEBUGGING_P && __find_in_stack_list (pd) == NULL)
-	/* Something is really wrong.  The descriptor for a still
-	   running thread is gone.  */
-	abort ();
-
       /* Free TPP data.  */
       if (__glibc_unlikely (pd->tpp != NULL))
 	{
diff --git a/nptl/pthread_sigqueue.c b/nptl/pthread_sigqueue.c
index 64bacfe41b..3ffb595489 100644
--- a/nptl/pthread_sigqueue.c
+++ b/nptl/pthread_sigqueue.c
@@ -31,11 +31,6 @@  pthread_sigqueue (pthread_t threadid, int signo, const union sigval value)
 #ifdef __NR_rt_tgsigqueueinfo
   struct pthread *pd = (struct pthread *) threadid;
 
-  /* Make sure the descriptor is valid.  */
-  if (DEBUGGING_P && INVALID_TD_P (pd))
-    /* Not a valid thread handle.  */
-    return ESRCH;
-
   /* Force load of pd->tid into local variable or register.  Otherwise
      if a thread exits between ESRCH test and tgkill, we might return
      EINVAL, because pd->tid would be cleared by the kernel.  */