Page MenuHomePhabricator

[ELF] - Support "INSERT AFTER" statement.
ClosedPublic

Authored by grimar on Feb 19 2018, 7:52 AM.

Details

Summary

Its PR35877 again.

This implements INSERT AFTER in a following way:

  • During reading scripts it collects all insert statements.
  • After we done and read all files it inserts statements into script commands list.

With that:

  1. Rest of code does know nothing about INSERT.
  2. Approach is straightforward and have no visible limitations.
  3. It is also easy to support INSERT BEFORE (was seen in clang code once).
  4. Should work for PR35877 and similar cases.

cons:

  • It assumes we have "main" scripts that describes sections.

So it does not work for case when we would have only script that do INSERT
to patch some "default bfd script" that we do not have in LLD anyways.

Diff Detail

Repository
rLLD LLVM Linker

Event Timeline

grimar created this revision.Feb 19 2018, 7:52 AM
ruiu added inline comments.Mar 2 2018, 2:09 PM
test/ELF/insert-after.s
4–14 ↗(On Diff #134913)

We should stop writing linker scripts using "echo". As I did, rewrite this file as a linker script. You could create Inputs/insert-after.s.

grimar planned changes to this revision.Mar 5 2018, 1:15 AM
grimar added inline comments.
test/ELF/insert-after.s
4–14 ↗(On Diff #134913)

This patch was posted a few weeks before your change :) I'll update it.

grimar updated this revision to Diff 136968.Mar 5 2018, 4:02 AM
  • Rebased.
  • Implemented test case as a script.
  • Adjusted code slightly.
test/ELF/insert-after.s
4–14 ↗(On Diff #134913)

Also, note there are 2 linker scripts required here.

ruiu added inline comments.Mar 7 2018, 4:54 PM
ELF/LinkerScript.cpp
206

used to -> is used to

213

InsertV is odd too -- maybe just W because it's next to V.

222

SectionCommands = std::move(V) is better I guess?

ELF/LinkerScript.h
292

needs -> need

294

PendingInsert sounds a bit odd. I'd name this InsertAfterCommands.

ruiu accepted this revision.Mar 7 2018, 4:55 PM

With that LGTM

This revision is now accepted and ready to land.Mar 7 2018, 4:55 PM

LGTM

George Rimar via Phabricator via llvm-commits
<llvm-commits@lists.llvm.org> writes:

grimar updated this revision to Diff 136968.
grimar added a comment.

  • Rebased.
  • Implemented test case as a script.
  • Adjusted code slightly.

https://reviews.llvm.org/D43468

Files:

ELF/Driver.cpp
ELF/LinkerScript.cpp
ELF/LinkerScript.h
ELF/ScriptParser.cpp
test/ELF/linkerscript/Inputs/insert-after.s
test/ELF/linkerscript/Inputs/insert-after.script
test/ELF/linkerscript/insert-after.test

Index: test/ELF/linkerscript/insert-after.test

  • test/ELF/linkerscript/insert-after.test

+++ test/ELF/linkerscript/insert-after.test
@@ -0,0 +1,29 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/insert-after.s -o %t1.o
+
+ Main linker script contains .text and .data sections. Here + we check that can use INSERT AFTER to insert sections .foo.data
+## and .foo.text at the right places.
+
+SECTIONS {
+ .foo.data : { *(.foo.data) }
+} INSERT AFTER .data;
+
+SECTIONS {
+ .foo.text : { *(.foo.text) }
+} INSERT AFTER .text;
+
+# RUN: ld.lld %t1.o -o %t1 --script %p/Inputs/insert-after.script --script %s
+# RUN: llvm-objdump -section-headers %t1 | FileCheck %s
+# CHECK: Sections:
+# CHECK-NEXT: Idx Name Size Address Type
+# CHECK-NEXT: 0 00000000 0000000000000000
+# CHECK-NEXT: 1 .text 00000008 0000000000000000 TEXT DATA
+# CHECK-NEXT: 2 .foo.text 00000008 0000000000000008 TEXT DATA
+# CHECK-NEXT: 3 .data 00000008 0000000000000010 DATA
+# CHECK-NEXT: 4 .foo.data 00000008 0000000000000018 DATA
+
+# RUN: not ld.lld %t1.o -o %t1 --script %s 2>&1 \
+# RUN: | FileCheck %s --check-prefix=ERR
+# ERR-DAG: error: unable to INSERT AFTER .text: section not defined
+# ERR-DAG: error: unable to INSERT AFTER .data: section not defined

Index: test/ELF/linkerscript/Inputs/insert-after.script

  • test/ELF/linkerscript/Inputs/insert-after.script

+++ test/ELF/linkerscript/Inputs/insert-after.script
@@ -0,0 +1,4 @@
+SECTIONS {
+ .text : { *(.text.*) }
+ .data : { *(.data.*) }
+}

Index: test/ELF/linkerscript/Inputs/insert-after.s

  • test/ELF/linkerscript/Inputs/insert-after.s

+++ test/ELF/linkerscript/Inputs/insert-after.s
@@ -0,0 +1,11 @@
+.section .foo.text,"ax"
+.quad 0
+
+.section .foo.data,"aw"
+.quad 0
+
+.section .text.1,"ax"
+.quad 0
+
+.section .data.1,"aw"
+.quad 0

Index: ELF/ScriptParser.cpp

  • ELF/ScriptParser.cpp

+++ ELF/ScriptParser.cpp
@@ -436,6 +436,7 @@

Config->SingleRoRx = true;
 
expect("{");

+ std::vector<BaseCommand *> V;

while (!errorCount() && !consume("}")) {
  StringRef Tok = next();
  BaseCommand *Cmd = readProvideOrAssignment(Tok);

@@ -445,8 +446,18 @@

  else
    Cmd = readOutputSectionDescription(Tok);
}
  • Script->SectionCommands.push_back(Cmd);

+ V.push_back(Cmd);

}

+
+ if (!atEOF() && consume("INSERT")) {
+ consume("AFTER");
+ std::vector<BaseCommand *> &Dest = Script->PendingInserts[next()];
+ Dest.insert(Dest.end(), V.begin(), V.end());
+ return;
+ }
+
+ Script->SectionCommands.insert(Script->SectionCommands.end(), V.begin(),
+ V.end());
}

