This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] x86-64: Align the stack in __tls_get_addr [BZ #21609]
On Tue, Jul 4, 2017 at 8:47 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 07/04/2017 05:16 PM, H.J. Lu wrote:
>
>>> Furthermore, the code GCC generates for stack realignment is really bad,
>>
>> GCC generates very good code for stack realignment when
>> -maccumulate-outgoing-args is used.
>
> I wonder why that isn't enabled by the force attribute.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81312
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81313
>>> # define __tls_get_addr __tls_get_addr_default
>>> +# include <elf/dl-tls.c>
>>> +
>>> +# undef __tls_get_addr_default
>> ^^^^^^^ Shouldn't it be __tls_get_addr?
>
> Looks this way. I'd have to re-test and see if it makes a difference.
>
>>> -typedef struct dl_tls_index
>>> -{
>>> - uint64_t ti_module;
>>> - uint64_t ti_offset;
>>> -} tls_index;
>
>> Is this sysdeps/x86_64/dl-tlsdesc.h change related to this?
>
> It is because the patch includes both <dl-tls.h> and <dl-tlsdesc.h> from
> the same file, causing a multiple definition error without the removal
> of the conflicting definition.
It is a cleanup. But I don't believe it is required here.
>> __tls_get_addr_compat:
>> + .type __tls_get_addr_compat,@function
>> + .global __tls_get_addr_compat
>> + strong_alias (__tls_get_addr_compat, __tls_get_addr)
>>
>> We can use ENTRY/END here. Why do we need __tls_get_addr_compat?
>> Can we just have __tls_get_addr?
>
> That confuses the linker once we add the .symver directive.
I don't think it is a case. We just define a different __tls_get_addr for
x86-64.
>> Since we are talking performance here, we should add __tls_get_addr_slow
>> to only handle slow paths.
>
> I'd prefer something that adds a new symbol version for the non-aligning
> implementation, so that we eventually move away from the aligning one.
I thought about it. There is no easy way to do it without linker help. We
can add ___tls_get_addr, similar to i386, which will take an aligned
stack. Linker must support ___tls_get_addr. Then we can use weakref
to redirect __tls_get_addr to ___tls_get_addr if linker supports
___tls_get_addr and GCC doesn't.
>> Here is the patch which implements those. It is tested on x86-64
>> and x32.
>
> Is this really needed on x32, too? Did GCC have the same bug?
>
Yes, it is needed for x32 since the bug is only fixed on I thiuGCC 4.9 branch
and above.
--
H.J.