[lld] 3f5dc57 - [LLD][ELF] - Don't keep empty output sections which have explicit program headers.

Georgii Rymar via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 2 00:22:46 PST 2020


Author: Georgii Rymar
Date: 2020-12-02T11:19:21+03:00
New Revision: 3f5dc57fd18106766f45216b5ddd4648d2fb4629

URL: https://github.com/llvm/llvm-project/commit/3f5dc57fd18106766f45216b5ddd4648d2fb4629
DIFF: https://github.com/llvm/llvm-project/commit/3f5dc57fd18106766f45216b5ddd4648d2fb4629.diff

LOG: [LLD][ELF] - Don't keep empty output sections which have explicit program headers.

This reverts a side effect introduced in the code cleanup patch D43571:
LLD started to emit empty output sections that are explicitly assigned to a segment.

This patch fixes the issue by removing the !sec.phdrs.empty() special case from
isDiscardable. As compensation, we add an early phdrs propagation step (see the inline comment).
This is similar to one that we do in adjustSectionsAfterSorting.

Differential revision: https://reviews.llvm.org/D92301

Added: 
    

Modified: 
    lld/ELF/LinkerScript.cpp
    lld/test/ELF/linkerscript/empty-relaplt-dyntags.test
    lld/test/ELF/linkerscript/implicit-program-header.test
    lld/test/ELF/linkerscript/orphan-phdrs.s

Removed: 
    


