[PATCH] D43999: [ELF] - Support moving location counter when MEMORY is used.

[Rafael Avila de Espindola via llvm-commits]

This is awesome!

LGTM

Thanks,
Rafael

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

> grimar created this revision.
> grimar added reviewers: ruiu, rafael.
> Herald added subscribers: arichardson, emaste.
>
> We do not expand memory region correctly for following scripts:
>
>   .foo.1 : 
>    {
>      *(.foo.1)
>      . += 0x1000;
>    } > ram
>
> Patch generalizes expanding of output sections and memory
> regions in one place and fixes the issue.
>
>
> https://reviews.llvm.org/D43999
>
> Files:
>   ELF/LinkerScript.cpp
>   ELF/LinkerScript.h
>   test/ELF/linkerscript/memory-loc-counter.test
>
> Index: test/ELF/linkerscript/memory-loc-counter.test
> ===================================================================
> --- test/ELF/linkerscript/memory-loc-counter.test
> +++ test/ELF/linkerscript/memory-loc-counter.test
> @@ -0,0 +1,37 @@
> +# REQUIRES: x86
> +
> +# RUN: echo ".section .foo,\"a\"" > %t.s
> +# RUN: echo ".quad 1" >> %t.s
> +# RUN: echo ".section .bar,\"a\"" >> %t.s
> +# RUN: echo ".quad 1" >> %t.s
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %t.s -o %t.o
> +
> +# RUN: ld.lld -o %t %t.o --script %s
> +# RUN: llvm-readelf -sections -program-headers %t | FileCheck %s
> +
> +## Check that we can produce output without errors,
> +## and .foo section has proper size.
> +# CHECK:      Section Headers:
> +# CHECK-NEXT:   [Nr] Name Type     Address          Off    Size
> +# CHECK-NEXT:   [ 0]      NULL     0000000000000000 000000 000000
> +# CHECK-NEXT:   [ 1] .foo PROGBITS 0000000000001000 001000 000108
> +# CHECK-NEXT:   [ 2] .bar PROGBITS 0000000000001108 001108 000008
> +
> +## Check that load address is correct.
> +# CHECK:      Program Headers:
> +# CHECK-NEXT:   Type Offset   VirtAddr           PhysAddr           FileSiz  MemSiz
> +# CHECK-NEXT:   LOAD 0x001000 0x0000000000001000 0x0000000000002000 0x000110 0x000110
> +
> +MEMORY {
> +  ram (rwx)   : org = 0x1000, len = 0x200
> +  flash (rwx) : org = 0x2000, len = 0x200
> +}
> +SECTIONS {
> +  .foo : {
> +    *(.foo)
> +    . += 0x100;
> +  } > ram AT>flash
> +  .bar : {
> +    *(.bar)
> +  } > ram AT>flash
> +}
> Index: ELF/LinkerScript.h
> ===================================================================
> --- ELF/LinkerScript.h
> +++ ELF/LinkerScript.h
> @@ -215,6 +215,7 @@
>    void addSymbol(SymbolAssignment *Cmd);
>    void assignSymbol(SymbolAssignment *Cmd, bool InSec);
>    void setDot(Expr E, const Twine &Loc, bool InSec);
> +  void expandOutputSection(uint64_t Size);
>  
>    std::vector<InputSection *>
>    computeInputSections(const InputSectionDescription *);
> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -102,16 +102,36 @@
>    return CmdRef;
>  }
>  
> +// Expands the memory region by the specified size.
> +static void expandMemoryRegion(MemoryRegion *MemRegion, uint64_t Size,
> +                               StringRef RegionName, StringRef SecName) {
> +  MemRegion->CurPos += Size;
> +  uint64_t NewSize = MemRegion->CurPos - MemRegion->Origin;
> +  if (NewSize > MemRegion->Length)
> +    error("section '" + SecName + "' will not fit in region '" + RegionName +
> +          "': overflowed by " + Twine(NewSize - MemRegion->Length) + " bytes");
> +}
> +
> +void LinkerScript::expandOutputSection(uint64_t Size) {
> +  Ctx->OutSec->Size += Size;
> +  if (Ctx->MemRegion)
> +    expandMemoryRegion(Ctx->MemRegion, Size, Ctx->MemRegion->Name,
> +                       Ctx->OutSec->Name);
> +  // FIXME: check LMA region overflow too.
> +  if (Ctx->LMARegion)
> +    Ctx->LMARegion->CurPos += Size;
> +}
> +
>  void LinkerScript::setDot(Expr E, const Twine &Loc, bool InSec) {
>    uint64_t Val = E().getValue();
>    if (Val < Dot && InSec)
>      error(Loc + ": unable to move location counter backward for: " +
>            Ctx->OutSec->Name);
> -  Dot = Val;
>  
>    // Update to location counter means update to section size.
>    if (InSec)
> -    Ctx->OutSec->Size = Dot - Ctx->OutSec->Addr;
> +    expandOutputSection(Val - Dot);
> +  Dot = Val;
>  }
>  
>  // Used for handling linker symbol assignments, for both finalizing
> @@ -621,35 +641,15 @@
>    return End;
>  }
>  
> -// Expands the memory region by the specified size.
> -static void expandMemoryRegion(MemoryRegion *MemRegion, uint64_t Size,
> -                               StringRef RegionName, StringRef SecName) {
> -  MemRegion->CurPos += Size;
> -  uint64_t NewSize = MemRegion->CurPos - MemRegion->Origin;
> -  if (NewSize > MemRegion->Length)
> -    error("section '" + SecName + "' will not fit in region '" + RegionName +
> -          "': overflowed by " + Twine(NewSize - MemRegion->Length) + " bytes");
> -}
> -
>  void LinkerScript::output(InputSection *S) {
>    uint64_t Before = advance(0, 1);
>    uint64_t Pos = advance(S->getSize(), S->Alignment);
>    S->OutSecOff = Pos - S->getSize() - Ctx->OutSec->Addr;
>  
>    // Update output section size after adding each section. This is so that
>    // SIZEOF works correctly in the case below:
>    // .foo { *(.aaa) a = SIZEOF(.foo); *(.bbb) }
> -  Ctx->OutSec->Size = Pos - Ctx->OutSec->Addr;
> -
> -  // If there is a memory region associated with this input section, then
> -  // place the section in that region and update the region index.
> -  if (Ctx->LMARegion)
> -    Ctx->LMARegion->CurPos += Pos - Before;
> -  // FIXME: should we also produce overflow errors for LMARegion?
> -
> -  if (Ctx->MemRegion)
> -    expandMemoryRegion(Ctx->MemRegion, Pos - Before, Ctx->MemRegion->Name,
> -                       Ctx->OutSec->Name);
> +  expandOutputSection(Pos - Before);
>  }
>  
>  void LinkerScript::switchTo(OutputSection *Sec) {
> @@ -739,12 +739,7 @@
>      if (auto *Cmd = dyn_cast<ByteCommand>(Base)) {
>        Cmd->Offset = Dot - Ctx->OutSec->Addr;
>        Dot += Cmd->Size;
> -      if (Ctx->MemRegion)
> -        expandMemoryRegion(Ctx->MemRegion, Cmd->Size, Ctx->MemRegion->Name,
> -                           Ctx->OutSec->Name);
> -      if (Ctx->LMARegion)
> -        Ctx->LMARegion->CurPos += Cmd->Size;
> -      Ctx->OutSec->Size = Dot - Ctx->OutSec->Addr;
> +      expandOutputSection(Cmd->Size);
>        continue;
>      }
>