[ELF] Linkerscript: allow adding start/end symbols to arbitrary section

[evgeny777]

Summary

Current revision does not allow adding symbols to special output sections, like .eh_frame, .eh_frame_hdr and others.
This patch implements this feature (which is required by our dynamic linker)

[ruiu]

This patch seems a bit tricky because it handles symbols at start or end of sections command specially. For example, it works as expected if a command is

.eh_frame_hdr { _foo = .; *(.eh_frame_hdr); _bar = .; }

but doesn't if

.eh_frame_hdr { _foo = .; *(.eh_frame_hdr); _bar1 = .; bar2 = .; }

I can see why you needed to do this (because special output sections are not a list of output sections), but I'm wondering if there's a better way than the way you implemented in this patch.

[evgeny777]

I agree, but unfortunately I don't see a lot of other options. What comes to my mind is:

• Implement more sophisticated logic, so it would handle your case.
• Check for symbol names (section name + "_end" or "_start"), and if there are then add start and/or end symbols.

Any other suggestions are appreciated, of course.

[evgeny777]

An attempt to make it slightly more clever.

[ruiu]

ELF/LinkerScript.cpp

308–328

I agree we need this function, but this is tricky, so this needs comments.

311

Add is used only once, so please inline.

[evgeny777]

Added support for expressions (see test case)

[evgeny777]

Addressed review comments

[ruiu]

ELF/LinkerScript.cpp

309

I'd also mention that special sections are not a list of regular output sections and therefore the regular mechanism to define symbols doesn't work. It's also good to mention that this approach is not perfect -- it only handles start and end symbols.

316

Use isa instead of dyn_cast.

316–319

Let's reduce the indentation level by using early continues.

if (isa<InputSectionDescription>(...)) {
  Start = false;
  continue;
}
auto *AssignCmd = dyn_cast...;
if (!AssignCmd)
  continue;

[evgeny777]

Addressed review comments

[ruiu]

ELF/LinkerScript.cpp

311

Format.

328–330

If an output section is defined as

.eh_frame { start = .; foo.o(.eh_frame); bar = .; bar.o(.eh_frame); end = . }

then bar is silently set so that it points to the end of .eh_frame section? I think we want to print an error message for that instead.

[evgeny777]

Added error checking + test case for error.

[evgeny777]

Rebased.

[evgeny777]

Fixed bug in addStartEndSymbols() which caused the following script to fail

.eh_frame_hdr : {
   *(.eh_frame_hdr)
   *(.eh_frame_hdr)
}

[ruiu]

ELF/LinkerScript.cpp

316

It seems unnecessarily complicated to me. Why don't you do something like this?

// Add start symbols.
for (int I = 0, E = Cmd->Commands.size(); I < E; ++I) {
  if (!isa<SymbolAssignment>(Cmd->Commands[I]))
    break;
  ....
}

// Add end symbols.
for (int I = Cmd->Commands.size() - 1; I >= 0; --I) {
  if (!isa<SymbolAssignment>(Cmd->Commands[I]))
    break;
  ....
}

[evgeny777]

ELF/LinkerScript.cpp

316

That's fine, but what about error diagnostics, you suggested to add last time?

[ruiu]

ELF/LinkerScript.cpp

316

Well, maybe something like this?

 // Add start symbols.
 int I = 0;
 for (int E = Cmd->Commands.size(); I < E; ++I) {
   if (!isa<SymbolAssignment>(Cmd->Commands[I]))
     break;
   ....
 }

 // Add end symbols.
 int J = Cmd->Commands.size() - 1;
 for (; J >= 0; --J) {
   if (!isa<SymbolAssignment>(Cmd->Commands[J]))
     break;
   ....
 }

// Warn on stray assignment expressions.
for (; I <= J; ++I)
  if (auto *Foo = dyn_cast<SymbolAssignment>(Cmd->Commands[I])
    ....

[evgeny777]

ELF/LinkerScript.cpp

316

The problem in this implementation is that it add 'end' symbols in reverse order. The following one will not work:

.eh_frame_hdr : {
    foo = .;
    *(.eh_frame_hdr)
    bar1 = .;
    bar2 = bar1 + 1;
}

[evgeny777]

Diff updated.
I've simplified the 'addStartEndSymbols' function and hope it's easier to understand now.

[ruiu]

LGTM

ELF/LinkerScript.cpp

for (InputSectionBase<ELFT> *S : F->getSections()) {
       std::tie(OutSec, IsNew) = Factory.create(S, getOutputSectionName(S));
       if (IsNew)
         OutputSections->push_back(OutSec);
       OutSec->addSection(S);
     }
   }
 }
 
