diff mbox series

[v3] nptl: Export libthread_db-used symbols under GLIBC_PRIVATE

Message ID 87zgvarwj8.fsf@oldenburg.str.redhat.com
State Committed
Commit fef400a2f976d1fd6a4639e6980f6c50ee13fbf5
Headers show
Series [v3] nptl: Export libthread_db-used symbols under GLIBC_PRIVATE | expand

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit fail Patch caused testsuite regressions

Commit Message

Florian Weimer June 28, 2021, 12:41 p.m. UTC
This allows distributions to strip debugging information from
libc.so.6 without impacting the debugging experience.

---
 nptl/Versions          | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++
 nptl/pthread_create.c  | 13 ++++++-----
 nptl_db/Makefile       |  2 +-
 nptl_db/db-symbols.awk |  6 ++++--
 4 files changed, 71 insertions(+), 8 deletions(-)

Comments

Carlos O'Donell June 28, 2021, 12:59 p.m. UTC | #1
On 6/28/21 8:41 AM, Florian Weimer wrote:
> This allows distributions to strip debugging information from
> libc.so.6 without impacting the debugging experience.

This looks good, no redundant checks. The only *MAIN* symbol
left in structs.def is from ld.so and we skip checking it
(because the test framework only processes libc.so's symbols).

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
 
> ---
>  nptl/Versions          | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  nptl/pthread_create.c  | 13 ++++++-----
>  nptl_db/Makefile       |  2 +-
>  nptl_db/db-symbols.awk |  6 ++++--
>  4 files changed, 71 insertions(+), 8 deletions(-)
> 
> diff --git a/nptl/Versions b/nptl/Versions
> index e4fae73c0b..060d8d5dec 100644
> --- a/nptl/Versions
> +++ b/nptl/Versions
> @@ -403,10 +403,14 @@ libc {
>      __nptl_deallocate_tsd;
>      __nptl_death_event;
>      __nptl_free_tcb;
> +    __nptl_last_event;
>      __nptl_nthreads;
> +    __nptl_rtld_global;
>      __nptl_setxid_sighandler;
>      __nptl_stack_list_add;
>      __nptl_stack_list_del;
> +    __nptl_threads_events;
> +    __nptl_version;
>      __pthread_attr_copy;
>      __pthread_attr_destroy;
>      __pthread_attr_init;
> @@ -430,6 +434,60 @@ libc {
>      __pthread_unwind;
>      __sched_fifo_max_prio;
>      __sched_fifo_min_prio;
> +    _thread_db___nptl_last_event;
> +    _thread_db___nptl_nthreads;
> +    _thread_db___nptl_rtld_global;
> +    _thread_db___pthread_keys;
> +    _thread_db_const_thread_area;
> +    _thread_db_dtv_dtv;
> +    _thread_db_dtv_slotinfo_gen;
> +    _thread_db_dtv_slotinfo_list_len;
> +    _thread_db_dtv_slotinfo_list_next;
> +    _thread_db_dtv_slotinfo_list_slotinfo;
> +    _thread_db_dtv_slotinfo_map;
> +    _thread_db_dtv_t_counter;
> +    _thread_db_dtv_t_pointer_val;
> +    _thread_db_link_map_l_tls_modid;
> +    _thread_db_link_map_l_tls_offset;
> +    _thread_db_list_t_next;
> +    _thread_db_list_t_prev;
> +    _thread_db_pthread_cancelhandling;
> +    _thread_db_pthread_dtvp;
> +    _thread_db_pthread_eventbuf;
> +    _thread_db_pthread_eventbuf_eventmask;
> +    _thread_db_pthread_eventbuf_eventmask_event_bits;
> +    _thread_db_pthread_key_data_data;
> +    _thread_db_pthread_key_data_level2_data;
> +    _thread_db_pthread_key_data_seq;
> +    _thread_db_pthread_key_struct_destr;
> +    _thread_db_pthread_key_struct_seq;
> +    _thread_db_pthread_list;
> +    _thread_db_pthread_nextevent;
> +    _thread_db_pthread_report_events;
> +    _thread_db_pthread_schedparam_sched_priority;
> +    _thread_db_pthread_schedpolicy;
> +    _thread_db_pthread_specific;
> +    _thread_db_pthread_start_routine;
> +    _thread_db_pthread_tid;
> +    _thread_db_register32;
> +    _thread_db_register32_thread_area;
> +    _thread_db_register64;
> +    _thread_db_register64_thread_area;
> +    _thread_db_rtld_global__dl_stack_used;
> +    _thread_db_rtld_global__dl_stack_user;
> +    _thread_db_rtld_global__dl_tls_dtv_slotinfo_list;
> +    _thread_db_sizeof_dtv_slotinfo;
> +    _thread_db_sizeof_dtv_slotinfo_list;
> +    _thread_db_sizeof_list_t;
> +    _thread_db_sizeof_pthread;
> +    _thread_db_sizeof_pthread_key_data;
> +    _thread_db_sizeof_pthread_key_data_level2;
> +    _thread_db_sizeof_pthread_key_struct;
> +    _thread_db_sizeof_td_eventbuf_t;
> +    _thread_db_sizeof_td_thr_events_t;
> +    _thread_db_td_eventbuf_t_eventdata;
> +    _thread_db_td_eventbuf_t_eventnum;
> +    _thread_db_td_thr_events_t_event_bits;
>    }
>  }
>  
> diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
> index 3f017f1e26..d1b6817a81 100644
> --- a/nptl/pthread_create.c
> +++ b/nptl/pthread_create.c
> @@ -43,21 +43,24 @@
>  
>  
>  /* Globally enabled events.  */
> -static td_thr_events_t __nptl_threads_events __attribute_used__;
> +td_thr_events_t __nptl_threads_events __attribute__ ((nocommon));
> +libc_hidden_proto (__nptl_threads_events)
> +libc_hidden_data_def (__nptl_threads_events)
>  
>  /* Pointer to descriptor with the last event.  */
> -static struct pthread *__nptl_last_event __attribute_used__;
> +struct pthread *__nptl_last_event __attribute__ ((nocommon));
> +libc_hidden_proto (__nptl_last_event)
> +libc_hidden_data_def (__nptl_last_event)
>  
>  #ifdef SHARED
>  /* This variable is used to access _rtld_global from libthread_db.  If
>     GDB loads libpthread before ld.so, it is not possible to resolve
>     _rtld_global directly during libpthread initialization.  */
> -static struct rtld_global *__nptl_rtld_global __attribute_used__
> -  = &_rtld_global;
> +struct rtld_global *__nptl_rtld_global = &_rtld_global;
>  #endif
>  
>  /* Version of the library, used in libthread_db to detect mismatches.  */
> -static const char nptl_version[] __attribute_used__ = VERSION;
> +const char __nptl_version[] = VERSION;
>  
>  /* This performs the initialization necessary when going from
>     single-threaded to multi-threaded mode for the first time.  */
> diff --git a/nptl_db/Makefile b/nptl_db/Makefile
> index 1f79c018a1..c04aa6140a 100644
> --- a/nptl_db/Makefile
> +++ b/nptl_db/Makefile
> @@ -55,7 +55,7 @@ include ../Rules
>  
>  $(objpfx)db-symbols.out: $(objpfx)db-symbols.v.i \
>  			 $(common-objpfx)libc.so
> -	LC_ALL=C $(READELF) -W -s $(filter %.so,$^) | $(AWK) -f $< > $@; \
> +	LC_ALL=C $(READELF) -W -D -s $(filter %.so,$^) | $(AWK) -f $< > $@; \
>  	$(evaluate-test)
>  
>  $(objpfx)db-symbols.v.i: db-symbols.awk
> diff --git a/nptl_db/db-symbols.awk b/nptl_db/db-symbols.awk
> index ef1d91b167..fe90d3b435 100644
> --- a/nptl_db/db-symbols.awk
> +++ b/nptl_db/db-symbols.awk
> @@ -1,4 +1,4 @@
> -# This script processes the output of 'readelf -W -s' on the libpthread.so
> +# This script processes the output of 'readelf -W -D -s' on the libc.so
>  # we've just built.  It checks for all the symbols used in td_symbol_list.
>  
>  BEGIN {
> @@ -12,7 +12,7 @@ BEGIN {
>     in_symtab = 0;
>  }
>  
> -/Symbol table '.symtab'/ { in_symtab=1; next }
> +/Symbol table for image/ { in_symtab=1; next }
>  NF == 0 { in_symtab=0; next }
>  
>  !in_symtab { next }
> @@ -23,6 +23,7 @@ END {
>    status = 0;
>  
>    for (s in required) {
> +    s = s "@@GLIBC_PRIVATE"
>      if (s in seen) print s, "ok";
>      else {
>        status = 1;
> @@ -32,6 +33,7 @@ END {
>  
>    any = "";
>    for (s in th_unique) {
> +    s = s "@@GLIBC_PRIVATE"
>      if (s in seen) {
>        any = s;
>        break;
>
Szabolcs Nagy June 29, 2021, 8:21 a.m. UTC | #2
The 06/28/2021 08:59, Carlos O'Donell via Libc-alpha wrote:
> On 6/28/21 8:41 AM, Florian Weimer wrote:
> > This allows distributions to strip debugging information from
> > libc.so.6 without impacting the debugging experience.
> 
> This looks good, no redundant checks. The only *MAIN* symbol
> left in structs.def is from ld.so and we skip checking it
> (because the test framework only processes libc.so's symbols).
> 
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>

after the previous two nptl_db commits i see

FAIL: nptl_db/db-symbols

i thought this third patch would fix it, but it didn't:

$ head nptl_db/db-symbols.out
_thread_db_pthread_eventbuf_eventmask_event_bits@@GLIBC_PRIVATE ***MISSING***
_thread_db_pthread_start_routine@@GLIBC_PRIVATE ***MISSING***
_thread_db_sizeof_list_t@@GLIBC_PRIVATE ***MISSING***
_thread_db_pthread_schedparam_sched_priority@@GLIBC_PRIVATE ***MISSING***
_thread_db_td_eventbuf_t_eventdata@@GLIBC_PRIVATE ***MISSING***
_thread_db_list_t_prev@@GLIBC_PRIVATE ***MISSING***
_thread_db_sizeof_dtv_slotinfo@@GLIBC_PRIVATE ***MISSING***
_thread_db_pthread_cancelhandling@@GLIBC_PRIVATE ***MISSING***
_thread_db___pthread_keys@@GLIBC_PRIVATE ***MISSING***
_thread_db_rtld_global__dl_tls_dtv_slotinfo_list@@GLIBC_PRIVATE ***MISSING***

is this expected?
Florian Weimer June 29, 2021, 9:09 a.m. UTC | #3
* Szabolcs Nagy:

> The 06/28/2021 08:59, Carlos O'Donell via Libc-alpha wrote:
>> On 6/28/21 8:41 AM, Florian Weimer wrote:
>> > This allows distributions to strip debugging information from
>> > libc.so.6 without impacting the debugging experience.
>> 
>> This looks good, no redundant checks. The only *MAIN* symbol
>> left in structs.def is from ld.so and we skip checking it
>> (because the test framework only processes libc.so's symbols).
>> 
>> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
> after the previous two nptl_db commits i see
>
> FAIL: nptl_db/db-symbols
>
> i thought this third patch would fix it, but it didn't:
>
> $ head nptl_db/db-symbols.out
> _thread_db_pthread_eventbuf_eventmask_event_bits@@GLIBC_PRIVATE ***MISSING***
> _thread_db_pthread_start_routine@@GLIBC_PRIVATE ***MISSING***
> _thread_db_sizeof_list_t@@GLIBC_PRIVATE ***MISSING***
> _thread_db_pthread_schedparam_sched_priority@@GLIBC_PRIVATE ***MISSING***
> _thread_db_td_eventbuf_t_eventdata@@GLIBC_PRIVATE ***MISSING***
> _thread_db_list_t_prev@@GLIBC_PRIVATE ***MISSING***
> _thread_db_sizeof_dtv_slotinfo@@GLIBC_PRIVATE ***MISSING***
> _thread_db_pthread_cancelhandling@@GLIBC_PRIVATE ***MISSING***
> _thread_db___pthread_keys@@GLIBC_PRIVATE ***MISSING***
> _thread_db_rtld_global__dl_tls_dtv_slotinfo_list@@GLIBC_PRIVATE ***MISSING***
>
> is this expected?

It is not expected.  The test runs even when cross-compiling, and I
don't see this failure in a default build-many-glibcs.py run.

Are these symbols present as dynamic symbols in your build of libc.so.6?

I suspect your readelf has different output not expected by the script.
Apparently older versions do not print symbol versioning information
with -D -s. 8-( I guess we should parse ELF directly, rather than
readelf output, because it is easier to maintain.

As a stop-gap measure, we should probably switch to objdump -T.

Thanks,
Florian
Szabolcs Nagy June 29, 2021, 9:25 a.m. UTC | #4
The 06/29/2021 11:09, Florian Weimer via Libc-alpha wrote:
> * Szabolcs Nagy:
> 
> > The 06/28/2021 08:59, Carlos O'Donell via Libc-alpha wrote:
> >> On 6/28/21 8:41 AM, Florian Weimer wrote:
> >> > This allows distributions to strip debugging information from
> >> > libc.so.6 without impacting the debugging experience.
> >> 
> >> This looks good, no redundant checks. The only *MAIN* symbol
> >> left in structs.def is from ld.so and we skip checking it
> >> (because the test framework only processes libc.so's symbols).
> >> 
> >> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> >
> > after the previous two nptl_db commits i see
> >
> > FAIL: nptl_db/db-symbols
> >
> > i thought this third patch would fix it, but it didn't:
> >
> > $ head nptl_db/db-symbols.out
> > _thread_db_pthread_eventbuf_eventmask_event_bits@@GLIBC_PRIVATE ***MISSING***
> > _thread_db_pthread_start_routine@@GLIBC_PRIVATE ***MISSING***
> > _thread_db_sizeof_list_t@@GLIBC_PRIVATE ***MISSING***
> > _thread_db_pthread_schedparam_sched_priority@@GLIBC_PRIVATE ***MISSING***
> > _thread_db_td_eventbuf_t_eventdata@@GLIBC_PRIVATE ***MISSING***
> > _thread_db_list_t_prev@@GLIBC_PRIVATE ***MISSING***
> > _thread_db_sizeof_dtv_slotinfo@@GLIBC_PRIVATE ***MISSING***
> > _thread_db_pthread_cancelhandling@@GLIBC_PRIVATE ***MISSING***
> > _thread_db___pthread_keys@@GLIBC_PRIVATE ***MISSING***
> > _thread_db_rtld_global__dl_tls_dtv_slotinfo_list@@GLIBC_PRIVATE ***MISSING***
> >
> > is this expected?
> 
> It is not expected.  The test runs even when cross-compiling, and I
> don't see this failure in a default build-many-glibcs.py run.
> 
> Are these symbols present as dynamic symbols in your build of libc.so.6?
> 
> I suspect your readelf has different output not expected by the script.
> Apparently older versions do not print symbol versioning information
> with -D -s. 8-( I guess we should parse ELF directly, rather than
> readelf output, because it is easier to maintain.
> 
> As a stop-gap measure, we should probably switch to objdump -T.

yes i can confirm that my readelf does not print symbol versions
with -D -s.

GNU readelf (GNU Binutils for Ubuntu) 2.31.1

i can try to update it if it's considered too old.
Florian Weimer June 29, 2021, 9:37 a.m. UTC | #5
* Szabolcs Nagy:

> yes i can confirm that my readelf does not print symbol versions
> with -D -s.
>
> GNU readelf (GNU Binutils for Ubuntu) 2.31.1
>
> i can try to update it if it's considered too old.

It's not too old.  I'm going to post a patch that re-uses the abilist
parser, which seems to be really stable.

Thanks,
Florian
diff mbox series

Patch

diff --git a/nptl/Versions b/nptl/Versions
index e4fae73c0b..060d8d5dec 100644
--- a/nptl/Versions
+++ b/nptl/Versions
@@ -403,10 +403,14 @@  libc {
     __nptl_deallocate_tsd;
     __nptl_death_event;
     __nptl_free_tcb;
+    __nptl_last_event;
     __nptl_nthreads;
+    __nptl_rtld_global;
     __nptl_setxid_sighandler;
     __nptl_stack_list_add;
     __nptl_stack_list_del;
+    __nptl_threads_events;
+    __nptl_version;
     __pthread_attr_copy;
     __pthread_attr_destroy;
     __pthread_attr_init;
@@ -430,6 +434,60 @@  libc {
     __pthread_unwind;
     __sched_fifo_max_prio;
     __sched_fifo_min_prio;
+    _thread_db___nptl_last_event;
+    _thread_db___nptl_nthreads;
+    _thread_db___nptl_rtld_global;
+    _thread_db___pthread_keys;
+    _thread_db_const_thread_area;
+    _thread_db_dtv_dtv;
+    _thread_db_dtv_slotinfo_gen;
+    _thread_db_dtv_slotinfo_list_len;
+    _thread_db_dtv_slotinfo_list_next;
+    _thread_db_dtv_slotinfo_list_slotinfo;
+    _thread_db_dtv_slotinfo_map;
+    _thread_db_dtv_t_counter;
+    _thread_db_dtv_t_pointer_val;
+    _thread_db_link_map_l_tls_modid;
+    _thread_db_link_map_l_tls_offset;
+    _thread_db_list_t_next;
+    _thread_db_list_t_prev;
+    _thread_db_pthread_cancelhandling;
+    _thread_db_pthread_dtvp;
+    _thread_db_pthread_eventbuf;
+    _thread_db_pthread_eventbuf_eventmask;
+    _thread_db_pthread_eventbuf_eventmask_event_bits;
+    _thread_db_pthread_key_data_data;
+    _thread_db_pthread_key_data_level2_data;
+    _thread_db_pthread_key_data_seq;
+    _thread_db_pthread_key_struct_destr;
+    _thread_db_pthread_key_struct_seq;
+    _thread_db_pthread_list;
+    _thread_db_pthread_nextevent;
+    _thread_db_pthread_report_events;
+    _thread_db_pthread_schedparam_sched_priority;
+    _thread_db_pthread_schedpolicy;
+    _thread_db_pthread_specific;
+    _thread_db_pthread_start_routine;
+    _thread_db_pthread_tid;
+    _thread_db_register32;
+    _thread_db_register32_thread_area;
+    _thread_db_register64;
+    _thread_db_register64_thread_area;
+    _thread_db_rtld_global__dl_stack_used;
+    _thread_db_rtld_global__dl_stack_user;
+    _thread_db_rtld_global__dl_tls_dtv_slotinfo_list;
+    _thread_db_sizeof_dtv_slotinfo;
+    _thread_db_sizeof_dtv_slotinfo_list;
+    _thread_db_sizeof_list_t;
+    _thread_db_sizeof_pthread;
+    _thread_db_sizeof_pthread_key_data;
+    _thread_db_sizeof_pthread_key_data_level2;
+    _thread_db_sizeof_pthread_key_struct;
+    _thread_db_sizeof_td_eventbuf_t;
+    _thread_db_sizeof_td_thr_events_t;
+    _thread_db_td_eventbuf_t_eventdata;
+    _thread_db_td_eventbuf_t_eventnum;
+    _thread_db_td_thr_events_t_event_bits;
   }
 }
 
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 3f017f1e26..d1b6817a81 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -43,21 +43,24 @@ 
 
 
 /* Globally enabled events.  */
-static td_thr_events_t __nptl_threads_events __attribute_used__;
+td_thr_events_t __nptl_threads_events __attribute__ ((nocommon));
+libc_hidden_proto (__nptl_threads_events)
+libc_hidden_data_def (__nptl_threads_events)
 
 /* Pointer to descriptor with the last event.  */
-static struct pthread *__nptl_last_event __attribute_used__;
+struct pthread *__nptl_last_event __attribute__ ((nocommon));
+libc_hidden_proto (__nptl_last_event)
+libc_hidden_data_def (__nptl_last_event)
 
 #ifdef SHARED
 /* This variable is used to access _rtld_global from libthread_db.  If
    GDB loads libpthread before ld.so, it is not possible to resolve
    _rtld_global directly during libpthread initialization.  */
-static struct rtld_global *__nptl_rtld_global __attribute_used__
-  = &_rtld_global;
+struct rtld_global *__nptl_rtld_global = &_rtld_global;
 #endif
 
 /* Version of the library, used in libthread_db to detect mismatches.  */
-static const char nptl_version[] __attribute_used__ = VERSION;
+const char __nptl_version[] = VERSION;
 
 /* This performs the initialization necessary when going from
    single-threaded to multi-threaded mode for the first time.  */
diff --git a/nptl_db/Makefile b/nptl_db/Makefile
index 1f79c018a1..c04aa6140a 100644
--- a/nptl_db/Makefile
+++ b/nptl_db/Makefile
@@ -55,7 +55,7 @@  include ../Rules
 
 $(objpfx)db-symbols.out: $(objpfx)db-symbols.v.i \
 			 $(common-objpfx)libc.so
-	LC_ALL=C $(READELF) -W -s $(filter %.so,$^) | $(AWK) -f $< > $@; \
+	LC_ALL=C $(READELF) -W -D -s $(filter %.so,$^) | $(AWK) -f $< > $@; \
 	$(evaluate-test)
 
 $(objpfx)db-symbols.v.i: db-symbols.awk
diff --git a/nptl_db/db-symbols.awk b/nptl_db/db-symbols.awk
index ef1d91b167..fe90d3b435 100644
--- a/nptl_db/db-symbols.awk
+++ b/nptl_db/db-symbols.awk
@@ -1,4 +1,4 @@ 
-# This script processes the output of 'readelf -W -s' on the libpthread.so
+# This script processes the output of 'readelf -W -D -s' on the libc.so
 # we've just built.  It checks for all the symbols used in td_symbol_list.
 
 BEGIN {
@@ -12,7 +12,7 @@  BEGIN {
    in_symtab = 0;
 }
 
-/Symbol table '.symtab'/ { in_symtab=1; next }
+/Symbol table for image/ { in_symtab=1; next }
 NF == 0 { in_symtab=0; next }
 
 !in_symtab { next }
@@ -23,6 +23,7 @@  END {
   status = 0;
 
   for (s in required) {
+    s = s "@@GLIBC_PRIVATE"
     if (s in seen) print s, "ok";
     else {
       status = 1;
@@ -32,6 +33,7 @@  END {
 
   any = "";
   for (s in th_unique) {
+    s = s "@@GLIBC_PRIVATE"
     if (s in seen) {
       any = s;
       break;