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]
- From: Florian Weimer <fweimer at redhat dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: GNU C Library <libc-alpha at sourceware dot org>, Nicholas Miell <nmiell at gmail dot com>, Jakub Jelinek <jakub at redhat dot com>
- Date: Tue, 4 Jul 2017 17:47:06 +0200
- Subject: Re: [PATCH] x86-64: Align the stack in __tls_get_addr [BZ #21609]
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=fweimer at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 879F68553F
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 879F68553F
- References: <f5ac0827-0c3a-cc1d-eb14-6efb905bd53d@redhat.com> <14a88e5f-a8c6-cd78-a363-1368d175f605@redhat.com> <20170703202551.GA17398@gmail.com> <3e579057-ebc3-a407-40c2-3956a017ff4e@redhat.com> <CAMe9rOp15N_5xrbx4uCdKY7v9iMiN7gQ0ZBOvaoEJOO2ZLM+TA@mail.gmail.com>
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.
>> # 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.
> __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.
> 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.
> 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?
Thanks,
Florian