-template <class ELFT> void assignOffsets(OutputSectionBase<ELFT> *Sec) {
+template <class ELFT>
+void addStartEndSymbols(OutputSectionCommand *Cmd,
+                        OutputSectionBase<ELFT> *Sec) {
+  auto Add = [&](std::unique_ptr<BaseCommand> &Base, bool Start) {
+    auto *AssignCmd = dyn_cast<SymbolAssignment>(Base.get());
+    if (AssignCmd && AssignCmd->Sym) {
+      auto *Sym = cast<DefinedSynthetic<ELFT>>(AssignCmd->Sym);
+      Sym->Section = Sec;
+      Sym->Value = Start ? 0 : DefinedSynthetic<ELFT>::SectionEnd;
+    }
+  };
+
+  if (!Cmd->Commands.empty())
+    Add(Cmd->Commands.front(), true);
+
+  if (Cmd->Commands.size() > 1)
+    Add(Cmd->Commands.back(), false);
+}
+
+template <class ELFT>
+void assignOffsets(OutputSectionCommand *Cmd, OutputSectionBase<ELFT> *Sec) {
   auto *OutSec = dyn_cast<OutputSection<ELFT>>(Sec);
   if (!OutSec) {
     Sec->assignOffsets();
+    addStartEndSymbols(Cmd, Sec);
     return;
   }
 
   typedef typename ELFT::uint uintX_t;
   uintX_t Off = 0;
 
   for (InputSection<ELFT> *I : OutSec->Sections) {
     if (auto *L = dyn_cast<LayoutInputSection<ELFT>>(I)) {
for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands) {
 
     if (Cmd->AlignExpr)
       Sec->updateAlignment(Cmd->AlignExpr(Dot));
 
     if ((Sec->getFlags() & SHF_TLS) && Sec->getType() == SHT_NOBITS) {
       uintX_t TVA = Dot + ThreadBssOffset;
       TVA = alignTo(TVA, Sec->getAlignment());
       Sec->setVA(TVA);
-      assignOffsets(Sec);
+      assignOffsets(Cmd, Sec);
       ThreadBssOffset = TVA - Dot + Sec->getSize();
       continue;
     }
 
     if (!(Sec->getFlags() & SHF_ALLOC)) {
-      Sec->assignOffsets();
+      assignOffsets(Cmd, Sec);
       continue;
     }
 
     Dot = alignTo(Dot, Sec->getAlignment());
     Sec->setVA(Dot);
-    assignOffsets(Sec);
+    assignOffsets(Cmd, Sec);
     MinVA = std::min(MinVA, Dot);
     Dot += Sec->getSize();
   }
 
   // ELF and Program headers need to be right before the first section in
   // memory. Set their addresses accordingly.
   MinVA = alignDown(MinVA - Out<ELFT>::ElfHeader->getSize() -
                         Out<ELFT>::ProgramHeaders->getSize(),

test/ELF/linkerscript/linkerscript-symbols-synthetic.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
 
 # Simple symbol assignment within input section list. The '.' symbol
 # is not location counter but offset from the beginning of output
 # section .foo
 # RUN: echo "SECTIONS { \
 # RUN:          .foo : { \
 # RUN:              begin_foo = .; \
 # RUN:              *(.foo) \
 # RUN:              end_foo = .; \
 # RUN:              size_foo_1 = SIZEOF(.foo); \
 # RUN:              . = ALIGN(0x1000); \
 # RUN:              begin_bar = .; \
 # RUN:              *(.bar) \
 # RUN:              end_bar = .; \
-# RUN:              size_foo_2 = SIZEOF(.foo); \ 
-# RUN:            } \
-# RUN:            size_foo_3 = SIZEOF(.foo); }" > %t.script
-# RUN: ld.lld -o %t1 --script %t.script %t
+# RUN:              size_foo_2 = SIZEOF(.foo); } \ 
+# RUN:            size_foo_3 = SIZEOF(.foo); \
+# RUN:          .eh_frame_hdr : { \
+# RUN:             __eh_frame_hdr_start = .; \
+# RUN:             *(.eh_frame_hdr) \
+# RUN:             __eh_frame_hdr_end = .; } \
+# RUN:         }" > %t.script
+# RUN: ld.lld -o %t1 --eh-frame-hdr --script %t.script %t
 # RUN: llvm-objdump -t %t1 | FileCheck --check-prefix=SIMPLE %s
-# SIMPLE:      0000000000000120         .foo    00000000 begin_foo
-# SIMPLE-NEXT: 0000000000000128         .foo    00000000 end_foo
+# SIMPLE:      0000000000000158         .foo    00000000 begin_foo
+# SIMPLE-NEXT: 0000000000000160         .foo    00000000 end_foo
 # SIMPLE-NEXT: 0000000000000008         .foo    00000000 size_foo_1
 # SIMPLE-NEXT: 0000000000001000         .foo    00000000 begin_bar
 # SIMPLE-NEXT: 0000000000001004         .foo    00000000 end_bar
-# SIMPLE-NEXT: 0000000000000ee4         .foo    00000000 size_foo_2
-# SIMPLE-NEXT: 0000000000000ee4         *ABS*   00000000 size_foo_3
+# SIMPLE-NEXT: 0000000000000eac         .foo    00000000 size_foo_2
+# SIMPLE-NEXT: 0000000000000eac         *ABS*   00000000 size_foo_3
+# SIMPLE-NEXT: 0000000000001004         .eh_frame_hdr     00000000 __eh_frame_hdr_start
+# SIMPLE-NEXT: 0000000000001018         .eh_frame_hdr     00000000 __eh_frame_hdr_end
+
 
 .global _start
 _start:
  nop
 
 .section .foo,"a"
  .quad 0
 
 .section .bar,"a"
  .long 0
+
+.section .dah,"ax",@progbits
+ .cfi_startproc
+ nop
+ .cfi_endproc
+