Improve LMARegion handling

[• espindola]

Summary

This fixes the crash reported at PR36083.

The issue is that we were trying to put all the sections in the same PT_LOAD and crashing trying to write past the end of the file.

This also adds accounting for used space in LMARegion, without it all 3 PT_LOADs would have the same physical address.

[• espindola]

Rebased.

[grimar]

Looks good to me. Minor cosmetic comments/suggestions below.

test/ELF/linkerscript/at3.s

18

Excessive empty line.

26

I would add header:

Type           Offset   VirtAddr           PhysAddr

To make it clear where is Offset and VirtAddr and where is PhysAddr.

28

Looks you want to use CHECK-NEXT here ?

[• espindola]

r323449.

ELF/LinkerScript.h

class LinkerScript final {
   // Temporary state used in processSectionCommands() and assignAddresses()
   // that must be reinitialized for each call to the above functions, and must
   // not be used outside of the scope of a call to the above functions.
   struct AddressState {
     AddressState();
     uint64_t ThreadBssOffset = 0;
     OutputSection *OutSec = nullptr;
     MemoryRegion *MemRegion = nullptr;
+    MemoryRegion *LMARegion = nullptr;
     std::function<uint64_t()> LMAOffset;
   };
 
   llvm::DenseMap<StringRef, OutputSection *> NameToOutputSection;
 
   void addSymbol(SymbolAssignment *Cmd);
   void assignSymbol(SymbolAssignment *Cmd, bool InSec);
   void setDot(Expr E, const Twine &Loc, bool InSec);

ELF/LinkerScript.cpp

void LinkerScript::output(InputSection *S) {
 
   // 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) {
     uint64_t &CurOffset = Ctx->MemRegion->CurPos;
     CurOffset += Pos - Before;
     uint64_t CurSize = CurOffset - Ctx->MemRegion->Origin;
     if (CurSize > Ctx->MemRegion->Length) {
       uint64_t OverflowAmt = CurSize - Ctx->MemRegion->Length;
       error("section '" + Ctx->OutSec->Name + "' will not fit in region '" +
             Ctx->MemRegion->Name + "': overflowed by " + Twine(OverflowAmt) +

 // for a single sections command (e.g. ".text { *(.text); }").
 void LinkerScript::assignOffsets(OutputSection *Sec) {
   if (!(Sec->Flags & SHF_ALLOC))
     Dot = 0;
   else if (Sec->AddrExpr)
     setDot(Sec->AddrExpr, Sec->Location, false);
 
   Ctx->MemRegion = Sec->MemRegion;
+  Ctx->LMARegion = Sec->LMARegion;
   if (Ctx->MemRegion)
     Dot = Ctx->MemRegion->CurPos;
 
   switchTo(Sec);
 
   if (Sec->LMAExpr) {
     uint64_t D = Dot;
     Ctx->LMAOffset = [=] { return Sec->LMAExpr().getValue() - D; };
   }
 
   if (MemoryRegion *MR = Sec->LMARegion) {
-    uint64_t Offset = MR->Origin - Dot;
+    uint64_t Offset = MR->CurPos - Dot;
     Ctx->LMAOffset = [=] { return Offset; };
   }
 
   // If neither AT nor AT> is specified for an allocatable section, the linker
   // 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
   if (Ctx->LMAOffset)
for (BaseCommand *Base : Sec->SectionCommands) {
     }
 
     // Handle BYTE(), SHORT(), LONG(), or QUAD().
     if (auto *Cmd = dyn_cast<ByteCommand>(Base)) {
       Cmd->Offset = Dot - Ctx->OutSec->Addr;
       Dot += Cmd->Size;
       if (Ctx->MemRegion)
         Ctx->MemRegion->CurPos += Cmd->Size;
+      if (Ctx->LMARegion)
+        Ctx->LMARegion->CurPos += Cmd->Size;
       Ctx->OutSec->Size = Dot - Ctx->OutSec->Addr;
       continue;
     }
 
     // Handle ASSERT().
     if (auto *Cmd = dyn_cast<AssertCommand>(Base)) {
       Cmd->Expression();
       continue;

ELF/Writer.cpp

       continue;
 
     // Segments are contiguous memory regions that has the same attributes
     // (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.
     uint64_t NewFlags = computeFlags(Sec->getPhdrFlags());
-    if (Sec->LMAExpr || Flags != NewFlags) {
+    if (Sec->LMAExpr || Sec->MemRegion != Load->FirstSec->MemRegion ||
+        Flags != NewFlags) {
       Load = AddHdr(PT_LOAD, NewFlags);
       Flags = NewFlags;
     }
 
     Load->add(Sec);
   }
 
   // Add a TLS segment if any.

test/ELF/linkerscript/at3.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: echo "MEMORY {                                  \
+# RUN:   FOO   (ax) : ORIGIN = 0x1000, LENGTH = 0x100  \
+# RUN:   BAR   (ax) : ORIGIN = 0x2000, LENGTH = 0x100  \
+# RUN:   ZED   (ax) : ORIGIN = 0x3000, LENGTH = 0x100  \
+# RUN:   FLASH (ax) : ORIGIN = 0x6000, LENGTH = 0x200  \
+# RUN: }                                               \
+# RUN: SECTIONS {                                      \
+# RUN:  .foo1 : { *(.foo1) }            > FOO AT>FLASH \
+# RUN:  .foo2 : { *(.foo2) BYTE(0x42) } > BAR AT>FLASH \
+# RUN:  .foo3 : { *(.foo3) }            > ZED AT>FLASH \
+# RUN: }" > %t.script
+# RUN: ld.lld %t.o --script %t.script -o %t
+# RUN: llvm-readelf -sections -program-headers %t | FileCheck %s
+
+
+# CHECK: .foo1             PROGBITS        0000000000001000 001000
+# CHECK: .foo2             PROGBITS        0000000000002000 002000
+# CHECK: .foo3             PROGBITS        0000000000003000 003000
+
+# CHECK: Program Headers:
+# CHECK-NOT: LOAD
+
+# CHECK: LOAD  0x001000 0x0000000000001000 0x0000000000006000
+# CHECK: LOAD  0x002000 0x0000000000002000 0x0000000000006008
+# CHECK: LOAD  0x003000 0x0000000000003000 0x0000000000006011
+
+# CHECK-NOT: LOAD
+
+.section .foo1, "a"
+.quad 0
+
+.section .foo2, "ax"
+.quad 0
+
+.section .foo3, "ax"
+.quad 0