[PATCH] D30163: [ELF] - Postpone evaluation of LMA offset.

[Rafael Avila de Espindola via llvm-commits]

This is a bit confusing. Why do we need to hold information in
LinkerScript.h? OutputSectionCommand *Cmd holds the expression, can't we
just evaluate it once the virtual addresses are computed?

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

> grimar created this revision.
>
> Previously we evaluated the values of LMA incorrectly for next cases:
>
>   .text : AT(ADDR(.text) - 0xffffffff80000000) { ... }
>   .data : AT(ADDR(.data) - 0xffffffff80000000) { .. }
>   .init.begin : AT(ADDR(.init.begin) - 0xffffffff80000000) { ... }
>
> Reason was that we evaluated offset when VA was not assigned. For case above
> we ended up with 3 loads that has similar LMA and it was incorrect.
> That is critical for linux kernel.
>
> Patch updates the offset after VA calculation. That fixes the issue I observing.
>
>
> https://reviews.llvm.org/D30163
>
> Files:
>   ELF/LinkerScript.cpp
>   ELF/LinkerScript.h
>   test/ELF/linkerscript/at-addr.s
>
> Index: test/ELF/linkerscript/at-addr.s
> ===================================================================
> --- test/ELF/linkerscript/at-addr.s
> +++ test/ELF/linkerscript/at-addr.s
> @@ -0,0 +1,102 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
> +# RUN: echo "SECTIONS { . = 0x1000; \
> +# RUN:  .aaa : AT(ADDR(.aaa) - 0x500) { *(.aaa) } \
> +# RUN:  .bbb : AT(ADDR(.bbb) - 0x500) { *(.bbb) } \
> +# RUN:  .ccc : AT(ADDR(.ccc) - 0x500) { *(.ccc) } \
> +# RUN: }" > %t.script
> +# RUN: ld.lld %t --script %t.script -o %t2
> +# RUN: llvm-readobj -program-headers %t2 | FileCheck %s
> +
> +# CHECK:      ProgramHeaders [
> +# CHECK-NEXT:   ProgramHeader {
> +# CHECK-NEXT:     Type: PT_PHDR
> +# CHECK-NEXT:     Offset: 0x40
> +# CHECK-NEXT:     VirtualAddress: 0x40
> +# CHECK-NEXT:     PhysicalAddress: 0x40
> +# CHECK-NEXT:     FileSize:
> +# CHECK-NEXT:     MemSize:
> +# CHECK-NEXT:     Flags [
> +# CHECK-NEXT:       PF_R
> +# CHECK-NEXT:     ]
> +# CHECK-NEXT:     Alignment: 8
> +# CHECK-NEXT:   }
> +# CHECK-NEXT:   ProgramHeader {
> +# CHECK-NEXT:     Type: PT_LOAD
> +# CHECK-NEXT:     Offset: 0x0
> +# CHECK-NEXT:     VirtualAddress: 0x0
> +# CHECK-NEXT:     PhysicalAddress: 0x0
> +# CHECK-NEXT:     FileSize:
> +# CHECK-NEXT:     MemSize:
> +# CHECK-NEXT:     Flags [
> +# CHECK-NEXT:       PF_R
> +# CHECK-NEXT:       PF_X
> +# CHECK-NEXT:     ]
> +# CHECK-NEXT:     Alignment:
> +# CHECK-NEXT:   }
> +# CHECK-NEXT:   ProgramHeader {
> +# CHECK-NEXT:     Type: PT_LOAD
> +# CHECK-NEXT:     Offset: 0x1000
> +# CHECK-NEXT:     VirtualAddress: 0x1000
> +# CHECK-NEXT:     PhysicalAddress: 0xB00
> +# CHECK-NEXT:     FileSize: 8
> +# CHECK-NEXT:     MemSize: 8
> +# CHECK-NEXT:     Flags [
> +# CHECK-NEXT:       PF_R
> +# CHECK-NEXT:       PF_X
> +# CHECK-NEXT:     ]
> +# CHECK-NEXT:     Alignment:
> +# CHECK-NEXT:   }
> +# CHECK-NEXT:   ProgramHeader {
> +# CHECK-NEXT:     Type: PT_LOAD
> +# CHECK-NEXT:     Offset: 0x1008
> +# CHECK-NEXT:     VirtualAddress: 0x1008
> +# CHECK-NEXT:     PhysicalAddress: 0xB08
> +# CHECK-NEXT:     FileSize: 8
> +# CHECK-NEXT:     MemSize: 8
> +# CHECK-NEXT:     Flags [
> +# CHECK-NEXT:       PF_R
> +# CHECK-NEXT:       PF_X
> +# CHECK-NEXT:     ]
> +# CHECK-NEXT:     Alignment: 4096
> +# CHECK-NEXT:   }
> +# CHECK-NEXT:   ProgramHeader {
> +# CHECK-NEXT:     Type: PT_LOAD
> +# CHECK-NEXT:     Offset: 0x1010
> +# CHECK-NEXT:     VirtualAddress: 0x1010
> +# CHECK-NEXT:     PhysicalAddress: 0xB10
> +# CHECK-NEXT:     FileSize: 9
> +# CHECK-NEXT:     MemSize: 9
> +# CHECK-NEXT:     Flags [
> +# CHECK-NEXT:       PF_R
> +# CHECK-NEXT:       PF_X
> +# CHECK-NEXT:     ]
> +# CHECK-NEXT:     Alignment: 4096
> +# CHECK-NEXT:   }
> +# CHECK-NEXT:   ProgramHeader {
> +# CHECK-NEXT:     Type: PT_GNU_STACK
> +# CHECK-NEXT:     Offset:
> +# CHECK-NEXT:     VirtualAddress: 0x0
> +# CHECK-NEXT:     PhysicalAddress: 0x0
> +# CHECK-NEXT:     FileSize:
> +# CHECK-NEXT:     MemSize:
> +# CHECK-NEXT:     Flags [
> +# CHECK-NEXT:       PF_R
> +# CHECK-NEXT:       PF_W
> +# CHECK-NEXT:     ]
> +# CHECK-NEXT:     Alignment: 0
> +# CHECK-NEXT:   }
> +# CHECK-NEXT: ]
> +
> +.global _start
> +_start:
> + nop
> +
> +.section .aaa, "a"
> +.quad 0
> +
> +.section .bbb, "a"
> +.quad 0
> +
> +.section .ccc, "a"
> +.quad 0
> Index: ELF/LinkerScript.h
> ===================================================================
> --- ELF/LinkerScript.h
> +++ ELF/LinkerScript.h
> @@ -295,7 +295,7 @@
>                                   OutputSectionBase *Sec);
>  
>    uintX_t Dot;
> -  uintX_t LMAOffset = 0;
> +  std::pair<Expr, uintX_t> CurLMA;
>    OutputSectionBase *CurOutSec = nullptr;
>    MemoryRegion *CurMemRegion = nullptr;
>    uintX_t ThreadBssOffset = 0;
> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -462,7 +462,9 @@
>    // will set the LMA such that the difference between VMA and LMA for the
>    // section is the same as the preceding output section in the same region
>    // https://sourceware.org/binutils/docs-2.20/ld/Output-Section-LMA.html
> -  CurOutSec->setLMAOffset(LMAOffset);
> +  Expr LMAExpr = CurLMA.first;
> +  if (LMAExpr)
> +    CurOutSec->setLMAOffset(LMAExpr(CurLMA.second) - CurLMA.second);
>  }
>  
>  template <class ELFT> void LinkerScript<ELFT>::process(BaseCommand &Base) {
> @@ -561,7 +563,7 @@
>  template <class ELFT>
>  void LinkerScript<ELFT>::assignOffsets(OutputSectionCommand *Cmd) {
>    if (Cmd->LMAExpr)
> -    LMAOffset = Cmd->LMAExpr(Dot) - Dot;
> +    CurLMA = {Cmd->LMAExpr, Dot};
>    OutputSectionBase *Sec = findSection<ELFT>(Cmd->Name, *OutputSections);
>    if (!Sec)
>      return;