[PATCH] D44377: [ELF] - Drop SHF_LINK_ORDER flag from the output.

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 13 11:42:07 PDT 2018


Not sure this is worth it.

Even having a section table on output files is technically optional. In
practice it is necessary for static linkers, but anything other than the
symbol table is there for the benefit of a human looking at the output.

As such, SHF_LINK_ORDER has some meaning: The input sections had this
flag set. In my opinion it is OK to drop it or keep it, so we should do
whatever is simpler.

Cheers,
Rafael

George Rimar via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:

> grimar created this revision.
> grimar added reviewers: ruiu, psmith, espindola.
> Herald added subscribers: kristof.beyls, arichardson, emaste.
>
> It should be useless to leave SHF_LINK_ORDER flag in the 
> non-relocatable output because this flag is used only by the linker.
> This patch removes it to make output cleaner.
>
> I had to do an exception for SHT_ARM_EXIDX sections though
> because otherwise, GNU strip tool reports a 
> "sh_link not set for section `.ARM.exidx'" warning. 
> This warning itself seems wrong/useless to me.
>
>
> https://reviews.llvm.org/D44377
>
> Files:
>   ELF/Writer.cpp
>   test/ELF/link-order-flag.s
>
>
> Index: test/ELF/link-order-flag.s
> ===================================================================
> --- test/ELF/link-order-flag.s
> +++ test/ELF/link-order-flag.s
> @@ -0,0 +1,31 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
> +
> +## Check we keep SHF_LINK_ORDER flag for -r
> +# RUN: ld.lld -r %t.o -o %t
> +# RUN: llvm-readobj -s %t | FileCheck %s --check-prefix=NODROP
> +# NODROP:      Section {
> +# NODROP:        Index:
> +# NODROP:        Name: .bar
> +# NODROP-NEXT:   Type: SHT_PROGBITS
> +# NODROP-NEXT:   Flags [
> +# NODROP-NEXT:     SHF_ALLOC
> +# NODROP-NEXT:     SHF_LINK_ORDER
> +# NODROP-NEXT:   ]
> +
> +## Check we drop SHF_LINK_ORDER flag for regular output.
> +# RUN: ld.lld %t.o -o %t
> +# RUN: llvm-readobj -s %t | FileCheck %s --check-prefix=DROP
> +# DROP:      Section {
> +# DROP:        Index:
> +# DROP:        Name: .bar
> +# DROP-NEXT:   Type: SHT_PROGBITS
> +# DROP-NEXT:   Flags [
> +# DROP-NEXT:     SHF_ALLOC
> +# DROP-NEXT:   ]
> +
> +.section .foo,"a"
> +.quad 0
> +
> +.section .bar,"ao", at progbits,.foo
> +.quad 0
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -1351,6 +1351,13 @@
>      for (BaseCommand *Base : Sec->SectionCommands)
>        if (auto *ISD = dyn_cast<InputSectionDescription>(Base))
>          llvm::erase_if(ISD->Sections, [](InputSection *IS) { return !IS; });
> +
> +    // In theory, there should not make sense to have this flag in the
> +    // non-relocatable output because the flag is used by linker only.
> +    // Binutils strip tool reports a warning when sh_link is not set for
> +    // SHT_ARM_EXIDX though. So we have to keep it to workaround warning.
> +    if (!Config->Relocatable && Sec->Type != SHT_ARM_EXIDX)
> +      Sec->Flags &= ~SHF_LINK_ORDER;
>    }
>  }
>  
>
>
> Index: test/ELF/link-order-flag.s
> ===================================================================
> --- test/ELF/link-order-flag.s
> +++ test/ELF/link-order-flag.s
> @@ -0,0 +1,31 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
> +
> +## Check we keep SHF_LINK_ORDER flag for -r
> +# RUN: ld.lld -r %t.o -o %t
> +# RUN: llvm-readobj -s %t | FileCheck %s --check-prefix=NODROP
> +# NODROP:      Section {
> +# NODROP:        Index:
> +# NODROP:        Name: .bar
> +# NODROP-NEXT:   Type: SHT_PROGBITS
> +# NODROP-NEXT:   Flags [
> +# NODROP-NEXT:     SHF_ALLOC
> +# NODROP-NEXT:     SHF_LINK_ORDER
> +# NODROP-NEXT:   ]
> +
> +## Check we drop SHF_LINK_ORDER flag for regular output.
> +# RUN: ld.lld %t.o -o %t
> +# RUN: llvm-readobj -s %t | FileCheck %s --check-prefix=DROP
> +# DROP:      Section {
> +# DROP:        Index:
> +# DROP:        Name: .bar
> +# DROP-NEXT:   Type: SHT_PROGBITS
> +# DROP-NEXT:   Flags [
> +# DROP-NEXT:     SHF_ALLOC
> +# DROP-NEXT:   ]
> +
> +.section .foo,"a"
> +.quad 0
> +
> +.section .bar,"ao", at progbits,.foo
> +.quad 0
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -1351,6 +1351,13 @@
>      for (BaseCommand *Base : Sec->SectionCommands)
>        if (auto *ISD = dyn_cast<InputSectionDescription>(Base))
>          llvm::erase_if(ISD->Sections, [](InputSection *IS) { return !IS; });
> +
> +    // In theory, there should not make sense to have this flag in the
> +    // non-relocatable output because the flag is used by linker only.
> +    // Binutils strip tool reports a warning when sh_link is not set for
> +    // SHT_ARM_EXIDX though. So we have to keep it to workaround warning.
> +    if (!Config->Relocatable && Sec->Type != SHT_ARM_EXIDX)
> +      Sec->Flags &= ~SHF_LINK_ORDER;
>    }
>  }
>  
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list