[ELF] Refactor the way we handle -plugin-opt= (GCC collect2 or clang LTO related options)

[MaskRay]

Summary

GCC collect2 passes several options to the linker even if LTO is not used
(note, lld does not support GCC LTO). The lto-wrapper may be a relative
path (especially during development, when gcc is in a build directory), e.g.

-plugin-opt=relative/path/to/lto-wrapper

We need to ignore such options, which are currently interpreted by
cl::ParseCommandLineOptions() and will fail with error: --plugin-opt: ld.lld: Unknown command line argument 'relative/path/to/lto-wrapper'
because the path is apparently not an option registered by an llvm::cl::opt.

See lto-plugin-ignore.s for how we interpret various -plugin-opt= options now.

[MaskRay]

Improve comments.

[tejohnson]

I'm a little concerned that options will be silently ignored, e.g. if there are any other gold-plugin options that aren't yet supported. Would it be possible to capture these options and issue warnings for all of the ones that get ignored?

[MaskRay]

Check if '/' is in -plugin-opt=

[tejohnson]

lld/ELF/Driver.cpp

1046

This looks better to me. I suppose the ignore checking could be an even tighter, e.g. it contains either liblto_plugin.so or lto-wrapper - does that seem reasonable? Probably don't even need to check for '/' in that case.

[MaskRay]

Just special case lto-wrapper

It seems that this is the only option

[MaskRay]

Forgot to add -plugin path/to/liblto_plugin.so

[tejohnson]

lgtm

[grimar]

LG with 2 minor nits.

lld/ELF/Options.td

573–583

Why uppercase? Seems inconsistent with: "lto_obj_path_eq", "thinlto_index_only_eq" and others.

583

The same.

[MaskRay]

Use lowercase OPT_plugin_opt_eq_minus and OPT_plugin_opt_eq

lld/ELF/Driver.cpp