################################################################################
diff  --git a/lld/ELF/LinkerScript.cpp b/lld/ELF/LinkerScript.cpp
index 5bb977d6882b..201f1e48f1fb 100644
--- a/lld/ELF/LinkerScript.cpp
+++ b/lld/ELF/LinkerScript.cpp
@@ -987,11 +987,6 @@ static bool isDiscardable(OutputSection &sec) {
   if (sec.name == "/DISCARD/")
     return true;
 
-  // We do not remove empty sections that are explicitly
-  // assigned to any segment.
-  if (!sec.phdrs.empty())
-    return false;
-
   // We do not want to remove OutputSections with expressions that reference
   // symbols even if the OutputSection is empty. We want to ensure that the
   // expressions can be evaluated and report an error if they cannot.
@@ -1017,6 +1012,18 @@ static bool isDiscardable(OutputSection &sec) {
   return true;
 }
 
+static void maybePropagatePhdrs(OutputSection &sec,
+                                std::vector<StringRef> &phdrs) {
+  if (sec.phdrs.empty()) {
+    // To match the bfd linker script behaviour, only propagate program
+    // headers to sections that are allocated.
+    if (sec.flags & SHF_ALLOC)
+      sec.phdrs = phdrs;
+  } else {
+    phdrs = sec.phdrs;
+  }
+}
+
 void LinkerScript::adjustSectionsBeforeSorting() {
   // If the output section contains only symbol assignments, create a
   // corresponding output section. The issue is what to do with linker script
@@ -1040,6 +1047,7 @@ void LinkerScript::adjustSectionsBeforeSorting() {
   // the previous sections. Only a few flags are needed to keep the impact low.
   uint64_t flags = SHF_ALLOC;
 
+  std::vector<StringRef> defPhdrs;
   for (BaseCommand *&cmd : sectionCommands) {
     auto *sec = dyn_cast<OutputSection>(cmd);
     if (!sec)
@@ -1062,6 +1070,18 @@ void LinkerScript::adjustSectionsBeforeSorting() {
       sec->flags = flags & ((sec->nonAlloc ? 0 : (uint64_t)SHF_ALLOC) |
                             SHF_WRITE | SHF_EXECINSTR);
 
+    // The code below may remove empty output sections. We should save the
+    // specified program headers (if exist) and propagate them to subsequent
+    // sections which do not specify program headers.
+    // An example of such a linker script is:
+    // SECTIONS { .empty : { *(.empty) } :rw
+    //            .foo : { *(.foo) } }
+    // Note: at this point the order of output sections has not been finalized,
+    // because orphans have not been inserted into their expected positions. We
+    // will handle them in adjustSectionsAfterSorting().
+    if (sec->sectionIndex != UINT32_MAX)
+      maybePropagatePhdrs(*sec, defPhdrs);
+
     if (isEmpty && isDiscardable(*sec)) {
       sec->markDead();
       cmd = nullptr;
@@ -1106,20 +1126,9 @@ void LinkerScript::adjustSectionsAfterSorting() {
 
   // Walk the commands and propagate the program headers to commands that don't
   // explicitly specify them.
-  for (BaseCommand *base : sectionCommands) {
-    auto *sec = dyn_cast<OutputSection>(base);
-    if (!sec)
-      continue;
-
-    if (sec->phdrs.empty()) {
-      // To match the bfd linker script behaviour, only propagate program
-      // headers to sections that are allocated.
-      if (sec->flags & SHF_ALLOC)
-        sec->phdrs = defPhdrs;
-    } else {
-      defPhdrs = sec->phdrs;
-    }
-  }
+  for (BaseCommand *base : sectionCommands)
+    if (auto *sec = dyn_cast<OutputSection>(base))
+      maybePropagatePhdrs(*sec, defPhdrs);
 }
 
 static uint64_t computeBase(uint64_t min, bool allocateHeaders) {

diff  --git a/lld/test/ELF/linkerscript/empty-relaplt-dyntags.test b/lld/test/ELF/linkerscript/empty-relaplt-dyntags.test
index 3836dfb02d02..83bb9f15451f 100644
--- a/lld/test/ELF/linkerscript/empty-relaplt-dyntags.test
+++ b/lld/test/ELF/linkerscript/empty-relaplt-dyntags.test
@@ -3,31 +3,11 @@
 # RUN: ld.lld -shared %t.o -T %s -o %t
 # RUN: llvm-readobj --dynamic-table --sections %t | FileCheck %s
 
-## In spite of .rela.plt is empty, it might have been preserved because it is
-## mentioned in the linker script. However, even in that case, we should not
-## produce DT_JMPREL and DT_PLTGOT tags because this can cause a dynamic loader
-## to write into slots in .got.plt it considers reserved to support lazy symbol
-## resolution. In fact, as .got.plt is also empty, that memory might be
-## allocated for something else.
+## Check that we remove the empty .rela.plt section even when it
+## is explicitly assigned to a program header.
+## Check that no related dynamic tags are produced.
 
-# CHECK: Sections [
-# CHECK:      Name: .rela.plt
-# CHECK-NEXT: Type: SHT_RELA
-# CHECK-NEXT: Flags [
-# CHECK-NEXT:   SHF_ALLOC
-# CHECK-NEXT: ]
-# CHECK-NEXT: Address:
-# CHECK-NEXT: Offset:
-# CHECK-NEXT: Size: 0
-# CHECK:      Name: .got.plt
-# CHECK-NEXT: Type: SHT_PROGBITS
-# CHECK-NEXT: Flags [
-# CHECK-NEXT:   SHF_ALLOC
-# CHECK-NEXT:   SHF_WRITE
-# CHECK-NEXT: ]
-# CHECK-NEXT: Address:
-# CHECK-NEXT: Offset:
-# CHECK-NEXT: Size: 0
+# CHECK-NOT: Name: .rela.plt
 
 # CHECK: DynamicSection [
 # CHECK-NOT: JMPREL

diff  --git a/lld/test/ELF/linkerscript/implicit-program-header.test b/lld/test/ELF/linkerscript/implicit-program-header.test
index 8a3a4c6684af..d42e52532269 100644
--- a/lld/test/ELF/linkerscript/implicit-program-header.test
+++ b/lld/test/ELF/linkerscript/implicit-program-header.test
@@ -7,16 +7,18 @@
 # RUN: llvm-readelf -l %t1 | FileCheck %s
 
 # CHECK:      Segment Sections...
-# CHECK-NEXT:   00     .dynsym .hash .dynstr .bar .foo .text .dynamic
-# CHECK-NEXT:   01     .bar .foo
+# CHECK-NEXT:   00     .dynsym .hash .dynstr .foo .text .dynamic
+# CHECK-NEXT:   01     .foo
+# CHECK-NEXT:   02     .foo
 
 PHDRS {
   ph_write PT_LOAD FLAGS(2);
   ph_exec  PT_LOAD FLAGS(1);
+  ph_note  PT_NOTE;
 }
 
 SECTIONS {
-  .bar : { *(.bar) } : ph_exec
+  .bar : { *(.bar) } : ph_exec : ph_note
   .foo : { *(.foo) }
   .text : { *(.text) } : ph_write
 }

diff  --git a/lld/test/ELF/linkerscript/orphan-phdrs.s b/lld/test/ELF/linkerscript/orphan-phdrs.s
index 8bfa24b0d033..105a5ea012a9 100644
--- a/lld/test/ELF/linkerscript/orphan-phdrs.s
+++ b/lld/test/ELF/linkerscript/orphan-phdrs.s
@@ -18,12 +18,11 @@
 # CHECK: Section Headers
 # CHECK: .text
 # CHECK-NEXT: .orphan
-# CHECK-NEXT: .empty
 # CHECK-NEXT: .rw
 
 # CHECK: Segment Sections
 # CHECK-NEXT: .text .orphan
-# CHECK-NEXT: .empty .rw
+# CHECK-NEXT: .rw
 
 .section .text,"ax"
  ret


        


More information about the llvm-commits mailing list