Re: [PATCH] nptl: Move pthread_atfork to libc_nonshared.a

[Carlos O'Donell <carlos at redhat dot com>]

On 02/28/2018 07:16 AM, Florian Weimer wrote:
> libpthread_nonshared.a is unused after this, so remove it from the
> build.

Thanks for pushing this into upstream. This is more elegant than my
proposal in 2013: https://sourceware.org/ml/libc-alpha/2013-10/msg00065.html

> There is no ABI impact because pthread_atfork was implemented using
> __register_atfork in libc even before this change.

So just to be clear the __pthread_atfork moves to libc_nonshared.a?
Thus making it available for all applications using libc?

> pthread_atfork has to be a weak alias because pthread_* names are not
> reserved in libc.

OK.

> 2018-02-28  Florian Weimer  <fweimer@redhat.com>
> 
> 	Move pthread_atfork to libc.  Remove libpthread_nonshared.a.
> 	* nptl/Makefile (routines): Add pthread_atfork.
> 	(static-only-routines): Set to pthread_atfork.
> 	(libpthread-routines): Remove pthread_atfork.
> 	(libpthread-static-only-routines): Remove.
> 	(install): Update comment.
> 	(libpthread.so): Do not install libpthread_nonshared.a.
> 	(tests): Do not link with libpthread_nonshared.a.
> 	(generated): Remove libpthread_nonshared.a.
> 	* nptl/pthread_atfork.c (pthread_atfork): Turn into weak alias.
> 	* sysdeps/nptl/Makeconfig (shared-thread-library): Do not link
> 	with libpthread_nonshared.a.

This is less code than my original patch and that makes it better :-)

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> diff --git a/nptl/Makefile b/nptl/Makefile
> index de37fb4680..ba586c247e 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -30,8 +30,9 @@ install-lib-ldscripts := libpthread.so
>  
>  routines = alloca_cutoff forward libc-lowlevellock libc-cancellation \
>  	   libc-cleanup libc_pthread_init libc_multiple_threads \
> -	   register-atfork pthread_self
> +	   register-atfork pthread_atfork pthread_self

OK.

>  shared-only-routines = forward
> +static-only-routines = pthread_atfork

OK.

>  
>  # We need to provide certain routines for compatibility with existing
>  # binaries.
> @@ -106,7 +107,7 @@ libpthread-routines = nptl-init vars events version pt-interp \
>  		      pthread_cancel pthread_testcancel \
>  		      pthread_setcancelstate pthread_setcanceltype \
>  		      pthread_once \
> -		      old_pthread_atfork pthread_atfork \
> +		      old_pthread_atfork \

OK.

>  		      pthread_getcpuclockid \
>  		      pthread_clock_gettime pthread_clock_settime \
>  		      shm-directory \
> @@ -147,7 +148,6 @@ libpthread-routines = nptl-init vars events version pt-interp \
>  
>  libpthread-shared-only-routines = version pt-interp pt-allocrtsig \
>  				  unwind-forcedunwind
> -libpthread-static-only-routines = pthread_atfork

OK.

>  
>  # Since cancellation handling is in large parts handled using exceptions
>  # we have to compile some files with exception handling enabled, some
> @@ -476,16 +476,12 @@ lib-noranlib: $(addprefix $(objpfx),$(extra-objs))
>  
>  # What we install as libpthread.so for programs to link against is in fact a
>  # link script.  It contains references for the various libraries we need.
> -# The libpthread.so object is not complete since some functions are only
> -# defined in libpthread_nonshared.a.
>  # We need to use absolute paths since otherwise local copies (if they exist)
>  # of the files are taken by the linker.
>  install: $(inst_libdir)/libpthread.so
>  
>  $(inst_libdir)/libpthread.so: $(common-objpfx)format.lds \
>  			      $(objpfx)libpthread.so$(libpthread.so-version) \
> -			      $(inst_libdir)/$(patsubst %,$(libtype.oS),\
> -							$(libprefix)pthread) \

OK.

>  			      $(+force)
>  	(echo '/* GNU ld script';\
>  	 echo '   Use the shared library, but some functions are only in';\
> @@ -644,14 +640,12 @@ $(addprefix $(objpfx), \
>    $(filter-out $(tests-static) $(xtests-static) $(tests-reverse) \
>      $(tests-nolibpthread), \
>      $(tests) $(tests-internal) $(xtests) $(test-srcs))): \
> -	$(objpfx)libpthread.so \
> -	$(objpfx)libpthread_nonshared.a
> +	$(objpfx)libpthread.so

OK.

>  $(objpfx)tst-unload: $(libdl)
>  # $(objpfx)../libc.so is used instead of $(common-objpfx)libc.so,
>  # since otherwise libpthread.so comes before libc.so when linking.
>  $(addprefix $(objpfx), $(tests-reverse)): \
> -  $(objpfx)../libc.so $(objpfx)libpthread.so \
> -  $(objpfx)libpthread_nonshared.a
> +  $(objpfx)../libc.so $(objpfx)libpthread.so

OK.

>  $(objpfx)../libc.so: $(common-objpfx)libc.so ;
>  $(addprefix $(objpfx),$(tests-static) $(xtests-static)): $(objpfx)libpthread.a
>  
> @@ -681,8 +675,7 @@ $(objpfx)$(multidir)/crtn.o: $(objpfx)crtn.o $(objpfx)$(multidir)/
>  	ln -f $< $@
>  endif
>  
> -generated += libpthread_nonshared.a \
> -	     multidir.mk tst-atfork2.mtrace tst-cancel-wrappers.out \
> +generated += multidir.mk tst-atfork2.mtrace tst-cancel-wrappers.out \

OK.

>  	     tst-tls6.out
>  
>  generated += $(objpfx)tst-atfork2.mtrace \
> diff --git a/nptl/pthread_atfork.c b/nptl/pthread_atfork.c
> index 1c4ba23b8a..5ac62bf879 100644
> --- a/nptl/pthread_atfork.c
> +++ b/nptl/pthread_atfork.c
> @@ -53,5 +53,5 @@ __pthread_atfork (void (*prepare) (void), void (*parent) (void),
>  #ifndef __pthread_atfork
>  extern int pthread_atfork (void (*prepare) (void), void (*parent) (void),
>  			   void (*child) (void)) attribute_hidden;
> -strong_alias (__pthread_atfork, pthread_atfork)
> +weak_alias (__pthread_atfork, pthread_atfork)

OK.

>  #endif
> diff --git a/sysdeps/nptl/Makeconfig b/sysdeps/nptl/Makeconfig
> index 5595f356b1..ce8998bbf5 100644
> --- a/sysdeps/nptl/Makeconfig
> +++ b/sysdeps/nptl/Makeconfig
> @@ -21,8 +21,7 @@
>  
>  have-thread-library = yes
>  
> -shared-thread-library = $(common-objpfx)nptl/libpthread_nonshared.a \
> -			$(common-objpfx)nptl/libpthread.so
> +shared-thread-library = $(common-objpfx)nptl/libpthread.so

OK.

>  static-thread-library = $(common-objpfx)nptl/libpthread.a
>  
>  rpath-dirs += nptl
> 


-- 
Cheers,
Carlos.