[PATCH] D40652: [ELF] - Produce relocation section name consistent with output section name when --emit-reloc used with linker script.

[Rafael Avila de Espindola via llvm-commits]

LGTM, but wait for Rui's LGTM too.

Cheers,
Rafael


George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar created this revision.
> Herald added a subscriber: emaste.
>
> This is for "Bug 35474 - --emit-relocs produces wrongly-named reloc sections".
>
> LLD currently for scripts like:
>
>   .text.boot : { *(.text.boot) }
>
> emits relocation section with name `.rela.text` because does not take
> redefined name of output section into account and builds section name
> using rules for non-scripted case. Patch fixes this oddness.
>
>
> https://reviews.llvm.org/D40652
>
> Files:
>   ELF/LinkerScript.cpp
>   ELF/SyntheticSections.cpp
>   ELF/Writer.cpp
>   ELF/Writer.h
>   test/ELF/linkerscript/emit-reloc-section-names.s
>   test/ELF/linkerscript/emit-relocs-multiple.s
>
> Index: test/ELF/linkerscript/emit-relocs-multiple.s
> ===================================================================
> --- test/ELF/linkerscript/emit-relocs-multiple.s
> +++ test/ELF/linkerscript/emit-relocs-multiple.s
> @@ -5,7 +5,7 @@
>  # RUN: llvm-readobj -r %t1 | FileCheck %s
>  
>  # CHECK:      Relocations [
> -# CHECK-NEXT:   Section {{.*}} .rela.foo {
> +# CHECK-NEXT:   Section {{.*}} .rela.zed {
>  # CHECK-NEXT:     0x1 R_X86_64_32 .zed 0x0
>  # CHECK-NEXT:     0x6 R_X86_64_32 .zed 0x5
>  # CHECK-NEXT:   }
> Index: test/ELF/linkerscript/emit-reloc-section-names.s
> ===================================================================
> --- test/ELF/linkerscript/emit-reloc-section-names.s
> +++ test/ELF/linkerscript/emit-reloc-section-names.s
> @@ -0,0 +1,22 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
> +# RUN: echo "SECTIONS { .text.zed : { *(.text.foo) } \
> +# RUN:                  .text.qux : { *(.text.bar) } }" > %t.script
> +# RUN: ld.lld -T %t.script --emit-relocs %t.o -o %t
> +# RUN: llvm-objdump -section-headers %t | FileCheck %s
> +
> +## Check we name relocation sections in according to
> +## their target sections names.
> +
> +# CHECK: .text.zed
> +# CHECK: .text.qux
> +# CHECK: .rela.text.zed
> +# CHECK: .rela.text.qux
> +
> +.section .text.foo,"ax"
> +foo:
> + mov $bar, %rax
> +
> +.section .text.bar,"ax"
> +bar:
> + mov $foo, %rax
> Index: ELF/Writer.h
> ===================================================================
> --- ELF/Writer.h
> +++ ELF/Writer.h
> @@ -46,7 +46,7 @@
>    bool HasLMA = false;
>  };
>  
> -llvm::StringRef getOutputSectionName(llvm::StringRef Name);
> +llvm::StringRef getOutputSectionName(InputSectionBase *S);
>  
>  template <class ELFT> uint32_t calcMipsEFlags();
>  
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -87,22 +87,28 @@
>  };
>  } // anonymous namespace
>  
> -StringRef elf::getOutputSectionName(StringRef Name) {
> +StringRef elf::getOutputSectionName(InputSectionBase *S) {
> +  StringRef Name = S->Name;
> +
>    // ".zdebug_" is a prefix for ZLIB-compressed sections.
>    // Because we decompressed input sections, we want to remove 'z'.
>    if (Name.startswith(".zdebug_"))
>      return Saver.save("." + Name.substr(2));
>  
>    if (Config->Relocatable)
>      return Name;
>  
> -  // This is for --emit-relocs. If .text.foo is emitted as .text, we want to
> -  // emit .rela.text.foo as .rel.text for consistency (this is not technically
> -  // required, but not doing it is odd). This code guarantees that.
> -  if (Name.startswith(".rel."))
> -    return Saver.save(".rel" + getOutputSectionName(Name.substr(4)));
> -  if (Name.startswith(".rela."))
> -    return Saver.save(".rela" + getOutputSectionName(Name.substr(5)));
> +  // This is for --emit-relocs. If .text.foo is emitted as .text.bar, we want
> +  // to emit .rela.text.foo as .rela.text.bar for consistency (this is not
> +  // technically required, but not doing it is odd). This code guarantees that.
> +  if ((S->Type == SHT_REL || S->Type == SHT_RELA) &&
> +      !isa<SyntheticSection>(S)) {
> +    OutputSection *Out =
> +        cast<InputSection>(S)->getRelocatedSection()->getOutputSection();
> +    if (S->Type == SHT_RELA)
> +      return Saver.save(".rela" + Out->Name);
> +    return Saver.save(".rel" + Out->Name);
> +  }
>  
>    for (StringRef V :
>         {".text.", ".rodata.", ".data.rel.ro.", ".data.", ".bss.rel.ro.",
> Index: ELF/SyntheticSections.cpp
> ===================================================================
> --- ELF/SyntheticSections.cpp
> +++ ELF/SyntheticSections.cpp
> @@ -2528,7 +2528,7 @@
>      if (!MS->Live)
>        continue;
>  
> -    StringRef OutsecName = getOutputSectionName(MS->Name);
> +    StringRef OutsecName = getOutputSectionName(MS);
>      uint32_t Alignment = std::max<uint32_t>(MS->Alignment, MS->Entsize);
>  
>      auto I = llvm::find_if(MergeSections, [=](MergeSyntheticSection *Sec) {
> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -533,7 +533,7 @@
>      if (!S->Live || S->Parent)
>        continue;
>  
> -    StringRef Name = getOutputSectionName(S->Name);
> +    StringRef Name = getOutputSectionName(S);
>  
>      if (Config->OrphanHandling == OrphanHandlingPolicy::Error)
>        error(toString(S) + " is being placed in '" + Name + "'");