[LLD] Only increase LMARegion if different from MemRegion

[kschwarz]

Summary

If both the MemRegion and LMARegion are set for an output section in a linker script, we should only increase the LMARegion if it is different from the MemRegion. Otherwise, we reserve the memory twice.

[grimar]

Overall looks good to me. Comments are below.
(I would rename the test from at8.s to at6.s though, since we do not have at6.s yet.)

test/ELF/linkerscript/at8.test

10 ↗

(On Diff #158258)

We usually try to reduce the test cases.
This alias seems to be excessive here, so I would remove.
Otherwise, it looks like having an alias is important, though it is not.

17 ↗

(On Diff #158258)

Please add a brief comment describing what are you checking in this test case.

I would also add a header to show what is 0000000020000000 and 001000.
(like you have for program headers below).

21 ↗

(On Diff #158258)

I think this line does not make sense. All LOADs are always listed after the header:
Type Offset VirtAddr PhysAddr
and you have CHECK-NEXT after it.

[kschwarz]

Address review comments.

This patch was split from https://reviews.llvm.org/D50052, which adds at6.test and at7.test

[ruiu]

test/ELF/linkerscript/at8.test

11 ↗

(On Diff #158276)

This seems to be equivalent to .text : { *(.text) } > RAM. What is the point of appending AT> RAM?

[kschwarz]

test/ELF/linkerscript/at8.test

11 ↗

(On Diff #158276)

If used together with the REGION_ALIAS feature, this becomes more clear.
Often, the MEMORY commands and SECTIONS commands are kept in different linker scripts and the REGION_ALIAS feature is used to keep them independent.
This way, we can use the same linker script containing the SECTIONS commands and just switch the linker script containing the MEMORY commands for different targets.
Then, one target might map RAM2 to some flash region, while the other target simply maps to the same ram region.

[grimar]

LGTM with test case renamed to at6.s (since D50052 is not yet landed).

test/ELF/linkerscript/at8.test

25 ↗

(On Diff #158276)

Excessive empty line.

29 ↗

(On Diff #158276)

Excessive empty line.

[kschwarz]

Rename at8.test to at6.test

lld/trunk/ELF/LinkerScript.cpp

if (NewSize > MemRegion->Length)
     error("section '" + SecName + "' will not fit in region '" + RegionName +
           "': overflowed by " + Twine(NewSize - MemRegion->Length) + " bytes");
 }
 
 void LinkerScript::expandMemoryRegions(uint64_t Size) {
   if (Ctx->MemRegion)
     expandMemoryRegion(Ctx->MemRegion, Size, Ctx->MemRegion->Name,
                        Ctx->OutSec->Name);
-  if (Ctx->LMARegion)
+  // Only expand the LMARegion if it is different from MemRegion.
+  if (Ctx->LMARegion && Ctx->MemRegion != Ctx->LMARegion)
     expandMemoryRegion(Ctx->LMARegion, Size, Ctx->LMARegion->Name,
                        Ctx->OutSec->Name);
 }
 
 void LinkerScript::expandOutputSection(uint64_t Size) {
   Ctx->OutSec->Size += Size;
   expandMemoryRegions(Size);
 }

lld/trunk/test/ELF/linkerscript/Inputs/at7.s

+.global _start
+.text
+_start:
+nop
+
+.section .sec, "aw"
+.word 4

lld/trunk/test/ELF/linkerscript/at7.test

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/at7.s -o %t.o
+# RUN: ld.lld %t.o --script %s -o %t
+# RUN: llvm-readelf -sections -program-headers %t | FileCheck %s
+
+MEMORY {
+  RAM    : ORIGIN = 0x20000000, LENGTH = 0x200
+}
+
+SECTIONS {
+ .text : { *(.text) } > RAM AT> RAM
+ .sec  : { *(.sec)  } > RAM
+}
+
+# Make sure the memory for the .text section is only reserved once.
+# Previously, the location counter for both MemRegion and LMARegion
+# was increased unconditionally.
+
+
+# CHECK: Name              Type            Address          Off
+# CHECK: .text             PROGBITS        0000000020000000 001000
+# CHECK: .sec             PROGBITS        0000000020000001 001001
+
+# CHECK: Program Headers:
+# CHECK:      Type  Offset   VirtAddr           PhysAddr
+# CHECK-NEXT: LOAD  0x001000 0x0000000020000000 0x0000000020000000
+# CHECK-NEXT: LOAD  0x001001 0x0000000020000001 0x0000000020000001
+# CHECK-NOT: LOAD