Message ID | 20200311212713.vruwfgxhgjwxibmg@google.com |
---|---|
State | New |
Headers | show |
Series | Fix section type of .eh_frame on Linux x86_64 | expand |
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" \