[ELF] Don't create empty output section for unreferenced PROVIDE symbol

[jhenderson]

Summary

LLD removes empty output sections otherwise specified in the linker script. Prior to this change however, if section descriptions included ANY kind of symbol assignment, then the consequent output section would not be removed, even if the assignment was marked with PROVIDE and not actually triggered (i.e. the symbol was never referenced). This change modifies the isDiscarable function to ignore such directives when determining whether a section should be discarded, in keeping with bfd's behaviour. Symbol assignments that do result in a symbol definition will continue to result in a kept section (this is not actually the same as bfd's behaviour, but it is simpler, and probably makes more sense).

[grimar]

LGTM with few nits.

test/ELF/linkerscript/provide-empty-section.s

5 ↗

(On Diff #153486)

I would avoid using additional input file and -defsym and be more explicit
(Just echoing code to llvm-mc is the common way when there is a little amount of code I believe).

# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %tundefined.o
# RUN: echo "foo=42" | llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %tdefined.o
# RUN: echo "call foo" | llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %treference.o

11 ↗

(On Diff #153486)

Let's do it a single line. That is what we usually try to do if possible and also it avoids multiple echo calls.

# RUN: echo "SECTIONS { .bar : { PROVIDE(foo = .); } }" > %t.script

[grimar]

ELF/LinkerScript.cpp

841 ↗

(On Diff #153486)

I also was wondered if we might want to abstract from Provide here and use something like

if (Cmd->Name != "." && !Cmd->Sym)
...

But I am not sure it worth doing.

[jhenderson]

Rather than rush this in before I go home today, I'll wait until Monday before putting it in, assuming there are no objections by then.

ELF/LinkerScript.cpp

841 ↗

(On Diff #153486)

My one concern with this change is that it relies on us setting Provide to false when we define a symbol because of it. I'll try with your suggested change on Monday before putting this in, as I think it feels a little safer. Are you happy with it going straight in if I make that change?

test/ELF/linkerscript/provide-empty-section.s

5 ↗

(On Diff #153486)

Ah, I didn't know about that technique. I'll make that change, no problem.

11 ↗

(On Diff #153486)

Will do.

[grimar]

ELF/LinkerScript.cpp

841 ↗

(On Diff #153486)

Yep.

lld/trunk/ELF/LinkerScript.cpp

if (!Sec.Phdrs.empty())
     return false;
 
   // 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 the output, hence have to keep them.
   if (Sec.ExpressionsUseSymbols)
     return false;
 
-  for (BaseCommand *Base : Sec.SectionCommands)
+  for (BaseCommand *Base : Sec.SectionCommands) {
+    if (auto Cmd = dyn_cast<SymbolAssignment>(Base))
+      // Don't create empty output sections just for unreferenced PROVIDE
+      // symbols.
+      if (Cmd->Name != "." && !Cmd->Sym)
+        continue;
+
     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

lld/trunk/test/ELF/linkerscript/provide-empty-section.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %tundefined.o
+# RUN: echo "foo=42" | llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %tdefined.o
+# RUN: echo "call foo" | llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %treference.o
+
+# RUN: echo "SECTIONS { .bar : { PROVIDE(foo = .); } }" > %t.script
+
+# Case 1: Provided symbol is undefined and not referenced - empty section should be removed.
+# RUN: ld.lld %tundefined.o -T %t.script -o %t1.elf
+# RUN: llvm-readobj -sections %t1.elf | FileCheck %s --check-prefix=NOSECTION
+
+# Case 2: Provided symbol is undefined and referenced - empty section should not be removed.
+# RUN: ld.lld %tundefined.o %treference.o -T %t.script -o %t2.elf
+# RUN: llvm-readobj -sections %t2.elf | FileCheck %s --check-prefix=SECTION
+
+# Case 3: Provided symbol is defined and not referenced - empty section should be removed.
+# RUN: ld.lld %tdefined.o -T %t.script -o %t3.elf
+# RUN: llvm-readobj -sections %t3.elf | FileCheck %s --check-prefix=NOSECTION
+
+# Case 4: Provided symbol is defined and referenced - empty section should not be removed.
+# RUN: ld.lld %tdefined.o %treference.o -T %t.script -o %t4.elf
+# RUN: llvm-readobj -sections %t4.elf | FileCheck %s --check-prefix=SECTION
+
+.global _start
+_start:
+    ret
+
+# SECTION: .bar
+# NOSECTION-NOT: .bar