[ELF] - Postpone the evaluation of DefinedSynthetic value

[grimar]

Summary

After applying D29332 and trying to link linux kernel I found next issue.
We still produced symbols value wrong.

Having the next script:

vvar_start = . - 2 * (1 << 12);
vvar_page = vvar_start;
vvar_vsyscall_gtod_data = vvar_page + 128;

pvclock_page = vvar_start + PAGE_SIZE;
. = SIZEOF_HEADERS;
.hash		: { *(.hash) }			:text
...

LLD output was:

Section Headers:
  [Nr] Name              Type             Address           Offset
  [ 1] .hash             HASH             0000000000000120  00000120
       0000000000000050  0000000000000004   A       3     0     4

Symbol table '.symtab' contains 18 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     3: ffffffffffffe1a0     0 NOTYPE  LOCAL  HIDDEN     1 vvar_vsyscall_gtod_data
     5: ffffffffffffe120     0 NOTYPE  LOCAL  DEFAULT    1 vvar_start
     6: ffffffffffffe120     0 NOTYPE  LOCAL  DEFAULT    1 vvar_page

BFD was:

28: ffffffffffffe000     0 NOTYPE  LOCAL  DEFAULT  ABS vvar_page
33: ffffffffffffe000     0 NOTYPE  LOCAL  DEFAULT    1 vvar_start
35: ffffffffffffe080     0 NOTYPE  LOCAL  DEFAULT  ABS vvar_vsyscall_gtod_data

GOLD was:

6: ffffffffffffe000     0 NOTYPE  LOCAL  DEFAULT  ABS vvar_start
7: ffffffffffffe000     0 NOTYPE  LOCAL  DEFAULT  ABS vvar_page
8: ffffffffffffe080     0 NOTYPE  LOCAL  DEFAULT  ABS vvar_vsyscall_gtod_data

The problem was in assignSectionSymbol(). We used Body->Section->Addr too early.
At the point of evaluation Addr was unset and == 0x0. Solution I suggest - to postpone the evaluation.
After that patch values we produce are correct for case above.

[ruiu]

ELF/LinkerScript.cpp

387

I don't think I understand this comment. Why Sym value is affected by the following .sec?

I do understand that Sym is affected if it is .sec : { ... }; Sym = ., but this is Sym = .; .sec { ... };. Can a following expression affects the results of preceding expressions?

By the way, why + 0x1?

[grimar]

ELF/LinkerScript.cpp

387

but this is Sym = .; .sec { ... };. Can a following expression affects the results of preceding expressions?

Yes. Please take a look on non-absolute.s:

{ A = . - 0x10; B = A + 0x1; . += 0x1000; }

B is non-absolute expression, because depends on A which uses location counter for evaluation.
That way B is non absolute symbol and belongs to next section, which is .text in testcase.

That is a different behavior in compare with gold/bfd. As you can see in description BFD makes vvar_start to belong to .text,
but other symbols are ABS, gold makes all symbols as ABS. It was discussed with Rafael in D29332 thread that it looks as a bug of gnu linkers.

By the way, why + 0x1?

Just shows the common case. Some expression that has location counter and operation. I think it is pretty common case in scripts. I can replace with "Sym = .;" if you want, though my way probably a bit more readable and common to see.

[ruiu]

ELF/LinkerScript.cpp

387

I think that doesn't answer my question. My question is how a following expression can affect a preceding expression. Your expression,

A = . - 0x10; B = A + 0x1; . += 0x1000;

can be computed from left to right, so it doesn't seems to be a counter example.

[grimar]

ELF/LinkerScript.cpp

387

It should not affect on final value and that what patch fixes.

Current non-absolute.s contains:

SECTIONS { A = . - 0x10; B = A + 0x1; }
# SYMBOL:        Name: B
# SYMBOL-NEXT:   Value: 0xFFFFFFFFFFFFFFF1

If you just do: "SECTIONS { A = . - 0x10; B = A + 0x1; . += 0x1234; }"
and run test, you'll see that value of B changed:

