[3/8] nptl: Remove always-disabled debugging support
Checks
Commit Message
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
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.
@@ -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;
@@ -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))
{
@@ -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. */