[ELF] Change -z unknown from error to warning

[MaskRay]

Summary

There is a trend of having more optional options (usually security
hardening related) like -z cet-report=, -z bti-report=, -z force-bti.
If ld.lld 14.0.0 uses a warning, in 15/16/17/... timeframe when people
add new options to software, they can worry less about linker errors on ld.lld 14.0.0.

In some cases -z foo does essential work where a silent ignore can be
problematic, but the user has received a warning. From my observation, the
doing-essential-work -z foo is much fewer than the converse. In addition,
the user who cares can use --fatal-warnings (Note: GNU ld doesn't upgrade warnings to errors).
It is unclear whether we need something like clang -Wunknown-warning-option.

If we ever run into unfortunate transition like -z start-stop-gc, the
affected software (e.g. ldc is a compiler which passes linker options to the underlying ld)
can blindly add the -z option, without worrying it may cause a linker error to LLD 14.0.0.

[jrtc27]

Probably worth pointing out this is what GNU ld does, so there's precedent (that's not just what Clang does)

lld/test/ELF/driver.test

66–70

Should there be --fatal-warnings versions of these? Might be overkill given it's just a call to warn but at the same time doesn't hurt to be extra safe.

[MaskRay]

Move fatalWarnings and test it

[MaskRay]

update a comment

[jrtc27]

But I suggest you get an approval from more than just me

[peter.smith]

LGTM too, good to match GNU ld behaviour where possible. One stale comment spotted but easy to fix up prior to commit.

lld/ELF/Driver.cpp

464

Comment is now stale as we're not reporting an error. Perhaps
// Report a warning for an unknown -z option to match ld.bfd behavior.

[MaskRay]

Update stale comment

lld/ELF/Driver.cpp

return s == "combreloc" || s == "copyreloc" || s == "defs" ||
          s == "rela" || s == "relro" || s == "retpolineplt" ||
          s == "rodynamic" || s == "shstk" || s == "text" || s == "undefs" ||
          s == "wxneeded" || s.startswith("common-page-size=") ||
          s.startswith("dead-reloc-in-nonalloc=") ||
          s.startswith("max-page-size=") || s.startswith("stack-size=") ||
          s.startswith("start-stop-visibility=");
 }
 
-// Report an error for an unknown -z option.
+// Report a warning for an unknown -z option.
 static void checkZOptions(opt::InputArgList &args) {
   for (auto *arg : args.filtered(OPT_z))
     if (!isKnownZFlag(arg->getValue()))
-      error("unknown -z value: " + StringRef(arg->getValue()));
+      warn("unknown -z value: " + StringRef(arg->getValue()));
 }
 
 void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   ELFOptTable parser;
   opt::InputArgList args = parser.parse(argsArr.slice(1));
 
-  // Interpret this flag early because error() depends on them.
+  // Interpret the flags early because error()/warn() depend on them.
   errorHandler().errorLimit = args::getInteger(args, OPT_error_limit, 20);
+  errorHandler().fatalWarnings =
+      args.hasFlag(OPT_fatal_warnings, OPT_no_fatal_warnings, false);
   checkZOptions(args);
 
   // Handle -help
   if (args.hasArg(OPT_help)) {
     printHelp();
     return;
   }
 
if (cl::ParseCommandLineOptions(2, argv, "", &os))
     return;
   os.flush();
   error(msg + ": " + StringRef(err).trim());
 }
 
 // Initializes Config members by the command line options.
 static void readConfigs(opt::InputArgList &args) {
   errorHandler().verbose = args.hasArg(OPT_verbose);
-  errorHandler().fatalWarnings =
-      args.hasFlag(OPT_fatal_warnings, OPT_no_fatal_warnings, false);
   errorHandler().vsDiagnostics =
       args.hasArg(OPT_visual_studio_diagnostics_format, false);
 
   config->allowMultipleDefinition =
       args.hasFlag(OPT_allow_multiple_definition,
                    OPT_no_allow_multiple_definition, false) ||
       hasZOption(args, "muldefs");
   config->auxiliaryList = args::getStrings(args, OPT_auxiliary);

lld/test/ELF/driver.test

 ## "--output=foo" is equivalent to "-o foo".
 # RUN: not ld.lld %t --output=/no/such/file 2>&1 | FileCheck -check-prefix=ERR8 %s
 # ERR8: cannot open output file /no/such/file
 
 ## "-output=foo" is equivalent to "-o utput=foo".
 # RUN: not ld.lld %t -output=/no/such/file 2>&1 | FileCheck -check-prefix=ERR9 %s
 # ERR9: cannot open output file utput=/no/such/file
 
-# RUN: not ld.lld %t -z foo 2>&1 | FileCheck -check-prefix=ERR10 %s
-# RUN: not ld.lld %t -z foo --version 2>&1 | FileCheck -check-prefix=ERR10 %s
-# ERR10: unknown -z value: foo
+# RUN: ld.lld %t -z foo 2>&1 | FileCheck -check-prefix=ERR10 %s
+# RUN: ld.lld %t -z foo --version 2>&1 | FileCheck -check-prefix=ERR10 %s
+# ERR10: warning: unknown -z value: foo
 
 ## Check we report "unknown -z value" error even with -v.
-# RUN: not ld.lld %t -z foo -v 2>&1 | FileCheck -check-prefix=ERR10 %s
+# RUN: ld.lld %t -z foo -v 2>&1 | FileCheck -check-prefix=ERR10 %s
+
+## Note: in GNU ld, --fatal-warning still leads to a warning.
+# RUN: not ld.lld %t -z foo --fatal-warnings 2>&1 | FileCheck --check-prefix=ERR10-FATAL %s
+# ERR10-FATAL: error: unknown -z value: foo
 
 # RUN: not ld.lld %t -z max-page-size 2>&1 | FileCheck -check-prefix=ERR11 %s
 # ERR11: unknown -z value: max-page-size
 
 ## Attempt to use -r and --export-dynamic together
 # RUN: not ld.lld -r -export-dynamic %t -o /dev/null 2>&1 | FileCheck -check-prefix=ERR12 %s
 # ERR12: -r and --export-dynamic may not be used together
 
 .globl _start
 _start:
   nop