[lld] r371216 - Reland D66717 [ELF] Do not ICF two sections with different output sections (by SECTIONS commands)

[Fangrui Song via llvm-commits]

Author: maskray
Date: Fri Sep  6 08:57:44 2019
New Revision: 371216

URL: http://llvm.org/viewvc/llvm-project?rev=371216&view=rev
Log:
Reland D66717 [ELF] Do not ICF two sections with different output sections (by SECTIONS commands)

Recommit r370635 (reverted by r371202), with one change: move addOrphanSections() before ICF.

Before, orphan sections in two different partitions may be folded and
moved to the main partition.

Now, InputSection->OutputSection assignment for orphans happens before
ICF. ICF does not fold input sections with different output sections.

With the PR43241 reproduce,
`llvm-objcopy --extract-partition libvr.so libchrome__combined.so libvr.so` => no error

Updated description:

Fixes PR39418. Complements D47241 (the non-linker-script case).

processSectionCommands() assigns input sections to output sections.
ICF is called before it, so .text.foo and .text.bar may be folded even if
their output sections are made different by SECTIONS commands.

```
markLive<ELFT>()
doIcf<ELFT>()                      // During ICF, we don't know the output sections
writeResult()
  combineEhSections<ELFT>()
  script->processSectionCommands() // InputSection -> OutputSection assignment
```

This patch splits processSectionCommands() into processSectionCommands()
and processSymbolAssignments(), and moves
processSectionCommands()/addOrphanSections() before ICF:

```
markLive<ELFT>()
combineEhSections<ELFT>()
script->processSectionCommands()
script->addOrphanSections();
doIcf<ELFT>()                      // should remove folded input sections
writeResult()
  script->processSymbolAssignments()
```

An alternative approach is to unfold a section `sec` in
processSectionCommands() when we find `sec` and `sec->repl` belong to
different output sections. I feel this patch is superior because this
can fold more sections and the decouple of
SectionCommand/SymbolAssignment gives flexibility:

* An ExprValue can't be evaluated before its section is assigned to an
  output section -> we can delete getOutputSectionVA and simplify
  another place where we had to check if the output section is null.
  Moreover, a case in linkerscript/early-assign-symbol.s can be handled
  now.
* processSectionCommands/processSymbolAssignments can be freely moved
  around.

Modified:
    lld/trunk/ELF/Driver.cpp
    lld/trunk/ELF/ICF.cpp
    lld/trunk/ELF/Writer.cpp
    lld/trunk/test/ELF/partition-icf.s

Modified: lld/trunk/ELF/Driver.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Driver.cpp?rev=371216&r1=371215&r2=371216&view=diff
==============================================================================
--- lld/trunk/ELF/Driver.cpp (original)
+++ lld/trunk/ELF/Driver.cpp Fri Sep  6 08:57:44 2019
@@ -1919,6 +1919,11 @@ template <class ELFT> void LinkerDriver:
   // Create output sections described by SECTIONS commands.
   script->processSectionCommands();
 
+  // Linker scripts control how input sections are assigned to output sections.
+  // Input sections that were not handled by scripts are called "orphans", and
+  // they are assigned to output sections by the default rule. Process that.
+  script->addOrphanSections();
+
   // Two input sections with different output sections should not be folded.
   // ICF runs after processSectionCommands() so that we know the output sections.
   if (config->icf != ICFLevel::None) {

Modified: lld/trunk/ELF/ICF.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/ICF.cpp?rev=371216&r1=371215&r2=371216&view=diff
==============================================================================
--- lld/trunk/ELF/ICF.cpp (original)
+++ lld/trunk/ELF/ICF.cpp Fri Sep  6 08:57:44 2019
@@ -306,8 +306,8 @@ bool ICF<ELFT>::equalsConstant(const Inp
     return false;
 
   // If two sections have different output sections, we cannot merge them.
-  if (getOutputSectionName(a) != getOutputSectionName(b) ||
-      a->getParent() != b->getParent())
+  assert(a->getParent() && b->getParent());
+  if (a->getParent() != b->getParent())
     return false;
 
   if (a->areRelocsRela)

Modified: lld/trunk/ELF/Writer.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=371216&r1=371215&r2=371216&view=diff
==============================================================================
--- lld/trunk/ELF/Writer.cpp (original)
+++ lld/trunk/ELF/Writer.cpp Fri Sep  6 08:57:44 2019
@@ -534,11 +534,6 @@ template <class ELFT> void elf::createSy
 
 // The main function of the writer.
 template <class ELFT> void Writer<ELFT>::run() {
-  // Linker scripts controls how input sections are assigned to output sections.
-  // Input sections that were not handled by scripts are called "orphans", and
-  // they are assigned to output sections by the default rule. Process that.
-  script->addOrphanSections();
-
   if (config->discard != DiscardPolicy::All)
     copyLocalSymbols();
 

Modified: lld/trunk/test/ELF/partition-icf.s
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/partition-icf.s?rev=371216&r1=371215&r2=371216&view=diff
==============================================================================
--- lld/trunk/test/ELF/partition-icf.s (original)
+++ lld/trunk/test/ELF/partition-icf.s Fri Sep  6 08:57:44 2019
@@ -3,15 +3,16 @@
 // RUN: ld.lld %t.o -o %t --export-dynamic --gc-sections --icf=all
 // RUN: llvm-readelf -S -s %t | FileCheck %s
 
-// CHECK: [[MAIN:[0-9]+]]] .text
+// CHECK: part1
 // CHECK: [[P1:[0-9]+]]] .text
+// CHECK: part2
 // CHECK: [[P2:[0-9]+]]] .text
 
 // CHECK: Symbol table '.symtab'
 // CHECK:   [[P1]] f1
 // CHECK:   [[P2]] f2
-// CHECK: [[MAIN]] g1
-// CHECK: [[MAIN]] g2
+// CHECK:   [[P1]] g1
+// CHECK:   [[P2]] g2
 
 .section .llvm_sympart.f1,"", at llvm_sympart
 .asciz "part1"