[PATCH] Fix section type of .eh_frame on Linux x86_64

[H.J. Lu]

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.

-- 
H.J.