[WebAssembly] Add support for response file parsing

[sbc100]

Summary

Fixes PR37620

[alexcrichton]

Awesome, thanks for the quick fix!

[ruiu]

wasm/Options.td

76–77

Do you really need to support both Windows and Unix-style quoting rules? IMO Windows' quoting rule is pretty hairly, and you should always use Unix-style quoting rule if possible. Since you are the only generator who create response files, you can just create response files in Unix style, no?

[sbc100]

wasm/Options.td

76–77

My understanding is that the higher level toolchain driver can generate a repsonse file. e.g. visual studio CMake, or ninja? I would have thought that we want to at least use windows quoting when running on windows even if we don't really need to the command line flag.

Perhaps you are right though and we should add this if and when we get our first windows user who wants to include unquoted filenames.

Also, I'm not sure why you mention complexity is an issue. We are not generating these files, only consuming them and it seems that llvm already handle both types of quoting.

[ruiu]

wasm/Options.td

76–77

I'd support only Unix-style first. We can support Windows-style quoting later when it is proved to be needed.

As to the complexity, adding a new command line option or a new feature always adds some complexity. Once we start supporting some feature, we'll have to support it virtually forever. Also, by adding this option, you allow users to write response files in two different styles, whose default choice depends on your platform. It is not hard to imagine that that could theoretically allow you to write code that works on your platform but doesn't on the other. Having less number of moving parts is generally preferred.

[sbc100]

• feedback

[ruiu]

wasm/Driver.cpp

159

I think you should add ExpandResponseFiles before this line.

[sbc100]

wasm/Driver.cpp

159

ParseArgs is called twice, once before and once after. This is the how COFF and ELF do it too.

[ruiu]

wasm/Driver.cpp

159

But you don't need to cargo-cult that, no?

[sbc100]

• feedback

wasm/Driver.cpp

159

Oh, sorry yes. The double parsing is only needed in order to handle OPT_rsp_quoting which I removed from this change.

[ruiu]

LGTM

test/wasm/responsefile.test

4

It is perhaps better to move %t.o before -o so that -o would never be interpreted as an argument for the echo command.

10–11

Looks like this can fit in one line.

[sbc100]

• feedback

test/wasm/responsefile.test

+RUN: llc -filetype=obj -o %t.o %p/Inputs/ret32.ll
+
+RUN: echo -o %t.wasm -e ret32 %t.o > %t.rsp
+RUN: wasm-ld @%t.rsp --initial-memory=655360
+RUN: llvm-readobj --sections %t.wasm | FileCheck %s
+CHECK: InitialPages: 10
+
+RUN: not wasm-ld --rsp-quoting=foobar @%t.rsp 2>&1 | \
+RUN:     FileCheck --check-prefix=INVRSP %s
+INVRSP: invalid response file quoting: foobar
+
+RUN: echo "blah/foo" > %t.rsp
+RUN: not wasm-ld @%t.rsp -o dummy 2>&1 | \
+RUN:     FileCheck --check-prefix=DEFRSP %s
+DEFRSP: error: cannot open blah/foo: No such file or directory
+
+RUN: echo "blah\foo" > %t.rsp
+RUN: not wasm-ld --rsp-quoting=windows @%t.rsp 2>&1 | \
+RUN:     FileCheck --check-prefix=WINRSP %s
+WINRSP: error: cannot open blah\foo: No such file or directory
+
+RUN: echo "blah\foo" > %t.rsp
+RUN: not wasm-ld --rsp-quoting=posix @%t.rsp 2>&1 | \
+RUN:     FileCheck --check-prefix=POSRSP %s
+POSRSP: error: cannot open blahfoo: No such file or directory

wasm/Driver.cpp

if (S == "always")
       errorHandler().ColorDiagnostics = true;
     else if (S == "never")
       errorHandler().ColorDiagnostics = false;
     else if (S != "auto")
       error("unknown option: --color-diagnostics=" + S);
   }
 }
 
+static cl::TokenizerCallback getQuotingStyle(opt::InputArgList &Args) {
+  if (auto *Arg = Args.getLastArg(OPT_rsp_quoting)) {
+    StringRef S = Arg->getValue();
+    if (S != "windows" && S != "posix")
+      error("invalid response file quoting: " + S);
+    if (S == "windows")
+      return cl::TokenizeWindowsCommandLine;
+    return cl::TokenizeGNUCommandLine;
+  }
+  if (Triple(sys::getProcessTriple()).getOS() == Triple::Win32)
+    return cl::TokenizeWindowsCommandLine;
+  return cl::TokenizeGNUCommandLine;
+}
+
 // Find a file by concatenating given paths.
 static Optional<std::string> findFile(StringRef Path1, const Twine &Path2) {
   SmallString<128> S;
   path::append(S, Path1, Path2);
   if (fs::exists(S))
     return S.str().str();
   return None;
 }
 
 opt::InputArgList WasmOptTable::parse(ArrayRef<const char *> Argv) {
   SmallVector<const char *, 256> Vec(Argv.data(), Argv.data() + Argv.size());
 
   unsigned MissingIndex;
   unsigned MissingCount;
   opt::InputArgList Args = this->ParseArgs(Vec, MissingIndex, MissingCount);
 
+  // Expand response files (arguments in the form of @<filename>)
+  // and then parse the argument again.
+  cl::ExpandResponseFiles(Saver, getQuotingStyle(Args), Vec);
+  Args = this->ParseArgs(Vec, MissingIndex, MissingCount);
+
   handleColorDiagnostics(Args);
   for (auto *Arg : Args.filtered(OPT_UNKNOWN))
     error("unknown argument: " + Arg->getSpelling());
   return Args;
 }
 
 // Currently we allow a ".imports" to live alongside a library. This can
 // be used to specify a list of symbols which can be undefined at link

wasm/Options.td

 def O: JoinedOrSeparate<["-"], "O">, HelpText<"Optimize output file size">;
 
 defm print_gc_sections: B<"print-gc-sections",
     "List removed unused sections",
     "Do not list removed unused sections">;
 
 def relocatable: F<"relocatable">, HelpText<"Create relocatable object file">;
 
+defm rsp_quoting: Eq<"rsp-quoting">,
+  HelpText<"Quoting style for response files. Values supported: windows|posix">;
+
 def strip_all: F<"strip-all">, HelpText<"Strip all symbols">;
 
 def strip_debug: F<"strip-debug">, HelpText<"Strip debugging information">;
 
 def threads: F<"threads">, HelpText<"Run the linker multi-threaded">;
 
 defm undefined: Eq<"undefined">,
   HelpText<"Force undefined symbol during linking">;