[ELF] - Report unimplemented -z options.

[grimar]

Summary

This is https://bugs.llvm.org/show_bug.cgi?id=37881,

it is about that LLD currently does not report any invalid/unsupported -z options.

There are few possible ways how this can be supported. A whitelist would be straightforward
but it does not look nice to list all of these options again since such list would be large.
The idea of this patch is to claim the options we check in the driver and hence mark them as used.
All unclaimed options than can be treated as unsupported and we can report them.

[ruiu]

I wonder if this is the best way to do an error check. Did you try something like this?

for (auto *Arg : Args.filtered(OPT_z)) {
  StringRef S = Arg->getValue();
  if (S != "mudefs" && S != "hazardplt" && S != "relro" && ...)
    error("unknown -z value:  " + S);
}

This is simple-minded and less-abstracted, which is usually preferred in lld.

ELF/Driver.cpp

310

I'd make this function to take just one Key instead of arbitrary number of keys. I'd also rename this just claim.

[grimar]

Abandoning in favor of https://reviews.llvm.org/D48621

[dim]

Ah, only now I've noticed that in the FreeBSD kernel link, we've been using the unsupported -z common-page-size= option:

--- kernel.full ---
linking kernel.full
ld: error: unknown -z value: common-page-size=4096
*** [kernel.full] Error code 1

This flag was added for ifunc support, but I'm not 100% sure if it is needed at all. Does lld always assume a common page size of 4096 (or whatever the OS reports as PAGESIZE)?

[grimar]

In D48433#1188567, @dim wrote:

Ah, only now I've noticed that in the FreeBSD kernel link, we've been using the unsupported -z common-page-size= option:

--- kernel.full ---
linking kernel.full
ld: error: unknown -z value: common-page-size=4096
*** [kernel.full] Error code 1

This flag was added for ifunc support, but I'm not 100% sure if it is needed at all. Does lld always assume a common page size of 4096 (or whatever the OS reports as PAGESIZE)?

I believe, it is always a 4096:
https://github.com/llvm-mirror/lld/blob/master/ELF/Target.h#L80
Except for sparc:
https://github.com/llvm-mirror/lld/blob/master/ELF/Arch/SPARCV9.cpp#L44

[ruiu]

This flag was added for ifunc support, but I'm not 100% sure if it is needed at all. Does lld always assume a common page size of 4096 (or whatever the OS reports as PAGESIZE)?

We generally don't depend on any value that the host OS reports to make lld work exactly the same way on any platform when the same set of files and command line options are given, which should help cross linking.

Common/Args.cpp

std::vector<StringRef> lld::args::getStrings(opt::InputArgList &Args, int Id) {
   std::vector<StringRef> V;
   for (auto *Arg : Args.filtered(Id))
     V.push_back(Arg->getValue());
   return V;
 }
 
 uint64_t lld::args::getZOptionValue(opt::InputArgList &Args, int Id,
                                     StringRef Key, uint64_t Default) {
+  uint64_t Result = Default;
   for (auto *Arg : Args.filtered(Id)) {
     std::pair<StringRef, StringRef> KV = StringRef(Arg->getValue()).split('=');
     if (KV.first == Key) {
-      uint64_t Result = Default;
       if (!to_integer(KV.second, Result))
         error("invalid " + Key + ": " + KV.second);
-      return Result;
+      Arg->claim();
     }
   }
-  return Default;
+  return Result;
 }
 
 std::vector<StringRef> lld::args::getLines(MemoryBufferRef MB) {
   SmallVector<StringRef, 0> Arr;
   MB.getBuffer().split(Arr, '\n');
 
   std::vector<StringRef> Ret;
   for (StringRef S : Arr) {
     S = S.trim();
     if (!S.empty() && S[0] != '#')
       Ret.push_back(S);
   }
   return Ret;
 }

ELF/Driver.cpp

 }
 
 static const char *getReproduceOption(opt::InputArgList &Args) {
   if (auto *Arg = Args.getLastArg(OPT_reproduce))
     return Arg->getValue();
   return getenv("LLD_REPRODUCE");
 }
 
