[PATCH] D27516: [ELF] - Place ".text" to be first section if -Ttext is used.

[Rafael Avila de Espindola via llvm-commits]

Oh, is the loader trying to set the address of the section .text or of
the PT_LOAD? If the section, what exactly is it trying to do?

If it has expectations on the section order it seems better to use a
linker script.

If what it really wants is to set a particular section at a particular
address, I think we can implement that, but as Rui points out we should
have a general solution.

One such solution would be to put all section with specified addresses
before other sections and to order them based of the address value.

I don't think we should try to do something fancier like looking for an
empty spot when placing each section.


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

> grimar created this revision.
> grimar added reviewers: ruiu, rafael.
> grimar added subscribers: llvm-commits, grimar, evgeny777.
>
> Some loaders, for example one from PR31295 use -N -Ttext combination.
> Previously our output was in next order:
>
>   .rodata
>   .text
>   ...
>
> That caused an issue. Imagine .rodata was assigned to 0x10000 and then -Ttext sets .text below that.
> That could cause sections addresses intersections.
> Both GNU linkers able handles that correctly.
>
> gold just places .text to be first section. In that case
> we can start assigning VA from -Ttext value if was specified and do that in a forward loop, just like we 
> already do in void Writer<ELFT>::assignAddresses().
> That is what this patch implements.
>
>
> https://reviews.llvm.org/D27516
>
> Files:
>   ELF/Writer.cpp
>   test/ELF/ttext.s
>
>
> Index: test/ELF/ttext.s
> ===================================================================
> --- test/ELF/ttext.s
> +++ test/ELF/ttext.s
> @@ -0,0 +1,27 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
> +
> +# RUN: ld.lld -N %t -o %t1
> +# RUN: llvm-objdump -section-headers %t1 | FileCheck %s
> +# CHECK:      Sections:
> +# CHECK-NEXT: Idx Name         Size      Address         Type
> +# CHECK-NEXT:   0              00000000 0000000000000000
> +# CHECK-NEXT:   1 .rodata      00000008 00000000002000e8 DATA
> +# CHECK-NEXT:   2 .text        00000001 00000000002000f0 TEXT DATA
> +
> +## Check that sections does not intersect if -Ttext used.
> +# RUN: ld.lld -N -Ttext 0x2000e8 %t -o %t1
> +# RUN: llvm-objdump -section-headers %t1 | FileCheck %s --check-prefix=TEXT
> +# TEXT:      Sections:
> +# TEXT-NEXT: Idx Name       Size      Address         Type
> +# TEXT-NEXT:   0            00000000 0000000000000000
> +# TEXT-NEXT:   1 .text      00000001 00000000002000e8 TEXT DATA
> +# TEXT-NEXT:   2 .rodata    00000008 00000000002000e9 DATA
> +
> +.text
> +.globl _start
> +_start:
> + nop
> +
> +.section .rodata,"a"
> + .quad 1
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -482,7 +482,18 @@
>  template <class ELFT>
>  static bool compareSectionsNonScript(const OutputSectionBase *A,
>                                       const OutputSectionBase *B) {
> -  // Put .interp first because some loaders want to see that section
> +  // Some loaders use -Ttext option to set .text section address.
> +  // In that case we place it at the begining to be able to
> +  // assign VA in a simple forward loop later. That is consistent
> +  // with what GNU gold do.
> +  if (Config->SectionStartMap.count(".text")) {
> +    bool AIsText = A->getName() == ".text";
> +    bool BIsText = B->getName() == ".text";
> +    if (AIsText != BIsText)
> +      return AIsText;
> +  }
> +
> +  // Put .interp early because some loaders want to see that section
>    // on the first page of the executable file when loaded into memory.
>    bool AIsInterp = A->getName() == ".interp";
>    bool BIsInterp = B->getName() == ".interp";
>
>
> Index: test/ELF/ttext.s
> ===================================================================
> --- test/ELF/ttext.s
> +++ test/ELF/ttext.s
> @@ -0,0 +1,27 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
> +
> +# RUN: ld.lld -N %t -o %t1
> +# RUN: llvm-objdump -section-headers %t1 | FileCheck %s
> +# CHECK:      Sections:
> +# CHECK-NEXT: Idx Name         Size      Address         Type
> +# CHECK-NEXT:   0              00000000 0000000000000000
> +# CHECK-NEXT:   1 .rodata      00000008 00000000002000e8 DATA
> +# CHECK-NEXT:   2 .text        00000001 00000000002000f0 TEXT DATA
> +
> +## Check that sections does not intersect if -Ttext used.
> +# RUN: ld.lld -N -Ttext 0x2000e8 %t -o %t1
> +# RUN: llvm-objdump -section-headers %t1 | FileCheck %s --check-prefix=TEXT
> +# TEXT:      Sections:
> +# TEXT-NEXT: Idx Name       Size      Address         Type
> +# TEXT-NEXT:   0            00000000 0000000000000000
> +# TEXT-NEXT:   1 .text      00000001 00000000002000e8 TEXT DATA
> +# TEXT-NEXT:   2 .rodata    00000008 00000000002000e9 DATA
> +
> +.text
> +.globl _start
> +_start:
> + nop
> +
> +.section .rodata,"a"
> + .quad 1
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -482,7 +482,18 @@
>  template <class ELFT>
>  static bool compareSectionsNonScript(const OutputSectionBase *A,
>                                       const OutputSectionBase *B) {
> -  // Put .interp first because some loaders want to see that section
> +  // Some loaders use -Ttext option to set .text section address.
> +  // In that case we place it at the begining to be able to
> +  // assign VA in a simple forward loop later. That is consistent
> +  // with what GNU gold do.
> +  if (Config->SectionStartMap.count(".text")) {
> +    bool AIsText = A->getName() == ".text";
> +    bool BIsText = B->getName() == ".text";
> +    if (AIsText != BIsText)
> +      return AIsText;
> +  }
> +
> +  // Put .interp early because some loaders want to see that section
>    // on the first page of the executable file when loaded into memory.
>    bool AIsInterp = A->getName() == ".interp";
>    bool BIsInterp = B->getName() == ".interp";