[PATCH] D43284: [ELF] Simplify handling of AT section attribute.

[Rafael Avila de Espindola via llvm-commits]

LGTM

Igor Kudrin via Phabricator <reviews at reviews.llvm.org> writes:

> ikudrin updated this revision to Diff 134376.
> ikudrin added a comment.
>
> Update according to Rafael's comments:
>
> - Replace `!FirstLoadSection` with `Load->LastSec != Out::ProgramHeaders`.
> - Add a corresponding comment.
>
>
> https://reviews.llvm.org/D43284
>
> Files:
>   ELF/Writer.cpp
>   ELF/Writer.h
>   test/ELF/linkerscript/at5.s
>
>
> Index: test/ELF/linkerscript/at5.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/linkerscript/at5.s
> @@ -0,0 +1,36 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
> +# RUN: echo "SECTIONS { \
> +# RUN:  . = 0x1000; \
> +# RUN:  .aaa : { *(.aaa) } \
> +# RUN:  .bbb : AT(0x2008) { *(.bbb) } \
> +# RUN:  .ccc : { *(.ccc) } \
> +# RUN: }" > %t.script
> +# RUN: ld.lld %t --script %t.script -o %t2
> +# RUN: llvm-readobj -program-headers %t2 | FileCheck %s
> +
> +# CHECK:        Type: PT_LOAD
> +# CHECK-NEXT:     Offset: 0x1000
> +# CHECK-NEXT:     VirtualAddress: 0x1000
> +# CHECK-NEXT:     PhysicalAddress: 0x1000
> +# CHECK-NEXT:     FileSize: 8
> +# CHECK-NEXT:     MemSize: 8
> +# CHECK:        Type: PT_LOAD
> +# CHECK-NEXT:     Offset: 0x1008
> +# CHECK-NEXT:     VirtualAddress: 0x1008
> +# CHECK-NEXT:     PhysicalAddress: 0x2008
> +# CHECK-NEXT:     FileSize: 17
> +# CHECK-NEXT:     MemSize: 17
> +
> +.global _start
> +_start:
> + nop
> +
> +.section .aaa, "a"
> +.quad 0
> +
> +.section .bbb, "a"
> +.quad 0
> +
> +.section .ccc, "a"
> +.quad 0
> Index: ELF/Writer.h
> ===================================================================
> --- ELF/Writer.h
> +++ ELF/Writer.h
> @@ -45,11 +45,6 @@
>    OutputSection *LastSec = nullptr;
>    bool HasLMA = false;
>  
> -  // True if one of the sections in this program header has a LMA specified via
> -  // linker script: AT(addr). We never allow 2 or more sections with LMA in the
> -  // same program header.
> -  bool ASectionHasLMA = false;
> -
>    uint64_t LMAOffset = 0;
>  };
>  
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -821,8 +821,6 @@
>    p_align = std::max(p_align, Sec->Alignment);
>    if (p_type == PT_LOAD)
>      Sec->PtLoad = this;
> -  if (Sec->LMAExpr)
> -    ASectionHasLMA = true;
>  }
>  
>  // The beginning and the ending of .rel[a].plt section are marked
> @@ -1697,9 +1695,11 @@
>      // (e.g. executable or writable). There is one phdr for each segment.
>      // Therefore, we need to create a new phdr when the next section has
>      // different flags or is loaded at a discontiguous address using AT linker
> -    // script command.
> +    // script command. At the same time, we don't want to create a separate
> +    // load segment for the headers, even if the first output section has
> +    // an AT attribute.
>      uint64_t NewFlags = computeFlags(Sec->getPhdrFlags());
> -    if ((Sec->LMAExpr && Load->ASectionHasLMA) ||
> +    if ((Sec->LMAExpr && Load->LastSec != Out::ProgramHeaders) ||
>          Sec->MemRegion != Load->FirstSec->MemRegion || Flags != NewFlags) {
>  
>        Load = AddHdr(PT_LOAD, NewFlags);
>
>
> Index: test/ELF/linkerscript/at5.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/linkerscript/at5.s
> @@ -0,0 +1,36 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
> +# RUN: echo "SECTIONS { \
> +# RUN:  . = 0x1000; \
> +# RUN:  .aaa : { *(.aaa) } \
> +# RUN:  .bbb : AT(0x2008) { *(.bbb) } \
> +# RUN:  .ccc : { *(.ccc) } \
> +# RUN: }" > %t.script
> +# RUN: ld.lld %t --script %t.script -o %t2
> +# RUN: llvm-readobj -program-headers %t2 | FileCheck %s
> +
> +# CHECK:        Type: PT_LOAD
> +# CHECK-NEXT:     Offset: 0x1000
> +# CHECK-NEXT:     VirtualAddress: 0x1000
> +# CHECK-NEXT:     PhysicalAddress: 0x1000
> +# CHECK-NEXT:     FileSize: 8
> +# CHECK-NEXT:     MemSize: 8
> +# CHECK:        Type: PT_LOAD
> +# CHECK-NEXT:     Offset: 0x1008
> +# CHECK-NEXT:     VirtualAddress: 0x1008
> +# CHECK-NEXT:     PhysicalAddress: 0x2008
> +# CHECK-NEXT:     FileSize: 17
> +# CHECK-NEXT:     MemSize: 17
> +
> +.global _start
> +_start:
> + nop
> +
> +.section .aaa, "a"
> +.quad 0
> +
> +.section .bbb, "a"
> +.quad 0
> +
> +.section .ccc, "a"
> +.quad 0
> Index: ELF/Writer.h
> ===================================================================
> --- ELF/Writer.h
> +++ ELF/Writer.h
> @@ -45,11 +45,6 @@
>    OutputSection *LastSec = nullptr;
>    bool HasLMA = false;
>  
> -  // True if one of the sections in this program header has a LMA specified via
> -  // linker script: AT(addr). We never allow 2 or more sections with LMA in the
> -  // same program header.
> -  bool ASectionHasLMA = false;
> -
>    uint64_t LMAOffset = 0;
>  };
>  
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -821,8 +821,6 @@
>    p_align = std::max(p_align, Sec->Alignment);
>    if (p_type == PT_LOAD)
>      Sec->PtLoad = this;
> -  if (Sec->LMAExpr)
> -    ASectionHasLMA = true;
>  }
>  
>  // The beginning and the ending of .rel[a].plt section are marked
> @@ -1697,9 +1695,11 @@
>      // (e.g. executable or writable). There is one phdr for each segment.
>      // Therefore, we need to create a new phdr when the next section has
>      // different flags or is loaded at a discontiguous address using AT linker
> -    // script command.
> +    // script command. At the same time, we don't want to create a separate
> +    // load segment for the headers, even if the first output section has
> +    // an AT attribute.
>      uint64_t NewFlags = computeFlags(Sec->getPhdrFlags());
> -    if ((Sec->LMAExpr && Load->ASectionHasLMA) ||
> +    if ((Sec->LMAExpr && Load->LastSec != Out::ProgramHeaders) ||
>          Sec->MemRegion != Load->FirstSec->MemRegion || Flags != NewFlags) {
>  
>        Load = AddHdr(PT_LOAD, NewFlags);