[PATCH] D69968: [COFF] Don't error if the only inputs are from /wholearchive:

[Reid Kleckner via Phabricator via llvm-commits]

rnk marked 3 inline comments as done.
rnk added inline comments.


================
Comment at: lld/COFF/Driver.cpp:1683
   // Handle generation of import library from a def file.
   if (!args.hasArg(OPT_INPUT)) {
     fixupExports();
----------------
This also ended up being a bug caught by the entry inference test.


================
Comment at: lld/COFF/Driver.cpp:1730
     config->outputFile =
         getOutputPath((*args.filtered(OPT_INPUT).begin())->getValue());
   }
----------------
amccarth wrote:
> This seems like a possible problem.  Before, you couldn't get here without OPT_INPUTs--you either had some, or you had a DEF file which would provide the output file name.  Now we can get here with just an archive, so we might be dereferencing an iterator at the beginning of an empty list.
Thanks, that's a real bug. I checked the Visual C++ linker's behavior, and they name the output after the first wholearchive argument. I did this: `link -wholearchive:foo.lib` and it produced foo.exe.


================
Comment at: lld/test/COFF/wholearchive.s:18
+# PR43744: Test no inputs except a whole archive.
+# RUN: lld-link -dll -out:%t.dll -noentry -wholearchive:%t.archive.lib -implib:%t.lib
+# RUN: llvm-readobj %t.lib | FileCheck %s -check-prefix CHECK-IMPLIB
----------------
amccarth wrote:
> I think the `-out` flag is preventing the potential crasher I mentioned above.  Can you try it without the `-out`?
> 
> Maybe we need one with a DEF file as well.
I updated two existing tests that deal with infering things from inputs:
1. out.test, this infers the .exe name from the wholearchive name
2. entry-inference.test, this deals with inferring the entry point, which interacts with deffile and noentry.

I don't think the case of -def: -wholearchive: is interesting, the def file is going to control the name of the DLL in that case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69968/new/

https://reviews.llvm.org/D69968