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

[Adrian McCarthy via Phabricator via llvm-commits]

amccarth requested changes to this revision.
amccarth added a comment.
This revision now requires changes to proceed.

This looks like the right approach, but I have a concern or two about the ramifications as noted inline.  Let me know if my concerns are unwarranted.



================
Comment at: lld/COFF/Driver.cpp:1168
     if (args.hasArg(OPT_deffile))
       config->noEntry = true;
     else
----------------
I don't entirely understand the meaning of `noEntry` here, but your change affects whether this happens or not.  Is that important?  I don't know.  See my comment on your new test.


================
Comment at: lld/COFF/Driver.cpp:1730
     config->outputFile =
         getOutputPath((*args.filtered(OPT_INPUT).begin())->getValue());
   }
----------------
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.


================
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
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69968