| Message ID | 20200311212713.vruwfgxhgjwxibmg@google.com |
|---|---|
| State | Dropped |
| Headers |
Return-Path: <maskray@google.com> X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-pl1-x62f.google.com (mail-pl1-x62f.google.com [IPv6:2607:f8b0:4864:20::62f]) by sourceware.org (Postfix) with ESMTPS id 711ED3943550 for <libc-alpha@sourceware.org>; Wed, 11 Mar 2020 21:27:17 +0000 (GMT) Received: by mail-pl1-x62f.google.com with SMTP id h11so1694578plk.7 for <libc-alpha@sourceware.org>; Wed, 11 Mar 2020 14:27:17 -0700 (PDT) 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:subject:message-id:mime-version :content-disposition; bh=t3K6oFvOdKVxtIxkkDmBaEHOQPxPRhrzxejm0rcrvuY=; b=Kt/ksNJUbwehOW9svAA2vhJX0lV7D13qcg6P8QmGIDfMVDJKYFQiMzpz+AIn6ubDFg TzIMgOZzAZLr7uFUzFejNqOnWbAeygYm8YgOtR2g5dKLbLdHbqGjq/Twaz1YQ1tyMEta ab8v2LXgW7ih7opXD9AOer3kwCBPL+uTFiWkwrbX0qWnJgW+QaLgipJ/F7D9Atbydia8 Z2rR7LsedXqDefsQGrR/+o+Vi5J/XbxZNSr9Xrym5dthPvVhOez80ig0aTv7EkvCn9gO 3tHCq9EepDhPgf7UYtEc9MOGQk5YHf89MdpTgLy8WyO6fjuMU/PbrKZ9fSBEwhgRdhxY ofjg== X-Gm-Message-State: ANhLgQ1EsxE+T0kjMx3EFhdORymCPFlilhglvjOKjzt4qXnKpc+LQw6D po6BmcDYSuf5e0w8xpCDK4kkBHAeD9ordA== X-Google-Smtp-Source: ADFU+vuQTQG6OZEldu1zakC5edMmN7NRb80yUOKb8S4Y0emf8LwatqP37Tnh7cW684Jh8B3n8qDvsw== X-Received: by 2002:a17:90a:6545:: with SMTP id f5mr750886pjs.42.1583962036056; Wed, 11 Mar 2020 14:27:16 -0700 (PDT) Received: from google.com ([2620:15c:2ce:0:9efe:9f1:9267:2b27]) by smtp.gmail.com with ESMTPSA id v5sm26772621pfn.64.2020.03.11.14.27.15 for <libc-alpha@sourceware.org> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Mar 2020 14:27:15 -0700 (PDT) Date: Wed, 11 Mar 2020 14:27:13 -0700 From: Fangrui Song <maskray@google.com> To: libc-alpha@sourceware.org Subject: [PATCH] Fix section type of .eh_frame on Linux x86_64 Message-ID: <20200311212713.vruwfgxhgjwxibmg@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-41.7 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, FSL_HELO_FAKE, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org> List-Unsubscribe: <http://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <http://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <http://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> X-List-Received-Date: Wed, 11 Mar 2020 21:27:18 -0000 |
| Series |
Fix section type of .eh_frame on Linux x86_64
|
|
Commit Message
Fangrui Song
March 11, 2020, 9:27 p.m. UTC
Clang since https://reviews.llvm.org/D73999 will error for the wrong sh_type. --- sysdeps/unix/sysv/linux/x86_64/sigaction.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On 11/03/2020 18:27, Fangrui Song via Libc-alpha wrote: > Clang since https://reviews.llvm.org/D73999 will error for the wrong > sh_type. It will make eh_frame section of sigaction object to have the SHT_X86_64_UNWIND, but it seems that 'ld.bfd' at least ignore and set the resulting eh_frame sh_type for libc.so to SHT_PROGBITS anyway. The 'as' also still generate SHT_PROGBITS for code that requires a eh_frame (C++ with exception handling that emits a gcc_except_table section, for instance). Setting the eh_frame in assembly routines is not a common practice, the only other code that I could find that actually does it is Linux. For i686 vDSO is also uses 'progbits': arch/x86/entry/vdso/vdso32/sigreturn.S 36 .section .eh_frame,"a",@progbits 37 .LSTARTFRAMEDLSI1: The change should be ok, but I would like to understand better why exactly SHT_X86_64_UNWIND should be used for eh_frame, why binutils does not seems to use it to eh_frame, and why clang is now not accepting the eh_frame with SHT_PROGBITS type. > --- > sysdeps/unix/sysv/linux/x86_64/sigaction.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sysdeps/unix/sysv/linux/x86_64/sigaction.c b/sysdeps/unix/sysv/linux/x86_64/sigaction.c > index c58a77c5c6..3b730bc9e3 100644 > --- a/sysdeps/unix/sysv/linux/x86_64/sigaction.c > +++ b/sysdeps/unix/sysv/linux/x86_64/sigaction.c > @@ -80,7 +80,7 @@ asm \ > " movq $" #syscall ", %rax\n" \ > " syscall\n" \ > ".LEND_" #name ":\n" \ > - ".section .eh_frame,\"a\",@progbits\n" \ > + ".section .eh_frame,\"a\",@unwind\n" \ > ".LSTARTFRAME_" #name ":\n" \ > " .long .LENDCIE_" #name "-.LSTARTCIE_" #name "\n" \ > ".LSTARTCIE_" #name ":\n" \ >
On Fri, Mar 13, 2020 at 5:28 AM Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> wrote: > > > > On 11/03/2020 18:27, Fangrui Song via Libc-alpha wrote: > > Clang since https://reviews.llvm.org/D73999 will error for the wrong > > sh_type. > > It will make eh_frame section of sigaction object to have the > SHT_X86_64_UNWIND, but it seems that 'ld.bfd' at least ignore and > set the resulting eh_frame sh_type for libc.so to SHT_PROGBITS > anyway. The 'as' also still generate SHT_PROGBITS for code that > requires a eh_frame (C++ with exception handling that emits a > gcc_except_table section, for instance). SHT_X86_64_UNWIND was added so that linker can group all SHT_X86_64_UNWIND sections together without checking .eh_frame section name. Other than that, linker treats .eh_frame like other SHT_PROGBITS sections. Since .eh_frame is also listed as a special section, similar to .got and .plt, SHT_X86_64_UNWIND isn't really required. > Setting the eh_frame in assembly routines is not a common practice, > the only other code that I could find that actually does it is > Linux. For i686 vDSO is also uses 'progbits': > > arch/x86/entry/vdso/vdso32/sigreturn.S > > 36 .section .eh_frame,"a",@progbits > 37 .LSTARTFRAMEDLSI1: > > The change should be ok, but I would like to understand better why > exactly SHT_X86_64_UNWIND should be used for eh_frame, why binutils > does not seems to use it to eh_frame, and why clang is now not > accepting the eh_frame with SHT_PROGBITS type. > Linker should check .eh_frame section name, and optionally check SHT_X86_64_UNWIND. Because of this, i386 psABI only specifies special .eh_frame section without SHT_386_UNWIND. So I think there is no need to change glibc and lld should be changed.
On 13/03/2020 10:12, H.J. Lu wrote: > On Fri, Mar 13, 2020 at 5:28 AM Adhemerval Zanella via Libc-alpha > <libc-alpha@sourceware.org> wrote: >> >> >> >> On 11/03/2020 18:27, Fangrui Song via Libc-alpha wrote: >>> Clang since https://reviews.llvm.org/D73999 will error for the wrong >>> sh_type. >> >> It will make eh_frame section of sigaction object to have the >> SHT_X86_64_UNWIND, but it seems that 'ld.bfd' at least ignore and >> set the resulting eh_frame sh_type for libc.so to SHT_PROGBITS >> anyway. The 'as' also still generate SHT_PROGBITS for code that >> requires a eh_frame (C++ with exception handling that emits a >> gcc_except_table section, for instance). > > SHT_X86_64_UNWIND was added so that linker can group all > SHT_X86_64_UNWIND sections together without checking > .eh_frame section name. Other than that, linker treats .eh_frame like > other SHT_PROGBITS sections. Since .eh_frame is also listed as a > special section, similar to .got and .plt, SHT_X86_64_UNWIND isn't > really required. Thanks for the explanation. > >> Setting the eh_frame in assembly routines is not a common practice, >> the only other code that I could find that actually does it is >> Linux. For i686 vDSO is also uses 'progbits': >> >> arch/x86/entry/vdso/vdso32/sigreturn.S >> >> 36 .section .eh_frame,"a",@progbits >> 37 .LSTARTFRAMEDLSI1: >> >> The change should be ok, but I would like to understand better why >> exactly SHT_X86_64_UNWIND should be used for eh_frame, why binutils >> does not seems to use it to eh_frame, and why clang is now not >> accepting the eh_frame with SHT_PROGBITS type. >> > > Linker should check .eh_frame section name, and optionally check > SHT_X86_64_UNWIND. Because of this, i386 psABI only specifies > special .eh_frame section without SHT_386_UNWIND. > > So I think there is no need to change glibc and lld should be changed. > I was confused why binutils was not throwing a similar issue. So the idea is SHT_X86_64_UNWIND is optional on eh_frame definitions, not mandatory, and being optional I agree that compiler/linker should not emit an error in such case.
On Fri, Mar 13, 2020 at 7:04 AM Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote: > > > > On 13/03/2020 10:12, H.J. Lu wrote: > > On Fri, Mar 13, 2020 at 5:28 AM Adhemerval Zanella via Libc-alpha > > <libc-alpha@sourceware.org> wrote: > >> > >> > >> > >> On 11/03/2020 18:27, Fangrui Song via Libc-alpha wrote: > >>> Clang since https://reviews.llvm.org/D73999 will error for the wrong > >>> sh_type. > >> > >> It will make eh_frame section of sigaction object to have the > >> SHT_X86_64_UNWIND, but it seems that 'ld.bfd' at least ignore and > >> set the resulting eh_frame sh_type for libc.so to SHT_PROGBITS > >> anyway. The 'as' also still generate SHT_PROGBITS for code that > >> requires a eh_frame (C++ with exception handling that emits a > >> gcc_except_table section, for instance). > > > > SHT_X86_64_UNWIND was added so that linker can group all > > SHT_X86_64_UNWIND sections together without checking > > .eh_frame section name. Other than that, linker treats .eh_frame like > > other SHT_PROGBITS sections. Since .eh_frame is also listed as a > > special section, similar to .got and .plt, SHT_X86_64_UNWIND isn't > > really required. > > Thanks for the explanation. > > > > >> Setting the eh_frame in assembly routines is not a common practice, > >> the only other code that I could find that actually does it is > >> Linux. For i686 vDSO is also uses 'progbits': > >> > >> arch/x86/entry/vdso/vdso32/sigreturn.S > >> > >> 36 .section .eh_frame,"a",@progbits > >> 37 .LSTARTFRAMEDLSI1: > >> > >> The change should be ok, but I would like to understand better why > >> exactly SHT_X86_64_UNWIND should be used for eh_frame, why binutils > >> does not seems to use it to eh_frame, and why clang is now not > >> accepting the eh_frame with SHT_PROGBITS type. > >> > > > > Linker should check .eh_frame section name, and optionally check > > SHT_X86_64_UNWIND. Because of this, i386 psABI only specifies > > special .eh_frame section without SHT_386_UNWIND. > > > > So I think there is no need to change glibc and lld should be changed. > > > > I was confused why binutils was not throwing a similar issue. So > the idea is SHT_X86_64_UNWIND is optional on eh_frame definitions, > not mandatory, and being optional I agree that compiler/linker > should not emit an error in such case. It is unfortunate that x86-64 psABI specifies both SHT_X86_64_UNWIND and .eh_frame. Only one is needed, not both.
On 2020-03-13, H.J. Lu wrote: >On Fri, Mar 13, 2020 at 7:04 AM Adhemerval Zanella ><adhemerval.zanella@linaro.org> wrote: >> >> >> >> On 13/03/2020 10:12, H.J. Lu wrote: >> > On Fri, Mar 13, 2020 at 5:28 AM Adhemerval Zanella via Libc-alpha >> > <libc-alpha@sourceware.org> wrote: >> >> >> >> >> >> >> >> On 11/03/2020 18:27, Fangrui Song via Libc-alpha wrote: >> >>> Clang since https://reviews.llvm.org/D73999 will error for the wrong >> >>> sh_type. >> >> >> >> It will make eh_frame section of sigaction object to have the >> >> SHT_X86_64_UNWIND, but it seems that 'ld.bfd' at least ignore and >> >> set the resulting eh_frame sh_type for libc.so to SHT_PROGBITS >> >> anyway. The 'as' also still generate SHT_PROGBITS for code that >> >> requires a eh_frame (C++ with exception handling that emits a >> >> gcc_except_table section, for instance). >> > >> > SHT_X86_64_UNWIND was added so that linker can group all >> > SHT_X86_64_UNWIND sections together without checking >> > .eh_frame section name. Other than that, linker treats .eh_frame like >> > other SHT_PROGBITS sections. Since .eh_frame is also listed as a >> > special section, similar to .got and .plt, SHT_X86_64_UNWIND isn't >> > really required. >> >> Thanks for the explanation. >> >> > >> >> Setting the eh_frame in assembly routines is not a common practice, >> >> the only other code that I could find that actually does it is >> >> Linux. For i686 vDSO is also uses 'progbits': >> >> >> >> arch/x86/entry/vdso/vdso32/sigreturn.S >> >> >> >> 36 .section .eh_frame,"a",@progbits >> >> 37 .LSTARTFRAMEDLSI1: >> >> >> >> The change should be ok, but I would like to understand better why >> >> exactly SHT_X86_64_UNWIND should be used for eh_frame, why binutils >> >> does not seems to use it to eh_frame, and why clang is now not >> >> accepting the eh_frame with SHT_PROGBITS type. >> >> >> > >> > Linker should check .eh_frame section name, and optionally check >> > SHT_X86_64_UNWIND. Because of this, i386 psABI only specifies >> > special .eh_frame section without SHT_386_UNWIND. >> > >> > So I think there is no need to change glibc and lld should be changed. >> > clang integrated assembler is more rigid. https://github.com/llvm/llvm-project/blob/master/llvm/lib/MC/MCObjectFileInfo.cpp#L488 .eh_frame is a pre-allocated recognized section. "Redeclaring" (via .section directive) it with different sh_flags or sh_type will error. >> I was confused why binutils was not throwing a similar issue. So >> the idea is SHT_X86_64_UNWIND is optional on eh_frame definitions, >> not mandatory, and being optional I agree that compiler/linker >> should not emit an error in such case. > >It is unfortunate that x86-64 psABI specifies both SHT_X86_64_UNWIND >and .eh_frame. Only one is needed, not both. I know little about GNU as internals (https://sourceware.org/legacy-ml/binutils/2020-02/msg00129.html), but it seems .cfi* generated .eh_frame contents don't conflict with .section declared .eh_frame Though, .section .eh_frame,"a",@unwind; .section .eh_frame,"a",@progbits will error as well. https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d2b2c203e1ff2daf8cfeab0906084f9389e66246 (2004) added SHT_X86_64_UNWIND to binutils. There was no further information why it was added, though.
On Fri, Mar 13, 2020 at 9:07 AM Fangrui Song <maskray@google.com> wrote: > > On 2020-03-13, H.J. Lu wrote: > >On Fri, Mar 13, 2020 at 7:04 AM Adhemerval Zanella > ><adhemerval.zanella@linaro.org> wrote: > >> > >> > >> > >> On 13/03/2020 10:12, H.J. Lu wrote: > >> > On Fri, Mar 13, 2020 at 5:28 AM Adhemerval Zanella via Libc-alpha > >> > <libc-alpha@sourceware.org> wrote: > >> >> > >> >> > >> >> > >> >> On 11/03/2020 18:27, Fangrui Song via Libc-alpha wrote: > >> >>> Clang since https://reviews.llvm.org/D73999 will error for the wrong > >> >>> sh_type. > >> >> > >> >> It will make eh_frame section of sigaction object to have the > >> >> SHT_X86_64_UNWIND, but it seems that 'ld.bfd' at least ignore and > >> >> set the resulting eh_frame sh_type for libc.so to SHT_PROGBITS > >> >> anyway. The 'as' also still generate SHT_PROGBITS for code that > >> >> requires a eh_frame (C++ with exception handling that emits a > >> >> gcc_except_table section, for instance). > >> > > >> > SHT_X86_64_UNWIND was added so that linker can group all > >> > SHT_X86_64_UNWIND sections together without checking > >> > .eh_frame section name. Other than that, linker treats .eh_frame like > >> > other SHT_PROGBITS sections. Since .eh_frame is also listed as a > >> > special section, similar to .got and .plt, SHT_X86_64_UNWIND isn't > >> > really required. > >> > >> Thanks for the explanation. > >> > >> > > >> >> Setting the eh_frame in assembly routines is not a common practice, > >> >> the only other code that I could find that actually does it is > >> >> Linux. For i686 vDSO is also uses 'progbits': > >> >> > >> >> arch/x86/entry/vdso/vdso32/sigreturn.S > >> >> > >> >> 36 .section .eh_frame,"a",@progbits > >> >> 37 .LSTARTFRAMEDLSI1: > >> >> > >> >> The change should be ok, but I would like to understand better why > >> >> exactly SHT_X86_64_UNWIND should be used for eh_frame, why binutils > >> >> does not seems to use it to eh_frame, and why clang is now not > >> >> accepting the eh_frame with SHT_PROGBITS type. > >> >> > >> > > >> > Linker should check .eh_frame section name, and optionally check > >> > SHT_X86_64_UNWIND. Because of this, i386 psABI only specifies > >> > special .eh_frame section without SHT_386_UNWIND. > >> > > >> > So I think there is no need to change glibc and lld should be changed. > >> > > > clang integrated assembler is more rigid. > https://github.com/llvm/llvm-project/blob/master/llvm/lib/MC/MCObjectFileInfo.cpp#L488 > .eh_frame is a pre-allocated recognized section. > "Redeclaring" (via .section directive) it with different sh_flags or sh_type will error. > > >> I was confused why binutils was not throwing a similar issue. So > >> the idea is SHT_X86_64_UNWIND is optional on eh_frame definitions, > >> not mandatory, and being optional I agree that compiler/linker > >> should not emit an error in such case. > > > >It is unfortunate that x86-64 psABI specifies both SHT_X86_64_UNWIND > >and .eh_frame. Only one is needed, not both. > > I know little about GNU as internals > (https://sourceware.org/legacy-ml/binutils/2020-02/msg00129.html), > but it seems .cfi* generated .eh_frame contents don't conflict with > .section declared .eh_frame > > Though, .section .eh_frame,"a",@unwind; .section .eh_frame,"a",@progbits > will error as well. > > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d2b2c203e1ff2daf8cfeab0906084f9389e66246 (2004) added SHT_X86_64_UNWIND to binutils. > There was no further information why it was added, though. I guess it was for Solaris.
On 13/03/2020 13:07, Fangrui Song wrote: > On 2020-03-13, H.J. Lu wrote: >> On Fri, Mar 13, 2020 at 7:04 AM Adhemerval Zanella >> <adhemerval.zanella@linaro.org> wrote: >>> >>> >>> >>> On 13/03/2020 10:12, H.J. Lu wrote: >>> > On Fri, Mar 13, 2020 at 5:28 AM Adhemerval Zanella via Libc-alpha >>> > <libc-alpha@sourceware.org> wrote: >>> >> >>> >> >>> >> >>> >> On 11/03/2020 18:27, Fangrui Song via Libc-alpha wrote: >>> >>> Clang since https://reviews.llvm.org/D73999 will error for the wrong >>> >>> sh_type. >>> >> >>> >> It will make eh_frame section of sigaction object to have the >>> >> SHT_X86_64_UNWIND, but it seems that 'ld.bfd' at least ignore and >>> >> set the resulting eh_frame sh_type for libc.so to SHT_PROGBITS >>> >> anyway. The 'as' also still generate SHT_PROGBITS for code that >>> >> requires a eh_frame (C++ with exception handling that emits a >>> >> gcc_except_table section, for instance). >>> > >>> > SHT_X86_64_UNWIND was added so that linker can group all >>> > SHT_X86_64_UNWIND sections together without checking >>> > .eh_frame section name. Other than that, linker treats .eh_frame like >>> > other SHT_PROGBITS sections. Since .eh_frame is also listed as a >>> > special section, similar to .got and .plt, SHT_X86_64_UNWIND isn't >>> > really required. >>> >>> Thanks for the explanation. >>> >>> > >>> >> Setting the eh_frame in assembly routines is not a common practice, >>> >> the only other code that I could find that actually does it is >>> >> Linux. For i686 vDSO is also uses 'progbits': >>> >> >>> >> arch/x86/entry/vdso/vdso32/sigreturn.S >>> >> >>> >> 36 .section .eh_frame,"a",@progbits >>> >> 37 .LSTARTFRAMEDLSI1: >>> >> >>> >> The change should be ok, but I would like to understand better why >>> >> exactly SHT_X86_64_UNWIND should be used for eh_frame, why binutils >>> >> does not seems to use it to eh_frame, and why clang is now not >>> >> accepting the eh_frame with SHT_PROGBITS type. >>> >> >>> > >>> > Linker should check .eh_frame section name, and optionally check >>> > SHT_X86_64_UNWIND. Because of this, i386 psABI only specifies >>> > special .eh_frame section without SHT_386_UNWIND. >>> > >>> > So I think there is no need to change glibc and lld should be changed. >>> > > > clang integrated assembler is more rigid. > https://github.com/llvm/llvm-project/blob/master/llvm/lib/MC/MCObjectFileInfo.cpp#L488 > .eh_frame is a pre-allocated recognized section. > "Redeclaring" (via .section directive) it with different sh_flags or sh_type will error. The question now is whether this specific test make sense. From H.J.Lu explanation, it seems clang strictness in not allowing eh_frame to be defined as SHT_PROGBITS does not make much sense because the section will be set a SHT_PROGBITS in any case (even if a module defines it as SHT_X86_64_UNWIND). > >>> I was confused why binutils was not throwing a similar issue. So >>> the idea is SHT_X86_64_UNWIND is optional on eh_frame definitions, >>> not mandatory, and being optional I agree that compiler/linker >>> should not emit an error in such case. >> >> It is unfortunate that x86-64 psABI specifies both SHT_X86_64_UNWIND >> and .eh_frame. Only one is needed, not both. > > I know little about GNU as internals > (https://sourceware.org/legacy-ml/binutils/2020-02/msg00129.html), > but it seems .cfi* generated .eh_frame contents don't conflict with > .section declared .eh_frame > > Though, .section .eh_frame,"a",@unwind; .section .eh_frame,"a",@progbits > will error as well. > > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d2b2c203e1ff2daf8cfeab0906084f9389e66246 (2004) added SHT_X86_64_UNWIND to binutils. > There was no further information why it was added, though.
On 2020-03-13, Adhemerval Zanella wrote: > > >On 13/03/2020 13:07, Fangrui Song wrote: >> On 2020-03-13, H.J. Lu wrote: >>> On Fri, Mar 13, 2020 at 7:04 AM Adhemerval Zanella >>> <adhemerval.zanella@linaro.org> wrote: >>>> >>>> >>>> >>>> On 13/03/2020 10:12, H.J. Lu wrote: >>>> > On Fri, Mar 13, 2020 at 5:28 AM Adhemerval Zanella via Libc-alpha >>>> > <libc-alpha@sourceware.org> wrote: >>>> >> >>>> >> >>>> >> >>>> >> On 11/03/2020 18:27, Fangrui Song via Libc-alpha wrote: >>>> >>> Clang since https://reviews.llvm.org/D73999 will error for the wrong >>>> >>> sh_type. >>>> >> >>>> >> It will make eh_frame section of sigaction object to have the >>>> >> SHT_X86_64_UNWIND, but it seems that 'ld.bfd' at least ignore and >>>> >> set the resulting eh_frame sh_type for libc.so to SHT_PROGBITS >>>> >> anyway. The 'as' also still generate SHT_PROGBITS for code that >>>> >> requires a eh_frame (C++ with exception handling that emits a >>>> >> gcc_except_table section, for instance). >>>> > >>>> > SHT_X86_64_UNWIND was added so that linker can group all >>>> > SHT_X86_64_UNWIND sections together without checking >>>> > .eh_frame section name. Other than that, linker treats .eh_frame like >>>> > other SHT_PROGBITS sections. Since .eh_frame is also listed as a >>>> > special section, similar to .got and .plt, SHT_X86_64_UNWIND isn't >>>> > really required. >>>> >>>> Thanks for the explanation. >>>> >>>> > >>>> >> Setting the eh_frame in assembly routines is not a common practice, >>>> >> the only other code that I could find that actually does it is >>>> >> Linux. For i686 vDSO is also uses 'progbits': >>>> >> >>>> >> arch/x86/entry/vdso/vdso32/sigreturn.S >>>> >> >>>> >> 36 .section .eh_frame,"a",@progbits >>>> >> 37 .LSTARTFRAMEDLSI1: >>>> >> >>>> >> The change should be ok, but I would like to understand better why >>>> >> exactly SHT_X86_64_UNWIND should be used for eh_frame, why binutils >>>> >> does not seems to use it to eh_frame, and why clang is now not >>>> >> accepting the eh_frame with SHT_PROGBITS type. >>>> >> >>>> > >>>> > Linker should check .eh_frame section name, and optionally check >>>> > SHT_X86_64_UNWIND. Because of this, i386 psABI only specifies >>>> > special .eh_frame section without SHT_386_UNWIND. >>>> > >>>> > So I think there is no need to change glibc and lld should be changed. >>>> > >> >> clang integrated assembler is more rigid. >> https://github.com/llvm/llvm-project/blob/master/llvm/lib/MC/MCObjectFileInfo.cpp#L488 >> .eh_frame is a pre-allocated recognized section. >> "Redeclaring" (via .section directive) it with different sh_flags or sh_type will error. > >The question now is whether this specific test make sense. From >H.J.Lu explanation, it seems clang strictness in not allowing >eh_frame to be defined as SHT_PROGBITS does not make much sense >because the section will be set a SHT_PROGBITS in any case >(even if a module defines it as SHT_X86_64_UNWIND). https://reviews.llvm.org/D73999 The rigidness does not come from more code, but rather, a much simpler check. if (Section->getType() != Type) Error(loc, "changed section type for " + SectionName + ", expected: 0x" + utohexstr(Section->getType())); if (Section->getFlags() != Flags) Error(loc, "changed section flags for " + SectionName + ", expected: 0x" + utohexstr(Section->getFlags())); I confirm that GNU as generate SHT_PROGBITS .eh_frame from .cfi* (Maybe we should teach it to generate SHT_X86_64_UNWIND instead?) clang generates SHT_X86_64_UNWIND. My intention is to keep the clang logic simple this way. As another example, GNU as special cases empty section flags, but we think the special case is not necessary: https://github.com/ClangBuiltLinux/linux/issues/913#issuecomment-594157005 Alternatively, we can remove SHT_X86_64_UNWIND from x86-64 psABI, so every arch will use SHT_PROGBITS. Using a non-special SHT_PROGBITS does not match ELF's spirit but a processor specific SHT_X86_64_UNWIND does not make things better (probably make things worse). On clang/LLVM side, I'll revert https://reviews.llvm.org/rL252300 and make SHT_X86_64_UNWIND Solaris specific again. I'll have to make `if (Section->getType() != Type)` uglier by special casing .eh_frame SHT_X86_64_UNWIND to keep existing object files compatible. >> >>>> I was confused why binutils was not throwing a similar issue. So >>>> the idea is SHT_X86_64_UNWIND is optional on eh_frame definitions, >>>> not mandatory, and being optional I agree that compiler/linker >>>> should not emit an error in such case. >>> >>> It is unfortunate that x86-64 psABI specifies both SHT_X86_64_UNWIND >>> and .eh_frame. Only one is needed, not both. >> >> I know little about GNU as internals >> (https://sourceware.org/legacy-ml/binutils/2020-02/msg00129.html), >> but it seems .cfi* generated .eh_frame contents don't conflict with >> .section declared .eh_frame >> >> Though, .section .eh_frame,"a",@unwind; .section .eh_frame,"a",@progbits >> will error as well. >> >> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d2b2c203e1ff2daf8cfeab0906084f9389e66246 (2004) added SHT_X86_64_UNWIND to binutils. >> There was no further information why it was added, though.
On Fri, Mar 13, 2020 at 9:56 AM Fangrui Song <maskray@google.com> wrote: > > On 2020-03-13, Adhemerval Zanella wrote: > > > > > >On 13/03/2020 13:07, Fangrui Song wrote: > >> On 2020-03-13, H.J. Lu wrote: > >>> On Fri, Mar 13, 2020 at 7:04 AM Adhemerval Zanella > >>> <adhemerval.zanella@linaro.org> wrote: > >>>> > >>>> > >>>> > >>>> On 13/03/2020 10:12, H.J. Lu wrote: > >>>> > On Fri, Mar 13, 2020 at 5:28 AM Adhemerval Zanella via Libc-alpha > >>>> > <libc-alpha@sourceware.org> wrote: > >>>> >> > >>>> >> > >>>> >> > >>>> >> On 11/03/2020 18:27, Fangrui Song via Libc-alpha wrote: > >>>> >>> Clang since https://reviews.llvm.org/D73999 will error for the wrong > >>>> >>> sh_type. > >>>> >> > >>>> >> It will make eh_frame section of sigaction object to have the > >>>> >> SHT_X86_64_UNWIND, but it seems that 'ld.bfd' at least ignore and > >>>> >> set the resulting eh_frame sh_type for libc.so to SHT_PROGBITS > >>>> >> anyway. The 'as' also still generate SHT_PROGBITS for code that > >>>> >> requires a eh_frame (C++ with exception handling that emits a > >>>> >> gcc_except_table section, for instance). > >>>> > > >>>> > SHT_X86_64_UNWIND was added so that linker can group all > >>>> > SHT_X86_64_UNWIND sections together without checking > >>>> > .eh_frame section name. Other than that, linker treats .eh_frame like > >>>> > other SHT_PROGBITS sections. Since .eh_frame is also listed as a > >>>> > special section, similar to .got and .plt, SHT_X86_64_UNWIND isn't > >>>> > really required. > >>>> > >>>> Thanks for the explanation. > >>>> > >>>> > > >>>> >> Setting the eh_frame in assembly routines is not a common practice, > >>>> >> the only other code that I could find that actually does it is > >>>> >> Linux. For i686 vDSO is also uses 'progbits': > >>>> >> > >>>> >> arch/x86/entry/vdso/vdso32/sigreturn.S > >>>> >> > >>>> >> 36 .section .eh_frame,"a",@progbits > >>>> >> 37 .LSTARTFRAMEDLSI1: > >>>> >> > >>>> >> The change should be ok, but I would like to understand better why > >>>> >> exactly SHT_X86_64_UNWIND should be used for eh_frame, why binutils > >>>> >> does not seems to use it to eh_frame, and why clang is now not > >>>> >> accepting the eh_frame with SHT_PROGBITS type. > >>>> >> > >>>> > > >>>> > Linker should check .eh_frame section name, and optionally check > >>>> > SHT_X86_64_UNWIND. Because of this, i386 psABI only specifies > >>>> > special .eh_frame section without SHT_386_UNWIND. > >>>> > > >>>> > So I think there is no need to change glibc and lld should be changed. > >>>> > > >> > >> clang integrated assembler is more rigid. > >> https://github.com/llvm/llvm-project/blob/master/llvm/lib/MC/MCObjectFileInfo.cpp#L488 > >> .eh_frame is a pre-allocated recognized section. > >> "Redeclaring" (via .section directive) it with different sh_flags or sh_type will error. > > > >The question now is whether this specific test make sense. From > >H.J.Lu explanation, it seems clang strictness in not allowing > >eh_frame to be defined as SHT_PROGBITS does not make much sense > >because the section will be set a SHT_PROGBITS in any case > >(even if a module defines it as SHT_X86_64_UNWIND). > > https://reviews.llvm.org/D73999 > The rigidness does not come from more code, but rather, a much simpler check. > > if (Section->getType() != Type) > Error(loc, "changed section type for " + SectionName + ", expected: 0x" + > utohexstr(Section->getType())); > if (Section->getFlags() != Flags) > Error(loc, "changed section flags for " + SectionName + ", expected: 0x" + > utohexstr(Section->getFlags())); > > I confirm that GNU as generate SHT_PROGBITS .eh_frame from .cfi* (Maybe we should teach it to generate SHT_X86_64_UNWIND instead?) > clang generates SHT_X86_64_UNWIND. > > My intention is to keep the clang logic simple this way. > As another example, GNU as special cases empty section flags, but we > think the special case is not necessary: > https://github.com/ClangBuiltLinux/linux/issues/913#issuecomment-594157005 > > Alternatively, we can remove SHT_X86_64_UNWIND from x86-64 psABI, so every arch will use SHT_PROGBITS. https://groups.google.com/forum/#!topic/x86-64-abi/7sr4E6THl3g > Using a non-special SHT_PROGBITS does not match ELF's spirit but a processor specific SHT_X86_64_UNWIND does not make things better (probably make things worse). > Agree.
diff --git a/sysdeps/unix/sysv/linux/x86_64/sigaction.c b/sysdeps/unix/sysv/linux/x86_64/sigaction.c index c58a77c5c6..3b730bc9e3 100644 --- a/sysdeps/unix/sysv/linux/x86_64/sigaction.c +++ b/sysdeps/unix/sysv/linux/x86_64/sigaction.c @@ -80,7 +80,7 @@ asm \ " movq $" #syscall ", %rax\n" \ " syscall\n" \ ".LEND_" #name ":\n" \ - ".section .eh_frame,\"a\",@progbits\n" \ + ".section .eh_frame,\"a\",@unwind\n" \ ".LSTARTFRAME_" #name ":\n" \ " .long .LENDCIE_" #name "-.LSTARTCIE_" #name "\n" \ ".LSTARTCIE_" #name ":\n" \