[ELF] - Implemented --no-dynamic-linker option

[grimar]

Summary

Feature is used for producing static-linked PIE executables
(https://gcc.gnu.org/ml/gcc/2015-06/msg00008.html)
And was implemented in GNU ld https://gcc.gnu.org/ml/gcc/2015-08/msg00099.html

I also found it in linux kernel build system: https://github.com/torvalds/linux/blob/master/arch/x86/boot/compressed/Makefile#L52.
Though I think that x86/x64 bootloader does not really rely on it, I believe it ignores all phdrs except PT_LOAD for now:
https://github.com/torvalds/linux/blob/master/arch/x86/boot/compressed/misc.c#L301

But it is used for PPC:
https://github.com/torvalds/linux/blob/ba6d973f78eb62ffebb32f6ef3334fc9a3b33d22/arch/powerpc/kernel/kexec_elf_64.c#L486

[ruiu]

We do not add .interp if no -dynamic-linker is given. So, do you really need -no-dynamic-linker?

[ruiu]

Also, if both -foo and -no-foo are given, the last one usually wins. But in this patch -no-dynamic-linker always wins no matter where that is in the command line. That is incosnsitent and confusing.

[grimar]

In D30258#683639, @ruiu wrote:

We do not add .interp if no -dynamic-linker is given. So, do you really need -no-dynamic-linker?

No, as I wrote in description for now for x86/x64 kernel I am looking into, I do not really need it,
they ignore everything except PT_LOAD segments.
For PPC it seems they explicitly fail if PT_INTERP is there (according to code I see).
So I implemented because "was there" and it was trivial.

[ruiu]

I wouldn't add this unless it is implemented in the same way as other -no-foo options. Basically you don't need to edit anything but Driver.cpp.

[grimar]

• Addressed review comment.

[ruiu]

LGTM with the following changes.

ELF/Driver.cpp

591 ↗

(On Diff #89474)

As you can see the code is roughly grouped in this function. Please keep it consistent. Probably you need to move the line here.

ELF/Options.td

152 ↗

(On Diff #89474)

Remove this blank line to keep the style consistent.

[grimar]

ELF/Driver.cpp

591 ↗

(On Diff #89474)

Ok, I did not move it there intentionally because we have

Config->Discard = getDiscardOption(Args);

at its place which confused me, probably I had to.

But aside of this I find this grouping is confusing. It mixes real and "calculated" options.
I mean:

• OFormatBinary is ok, just corresponds to OPT_oformat.
• SectionStartMap is different, there is no such option, it uses OPT_section_start/OPT_Ttext/OPT_Tdata/OPT_Tbss
• SortSection and Target2 are ok, because corresponds to OPT_sort_section/OPT_target2
• UnresolvedSymbols depends on OPT_noinhibit_exec/OPT_error_undef/OPT_warn_undef/OPT_no_undefined/OPT_unresolved_symbols/OPT_z

General grouping is also not ideal IMO.

What I would suggest is:

• Do not split Args.hasArg/getString/getInteger groups and sort options alphabetically. That probably much more useful.

I think code readers usually search option implementation by name and not by its type which is impossible to remember.

• Move OFormatBinary, SortSection, Target2 back to that list, because them are regular options connected with single command line parameter which equals to their variable name.
• Keep only SectionStartMap and UnresolvedSymbols outside in separate group.

ELF/Options.td

152 ↗

(On Diff #89474)

Blank line is intentional. Doesn't absence of blank line here

def no_export_dynamic: F<"no-export-dynamic">;
def no_fatal_warnings: F<"no-fatal-warnings">;

is a violation of style ? I think for regular options (not aliases and not ignored)
we always used blank line to separate them.

[grimar]

ELF/Options.td

152 ↗

(On Diff #89474)

Looks like these two options have no empty separating line because does not
have HelpText ? That is inconsistent styling then.
At least because LTO options has HelpText but also have no blank lines.

I think it worth to add help text for all options that are active (not ignored).
For ignored options probably makes sence to put "Option is ignored" or something
as help text too.

I added HelpText for no_dynamic_linker before commiting.

[ruiu]

ELF/Driver.cpp

591 ↗

(On Diff #89474)

Maybe. Let me think about that. Some options depend on other options, but from this code it is not very easy to see.

ELF/Options.td

152 ↗

(On Diff #89474)

This change was inconsistent in this local context. The entire context might be inconsistent, but it is not good to make more it more inconsistent by making an exception inside an exception (if it is exceptional).

lld/trunk/ELF/Driver.cpp

if (!Arg)
     return DiscardPolicy::Default;
   if (Arg->getOption().getID() == OPT_discard_all)
     return DiscardPolicy::All;
   if (Arg->getOption().getID() == OPT_discard_locals)
     return DiscardPolicy::Locals;
   return DiscardPolicy::None;
 }
 
+static StringRef getDynamicLinkerOption(opt::InputArgList &Args) {
+  auto *Arg = Args.getLastArg(OPT_dynamic_linker, OPT_no_dynamic_linker);
+  if (!Arg || Arg->getOption().getID() == OPT_no_dynamic_linker)
+    return "";
+  return Arg->getValue();
+}
+
 static StripPolicy getStripOption(opt::InputArgList &Args) {
   if (auto *Arg = Args.getLastArg(OPT_strip_all, OPT_strip_debug)) {
     if (Arg->getOption().getID() == OPT_strip_all)
       return StripPolicy::All;
     return StripPolicy::Debug;
   }
   return StripPolicy::None;
 }
void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   Config->SaveTemps = Args.hasArg(OPT_save_temps);
   Config->SingleRoRx = Args.hasArg(OPT_no_rosegment);
   Config->Shared = Args.hasArg(OPT_shared);
   Config->Target1Rel = getArg(Args, OPT_target1_rel, OPT_target1_abs, false);
   Config->Threads = getArg(Args, OPT_threads, OPT_no_threads, true);
   Config->Trace = Args.hasArg(OPT_trace);
   Config->Verbose = Args.hasArg(OPT_verbose);
   Config->WarnCommon = Args.hasArg(OPT_warn_common);
-
-  Config->DynamicLinker = getString(Args, OPT_dynamic_linker);
   Config->Entry = getString(Args, OPT_entry);
   Config->Fini = getString(Args, OPT_fini, "_fini");
   Config->Init = getString(Args, OPT_init, "_init");
   Config->LTOAAPipeline = getString(Args, OPT_lto_aa_pipeline);
   Config->LTONewPmPasses = getString(Args, OPT_lto_newpm_passes);
   Config->MapFile = getString(Args, OPT_Map);
   Config->OptRemarksFilename = getString(Args, OPT_opt_remarks_filename);
   Config->OutputFile = getString(Args, OPT_o);
void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   Config->ZNocopyreloc = hasZOption(Args, "nocopyreloc");
   Config->ZNodelete = hasZOption(Args, "nodelete");
   Config->ZNow = hasZOption(Args, "now");
   Config->ZOrigin = hasZOption(Args, "origin");
   Config->ZRelro = !hasZOption(Args, "norelro");
   Config->ZStackSize = getZOptionValue(Args, "stack-size", 0);
   Config->ZWxneeded = hasZOption(Args, "wxneeded");
 
+  Config->DynamicLinker = getDynamicLinkerOption(Args);
   Config->OFormatBinary = isOutputFormatBinary(Args);
   Config->SectionStartMap = getSectionStartMap(Args);
   Config->SortSection = getSortKind(Args);
   Config->Target2 = getTarget2Option(Args);
   Config->UnresolvedSymbols = getUnresolvedSymbolPolicy(Args);
 
   if (Args.hasArg(OPT_print_map))
     Config->MapFile = "-";

lld/trunk/ELF/Options.td

def no_color_diagnostics: F<"no-color-diagnostics">,
   HelpText<"Do not use colors in diagnostics">;
 
 def no_define_common: F<"no-define-common">,
   HelpText<"Do not assign space to common symbols">;
 
 def no_demangle: F<"no-demangle">,
   HelpText<"Do not demangle symbol names">;
 
+def no_dynamic_linker: F<"no-dynamic-linker">,
+  HelpText<"Inhibit output of .interp section">;
+
 def no_export_dynamic: F<"no-export-dynamic">;
 def no_fatal_warnings: F<"no-fatal-warnings">;
 
 def no_gc_sections: F<"no-gc-sections">,
   HelpText<"Disable garbage collection of unused sections">;
 
 def no_gnu_unique: F<"no-gnu-unique">,
   HelpText<"Disable STB_GNU_UNIQUE symbol binding">;

 def allow_shlib_undefined: F<"allow-shlib-undefined">;
 def cref: Flag<["--"], "cref">;
 def detect_odr_violations: F<"detect-odr-violations">;
 def g: Flag<["-"], "g">;
 def no_add_needed: F<"no-add-needed">;
 def no_allow_shlib_undefined: F<"no-allow-shlib-undefined">;
 def no_copy_dt_needed_entries: F<"no-copy-dt-needed-entries">,
   Alias<no_add_needed>;
-def no_dynamic_linker: F<"no-dynamic-linker">;
 def no_mmap_output_file: F<"no-mmap-output-file">;
 def no_warn_common: F<"no-warn-common">;
 def no_warn_mismatch: F<"no-warn-mismatch">;
 def rpath_link: S<"rpath-link">;
 def rpath_link_eq: J<"rpath-link=">;
 def sort_common: F<"sort-common">;
 def stats: F<"stats">;
 def warn_execstack: F<"warn-execstack">;

lld/trunk/test/ELF/no-dynamic-linker.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/shared.s -o %tso.o
+# RUN: ld.lld -shared %tso.o -o %t.so
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+
+# RUN: ld.lld -dynamic-linker foo --no-dynamic-linker %t.o %t.so -o %t
+# RUN: llvm-readobj --program-headers %t | FileCheck %s --check-prefix=NODL
+# NODL-NOT: PT_INTERP
+
+# RUN: ld.lld --no-dynamic-linker -dynamic-linker foo %t.o %t.so -o %t
+# RUN: llvm-readobj --program-headers %t | FileCheck %s --check-prefix=WITHDL
+# WITHDL: PT_INTERP