static int precedence(StringRef Op) {

Index: ELF/LinkerScript.h

  • ELF/LinkerScript.h

+++ ELF/LinkerScript.h
@@ -267,6 +267,9 @@

void processSectionCommands();
void declareSymbols();

+ // Used to handle INSERT AFTER statements.
+ void processInsertCommands();
+

// SECTIONS command list.
std::vector<BaseCommand *> SectionCommands;

@@ -285,6 +288,10 @@

// A list of symbols referenced by the script.
std::vector<llvm::StringRef> ReferencedSymbols;

+
+ Used to implement INSERT AFTER. Contains commands that needs
+
to be inserted into SECTIONS commands list.
+ llvm::DenseMap<StringRef, std::vector<BaseCommand *>> PendingInserts;
};

extern LinkerScript *Script;

Index: ELF/LinkerScript.cpp

  • ELF/LinkerScript.cpp

+++ ELF/LinkerScript.cpp
@@ -203,6 +203,25 @@

Cmd->Provide = false;

}

+ This method used to handle INSERT AFTER statement. Here we rebuild
+
list of script commands to mix section inserted into.
+void LinkerScript::processInsertCommands() {
+ std::vector<BaseCommand *> V;
+ for (BaseCommand *Base : SectionCommands) {
+ V.push_back(Base);
+ if (auto *Cmd = dyn_cast<OutputSection>(Base)) {
+ std::vector<BaseCommand *> &InsertV = PendingInserts[Cmd->Name];
+ V.insert(V.end(), InsertV.begin(), InsertV.end());
+ InsertV.clear();
+ }
+ }
+ for (std::pair<StringRef, std::vector<BaseCommand *>> &P : PendingInserts)
+ if (!P.second.empty())
+ error("unable to INSERT AFTER " + P.first + ": section not defined");
+
+ SectionCommands.swap(V);
+}
+
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.

Index: ELF/Driver.cpp

  • ELF/Driver.cpp

+++ ELF/Driver.cpp
@@ -1081,6 +1081,10 @@

if (errorCount())
  return;

+ Now when we read all script files, we want to finalize order of linker
+
script commands, which can be not yet final because of INSERT commands.
+ Script->processInsertCommands();
+

// We want to declare linker script's symbols early,
// so that we can version them.
// They also might be exported if referenced by DSOs.

llvm-commits mailing list
llvm-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.