+static void claimZOptions(opt::InputArgList &Args, ArrayRef<StringRef> Keys) {
+  for (auto *Arg : Args.filtered(OPT_z))
+    for (StringRef K : Keys)
+      if (Arg->getValue() == K)
+        Arg->claim();
+}
+
 static bool hasZOption(opt::InputArgList &Args, StringRef Key) {
+  claimZOptions(Args, {Key});
   for (auto *Arg : Args.filtered(OPT_z))
     if (Key == Arg->getValue())
       return true;
   return false;
 }
 
 static bool getZFlag(opt::InputArgList &Args, StringRef K1, StringRef K2,
                      bool Default) {
+  claimZOptions(Args, {K1, K2});
   for (auto *Arg : Args.filtered_reverse(OPT_z)) {
     if (K1 == Arg->getValue())
       return true;
     if (K2 == Arg->getValue())
       return false;
   }
   return Default;
 }

 static std::string getRpath(opt::InputArgList &Args) {
   std::vector<StringRef> V = args::getStrings(Args, OPT_rpath);
   return llvm::join(V.begin(), V.end(), ":");
 }
 
 // Determines what we should do if there are remaining unresolved
 // symbols after the name resolution.
 static UnresolvedPolicy getUnresolvedSymbolPolicy(opt::InputArgList &Args) {
+  claimZOptions(Args, {"defs"});
+
   if (Args.hasArg(OPT_relocatable))
     return UnresolvedPolicy::IgnoreAll;
 
   UnresolvedPolicy ErrorOrWarn = Args.hasFlag(OPT_error_unresolved_symbols,
                                               OPT_warn_unresolved_symbols, true)
                                      ? UnresolvedPolicy::ReportError
                                      : UnresolvedPolicy::Warn;
 

 // Initializes Config members by the command line options.
 void LinkerDriver::readConfigs(opt::InputArgList &Args) {
   errorHandler().Verbose = Args.hasArg(OPT_verbose);
   errorHandler().FatalWarnings =
       Args.hasFlag(OPT_fatal_warnings, OPT_no_fatal_warnings, false);
   ThreadsEnabled = Args.hasFlag(OPT_threads, OPT_no_threads, true);
 
   Config->AllowMultipleDefinition =
+      hasZOption(Args, "muldefs") ||
       Args.hasFlag(OPT_allow_multiple_definition,
-                   OPT_no_allow_multiple_definition, false) ||
-      hasZOption(Args, "muldefs");
+                   OPT_no_allow_multiple_definition, false);
   Config->AuxiliaryList = args::getStrings(Args, OPT_auxiliary);
   Config->Bsymbolic = Args.hasArg(OPT_Bsymbolic);
   Config->BsymbolicFunctions = Args.hasArg(OPT_Bsymbolic_functions);
   Config->CheckSections =
       Args.hasFlag(OPT_check_sections, OPT_no_check_sections, true);
   Config->Chroot = Args.getLastArgValue(OPT_chroot);
   Config->CompressDebugSections = getCompressDebugSections(Args);
   Config->Cref = Args.hasFlag(OPT_cref, OPT_no_cref, false);

 // Do actual linking. Note that when this function is called,
 // all linker scripts have already been parsed.
 template <class ELFT> void LinkerDriver::link(opt::InputArgList &Args) {
   Target = getTarget();
 
   Config->MaxPageSize = getMaxPageSize(Args);
   Config->ImageBase = getImageBase(Args);
 
+  for (auto *Arg : Args.filtered(OPT_z))
+    if (!Arg->isClaimed())
+      warn("unknown -z value: " + StringRef(Arg->getValue()));
+
   // If a -hash-style option was not given, set to a default value,
   // which varies depending on the target.
   if (!Args.hasArg(OPT_hash_style)) {
     if (Config->EMachine == EM_MIPS)
       Config->SysvHash = true;
     else
       Config->SysvHash = Config->GnuHash = true;
   }

test/ELF/invalid-z.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 # RUN: not ld.lld %t.o -o %t -z max-page-size 2>&1 | FileCheck %s
 # CHECK: invalid max-page-size
 # CHECK-NOT: error
 
+# RUN: ld.lld %t.o -o %t -z foobar 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=UNKNOWN
+# UNKNOWN: warning: unknown -z value: foobar
+
 .global _start
 _start:
   nop