[ELF] - Do not remove empty sections that use symbols in expressions.

[grimar]

Summary

This is PR36515.

Currenly if we have script like .debug_info 0 : { *(.debug_info) },
we would not remove this section and keep it in output.
That does not work, because it is common case for
debug sections to have zero address expression.
Patch changes behavior so that we remove only sections
that do not use symbols in its expressions.

[jhenderson]

test/ELF/linkerscript/empty-sections-expressions.s

12–14

I think I'd personally prefer the test define foo and then checks to make sure that the section is emitted in this case, as that demonstrates the end effect better. We could change the underlying mechanism to detect errors and still remove the section, but then this test doesn't actually test the claimed behaviour.

[grimar]

• Updated testcase in according to review comments.

[jhenderson]

LGTM, with nit.

@ruiu / @espindola should probably still review it, as I'm aware that there's a little bit of concern about adding extra class members sometimes.

ELF/OutputSections.cpp

89

defined in output -> defined in the output

ELF/OutputSections.h

public:
   std::vector<StringRef> Phdrs;
   llvm::Optional<uint32_t> Filler;
   ConstraintKind Constraint = ConstraintKind::NoConstraint;
   std::string Location;
   std::string MemoryRegionName;
   std::string LMARegionName;
   bool NonAlloc = false;
   bool Noload = false;
+  bool ExpressionsUseSymbols = false;
 
   template <class ELFT> void finalize();
   template <class ELFT> void writeTo(uint8_t *Buf);
   template <class ELFT> void maybeCompress();
 
   void sort(std::function<int(InputSectionBase *S)> Order);
   void sortInitFini();
   void sortCtorsDtors();

ELF/OutputSections.cpp

 }
 
 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)
+  // We do not want to remove sections that reference symbols in address and
+  // other expressions. We add script symbols as undefined, and want to ensure
+  // all of them are defined in output, hence have to keep them.
+  if (ExpressionsUseSymbols)
     return false;
 
   for (BaseCommand *Base : SectionCommands)
     if (!isa<InputSectionDescription>(*Base))
       return false;
   return true;
 }
 

ELF/ScriptParser.cpp

return [=] {
     return Alignment;
   };
 }
 
 OutputSection *ScriptParser::readOutputSectionDescription(StringRef OutSec) {
   OutputSection *Cmd =
       Script->createOutputSection(OutSec, getCurrentLocation());
 
+  size_t SymbolsReferenced = Script->ReferencedSymbols.size();
+
   if (peek() != ":")
     readSectionAddressType(Cmd);
   expect(":");
 
   std::string Location = getCurrentLocation();
   if (consume("AT"))
     Cmd->LMAExpr = readParenExpr();
   if (consume("ALIGN"))
OutputSection *ScriptParser::readOutputSectionDescription(StringRef OutSec) {
   if (consume("="))
     Cmd->Filler = parseFill(next());
   else if (peek().startswith("="))
     Cmd->Filler = parseFill(next().drop_front());
 
   // Consume optional comma following output section command.
   consume(",");
 
+  if (Script->ReferencedSymbols.size() > SymbolsReferenced)
+    Cmd->ExpressionsUseSymbols = true;
   return Cmd;
 }
 
 // Parses a given string as a octal/decimal/hexadecimal number and
 // returns it as a big-endian number. Used for `=<fillexp>`.
 // https://sourceware.org/binutils/docs/ld/Output-Section-Fill.html
 //
 // When reading a hexstring, ld.bfd handles it as a blob of arbitrary

test/ELF/linkerscript/empty-sections-expressions.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+
+## We remove empty sections that do not reference symbols in address,
+## LMA, align and subalign expressions. Here we check that.
+
+# RUN: echo "SECTIONS { .debug_info 0 : { *(.debug_info) } }" > %t.script
+# RUN: ld.lld -o %t --script %t.script %t.o
+# RUN: llvm-objdump -section-headers %t | FileCheck %s
+# CHECK-NOT: debug_info
+
+# RUN: echo "SECTIONS { .debug_info foo : { *(.debug_info) } }" > %t2.script
+# RUN: not ld.lld -o %t --script %t2.script %t.o 2>&1 | FileCheck %s --check-prefix=ERR
+# ERR: symbol not found: foo
+
+.text
+.globl _start
+_start:
+ nop