[ELF] Process linker scripts deeper when declaring symbols.

[ikudrin]

Summary

This is a follow-up to D41987.

We should process symbols inside output section declarations the same way as top-level ones.

[grimar]

Looks fine for me. Suggestions below.

ELF/LinkerScript.cpp

191

It's large. I would make it static helper instead then.
(probably would change argument type to SymbolAssignment* too)

test/ELF/linkerscript/version-script.s

8

It should be multiline, because exceeds 80 columns per line limit.

[ikudrin]

Address review comments.

[grimar]

LGTM with a nit.
(please wait for other reviewers approval)

test/ELF/linkerscript/version-script.s

16

Sorry for not being clear, I was mean it can not be single line because of 80 limit,
but we generally are trying to keep scripts short in testcases, it can be something,
just like:

echo "SECTIONS { .text : { bar = foo; *(.text) }} \
      VERSION { V { global: foo; bar; local: *; }; }" > %t.script

[ikudrin]

Shorten the script in the test.

@ruiu, @rafael is it OK to apply?

[ikudrin]

Ping. @espindola, @ruiu, is it OK to land?

[ikudrin]

Ping. Is there anything I can improve here?

@ruiu, @rafael, in spite of this revision is accepted by @grimar, he asked me to wait for your approval.

[ruiu]

LGTM

test/ELF/linkerscript/version-script.s

9

Can you write this way so %t.script consists of two lines instead of one?

1. RUN: echo "SECTIONS { .text : { bar = foo; *(.text) } }" > %t.script
2. RUN: echo "VERSION { V { global: foo; bar; local: *; }; }" >> %t.script

If something goes wrong, lld prints out an error message with "^" marker to point out the error location, but that is almost useless if a line is too long.

[ikudrin]

Thanks!

ELF/LinkerScript.cpp

void LinkerScript::addSymbol(SymbolAssignment *Cmd) {
   // write expressions like this: `alignment = 16; . = ALIGN(., alignment)`.
   uint64_t SymValue = Value.Sec ? 0 : Value.getValue();
 
   replaceSymbol<Defined>(Sym, nullptr, Cmd->Name, STB_GLOBAL, Visibility,
                          STT_NOTYPE, SymValue, 0, Sec);
   Cmd->Sym = cast<Defined>(Sym);
 }
 
