Keep flags from phantom synthetic sections

[• espindola]

Summary

This fixes pr36475.

I think this code can be simplified a bit, but I would like to check in the more direct fix if we are in agreement on the direction and then refactor.

This is not something that bfd does. The issue is not noticed in bfd because it keeps fewer sections from the linkerscript in the output.

The reasons why it seems reasonable to do this:

• As George noticed, we would still keep the flags if the output section had both an empty synthetic section and a regular section
• We need an heuristic to find the flags of output sections. Using the flags of a synthetic section that would have been there seems a reasonable heuristic.

[grimar]

LGTM

[jhenderson]

LGTM, with a couple of comment nits.

ELF/Writer.cpp

1386–1387

Might want to revert this comment back to what it used to say.

test/ELF/linkerscript/empty-synthetic-removed-flags.s

8

have -> has

[• espindola]

r326137

ELF/LinkerScript.cpp

for (InputSection *Sec : Cmd->Sections) {
       if (!Sec->Live)
         continue;
       assert(Ctx->OutSec == Sec->getParent());
       output(Sec);
     }
   }
 }
 
-static bool isAllSectionDescription(const OutputSection &Cmd) {
-  // We do not remove empty sections that are explicitly
-  // assigned to any segment.
-  if (!Cmd.Phdrs.empty())
-    return false;
-
-  // We do not want to remove sections that have custom address or align
-  // expressions set even if them are empty. We keep them because we
-  // want to be sure that any expressions can be evaluated and report
-  // an error otherwise.
-  if (Cmd.AddrExpr || Cmd.AlignExpr || Cmd.LMAExpr)
-    return false;
-
-  for (BaseCommand *Base : Cmd.SectionCommands)
-    if (!isa<InputSectionDescription>(*Base))
-      return false;
-  return true;
-}
-
 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
   // like ".foo : { symbol = 42; }". One option would be to convert it to
   // "symbol = 42;". That is, move the symbol out of the empty section
   // description. That seems to be what bfd does for this simple case. The
   // problem is that this is not completely general. bfd will give up and
   // create a dummy section too if there is a ". = . + 1" inside the section
for (BaseCommand *&Cmd : SectionCommands) {
     auto *Sec = dyn_cast<OutputSection>(Cmd);
     if (!Sec)
       continue;
     if (Sec->Live) {
       Flags = Sec->Flags & (SHF_ALLOC | SHF_WRITE | SHF_EXECINSTR);
       continue;
     }
 
-    if (!isAllSectionDescription(*Sec))
+    if (!Sec->isAllSectionDescription())
       Sec->Flags = Flags;
     else
       Cmd = nullptr;
   }
 
   // It is common practice to use very generic linker scripts. So for any
   // given run some of the output sections in the script will be empty.
   // We could create corresponding empty output sections, but that would

