[PATCH] x86-64: Align the stack in __tls_get_addr [BZ #21609]

[H.J. Lu]

On Mon, Jul 3, 2017 at 9:47 PM, Florian Weimer <fweimer@redhat.com> wrote:
> On 07/03/2017 10:25 PM, H.J. Lu wrote:
>> I prefer to let GCC realign the stack for us.  What do you think?
>
> I tried that first, but it does not work with all the GCC versions we

That is true.

> support.  We would have to add a configure check and hope that older GCC
> versions never generate code which needs an aligned stack.
>
> 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.

> and __tls_get_addr appears to be quite performance-critical because not
> all callers cache the result.
>

Some comments on the patch at

https://sourceware.org/ml/libc-alpha/2017-06/msg00922.html

# 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?

-#include <stdint.h>
-
 #ifndef _X86_64_DL_TLSDESC_H
-# define _X86_64_DL_TLSDESC_H 1
+#define _X86_64_DL_TLSDESC_H
+
+#include <stdint.h>
+#include <dl-tls.h>

 /* Type used to represent a TLS descriptor in the GOT.  */
 struct tlsdesc
@@ -39,12 +40,6 @@ struct tlsdesc
     };
 };

-typedef struct dl_tls_index
-{
-  uint64_t ti_module;
-  uint64_t ti_offset;
-} tls_index;
-
 /* Type used as the argument in a TLS descriptor for a symbol that
    needs dynamic TLS offsets.  */
 struct tlsdesc_dynamic_arg
@@ -59,12 +54,12 @@ extern ptrdiff_t attribute_hidden
   _dl_tlsdesc_resolve_rela(struct tlsdesc *on_rax),
   _dl_tlsdesc_resolve_hold(struct tlsdesc *on_rax);

-# ifdef SHARED
+#ifdef SHARED
 extern void *_dl_make_tlsdesc_dynamic (struct link_map *map,
        size_t ti_offset)
   internal_function attribute_hidden;

 extern ptrdiff_t attribute_hidden _dl_tlsdesc_dynamic(struct tlsdesc *);
-# endif
-
 #endif
+
+#endif /* _X86_64_DL_TLSDESC_H */

Is this sysdeps/x86_64/dl-tlsdesc.h change related to this?

__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?

Since we are talking performance here, we should add __tls_get_addr_slow
to only handle slow paths.

Here is the patch which implements those.  It is tested on x86-64
and x32.

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-x86-64-Align-the-stack-in-__tls_get_addr-BZ-21609.patch
Type: text/x-patch
Size: 7915 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20170704/c03a6e4f/attachment.bin>