[PATCH] D126497: [lld][WebAssemlby] Check for command line flags with missing arguments

[Sam Clegg via Phabricator via llvm-commits]

sbc100 created this revision.
Herald added subscribers: pmatos, asb, wingo, sunfish.
Herald added a project: All.
sbc100 requested review of this revision.
Herald added subscribers: llvm-commits, aheejin.
Herald added a project: LLVM.

I'm really not sure how this was overlooked when we first ported lld
to Wasm.  The upstream code in the ELF backend has these two lines but
for some reason they never make it into the Wasm version.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126497

Files:
  lld/test/wasm/driver.s
  lld/wasm/Driver.cpp
  lld/wasm/Options.td


Index: lld/wasm/Options.td
===================================================================
--- lld/wasm/Options.td
+++ lld/wasm/Options.td
@@ -1,5 +1,18 @@
 include "llvm/Option/OptParser.td"
 
+// Convenience classes for long options which only accept two dashes. For lld
+// specific or newer long options, we prefer two dashes to avoid collision with
+// short options. For many others, we have to accept both forms to be compatible
+// with GNU ld.
+class FF<string name> : Flag<["--"], name>;
+class JJ<string name>: Joined<["--"], name>;
+
+multiclass EEq<string name, string help> {
+  def NAME: Separate<["--"], name>;
+  def NAME # _eq: Joined<["--"], name # "=">, Alias<!cast<Separate>(NAME)>,
+    HelpText<help>;
+}
+
 // For options whose names are multiple letters, either one dash or
 // two can precede the option name except those that start with 'o'.
 class F<string name>: Flag<["--", "-"], name>;
@@ -99,7 +112,7 @@
 
 def relocatable: F<"relocatable">, HelpText<"Create relocatable object file">;
 
-defm reproduce: Eq<"reproduce", "Dump linker invocation and input files for debugging">;
+defm reproduce: EEq<"reproduce", "Dump linker invocation and input files for debugging">;
 
 defm rsp_quoting: Eq<"rsp-quoting", "Quoting style for response files">,
   MetaVarName<"[posix,windows]">;
Index: lld/wasm/Driver.cpp
===================================================================
--- lld/wasm/Driver.cpp
+++ lld/wasm/Driver.cpp
@@ -184,6 +184,9 @@
   args = this->ParseArgs(vec, missingIndex, missingCount);
 
   handleColorDiagnostics(args);
+  if (missingCount)
+    error(Twine(args.getArgString(missingIndex)) + ": missing argument");
+
   for (auto *arg : args.filtered(OPT_UNKNOWN))
     error("unknown argument: " + arg->getAsString(args));
   return args;
@@ -810,6 +813,12 @@
   });
 }
 
+static const char *getReproduceOption(opt::InputArgList &args) {
+  if (auto *arg = args.getLastArg(OPT_reproduce))
+    return arg->getValue();
+  return getenv("LLD_REPRODUCE");
+}
+
 static bool isKnownZFlag(StringRef s) {
   // For now, we only support a very limited set of -z flags
   return s.startswith("stack-size=");
@@ -847,8 +856,8 @@
   }
 
   // Handle --reproduce
-  if (auto *arg = args.getLastArg(OPT_reproduce)) {
-    StringRef path = arg->getValue();
+  // Note that --reproduce is a debug option so you can ignore it
+  if (const char *path = getReproduceOption(args)) {
     Expected<std::unique_ptr<TarWriter>> errOrWriter =
         TarWriter::create(path, path::stem(path));
     if (errOrWriter) {
Index: lld/test/wasm/driver.s
===================================================================
--- lld/test/wasm/driver.s
+++ lld/test/wasm/driver.s
@@ -5,6 +5,9 @@
   .functype _start () -> ()
   end_function
 
+# RUN: not wasm-ld %t -o 2>&1 | FileCheck --check-prefix=NO_O_VAL %s
+# NO_O_VAL: error: -o: missing argument
+
 # RUN: not wasm-ld -o %t.exe 2>&1 | FileCheck -check-prefix=IN %s
 # IN: error: no input files
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D126497.432368.patch
Type: text/x-patch
Size: 2979 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220526/07463ee7/attachment.bin>