Re: [PATCH][BZ #16214] Fix TLS access on S390 with -march=z10

[Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>]

Hi Siddhesh,

On 28/11/13 05:35, Siddhesh Poyarekar wrote:
> On Wed, Nov 27, 2013 at 10:01:46PM +0100, Andreas Krebbel wrote:
> Where is __tls_get_addr exported for GLIBC_2.3?  Or are we supposed to
> avoid exporting __tls_get_addr if we're exporting __tls_get_offset?

__tls_get_addr is not exported on System z and the patch tries to keep it that way.  That's why I
had to use that awkward wrapper:

>>  #ifdef SHARED
>> -/* This is the prototype for the GNU version.  */
>> -extern void *__tls_get_addr (tls_index *ti) attribute_hidden;
>> -extern unsigned long __tls_get_offset (unsigned long got_offset);
>>  
>>  # ifdef IS_IN_rtld
>> +
>> +#  include <shlib-compat.h>
>> +
>> +extern void *__tls_get_addr (tls_index *ti) attribute_hidden;
>> +
>> +void *
>> +__tls_get_addr_internal (tls_index *ti)
>> +{
>> +  return __tls_get_addr (ti);
>> +}
>> +versioned_symbol (ld, __tls_get_addr_internal, ___tls_get_addr, GLIBC_PRIVATE);
>> +
> 
> I think the canonical way to avoid PLT dereference within ld.so or
> libc.so is to have the function with the exported name and have an
> rtld_hidden_def?  So in this case the function should be
> ___tls_get_addr with a rtld_hidden_def() directive.  

The sequence above was the only way I found to export a tls_get_addr version privately without
exporting the original version but I'm eager to hear about better ways. I've failed with the following:

- Put a Versions script somewhere in the path which replaces GLIBC_2.3 on __tls_get_addr with
GLIBC_PRIVATE. No matter where I put it it appears after the original version definition and is
ignored that way.

- Adding a new symbol for __tls_get_addr and putting the GLIBC_PRIVATE version on it.  The new
symbol can only be exported when removing the hidden attribute from the original function but this
led to the original function to be exported as __tls_get_addr@@GLIBC_2.3 what I'm trying to prevent.

> Also (bikeshed) I
> believe __tls_get_addr_internal would be a nicer name instead of the
> extra underscore.

I've seen that x86 exports a _ _ _tls_get_addr and copied it. But I agree it is not ideal since it
is hard to distingiush from the normal version. I'll swap the name of the wrapper with the _ _ _ name.

> I am ambivalent about the use of r12 or another register for GOT for
> these test macros.  While the r12 usage emulates the ABI exactly,
> loading GOT in another register also does not break semantics of the
> test itself.  Maybe debugging any failures in this test may confuse a
> programmer looking at this for the first time since he would be
> looking for a specific sequence of instructions for TLS access.  But
> of course, it would mean that he didn't look at the code carefully
> enough ;)

The ABI requires the GOT pointer to be set up in r12 in some situations but it does not prevent it
from being loaded into some other GPR.  So the above would be a perfectly valid compiler
optimization and in fact for the compiler it makes sense in that case to replace the call-saved r12
with a call-clobbered GPR to get rid of save/restore ops.  So if the compiler isn't already doing
this it might do in the future and then the code matches what it would generate more closely.

Bye,

-Andreas-