[ELF] - Implement --orphan-handling option.

[grimar]

Summary

We have PR34946 about that LLD does not support this option.

Spec (http://man7.org/linux/man-pages/man1/ld.1.html) tells about --orphan-handling=MODE,
option where MODE can be one of four: "place", "discard", "warn", "error".
Currently we already report orphans when -verbose given, what becomes excessive with option implemented.

Given above I think reasonable behavior is to stop reporting orphans when -versbose is given,
and support "place", "warn" and "error" modes. I am not sure about usefulness of "discard"
and its a bit scary, so I suggest to not implement it. Patch demonstrate the change I am suggesting.

[grimar]

Ping. Are we going to support this ?

[ruiu]

ELF/LinkerScript.cpp

459–460 ↗

(On Diff #119304)

You don't need this if you use else if instead of else.

[grimar]

• Addressed review comments.

[ruiu]

LGTM

ELF/LinkerScript.cpp

461–464 ↗

(On Diff #120230)

nit: it is more natural if you swap the positions of warn and error.

test/ELF/linkerscript/orphan-report.s

46–48 ↗

(On Diff #120230)

In general, when you give a bogus value, you should give a value that is obviously bogus, because it conveys your meaning better. Instead of "discard", pass "foo", for example. This comment applies to other patches.

[grimar]

test/ELF/linkerscript/orphan-report.s

46–48 ↗

(On Diff #120230)

I passed "discard" here intentionally to document that we do not support this mode explicitly.
I can change to "foo" case as well, but I supposed "discard" case is useful itself.

[ruiu]

test/ELF/linkerscript/orphan-report.s

46–48 ↗

(On Diff #120230)

I don't think I agree. Testing that something is not supported seems a bit odd... Unit tests are to make sure that all your existing features are still working fine, but this test doesn't work for that purpose. We can, for example, add tests to make sure that some Solaris linker options are not supported in lld, but do we want to add such tests? I think the answer is no, because, it doesn't make sense as tests.

[grimar]

test/ELF/linkerscript/orphan-report.s

46–48 ↗

(On Diff #120230)

May be you are right. I have no strong opinion it seems, both positions seems reasonable for me.
My concern is that there is some difference between "feature is not supported" and "feature explicitly not supported".
If we choose the second then I think we take responsibility that it should not have unexpected behavior, like
we probably do not want "discard" be just ignored by mistake, what can require test.

I'll change to "foo" before commit though as I have no strong opinion on that point.

lld/trunk/ELF/Config.h

 enum class DiscardPolicy { Default, All, Locals, None };
 
 // For --strip-{all,debug}.
 enum class StripPolicy { None, All, Debug };
 
 // For --unresolved-symbols.
 enum class UnresolvedPolicy { ReportError, Warn, Ignore, IgnoreAll };
 
+// For --orphan-handling.
+enum class OrphanHandlingPolicy { Place, Warn, Error };
+
 // For --sort-section and linkerscript sorting rules.
 enum class SortSectionPolicy { Default, None, Alignment, Name, Priority };
 
 // For --target2
 enum class Target2Policy { Abs, Rel, GotRel };
 
 struct SymbolVersion {
   llvm::StringRef Name;
struct Configuration {
   bool ZNow;
   bool ZOrigin;
   bool ZRelro;
   bool ZRodynamic;
   bool ZText;
   bool ExitEarly;
   bool ZWxneeded;
   DiscardPolicy Discard;
+  OrphanHandlingPolicy OrphanHandling;
   SortSectionPolicy SortSection;
   StripPolicy Strip;
   UnresolvedPolicy UnresolvedSymbols;
   Target2Policy Target2;
   BuildIdKind BuildId = BuildIdKind::None;
   ELFKind EKind = ELFNoneKind;
   uint16_t DefaultSymbolVersion = llvm::ELF::VER_NDX_GLOBAL;
   uint16_t EMachine = llvm::ELF::EM_NONE;

lld/trunk/ELF/Driver.cpp

if (S == "alignment")
     return SortSectionPolicy::Alignment;
   if (S == "name")
     return SortSectionPolicy::Name;
   if (!S.empty())
     error("unknown --sort-section rule: " + S);
   return SortSectionPolicy::Default;
 }
 
+static OrphanHandlingPolicy getOrphanHandling(opt::InputArgList &Args) {
+  StringRef S = Args.getLastArgValue(OPT_orphan_handling, "place");
+  if (S == "warn")
+    return OrphanHandlingPolicy::Warn;
+  if (S == "error")
+    return OrphanHandlingPolicy::Error;
+  if (S != "place")
+    error("unknown --orphan-handling mode: " + S);
+  return OrphanHandlingPolicy::Place;
+}
+
 // Parse --build-id or --build-id=<style>. We handle "tree" as a
 // synonym for "sha1" because all our hash functions including
 // -build-id=sha1 are actually tree hashes for performance reasons.
 static std::pair<BuildIdKind, std::vector<uint8_t>>
 getBuildId(opt::InputArgList &Args) {
   auto *Arg = Args.getLastArg(OPT_build_id, OPT_build_id_eq);
   if (!Arg)
     return {BuildIdKind::None, {}};
void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   Config->NoUndefinedVersion = Args.hasArg(OPT_no_undefined_version);
   Config->NoinhibitExec = Args.hasArg(OPT_noinhibit_exec);
   Config->Nostdlib = Args.hasArg(OPT_nostdlib);
   Config->OFormatBinary = isOutputFormatBinary(Args);
   Config->Omagic = Args.hasArg(OPT_omagic);
   Config->OptRemarksFilename = Args.getLastArgValue(OPT_opt_remarks_filename);
   Config->OptRemarksWithHotness = Args.hasArg(OPT_opt_remarks_with_hotness);
   Config->Optimize = getInteger(Args, OPT_O, 1);
+  Config->OrphanHandling = getOrphanHandling(Args);
   Config->OutputFile = Args.getLastArgValue(OPT_o);
   Config->Pie = Args.hasFlag(OPT_pie, OPT_nopie, false);
   Config->PrintGcSections = Args.hasArg(OPT_print_gc_sections);
   Config->Rpath = getRpath(Args);
   Config->Relocatable = Args.hasArg(OPT_relocatable);
   Config->SaveTemps = Args.hasArg(OPT_save_temps);
   Config->SearchPaths = getArgs(Args, OPT_library_path);
   Config->SectionStartMap = getSectionStartMap(Args);

lld/trunk/ELF/LinkerScript.cpp

static OutputSection *findByName(ArrayRef<BaseCommand *> Vec,
                                  StringRef Name) {
   for (BaseCommand *Base : Vec)
     if (auto *Sec = dyn_cast<OutputSection>(Base))
       if (Sec->Name == Name)
         return Sec;
   return nullptr;
 }
 
+static void reportOrphan(InputSectionBase *IS, StringRef Name) {
+  if (Config->OrphanHandling == OrphanHandlingPolicy::Error)
+    error(toString(IS) + " is being placed in '" + Name + "'");
+  else if (Config->OrphanHandling == OrphanHandlingPolicy::Warn)
+    warn(toString(IS) + " is being placed in '" + Name + "'");
+}
+
 // Add sections that didn't match any sections command.
 void LinkerScript::addOrphanSections(OutputSectionFactory &Factory) {
   unsigned End = SectionCommands.size();
 
   for (InputSectionBase *S : InputSections) {
     if (!S->Live || S->Parent)
       continue;
 
     StringRef Name = getOutputSectionName(S->Name);
-    log(toString(S) + " is being placed in '" + Name + "'");
+    reportOrphan(S, Name);
 
     if (OutputSection *Sec =
             findByName(makeArrayRef(SectionCommands).slice(0, End), Name)) {
       Sec->addSection(cast<InputSection>(S));
       continue;
     }
 
     if (OutputSection *OS = Factory.addInputSec(S, Name))

lld/trunk/ELF/Options.td

def o: JoinedOrSeparate<["-"], "o">, MetaVarName<"<path>">,
   HelpText<"Path to file to write output">;
 
 def oformat: Separate<["--"], "oformat">, MetaVarName<"<format>">,
   HelpText<"Specify the binary format for the output object file">;
 
 def omagic: Flag<["--"], "omagic">, MetaVarName<"<magic>">,
   HelpText<"Set the text and data sections to be readable and writable">;
 
+defm orphan_handling: Eq<"orphan-handling">, 
+  HelpText<"Control how orphan sections are handled when linker script used">;
+
 def pie: F<"pie">, HelpText<"Create a position independent executable">;
 
 def print_gc_sections: F<"print-gc-sections">,
   HelpText<"List removed unused sections">;
 
 def print_map: F<"print-map">,
   HelpText<"Print a link map to the standard output">;
 

lld/trunk/test/ELF/linkerscript/orphan-report.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
 # RUN: echo "SECTIONS { .text : { *(.text.1) } }" > %t.script
-# RUN: ld.lld --hash-style=sysv -shared -o %t.out --script %t.script %t.o --verbose | FileCheck %s
 
-# CHECK:      {{.*}}.o:(.text) is being placed in '.text'
-# CHECK-NEXT: {{.*}}.o:(.text.2) is being placed in '.text'
-# CHECK-NEXT: <internal>:(.comment) is being placed in '.comment'
-# CHECK-NEXT: <internal>:(.bss) is being placed in '.bss'
-# CHECK-NEXT: <internal>:(.bss.rel.ro) is being placed in '.bss.rel.ro'
-# CHECK-NEXT: <internal>:(.dynsym) is being placed in '.dynsym'
-# CHECK-NEXT: <internal>:(.gnu.version) is being placed in '.gnu.version'
-# CHECK-NEXT: <internal>:(.gnu.version_r) is being placed in '.gnu.version_r'
-# CHECK-NEXT: <internal>:(.hash) is being placed in '.hash'
-# CHECK-NEXT: <internal>:(.dynamic) is being placed in '.dynamic'
-# CHECK-NEXT: <internal>:(.dynstr) is being placed in '.dynstr'
-# CHECK-NEXT: <internal>:(.rela.dyn) is being placed in '.rela.dyn'
-# CHECK-NEXT: <internal>:(.got) is being placed in '.got'
-# CHECK-NEXT: <internal>:(.got.plt) is being placed in '.got.plt'
-# CHECK-NEXT: <internal>:(.got.plt) is being placed in '.got.plt'
-# CHECK-NEXT: <internal>:(.rela.plt) is being placed in '.rela.plt'
-# CHECK-NEXT: <internal>:(.rela.plt) is being placed in '.rela.plt'
-# CHECK-NEXT: <internal>:(.plt) is being placed in '.plt'
-# CHECK-NEXT: <internal>:(.plt) is being placed in '.plt'
-# CHECK-NEXT: <internal>:(.eh_frame) is being placed in '.eh_frame'
-# CHECK-NEXT: <internal>:(.symtab) is being placed in '.symtab'
-# CHECK-NEXT: <internal>:(.shstrtab) is being placed in '.shstrtab'
-# CHECK-NEXT: <internal>:(.strtab) is being placed in '.strtab'
+## Check we do not report orphans by default even with -verbose.
+# RUN: ld.lld -shared -o %t.out --script %t.script %t.o 2>&1 -verbose \
+# RUN:   | FileCheck %s --check-prefix=DEFAULT
+# DEFAULT-NOT: placed
+
+## Check --orphan-handling=place has the same behavior as default.
+# RUN: ld.lld -shared --orphan-handling=place -o %t.out --script %t.script \
+# RUN:   %t.o 2>&1 -verbose  -error-limit=0 | FileCheck %s --check-prefix=DEFAULT
+
+## Check --orphan-handling=error reports errors about orphans.
+# RUN: not ld.lld -shared --orphan-handling=error -o %t.out --script %t.script \
+# RUN:   %t.o 2>&1 -verbose  -error-limit=0 | FileCheck %s --check-prefix=REPORT
+# REPORT:      {{.*}}.o:(.text) is being placed in '.text'
+# REPORT-NEXT: {{.*}}.o:(.text.2) is being placed in '.text'
+# REPORT-NEXT: <internal>:(.comment) is being placed in '.comment'
+# REPORT-NEXT: <internal>:(.bss) is being placed in '.bss'
+# REPORT-NEXT: <internal>:(.bss.rel.ro) is being placed in '.bss.rel.ro'
+# REPORT-NEXT: <internal>:(.dynsym) is being placed in '.dynsym'
+# REPORT-NEXT: <internal>:(.gnu.version) is being placed in '.gnu.version'
+# REPORT-NEXT: <internal>:(.gnu.version_r) is being placed in '.gnu.version_r'
+# REPORT-NEXT: <internal>:(.gnu.hash) is being placed in '.gnu.hash'
+# REPORT-NEXT: <internal>:(.hash) is being placed in '.hash'
+# REPORT-NEXT: <internal>:(.dynamic) is being placed in '.dynamic'
+# REPORT-NEXT: <internal>:(.dynstr) is being placed in '.dynstr'
+# REPORT-NEXT: <internal>:(.rela.dyn) is being placed in '.rela.dyn'
+# REPORT-NEXT: <internal>:(.got) is being placed in '.got'
+# REPORT-NEXT: <internal>:(.got.plt) is being placed in '.got.plt'
+# REPORT-NEXT: <internal>:(.got.plt) is being placed in '.got.plt'
+# REPORT-NEXT: <internal>:(.rela.plt) is being placed in '.rela.plt'
+# REPORT-NEXT: <internal>:(.rela.plt) is being placed in '.rela.plt'
+# REPORT-NEXT: <internal>:(.plt) is being placed in '.plt'
+# REPORT-NEXT: <internal>:(.plt) is being placed in '.plt'
+# REPORT-NEXT: <internal>:(.eh_frame) is being placed in '.eh_frame'
+# REPORT-NEXT: <internal>:(.symtab) is being placed in '.symtab'
+# REPORT-NEXT: <internal>:(.shstrtab) is being placed in '.shstrtab'
+# REPORT-NEXT: <internal>:(.strtab) is being placed in '.strtab'
+
+## Check --orphan-handling=warn reports warnings about orphans.
+# RUN: ld.lld -shared --orphan-handling=warn -o %t.out --script %t.script \
+# RUN:   %t.o 2>&1 -verbose | FileCheck %s --check-prefix=REPORT
+
+# RUN: not ld.lld --orphan-handling=foo -o %t.out --script %t.script %t.o 2>&1 \
+# RUN:   | FileCheck %s --check-prefix=UNKNOWN
+# UNKNOWN: unknown --orphan-handling mode: foo
 
 .section .text.1,"a"
  nop
 
 .section .text.2,"a"
  nop