-// Symbols defined in script should not be inlined by LTO. At the same time
-// we don't know their final values until late stages of link. Here we scan
-// over symbol assignment commands and create placeholder symbols if needed.
-void LinkerScript::declareSymbols() {
-  assert(!Ctx);
-  for (BaseCommand *Base : SectionCommands) {
-    auto *Cmd = dyn_cast<SymbolAssignment>(Base);
-    if (!Cmd || !shouldDefineSym(Cmd))
-      continue;
+// This function is called from LinkerScript::declareSymbols.
+// It creates a placeholder symbol if needed.
+static void declareSymbol(SymbolAssignment *Cmd) {
+  if (!shouldDefineSym(Cmd))
+    return;
 
-    // We can't calculate final value right now.
-    Symbol *Sym;
-    uint8_t Visibility = Cmd->Hidden ? STV_HIDDEN : STV_DEFAULT;
-    std::tie(Sym, std::ignore) =
-        Symtab->insert(Cmd->Name, /*Type*/ 0, Visibility,
+  std::tie(Sym, std::ignore) = Symtab->insert(Cmd->Name, /*Type*/ 0, Visibility,
                                               /*CanOmitFromDynSym*/ false,
                                               /*File*/ nullptr);
-    replaceSymbol<Defined>(Sym, nullptr, Cmd->Name, STB_GLOBAL, Visibility,
-                           STT_NOTYPE, 0, 0, nullptr);
-    Cmd->Sym = cast<Defined>(Sym);
-    Cmd->Provide = false;
-  }
+
+// Symbols defined in script should not be inlined by LTO. At the same time
+// we don't know their final values until late stages of link. Here we scan
+// over symbol assignment commands and create placeholder symbols if needed.
+void LinkerScript::declareSymbols() {
+  assert(!Ctx);
+  for (BaseCommand *Base : SectionCommands) {
+    if (auto *Cmd = dyn_cast<SymbolAssignment>(Base)) {
+      declareSymbol(Cmd);
+      continue;
+    }
+    auto *Sec = dyn_cast<OutputSection>(Base);
+    if (!Sec)
+      continue;
+    // If the output section directive has constraints,
+    // we can't say for sure if it is going to be included or not.
+    // Skip such sections for now. Improve the checks if we ever
+    // need symbols from that sections to be declared early.
+    if (Sec->Constraint != ConstraintKind::NoConstraint)
+      continue;
+    for (BaseCommand *Base2 : Sec->SectionCommands)
+      if (auto *Cmd = dyn_cast<SymbolAssignment>(Base2))
+        declareSymbol(Cmd);
+  }
 }
 
 // This function is called from assignAddresses, while we are
 // fixing the output section addresses. This function is supposed
 // to set the final value for a given symbol assignment.
 void LinkerScript::assignSymbol(SymbolAssignment *Cmd, bool InSec) {
   if (Cmd->Name == ".") {
     setDot(Cmd->Expression, Cmd->Location, InSec);

test/ELF/linkerscript/symbols-synthetic.s

 # RUN:        PROVIDE_HIDDEN(_end_sec = ADDR(.text) + SIZEOF(.text)); }" > %t.script
 # RUN: ld.lld -o %t1 --eh-frame-hdr --script %t.script %t
 # RUN: llvm-objdump -t %t1 | FileCheck --check-prefix=IN-SEC %s
 
 # SIMPLE:      0000000000000128         .foo    00000000 .hidden _end_sec
 # SIMPLE-NEXT: 0000000000000120         .foo    00000000 _begin_sec
 # SIMPLE-NEXT: 0000000000000128         *ABS*   00000000 _end_sec_abs
 # SIMPLE-NEXT: 0000000000001048         .text   00000000 _start
-# SIMPLE-NEXT: 0000000000000ee4         *ABS*   00000000 size_foo_3
 # SIMPLE-NEXT: 0000000000000120         .foo    00000000 begin_foo
 # SIMPLE-NEXT: 0000000000000128         .foo    00000000 end_foo
 # SIMPLE-NEXT: 0000000000000008         *ABS*   00000000 size_foo_1
 # SIMPLE-NEXT: 0000000000000008         *ABS*   00000000 size_foo_1_abs
 # SIMPLE-NEXT: 0000000000001000         .foo    00000000 begin_bar
 # SIMPLE-NEXT: 0000000000001004         .foo    00000000 end_bar
 # SIMPLE-NEXT: 0000000000000ee4         *ABS*   00000000 size_foo_2
+# SIMPLE-NEXT: 0000000000000ee4         *ABS*   00000000 size_foo_3
 # SIMPLE-NEXT: 0000000000001004         .eh_frame_hdr     00000000 __eh_frame_hdr_start
 # SIMPLE-NEXT: 0000000000001010         *ABS*             00000000 __eh_frame_hdr_start2
 # SIMPLE-NEXT: 0000000000001018         .eh_frame_hdr     00000000 __eh_frame_hdr_end
 # SIMPLE-NEXT: 0000000000001020         *ABS*             00000000 __eh_frame_hdr_end2
 
 # NO-SEC:       0000000000201000         .text     00000000 .hidden _begin_sec
 # NO-SEC-NEXT:  0000000000201001         .text     00000000 .hidden _end_sec
 

test/ELF/linkerscript/version-script.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
 
 # RUN: echo "bar = foo; VERSION { V { global: foo; bar; local: *; }; }" > %t.script
 # RUN: ld.lld -T %t.script -shared --no-undefined-version %t.o -o %t.so
 # RUN: llvm-readobj -V %t.so | FileCheck %s
 
+# RUN: echo "SECTIONS { .text : { bar = foo; *(.text) } }" > %t.script
+# RUN: echo "VERSION { V { global: foo; bar; local: *; }; }" >> %t.script
+# RUN: ld.lld -T %t.script -shared --no-undefined-version %t.o -o %t.so
+# RUN: llvm-readobj -V %t.so | FileCheck %s
+
 ## Check that we are able to version symbols defined in script.
 # CHECK:      Symbols [
 # CHECK-NEXT:   Symbol {
 # CHECK-NEXT:     Version: 0
 # CHECK-NEXT:     Name: @
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Symbol {
 # CHECK-NEXT:     Version: 0
 # CHECK-NEXT:     Name: und@
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Symbol {
 # CHECK-NEXT:     Version: 2