[ELF] Don't advance position in a memory region when assigning to the Dot

[MaskRay]

Summary

For memory5.test, ld.bfd appears to ignore . += 0x2000;, so the test was testing
a wrong behavior. After deleting the code added in rLLD336335, we match ld.bfd and thus fix PR41357.

PR37836 seems to have been fixed by another change.

[ruiu]

LGTM

Nice.

[grimar]

gold has a different behavior from bfd here btw.

[MaskRay]

In D62177#1509688, @grimar wrote:

gold has a different behavior from bfd here btw.

I think gold has a bug in its memory region matching. It uses boolean AND instead of boolean OR to find matching memory region... For example, in memory5.test, it fails to associate the two sections to the memory region and fails to report an error when there is no matching memory region...

[grimar]

I find the behavior of the GNU linkers wierd.

If we use bfd and have:

.section .foo,"ax";
nop;

.section .bar,"ax";
nop;

and script:

MEMORY {
  ram (ax) : ORIGIN = 0x42000, LENGTH = 0x100000
}
SECTIONS {
  .foo : { *(.foo*) }
  . += 0x2000;
  .bar : { *(.bar*) }
}

Then .foo section has address 0x42000 and .bar has 0x42001.
I.e. moving a dot does not change the address of the next section.

But if I remove MEMORY:

SECTIONS {
  .foo : { *(.foo*) }
  . += 0x2000;
  .bar : { *(.bar*) }
}

Then .foo is 0x0 and .bar is 0x2001 (as I would expect to see)

I think it is very strange that MEMORY command affects the Dot assignment behavior in that way.

Seems that original script from PR had to use a hack to move the second output section:

aligned_dot = ALIGN(0x10 * 1024);  
    
.data aligned_dot :
{
  *(.data*)
}

[MaskRay]

In D62177#1509729, @grimar wrote:

I find the behavior of the GNU linkers wierd.

If we use bfd and have:

.section .foo,"ax";
nop;

.section .bar,"ax";
nop;

and script:

MEMORY {
  ram (ax) : ORIGIN = 0x42000, LENGTH = 0x100000
}
SECTIONS {
  .foo : { *(.foo*) }
  . += 0x2000;
  .bar : { *(.bar*) }
}

Then .foo section has address 0x42000 and .bar has 0x42001.
I.e. moving a dot does not change the address of the next section.

But if I remove MEMORY:

SECTIONS {
  .foo : { *(.foo*) }
  . += 0x2000;
  .bar : { *(.bar*) }
}

Then .foo is 0x0 and .bar is 0x2001 (as I would expect to see)

I think it is very strange that MEMORY command affects the Dot assignment behavior in that way.

Seems that original script from PR had to use a hack to move the second output section:

aligned_dot = ALIGN(0x10 * 1024);  
    
.data aligned_dot :
{
  *(.data*)
}

I agree... The code near gold/script-sections.cc:2476 (in Output_section_definition::set_section_addresses) is responsible for this behavior

      vma_region = script_sections->find_memory_region(this, true, false, NULL);
      if (vma_region != NULL)  ////////// if it can find a matched memory region, dot_value will be ignored.
	address = vma_region->get_current_address()->eval(symtab, layout,
							  false);
      else
	address = *dot_value;

Our code responsible for this behavior is:

if (Ctx->MemRegion)
  Dot = Ctx->MemRegion->CurPos;

[peter.smith]

Thanks for the fix. The MEMORY command is indeed strange. I think that this behaviour is documented in https://sourceware.org/binutils/docs/ld/Output-Section-Address.html#Output-Section-Address

The output section address heuristic is as follows:

    If an output memory region is set for the section then it is added to this region and its address will be the next free address in that region.
    If the MEMORY command has been used to create a list of memory regions then the first region which has attributes compatible with the section is selected to contain it. The section’s output address will be the next free address in that region; MEMORY.
    If no memory regions were specified, or none match the section then the output address will be based on the current value of the location counter.

In the location counter page

The location counter may not be moved backwards inside an output section, and may not be moved backwards outside of an output section if so doing creates areas with overlapping LMAs.

So I think that the assignment to dot in the test case is ignored when setting the base address of .data due to Memory taking precedence in the above heuristic, as it doesn't create overlapping LMAs it is legal to move the location counter backwards.

lld/trunk/ELF/LinkerScript.cpp

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);
 
   // Update to location counter means update to section size.
   if (InSec)
     expandOutputSection(Val - Dot);
-  else if (Val > Dot)
-    expandMemoryRegions(Val - Dot);
 
   Dot = Val;
 }
 
 // Used for handling linker symbol assignments, for both finalizing
 // their values and doing early declarations. Returns true if symbol
 // should be defined from linker script.
 static bool shouldDefineSym(SymbolAssignment *Cmd) {

lld/trunk/test/ELF/linkerscript/memory5.test

 # REQUIRES: x86
 # RUN: echo ".section .text,\"ax\"; nop; .section .data,\"aw\"; nop;" \
 # RUN:   | llvm-mc -filetype=obj -triple=x86_64-pc-linux - -o %t.o
 # RUN: ld.lld -o %t.so --script %s %t.o
 # RUN: llvm-objdump -section-headers %t.so | FileCheck %s
 
 # CHECK:      1 .text         00000001 0000000000042000
-# CHECK-NEXT: 2 .data         00000001 0000000000044001
+# CHECK-NEXT: 2 .data         00000001 0000000000042001
 
-## Test that assign to Dot changes the position in a memory region.
+## Test that assigning to Dot does not change the position in a memory region.
 
 MEMORY {
   ram (wxa) : ORIGIN = 0x42000, LENGTH = 0x100000
 }
 SECTIONS {
   .text : { *(.text*) }
   . += 0x2000;
   .data : { *(.data*) }
 }