[ELF] Align the value if needed when computing the expression

[ruiu]

I think what you were looking for is ExprValue::getValue().

[phosek]

In D35651#815595, @ruiu wrote:

I think what you were looking for is ExprValue::getValue().

That's what I tried initially, but I don't think it's correct. The problem is that if I use {A.Sec, A.ForceAbsolute, A.getValue() + B.getValue(), A.Loc}, the section offset (if A.Sec != nullptr) will get applied twice. If I use A.getValue() + B.getValue() the resulting expression will be absolute and not section relative as it should be if A is.

[grimar]

Testcase ? :)

[phosek]

Added a test case, which revealed another issue: while trying PROVIDE_HIDDEN(newsym = ALIGN(_end, CONSTANT(MAXPAGESIZE)) + 5);, I noticed it was working fine in our project but not in LLD. Turned the problem is that we also use a linker script which defines PROVIDE_HIDDEN(_end = .). However, when using only input linker script, this breaks because LLD only finalizes the _end value in fixPredefinedSymbols after all symbol assignment commands have been processed, so newsym will get a wrong value. This change still fixes the case when script is being used.

[ruiu]

The test you added doesn't seem to exercise the code you added. Don't you need something like ALIGN(3, 8) + 10 to test that?

[phosek]

In D35651#816244, @ruiu wrote:

The test you added doesn't seem to exercise the code you added. Don't you need something like ALIGN(3, 8) + 10 to test that?

I have added few more test cases. This one is still useful because it checks that the relative case works with alignment (i.e. it'd break if we used A.getValue() + B.getValue() or A.getValue() - B.getValue()).

[ruiu]

LGTM

ELF/ScriptParser.cpp

static void moveAbsRight(ExprValue &A, ExprValue &B) {
   if (A.isAbsolute())
     std::swap(A, B);
   if (!B.isAbsolute())
     error(A.Loc + ": at least one side of the expression must be absolute");
 }
 
 static ExprValue add(ExprValue A, ExprValue B) {
   moveAbsRight(A, B);
-  return {A.Sec, A.ForceAbsolute, A.Val + B.getValue(), A.Loc};
+  uint64_t Val = alignTo(A.Val, A.Alignment) + B.getValue();
+  return {A.Sec, A.ForceAbsolute, Val, A.Loc};
 }
 
 static ExprValue sub(ExprValue A, ExprValue B) {
-  return {A.Sec, A.Val - B.getValue(), A.Loc};
+  uint64_t Val = alignTo(A.Val, A.Alignment) - B.getValue();
+  return {A.Sec, Val, A.Loc};
 }
 
 static ExprValue mul(ExprValue A, ExprValue B) {
   return A.getValue() * B.getValue();
 }
 
 static ExprValue div(ExprValue A, ExprValue B) {
   if (uint64_t BV = B.getValue())

test/ELF/linkerscript/Inputs/symbol-reserved.script

+SECTIONS
+{
+  .text : { *(.text) }
+  PROVIDE_HIDDEN(_end = .);
+}

test/ELF/linkerscript/symbol-reserved.s

 # SHARED: 0000000000000005 .dynsym 00000000 .hidden newsym
 
 # RUN: echo "PROVIDE_HIDDEN(newsym = ALIGN(__ehdr_start, CONSTANT(MAXPAGESIZE)) + 5);" > %t.script
 # RUN: ld.lld -o %t1 %t.script %t
 # RUN: llvm-objdump -t %t1 | FileCheck --check-prefix=ALIGNED %s
 
 # ALIGNED: 0000000000200005 .text 00000000 .hidden newsym
 
+# RUN: echo "PROVIDE_HIDDEN(newsym = ALIGN(3, 8) + 10);" > %t.script
+# RUN: ld.lld -o %t1 %t.script %t
+# RUN: llvm-objdump -t %t1 | FileCheck --check-prefix=ALIGN-ADD %s
+# ALIGN-ADD: 0000000000000012 *ABS* 00000000 .hidden newsym
+
+# RUN: echo "PROVIDE_HIDDEN(newsym = ALIGN(11, 8) - 10);" > %t.script
+# RUN: ld.lld -o %t1 %t.script %t
+# RUN: llvm-objdump -t %t1 | FileCheck --check-prefix=ALIGN-SUB %s
+# ALIGN-SUB: 0000000000000006 *ABS* 00000000 .hidden newsym
+
+# RUN: echo "PROVIDE_HIDDEN(newsym = ALIGN(_end, CONSTANT(MAXPAGESIZE)) + 5);" > %t.script
+# RUN: ld.lld -o %t1 --script %p/Inputs/symbol-reserved.script %t %t.script
+# RUN: llvm-objdump -t %t1 | FileCheck --check-prefix=RELATIVE-ADD %s
+# RELATIVE-ADD: 0000000000001005 .text 00000000 .hidden newsym
+# RELATIVE-ADD: 0000000000000007 .text 00000000 .hidden _end
+
+# RUN: echo "PROVIDE_HIDDEN(newsym = ALIGN(_end, CONSTANT(MAXPAGESIZE)) - 5);" > %t.script
+# RUN: ld.lld -o %t1 --script %p/Inputs/symbol-reserved.script %t %t.script
+# RUN: llvm-objdump -t %t1 | FileCheck --check-prefix=RELATIVE-SUB %s
+# RELATIVE-SUB: 0000000000000ffb .text 00000000 .hidden newsym
+# RELATIVE-SUB: 0000000000000007 .text 00000000 .hidden _end
+
 .global _start
 _start:
   lea newsym(%rip),%rax