ELF/OutputSections.h

 // This represents a section in an output file.
 // It is composed of multiple InputSections.
 // The writer creates multiple OutputSections and assign them unique,
 // non-overlapping file offsets and VAs.
 class OutputSection final : public BaseCommand, public SectionBase {
 public:
   OutputSection(StringRef Name, uint32_t Type, uint64_t Flags);
 
+  bool isAllSectionDescription() const;
+
   static bool classof(const SectionBase *S) {
     return S->kind() == SectionBase::Output;
   }
 
   static bool classof(const BaseCommand *C);
 
   uint64_t getLMA() const { return PtLoad ? Addr + PtLoad->LMAOffset : Addr; }
   template <typename ELFT> void writeHeaderTo(typename ELFT::Shdr *SHdr);

ELF/OutputSections.cpp

OutputSection::OutputSection(StringRef Name, uint32_t Type, uint64_t Flags)
     : BaseCommand(OutputSectionKind),
       SectionBase(Output, Name, Flags, /*Entsize*/ 0, /*Alignment*/ 1, Type,
                   /*Info*/ 0,
                   /*Link*/ 0),
       SectionIndex(INT_MAX) {
   Live = false;
 }
 
+bool OutputSection::isAllSectionDescription() const {
+  // We do not remove empty sections that are explicitly
+  // assigned to any segment.
+  if (!Phdrs.empty())
+    return false;
+
+  // We do not want to remove sections that have custom address or align
+  // expressions set even if them are empty. We keep them because we
+  // want to be sure that any expressions can be evaluated and report
+  // an error otherwise.
+  if (AddrExpr || AlignExpr || LMAExpr)
+    return false;
+
+  for (BaseCommand *Base : SectionCommands)
+    if (!isa<InputSectionDescription>(*Base))
+      return false;
+  return true;
+}
+
 // We allow sections of types listed below to merged into a
 // single progbits section. This is typically done by linker
 // scripts. Merging nobits and progbits will force disk space
 // to be allocated for nobits sections. Other ones don't require
 // any special treatment on top of progbits, so there doesn't
 // seem to be a harm in merging them.
 static bool canMergeToProgbits(unsigned Type) {
   return Type == SHT_NOBITS || Type == SHT_PROGBITS || Type == SHT_INIT_ARRAY ||

ELF/Writer.cpp

for (InputSectionBase *S : llvm::reverse(InputSections)) {
 
     // If we reach here, then SS is an unused synthetic section and we want to
     // remove it from corresponding input section description of output section.
     for (BaseCommand *B : OS->SectionCommands)
       if (auto *ISD = dyn_cast<InputSectionDescription>(B))
         llvm::erase_if(ISD->Sections,
                        [=](InputSection *IS) { return IS == SS; });
 
     // If there are no other alive input sections left, we remove output
     // section from the output.
-    if (getInputSections(OS).empty())
+    if (getInputSections(OS).empty() && OS->isAllSectionDescription())
       OS->Live = false;
   }
 }
 
 // Returns true if a symbol can be replaced at load-time by a symbol
 // with the same name defined in other ELF executable or DSO.
 static bool computeIsPreemptible(const Symbol &B) {
   assert(!B.isLocal());

test/ELF/linkerscript/empty-synthetic-removed-flags.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
 # RUN: echo "SECTIONS { .foo : { *(.foo) } .bar : { *(.got.plt) BYTE(0x11) }}" > %t.script
 # RUN: ld.lld -o %t --script %t.script %t.o
 # RUN: llvm-readobj -s %t | FileCheck %s
 
 ## We have ".got.plt" synthetic section with SHF_ALLOC|SHF_WRITE flags.
 ## It is empty, so linker removes it, but it have to keep ".got.plt" output
 ## section because of BYTE command. Here we check that result output section
-## gets the same flags as previous allocatable section and does not get
-## SHF_WRITE flag from removed syntethic input section.
+## still remembers what the flags of .got.plt are.
 
 # CHECK:     Section {
 # CHECK:       Index: 2
 # CHECK:       Name: .bar
 # CHECK-NEXT:  Type: SHT_PROGBITS
 # CHECK-NEXT:  Flags [
 # CHECK-NEXT:    SHF_ALLOC
-# CHECK-NEXT:    SHF_EXECINSTR
+# CHECK-NEXT:    SHF_WRITE
 # CHECK-NEXT:  ]
 
 ## Check flags are the same if we omit empty synthetic section in script.
 # RUN: echo "SECTIONS { .foo : { *(.foo) } .bar : { BYTE(0x11) }}" > %t.script
 # RUN: ld.lld -o %t --script %t.script %t.o
-# RUN: llvm-readobj -s %t | FileCheck %s
+# RUN: llvm-readobj -s %t | FileCheck --check-prefix=EMPTY %s
+
+# EMPTY:     Section {
+# EMPTY:       Index: 2
+# EMPTY:       Name: .bar
+# EMPTY-NEXT:  Type: SHT_PROGBITS
+# EMPTY-NEXT:  Flags [
+# EMPTY-NEXT:    SHF_ALLOC
+# EMPTY-NEXT:    SHF_EXECINSTR
+# EMPTY-NEXT:  ]
 
 .section .foo,"ax"
 .quad 0

test/ELF/pr36475.s

+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: echo "PHDRS {" > %t.script
+# RUN: echo " ph_text PT_LOAD FLAGS (0x1 | 0x4);" >> %t.script
+# RUN: echo " ph_data PT_LOAD FLAGS (0x2 | 0x4);" >> %t.script
+# RUN: echo "}" >> %t.script
+# RUN: echo "SECTIONS {" >> %t.script
+# RUN: echo " .text : { *(.text*) } : ph_text" >> %t.script
+# RUN: echo " . = ALIGN(0x4000);" >> %t.script
+# RUN: echo " .got.plt : { BYTE(42); *(.got); } : ph_data" >> %t.script
+# RUN: echo "}" >> %t.script
+# RUN: ld.lld -T %t.script %t.o -o %t.elf
+# RUN: llvm-readobj -l -elf-output-style=GNU %t.elf | FileCheck %s
+
+# CHECK: Section to Segment mapping:
+# CHECK-NEXT: Segment Sections...
+# CHECK-NEXT: 00 .text executable
+# CHECK-NEXT: 01 .got.plt
+
+.text
+.globl _start
+.type _start,@function
+_start:
+    callq custom_func
+    ret
+
+.section executable,"ax",@progbits
+.type custom_func,@function
+custom_func:
+    ret