[PATCH] D29663: [ELF] - Added partial support for --emit-relocs (no --gc-section case, no /DISCARD/ support) #3

[George Rimar via llvm-commits]

>> Index: test/ELF/linkerscript/emit-reloc-discard.s
>> ===================================================================
>> --- test/ELF/linkerscript/emit-reloc-discard.s
>> +++ test/ELF/linkerscript/emit-reloc-discard.s
>> @@ -0,0 +1,12 @@
>> +# REQUIRES: x86
>> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
>> +# RUN: echo "SECTIONS { /DISCARD/ : { *(.bbb) } }" > %t.script
>> +# RUN: not ld.lld --emit-relocs --script %t.script %t.o -o %t1 2>&1 | FileCheck %s
>> +
>> +# CHECK: can not discard target section .bbb due to an implementation limitation of --emit-relocs
>
>Leave this out. The idea is to split the patch so that the first one is
>simple. No point adding an error that will be immediately removed on
>the next patch.
>
>> Index: test/ELF/emit-relocs.s
>> ===================================================================
>> --- test/ELF/emit-relocs.s
>> +++ test/ELF/emit-relocs.s
>> @@ -0,0 +1,98 @@
>> +# REQUIRES: x86
>> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o
.> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/emit-relocs.s -o %t2.o
>
>Why do you need two files?
>
>> Index: test/ELF/emit-relocs-gc.s
>> ===================================================================
>> --- test/ELF/emit-relocs-gc.s
>> +++ test/ELF/emit-relocs-gc.s
.> @@ -0,0 +1,5 @@
>> +# REQUIRES: x86
>> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1.o
>> +# RUN: not ld.lld --gc-sections --emit-relocs %t1.o -o %t 2>&1 \
>> +# RUN:    | FileCheck -check-prefix=ERR %s
>> +# ERR: --emit-relocs and --gc-sections may not be used together
>
>Leave this out.
>
>> Index: ELF/OutputSections.cpp
>> ===================================================================
>> --- ELF/OutputSections.cpp
>> +++ ELF/OutputSections.cpp
>> @@ -116,7 +116,8 @@
>>    }
>>
>>    uint32_t Type = this->Type;
>> -  if (!Config->Relocatable || (Type != SHT_RELA && Type != SHT_REL))
>> +  if ((!Config->Relocatable && !Config->EmitRelocs) ||
>> +      (Type != SHT_RELA && Type != SHT_REL))
>>      return;
>
>It does look like a Config->copyRelocs() that return
>Config->Relocatable || Config->EmitRelocs would be nice.
>
>> Index: ELF/LinkerScript.cpp
>> ===================================================================
>> --- ELF/LinkerScript.cpp
>> +++ ELF/LinkerScript.cpp
>> @@ -263,11 +263,19 @@
>>    }
>>  }
>>
>> +template <class ELFT> static bool hasRelocs(InputSectionBase<ELFT> *S) {
>> +  return S->AreRelocsRela ? !S->relas().empty() : !S->relas().empty();
>> +}
>> +
>>  template <class ELFT>
>>  void LinkerScript<ELFT>::discard(ArrayRef<InputSectionBase<ELFT> *> V) {
>>    for (InputSectionBase<ELFT> *S : V) {
>>      S->Live = false;
>>      reportDiscarded(S);
>> +
>> +    if (Config->EmitRelocs && hasRelocs(S))
>> +      error("can not discard target section " + S->Name +
>> +            " due to an implementation limitation of --emit-relocs");
>
>Leave this out.
>
>
>>  template <class ELFT>
>>  template <class RelTy>
>>  void InputSection<ELFT>::copyRelocations(uint8_t *Buf, ArrayRef<RelTy> Rels) {
>> @@ -235,7 +235,11 @@
>>
>>      if (Config->Rela)
>>        P->r_addend = getAddend<ELFT>(Rel);
>> -    P->r_offset = RelocatedSection->getOffset(Rel.r_offset);
>> +
>> +    // Output section VA is zero for -r, so r_offset is an offset within the
>> +    // section, but for --emit-relocs it is an absolute address.
>> +    P->r_offset = RelocatedSection->OutSec->Addr +
>> +                  RelocatedSection->getOffset(Rel.r_offset);
>
>s/absolute/virtual/
>
>
>> Index: ELF/Driver.cpp
>> ===================================================================
>> --- ELF/Driver.cpp
>> +++ ELF/Driver.cpp
>> @@ -234,6 +234,11 @@
>>      if (Config->Pie)
>>        error("-r and -pie may not be used together");
>>    }
>> +
>> +  // -emit-relocs and -gc-sections don't conflict in theory, but LLD currently
>> +  // does not support the combination due to an implementation limitation.
>> +  if (Config->EmitRelocs && Config->GcSections)
>> +    error("--emit-relocs and --gc-sections may not be used together");
>
>Leave this out.
>
>Almost there :-)
>
>Cheers,
>Rafael

Thanks for review, all comments addressed in the latest diff.

George.