[lld] 03051f7 - [ELF] Preserve section order within an INSERT AFTER command

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2021-06-30T11:35:50-07:00
New Revision: 03051f7ac8a3e2eda44b8280290c90863a064f0e

URL: https://github.com/llvm/llvm-project/commit/03051f7ac8a3e2eda44b8280290c90863a064f0e
DIFF: https://github.com/llvm/llvm-project/commit/03051f7ac8a3e2eda44b8280290c90863a064f0e.diff

LOG: [ELF] Preserve section order within an INSERT AFTER command

For
```
SECTIONS {
  text.0 : {}
  text.1 : {}
  text.2 : {}
} INSERT AFTER .data;
```

the current order is `.data text.2 text.1 text.0`. It makes more sense to
preserve the specified order and thus improve compatibility with GNU ld.

For
```
SECTIONS { text.0 : {} } INSERT AFTER .data;
SECTIONS { text.3 : {} } INSERT AFTER .data;
```

GNU ld somehow collects sections with `INSERT AFTER .data` together (IMO
inconsistent) but I think it makes more sense to execute the commands in order
and get `.data text.3 text.0` instead.

Reviewed By: peter.smith

Differential Revision: https://reviews.llvm.org/D105158

Added: 
    lld/test/ELF/linkerscript/insert-multi.test

Modified: 
    lld/ELF/LinkerScript.cpp
    lld/ELF/LinkerScript.h
    lld/ELF/ScriptParser.cpp

Removed: 
    


################################################################################
diff  --git a/lld/ELF/LinkerScript.cpp b/lld/ELF/LinkerScript.cpp
index 6deb9a98cc4bb..a938984ad945e 100644
--- a/lld/ELF/LinkerScript.cpp
+++ b/lld/ELF/LinkerScript.cpp
@@ -251,30 +251,34 @@ getChangedSymbolAssignment(const SymbolAssignmentMap &oldValues) {
 // Process INSERT [AFTER|BEFORE] commands. For each command, we move the
 // specified output section to the designated place.
 void LinkerScript::processInsertCommands() {
+  std::vector<OutputSection *> moves;
   for (const InsertCommand &cmd : insertCommands) {
-    // If cmd.os is empty, it may have been discarded by
-    // adjustSectionsBeforeSorting(). We do not handle such output sections.
-    auto from = llvm::find_if(sectionCommands, [&](BaseCommand *base) {
-      return isa<OutputSection>(base) &&
-             cast<OutputSection>(base)->name == cmd.name;
-    });
-    if (from == sectionCommands.end())
-      continue;
-    OutputSection *osec = cast<OutputSection>(*from);
-    sectionCommands.erase(from);
+    for (StringRef name : cmd.names) {
+      // If base is empty, it may have been discarded by
+      // adjustSectionsBeforeSorting(). We do not handle such output sections.
+      auto from = llvm::find_if(sectionCommands, [&](BaseCommand *base) {
+        return isa<OutputSection>(base) &&
+               cast<OutputSection>(base)->name == name;
+      });
+      if (from == sectionCommands.end())
+        continue;
+      moves.push_back(cast<OutputSection>(*from));
+      sectionCommands.erase(from);
+    }
 
     auto insertPos = llvm::find_if(sectionCommands, [&cmd](BaseCommand *base) {
       auto *to = dyn_cast<OutputSection>(base);
       return to != nullptr && to->name == cmd.where;
     });
     if (insertPos == sectionCommands.end()) {
-      error("unable to insert " + osec->name +
+      error("unable to insert " + cmd.names[0] +
             (cmd.isAfter ? " after " : " before ") + cmd.where);
     } else {
       if (cmd.isAfter)
         ++insertPos;
-      sectionCommands.insert(insertPos, osec);
+      sectionCommands.insert(insertPos, moves.begin(), moves.end());
     }
+    moves.clear();
   }
 }
 

diff  --git a/lld/ELF/LinkerScript.h b/lld/ELF/LinkerScript.h
index 14cd145af5613..0592c52acb84d 100644
--- a/lld/ELF/LinkerScript.h
+++ b/lld/ELF/LinkerScript.h
@@ -227,7 +227,7 @@ struct ByteCommand : BaseCommand {
 };
 
 struct InsertCommand {
-  StringRef name;
+  std::vector<StringRef> names;
   bool isAfter;
   StringRef where;
 };

diff  --git a/lld/ELF/ScriptParser.cpp b/lld/ELF/ScriptParser.cpp
index 3d0d720014f25..4726dd1c6a2c9 100644
--- a/lld/ELF/ScriptParser.cpp
+++ b/lld/ELF/ScriptParser.cpp
@@ -596,9 +596,12 @@ void ScriptParser::readSections() {
   else if (!consume("BEFORE"))
     setError("expected AFTER/BEFORE, but got '" + next() + "'");
   StringRef where = next();
+  std::vector<StringRef> names;
   for (BaseCommand *cmd : v)
     if (auto *os = dyn_cast<OutputSection>(cmd))
-      script->insertCommands.push_back({os->name, isAfter, where});
+      names.push_back(os->name);
+  if (!names.empty())
+    script->insertCommands.push_back({std::move(names), isAfter, where});
 }
 
 void ScriptParser::readTarget() {

diff  --git a/lld/test/ELF/linkerscript/insert-multi.test b/lld/test/ELF/linkerscript/insert-multi.test
new file mode 100644
index 0000000000000..88c065371de0e
--- /dev/null
+++ b/lld/test/ELF/linkerscript/insert-multi.test
@@ -0,0 +1,53 @@
+# REQUIRES: x86
+## Sections in an INSERT command are in a unit. Their order is preserved.
+
+# RUN: split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %t/a.s -o %t/a.o
+# RUN: ld.lld -T %t/a.lds %t/a.o -o %t1
+# RUN: llvm-readelf -S -l %t1 | FileCheck %s
+
+# CHECK:      Name
+# CHECK-NEXT:        NULL
+# CHECK-NEXT: text.3
+# CHECK-NEXT: text.4
+# CHECK-NEXT: text.5
+# CHECK-NEXT: .text
+# CHECK-NEXT: .data
+# CHECK-NEXT: text.6
+# CHECK-NEXT: text.7
+# CHECK-NEXT: text.8
+# CHECK-NEXT: text.0
+# CHECK-NEXT: text.1
+# CHECK-NEXT: text.2
+
+#--- a.lds
+SECTIONS {
+  text.0 : {}
+  text.1 : {}
+  text.2 : {}
+} INSERT AFTER .data;
+
+SECTIONS {
+  text.3 : {}
+  text.4 : {}
+  text.5 : {}
+} INSERT BEFORE .text;
+
+SECTIONS {
+  text.6 : {}
+  text.7 : {}
+  text.8 : {}
+} INSERT AFTER .data;
+
+#--- a.s
+.text; nop
+.section text.0,"ax"; nop
+.section text.1,"ax"; nop
+.section text.2,"ax"; nop
+.section text.3,"ax"; nop
+.section text.4,"ax"; nop
+.section text.5,"ax"; nop
+.section text.6,"ax"; nop
+.section text.7,"ax"; nop
+.section text.8,"ax"; nop
+.data; .byte 0