[PATCH] D33780: [ELF] - Linkerscript - do not emit multiple relocation sections when using --emit-relocs.

[Rafael Avila de Espindola via llvm-commits]

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:
> Index: test/ELF/linkerscript/emit-relocs-multiple.s
> ===================================================================
> --- test/ELF/linkerscript/emit-relocs-multiple.s
> +++ test/ELF/linkerscript/emit-relocs-multiple.s
> @@ -0,0 +1,21 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
> +# RUN: echo "SECTIONS { .foo : { *(.foo) *(.bar) } }" > %t.script

Just for symmetry, rename one of the .foo to .zed, since otherwise one
has the impression that it is important that two have the same name.

> +# RUN: ld.lld --emit-relocs --script %t.script %t.o -o %t1
> +# RUN: llvm-readobj -s -r %t1 | FileCheck %s
> +
> +# CHECK-NOT:  .rela.bar
> +# CHECK:      Relocations [
> +# CHECK-NEXT:   Section {{.*}} .rela.foo {
> +# CHECK-NEXT:     0x1 R_X86_64_32 .foo 0x0
> +# CHECK-NEXT:     0x6 R_X86_64_32 .foo 0x5
> +# CHECK-NEXT:   }
> +# CHECK-NEXT: ]

I don't think you need the CHECK-NOT as you CHECK everything from [ to ].

> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -484,11 +484,35 @@
>    Opt.Commands = std::move(Commands);
>  }
>  
> +// Imagine .boo : { *(.foo) *(.bar) } script. Both foo and bar may have
> +// relocation sections .rela.foo and .rela.bar for example. Most tools does not
> +// allow to have multiple REL[A] sections for output section. Hence
> we should

Most tools do no allow multiple...

> +// combine these relocation sections into single output.
> +static void addRelocationSections(ArrayRef<InputSection *> Arr,
> +                                  OutputSectionFactory &Factory) {
> +  // Each regular output section may have single relocation section referenced
> +  // by. This mapping allows to find relocation section by output section.
> +  DenseMap<OutputSection *, OutputSection *> RelocationSections;
> +  for (InputSection *Sec : Arr) {
> +    OutputSection *Out = Sec->getRelocatedSection()->getOutputSection();
> +    OutputSection *&RelocationSec = RelocationSections[Out];
> +    StringRef Name = getOutputSectionName(Sec->Name);
> +    Factory.addInputSec(Sec, Name, RelocationSec);
> +  }
> +}
> +
>  // Add sections that didn't match any sections command.
>  void LinkerScript::addOrphanSections(OutputSectionFactory &Factory) {
> +  std::vector<InputSection *> RelocSections;
>    for (InputSectionBase *S : InputSections) {
>      if (!S->Live || S->Parent)
>        continue;
> +    if (!isa<SyntheticSection>(S) &&
> +        (S->Type == SHT_REL || S->Type == SHT_RELA)) {
> +      RelocSections.push_back(cast<InputSection>(S));
> +      continue;
> +    }
> +

The reason this is working without linker scripts is that we normally
have a naming convention on sections. That is a bit brittle and I think
that fixing that should fix the issue for both linker script and non
linker script cases.

For example, with the attached testcase and -emit-relocs we produce

  [ 1] .text             PROGBITS        00000000002000e8 0000e8 000010 00   A  0   0  1
  [ 2] .reloc.text.foo   RELA            0000000000000000 0000f8 000018 18      5   1  1
  [ 3] .reloc.text.bar   RELA            0000000000000000 000110 000018 18      5   1  1

Both bfd and gold can handle this. We should merge two reloc sections if
and only if the sections they relocate are merged. We should also
completely ignore their names.

Can you try fixing this first? I have the impression that with that fixed
this patch will be much simpler.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: test.yaml
Type: application/octet-stream
Size: 1027 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170601/e92c868a/attachment.obj>
-------------- next part --------------

Cheers,
Rafael