static void readConfigs(opt::InputArgList &args) {
   config->zText = getZFlag(args, "text", "notext", true);
   config->zWxneeded = hasZOption(args, "wxneeded");
 
   // Parse LTO options.
   if (auto *arg = args.getLastArg(OPT_plugin_opt_mcpu_eq))
     parseClangOption(saver.save("-mcpu=" + StringRef(arg->getValue())),
                      arg->getSpelling());
 
-  for (auto *arg : args.filtered(OPT_plugin_opt))
-    parseClangOption(arg->getValue(), arg->getSpelling());
+  for (opt::Arg *arg : args.filtered(OPT_plugin_opt_eq_minus))
+    parseClangOption(std::string("-") + arg->getValue(), arg->getSpelling());
+
+  // GCC collect2 passes -plugin-opt=path/to/lto-wrapper with an absolute or
+  // relative path. Just ignore. If not ended with "lto-wrapper", consider it an
+  // unsupported LLVMgold.so option and error.
+  for (opt::Arg *arg : args.filtered(OPT_plugin_opt_eq))
+    if (!StringRef(arg->getValue()).endswith("lto-wrapper"))
+      error(arg->getSpelling() + ": unknown plugin option '" + arg->getValue() +
+            "'");
 
   // Parse -mllvm options.
   for (auto *arg : args.filtered(OPT_mllvm))
     parseClangOption(arg->getValue(), arg->getSpelling());
 
   // --threads= takes a positive integer and provides the default value for
   // --thinlto-jobs=.
   if (auto *arg = args.getLastArg(OPT_threads)) {

lld/ELF/Options.td

 def opt_remarks_filename: Separate<["--"], "opt-remarks-filename">,
   HelpText<"YAML output file for optimization remarks">;
 def opt_remarks_passes: Separate<["--"], "opt-remarks-passes">,
   HelpText<"Regex for the passes that need to be serialized to the output file">;
 def opt_remarks_with_hotness: Flag<["--"], "opt-remarks-with-hotness">,
   HelpText<"Include hotness information in the optimization remarks file">;
 def opt_remarks_format: Separate<["--"], "opt-remarks-format">,
   HelpText<"The format used for serializing remarks (default: YAML)">;
-defm plugin_opt: Eq<"plugin-opt", "specifies LTO options for compatibility with GNU linkers">;
 def save_temps: F<"save-temps">;
 def lto_basicblock_sections: J<"lto-basicblock-sections=">,
   HelpText<"Enable basic block sections for LTO">;
 defm lto_unique_bb_section_names: B<"lto-unique-bb-section-names",
     "Give unique names to every basic block section for LTO",
     "Do not give unique names to every basic block section for LTO (default)">;
 def shuffle_sections: J<"shuffle-sections=">, MetaVarName<"<seed>">,
   HelpText<"Shuffle input sections using the given seed. If 0, use a random seed">;

 // clang -flto passes -plugin and -plugin-opt to the linker. This is required
 // for ld.gold and ld.bfd to get LTO working. But it's not for lld which doesn't
 // rely on a plugin. Instead of detecting which linker is used on clang side we
 // just ignore the option on lld side as it's easier. In fact, the linker could
 // be called 'ld' and understanding which linker is used would require parsing of
 // --version output.
 defm plugin: Eq<"plugin", "Ignored for compatibility with GNU linkers">;
 
-def plugin_opt_fresolution_eq: J<"plugin-opt=-fresolution=">;
-def plugin_opt_pass_through_eq: J<"plugin-opt=-pass-through=">;
-def plugin_opt_thinlto: J<"plugin-opt=thinlto">;
-def plugin_opt_slash: J<"plugin-opt=/">;
+def plugin_opt_eq_minus: J<"plugin-opt=-">,
+  HelpText<"Specify an LLVM option for compatibility with LLVMgold.so">;
+def: J<"plugin-opt=thinlto">;
+
+// Ignore GCC collect2 LTO plugin related options. Note that we don't support
+// GCC LTO, but GCC collect2 passes these options even in non-LTO mode.
+def: J<"plugin-opt=-fresolution=">;
+def: J<"plugin-opt=-pass-through=">;
+// This may be either an unhandled LLVMgold.so feature or GCC passed
+// -plugin-opt=path/to/{liblto_plugin.so,lto-wrapper}
+def plugin_opt_eq : J<"plugin-opt=">;
 
 // Options listed below are silently ignored for now for compatibility.
 def: F<"detect-odr-violations">;
 def: Flag<["-"], "g">;
 def: F<"long-plt">;
 def: F<"no-add-needed">;
 def: F<"no-copy-dt-needed-entries">;
 def: F<"no-ctors-in-init-array">;

lld/test/ELF/lto-plugin-ignore.s

 # REQUIRES: x86
+## Test we ignore some LTO related options from clang/GCC collect2.
 
-# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
-# RUN: ld.lld %t -plugin-opt=/foo/bar -plugin-opt=-fresolution=zed \
-# RUN:   -plugin-opt=-pass-through=-lgcc -plugin-opt=-function-sections \
-# RUN:   -plugin-opt=-data-sections -plugin-opt=thinlto -o /dev/null
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
 
-# RUN: not ld.lld %t -plugin-opt=-abc -plugin-opt=-xyz 2>&1 | FileCheck %s
-# CHECK: ld.lld: error: --plugin-opt: ld.lld{{.*}}: Unknown command line argument '-abc'
-# CHECK: ld.lld: error: --plugin-opt: ld.lld{{.*}}: Unknown command line argument '-xyz'
+## GCC collect2 passes several LTO related options to the linker even if -flto is not used.
+## We need to ignore them. Note that the lto-wrapper path can be relative.
+# RUN: ld.lld %t.o -o /dev/null \
+# RUN:   -plugin path/to/liblto_plugin.so \
+# RUN:   -plugin-opt=/path/to/lto-wrapper \
+# RUN:   -plugin-opt=relative/path/to/lto-wrapper \
+# RUN:   -plugin-opt=-fresolution=zed \
+# RUN:   -plugin-opt=-pass-through=-lgcc \
+# RUN:   -plugin-opt=-pass-through=-lgcc_eh \
+# RUN:   -plugin-opt=-pass-through=-lc
+
+## Clang LTO passes several options to the linker, which are intended to be consumed by
+## LLVMgold.so. We need to ignore them.
+# RUN: ld.lld %t.o -o /dev/null -plugin /path/to/LLVMgold.so -plugin-opt=thinlto
+
+## Other -plugin-opt=- prefixed options are passed through to cl::ParseCommandLineOptions.
+# RUN: not ld.lld %t.o -o /dev/null -plugin-opt=-abc -plugin-opt=-xyz 2>&1 | FileCheck %s
+# CHECK: ld.lld: error: -plugin-opt=-: ld.lld{{.*}}: Unknown command line argument '-abc'
+# CHECK: ld.lld: error: -plugin-opt=-: ld.lld{{.*}}: Unknown command line argument '-xyz'
+
+## Error if the option is an unhandled LLVMgold.so feature.
+# RUN: not ld.lld %t.o -o /dev/null -plugin-opt=LLVMgold-feature 2>&1 | FileCheck --check-prefix=GOLD %s
+# GOLD: ld.lld: error: -plugin-opt=: unknown plugin option 'LLVMgold-feature'