Re: [PATCH] tst-tlsopt-powerpc as a shared lib

[Alan Modra <amodra at gmail dot com>]

[cc trimmed]
On Mon, Jul 31, 2017 at 01:40:48PM -0300, Tulio Magno Quites Machado Filho wrote:
> Alan Modra <amodra@gmail.com> writes:
> 
> > On Fri, Jul 28, 2017 at 01:07:44PM +0000, Adam Conrad wrote:
> >> On Fri, Jul 28, 2017 at 06:32:46PM +0930, Alan Modra wrote:
> >> > 
> >> > Since tst-tlsopt-powerpc is supposed to test glibc dynamic relocation
> >> > processing, the body of the test ought to be put in a shared library
> >> > (*).  I cobbled together such a test, and TRY_STATIC_TLS works fine.
> >> > So, not a powerpc64 glibc bug.
> >> 
> >> Excellent.  Should I expect said cobbled test to replace tst-tlsopt-powerpc
> >> in glibc trunk soonish (obviously, I'll just XFAIL it for now locally).
> >
> > This makes the __tls_get_addr_opt test run as a shared library, and so
> > actually test that DTPMOD64/DTPREL64 pairs are processed by ld.so to
> > support the __tls_get_adfr_opt call stub fast return.  After a
> > 2017-01-24 patch (binutils f0158f4416) ld.bfd no longer emitted
> > unnecessary dynamic relocations against local thread variables,
> > instead setting up the __tls_index GOT entries for the call stub fast
> > return.  This meant tst-tlsopt-powerpc passed but did not check ld.so
> > relocation support.  After a 2017-07-16 patch (binutils 676ee2b5fa)
> > ld.bfd no longer set up the __tls_index GOT entries for the call stub
> > fast return, and tst-tlsopt-powerpc failed.
> >
> > Compiling mod-tlsopt-powerpc.c with -DSHARED exposed a bug in
> > powerpc64/tls-macros.h, which defines a __TLS_GET_ADDR macro that
> > clashes with one defined in dl-tls.h.  The tls-macros.h version is
> > only used in that file, so delete it and expand.
> >
> > Regression tested powerpc64le-linux.  Please verify the Makefile
> > changes.  The test passes with "make -j64 check", but I may well have
> > missed something there.
> 
> Tested on powerpc-linux and powerpc64-linux too.

Thanks!  I'll commit this after the 2.26 freeze is over.

> > 	* sysdeps/powerpc/mod-tlsopt-powerpc.c: Extract from
> > 	tst-tlsopt-powerpc.c with function name change and no test harness.
> > 	* sysdeps/powerpc/tst-tlsopt-powerpc.c: Remove body of test.
> > 	Call tls_get_addr_opt_test.
> > 	* sysdeps/powerpc/Makefile (LDFLAGS-tst-tlsopt-powerpc): Don't define.
> > 	(modules-names): Add mod-tlsopt-powerpc.
> > 	(mod-tlsopt-powerpc.so-no-z-defs): Define.
> > 	(tst-tlsopt-powerpc): Depend on .so.
> > 	* sysdeps/powerpc/powerpc64/tls-macros.h (__TLS_GET_ADDR): Don't
> > 	define.  Expand use in TLS_GD and TLS_LD.
> 
> Even if this patch doesn't fix bug 21847 [1], I think it makes sense to refer
> it here and close the bug, which was reported to glibc.
> 
> Looks good to me.
> 
> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=21847

No, 21847 is an entirely different issue.  I've moved it from
glibc/dynamic-linker to binutils/ld and mentioned the commits on the
binutils side that have already fixed the bug.

-- 
Alan Modra
Australia Development Lab, IBM