23>    Symbol {
23>      Name: B (1)
23>      Value: 0x1225

That is wrong and happens because B is DefinedSynthetic and it is relative to section VA.
Patch fixes the calculation of B.

[ruiu]

What I do not understand is why symbols outside an output section description should belong to an output section. Let's use this extremely simple linker script as an example.

SECTIONS { A = . }

This linker script defines symbol A. All output sections are orphan sections and laid out using the default layout rules. IIUC, what you said is symbol A should be defined relative to an orphan section that happens to become the first section of the output. I wonder why. I think symbol A should be defined relative to the image base instead of relative to some section. In other words, symbol A shouldn't belong to any section.

In general, any symbol that is not inside an output section description should be defined relative to the image base. I.e.

SECTIONS { A = . }

is defined relative to the image base, while

SECTIONS { .foo : { A = . } }

is defined relative to section .foo.

[grimar]

In D29391#665264, @ruiu wrote:

What I do not understand is why symbols outside an output section description should belong to an output section. Let's use this extremely simple linker script as an example.

SECTIONS { A = . }

This linker script defines symbol A. All output sections are orphan sections and laid out using the default layout rules. IIUC, what you said is symbol A should be defined relative to an orphan section that happens to become the first section of the output. I wonder why. I think symbol A should be defined relative to the image base instead of relative to some section. In other words, symbol A shouldn't belong to any section.

So it means you assume it should be absolute, right ?

In general, any symbol that is not inside an output section description should be defined relative to the image base. I.e.

SECTIONS { A = . }

is defined relative to the image base, while

SECTIONS { .foo : { A = . } }

is defined relative to section .foo.

I think Rafael wrote about the same situation in D28857 thread (18 jan):
"That is a bug in bfd. Its handling of what is absolute is broken. On ELF
absolute really means absolute, like a size. Anything that is a position
in the file is not absolute."

and

"The difference is that the '.' makes it a position in the file, and
therefore not absolute."

And patch where it was implemented was: https://reviews.llvm.org/D26161

[grimar]

• Addressed review comments.

[grimar]

D29607 was landed instead.

ELF/LinkerScript.cpp

 }
 
 template <class ELFT> static void assignSymbol(SymbolAssignment *Cmd) {
   // If there are sections, then let the value be assigned later in
   // `assignAddresses`.
   if (ScriptConfig->HasSections)
     return;
 
-  uint64_t Value = Cmd->Expression(0);
   if (Cmd->Expression.IsAbsolute()) {
-    cast<DefinedRegular<ELFT>>(Cmd->Sym)->Value = Value;
+    cast<DefinedRegular<ELFT>>(Cmd->Sym)->Value = Cmd->Expression(0);
   } else {
     const OutputSectionBase *Sec = Cmd->Expression.Section();
     if (Sec)
-      cast<DefinedSynthetic>(Cmd->Sym)->Value = Value - Sec->Addr;
+      cast<DefinedSynthetic>(Cmd->Sym)->GetValue = [=] {
+        return Cmd->Expression(0) - Sec->Addr;
+      };
   }
 }
 
 template <class ELFT> static void addSymbol(SymbolAssignment *Cmd) {
   if (Cmd->Name == ".")
     return;
 
   // If a symbol was in PROVIDE(), we need to define it only when

 // is an offset from beginning of section and regular
 // symbols whose value is absolute.
 template <class ELFT>
 static void assignSectionSymbol(SymbolAssignment *Cmd,
                                 typename ELFT::uint Value) {
   if (!Cmd->Sym)
     return;
 
+  // Sometimes Section->Addr is not yet set when we proccess the assign command.
+  // It can happen when non-absolute value is assigned before processing
+  // first section. Example: Sym = . + 0x1; .sec : { ... };
+  // In that case Sym contains offset depending on VA of the .sec, but VA value
+  // was not yet set. To solve it we postpone the evaluation of symbol value.
   if (auto *Body = dyn_cast<DefinedSynthetic>(Cmd->Sym)) {
     Body->Section = Cmd->Expression.Section();
-    Body->Value = Cmd->Expression(Value) - Body->Section->Addr;
+    Body->GetValue = [=] {
+      return Cmd->Expression(Value) - Body->Section->Addr;
+    };
     return;
   }
   auto *Body = cast<DefinedRegular<ELFT>>(Cmd->Sym);
   Body->Value = Cmd->Expression(Value);
 }
 
 template <class ELFT> static bool isTbss(OutputSectionBase *Sec) {
   return (Sec->Flags & SHF_TLS) && Sec->Type == SHT_NOBITS;

ELF/Symbols.h

 // takes an output section to calculate output VA, etc.
 // If Section is null, this symbol is relative to the image base.
 class DefinedSynthetic : public Defined {
 public:
   DefinedSynthetic(StringRef Name, uint64_t Value,
                    const OutputSectionBase *Section)
       : Defined(SymbolBody::DefinedSyntheticKind, Name, /*IsLocal=*/false,
                 llvm::ELF::STV_HIDDEN, 0 /* Type */),
-        Value(Value), Section(Section) {}
+        GetValue([=] { return Value; }), Section(Section) {}
 
   static bool classof(const SymbolBody *S) {
     return S->kind() == SymbolBody::DefinedSyntheticKind;
   }
 
-  uint64_t Value;
+  std::function<uint64_t()> GetValue;
   const OutputSectionBase *Section;
 };
 
 class Undefined : public SymbolBody {
 public:
   Undefined(StringRefZ Name, bool IsLocal, uint8_t StOther, uint8_t Type,
             InputFile *F);
 

ELF/Symbols.cpp

 static typename ELFT::uint getSymVA(const SymbolBody &Body,
                                     typename ELFT::uint &Addend) {
   typedef typename ELFT::uint uintX_t;
 
   switch (Body.kind()) {
   case SymbolBody::DefinedSyntheticKind: {
     auto &D = cast<DefinedSynthetic>(Body);
     const OutputSectionBase *Sec = D.Section;
+    uintX_t Value = D.GetValue();
     if (!Sec)
-      return D.Value;
-    if (D.Value == uintX_t(-1))
+      return Value;
+    if (Value == uintX_t(-1))
       return Sec->Addr + Sec->Size;
-    return Sec->Addr + D.Value;
+    return Sec->Addr + Value;
   }
   case SymbolBody::DefinedRegularKind: {
     auto &D = cast<DefinedRegular<ELFT>>(Body);
     InputSectionBase<ELFT> *IS = D.Section;
 
     // According to the ELF spec reference to a local symbol from outside
     // the group are not allowed. Unfortunately .eh_frame breaks that rule
     // and must be treated specially. For now we just replace the symbol with

ELF/Writer.cpp

 // This function is called after we have assigned address and size
 // to each section. This function fixes some predefined
 // symbol values that depend on section address and size.
 template <class ELFT> void Writer<ELFT>::fixPredefinedSymbols() {
   auto Set = [](DefinedSynthetic *S1, DefinedSynthetic *S2,
                 OutputSectionBase *Sec, uint64_t Value) {
     if (S1) {
       S1->Section = Sec;
-      S1->Value = Value;
+      S1->GetValue = [=] { return Value; };
     }
     if (S2) {
       S2->Section = Sec;
-      S2->Value = Value;
+      S2->GetValue = [=] { return Value; };
     }
   };
 
   // _etext is the first location after the last read-only loadable segment.
   // _edata is the first location after the last read-write loadable segment.
   // _end is the first location after the uninitialized data region.
   PhdrEntry *Last = nullptr;
   PhdrEntry *LastRO = nullptr;

test/ELF/linkerscript/non-absolute2.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t1.o
+# RUN: echo "SECTIONS { A = . - 0x9; . += 0x1000; }" > %t.script
+# RUN: ld.lld -shared %t1.o --script %t.script -o %t
+# RUN: llvm-objdump -section-headers %t | FileCheck %s --check-prefix=DUMP
+# RUN: llvm-readobj -t %t | FileCheck %s --check-prefix=SYMBOL
+
+# DUMP:       Sections:
+# DUMP-NEXT:   Idx Name          Size      Address
+# DUMP-NEXT:    0               00000000 0000000000000000
+# DUMP-NEXT:    1 .text         00000006 0000000000001000
+
+# SYMBOL:     Symbol {
+# SYMBOL:        Name: A
+# SYMBOL-NEXT:   Value: 0xFFFFFFFFFFFFFFF7
+# SYMBOL-NEXT:   Size: 0
+# SYMBOL-NEXT:   Binding: Local
+# SYMBOL-NEXT:   Type: None
+# SYMBOL-NEXT:   Other [
+# SYMBOL-NEXT:     STV_HIDDEN
+# SYMBOL-NEXT:   ]
+# SYMBOL-NEXT:   Section: .text
+# SYMBOL-NEXT: }
+
+.text
+ movl A(%rip), %eax
+
+.hidden A