From patchwork Mon Jul 3 20:25:51 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 21395 Received: (qmail 115214 invoked by alias); 3 Jul 2017 20:25:59 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 115198 invoked by uid 89); 3 Jul 2017 20:25:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=compensated X-HELO: mail-pf0-f193.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=CzBTiNC3np9bmyt1SIPR1dknUq4Kqd6iW6740VFt/TU=; b=Ulg3gFv+S4tRdrQTDlijbselUptltOo+cC499p/dNa+Tenq382Y9W0MVP54+gXZTu1 5mYu7ozT7fssg23Q8cg/UzFAwZhmKqhsjyqSBpUnys1LQlGYUjcsAZjYdj6QjzFRo7Gk CdsBUP7csUtzNHAfHkdRTxT3JkhrdcYB3ciB2KpT0JJqJt9MvLuJsmhuwJ6m8UOkueCE 3WP/+rylcmS/KL3b5z2T9jWc+7b5fk8+C7/0onbJbVUxtfF5A84mWhEkQe2CCowMK8zZ lTKFkn5GuV7hw6pkUpM4zR0VhXPsZX4aUYgftjUrURJpvP+XBap66ILyAXYLi1st7yaM V8Bw== X-Gm-Message-State: AIVw113eBVUdK5uprpw7yaRtq417E0esJ5Au8g4LLVghqpYOIOTdtHEF 35jFGtsppXCIfA== X-Received: by 10.99.96.132 with SMTP id u126mr11935459pgb.121.1499113553093; Mon, 03 Jul 2017 13:25:53 -0700 (PDT) Date: Mon, 3 Jul 2017 13:25:51 -0700 From: "H.J. Lu" To: Florian Weimer Cc: libc-alpha@sourceware.org, nmiell@gmail.com, Jakub Jelinek Subject: Re: [PATCH] x86-64: Align the stack in __tls_get_addr [BZ #21609] Message-ID: <20170703202551.GA17398@gmail.com> References: <14a88e5f-a8c6-cd78-a363-1368d175f605@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <14a88e5f-a8c6-cd78-a363-1368d175f605@redhat.com> User-Agent: Mutt/1.8.0 (2017-02-23) On Tue, Jun 20, 2017 at 03:59:12PM +0200, Florian Weimer wrote: > On 06/19/2017 12:18 PM, Florian Weimer wrote: > > This is a follow-up to bug 15128, where an unaligned stack was > > compensated for in the dynamic linker relocation trampoline. > > __tls_get_addr itself was not fixed, though, and the function can reach > > deeply into libc due to its malloc dependency, so can result crashes > > with an unaligned stack, too. > > > > The attached patch adds a compatibility implementation of __tls_get_addr > > which aligns the stack, but only on the slow path. Internal calls go to > > the default implementation (from elf/dl-tls.c) which does not perform > > stack realignment. > > > > I plan to submit a follow-up patch which adds a new symbol version for > > __tls_get_addr which bypasses the stack alignment for new binaries. > > > > In my patch, the CFI annotations need review. I have never written > > those before. > > I went over the CFI annotations with Jakub. He suggested some > simplifications, incorporated in the attached patch. We don't need > .cfi_restore because the %rbp value at the specified offset is protected > by the red zone until the function returns. > > Thanks, > Florian > x86-64: Align the stack in __tls_get_addr [BZ #21609] > > This change forces realignment of the stack pointer in > __tls_get_addr, so that old GCC-compiled binaries continue to work > even if vector instructions are used in glibc which require the > ABI stack realignment. > > The new assembler implementation of __tls_get_addr calls the default > implementation (from elf/dl-tls.c, now call __tls_get_addr_default) > after realigning the stack (but only does so on the slow path). > Internal calls go directly to __tls_get_addr_default because they do not > need stack realignment. > > 2017-06-20 Florian Weimer > > [BZ #21609] > __tls_get_addr implementation with stack alignment for older GCC. > * sysdeps/x86_64/dl-tls.h: Add multiple inclusion guards. > * sysdeps/x86_64/tls_get_addr_compat.c: New file. > * sysdeps/x86_64/dl-tls.c: Likewise. > * sysdeps/x86_64/rtld-offets.h: Likewise. > * sysdeps/x86_64/dl-tlsdesc.h: Include . > (tls_index): Remove definition. > * sysdeps/x86_64/tlsdesc.sym (TI_MODULE_OFFSET, TI_OFFSET_OFFSET): > Add. > * sysdeps/x86_64/Makefile (sysdep-dl-routines): Add > tls_get_addr_compat. > (gen-as-const-headers): Add rtld-offsets.sym. > Hi Florian, I prefer to let GCC realign the stack for us. What do you think? H.J. --- From 4781d00aa6656221d1ca82c863a07689b93172c0 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Mon, 3 Jul 2017 13:16:57 -0700 Subject: [PATCH] x86-64: Align the stack in __tls_get_addr [BZ #21609] This change forces realignment of the stack pointer in __tls_get_addr, so that old GCC-compiled binaries continue to work even if vector instructions are used in glibc which require the ABI stack realignment. The implementation of __tls_get_addr calls the default implementation (from elf/dl-tls.c, now calls __tls_get_addr_default) after realigning the stack. Internal calls go directly to __tls_get_addr_default because they do not need stack realignment. 2017-07-03 Florian Weimer H.J. Lu [BZ #21609] * sysdeps/x86_64/dl-tls.c: New file. * sysdeps/x86_64/dl-tls.h: Add multiple inclusion guards. --- sysdeps/x86_64/dl-tls.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ sysdeps/x86_64/dl-tls.h | 5 +++++ 2 files changed, 52 insertions(+) create mode 100644 sysdeps/x86_64/dl-tls.c diff --git a/sysdeps/x86_64/dl-tls.c b/sysdeps/x86_64/dl-tls.c new file mode 100644 index 0000000..64fd391 --- /dev/null +++ b/sysdeps/x86_64/dl-tls.c @@ -0,0 +1,47 @@ +/* Thread-local storage handling in the ELF dynamic linker. x86-64 version. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#ifdef SHARED +/* Work around GCC PR58066, due to which __tls_get_addr may be called + with an unaligned stack. The compat implementation is in + tls_get_addr-compat.S. */ + +# include + +/* Define __tls_get_addr within elf/dl-tls.c under a different + name. */ +extern __typeof__ (__tls_get_addr) __tls_get_addr_default; + +# define __tls_get_addr __tls_get_addr_default +# include +# undef __tls_get_addr + +hidden_ver (__tls_get_addr_default, __tls_get_addr) + +__attribute__ ((force_align_arg_pointer)) +void * +__tls_get_addr (tls_index *ti) +{ + return __tls_get_addr_default (ti); +} +#else + +/* No compatibility symbol needed. */ +# include + +#endif diff --git a/sysdeps/x86_64/dl-tls.h b/sysdeps/x86_64/dl-tls.h index 4a59d2a..c2fb56c 100644 --- a/sysdeps/x86_64/dl-tls.h +++ b/sysdeps/x86_64/dl-tls.h @@ -16,6 +16,9 @@ License along with the GNU C Library; if not, see . */ +#ifndef _X86_64_DL_TLS_H +#define _X86_64_DL_TLS_H + #include /* Type used for the representation of TLS information in the GOT. */ @@ -27,3 +30,5 @@ typedef struct dl_tls_index extern void *__tls_get_addr (tls_index *ti); + +#endif /* _X86_64_DL_TLS_H */