[ms] Add new option to llvm-lib: /llvmlibempty

[epastor]

Summary

Add a new option (/llvmlibempty). If passed and llvm-lib does not give an error, it will create a valid output archive even if empty.

By default, llvm-lib mimicks lib.exe: if given no input files, it doesn't create its output file at all. This is incompatible with some build systems, so we add a command-line option to toggle this compatibility behavior.

[thakis]

When creating a dll, we currently create an empty .lib file (for similar reasons as in the patch description), while link.exe only creates a .lib file if there actually are exports. I wonder if we should change that to match link.exe by default and give lld its current behavior if this new flag here is passed? Not sure, maybe @mstorsjo has an opinion (but maybe he doesn't care about link.exe compat all that much) :)

Otherwise, this looks fine to me once it has a test.

[epastor]

Add functionality test

[thakis]

Thoughts on making this affect the export lib creation as well?

[epastor]

In D78894#2010440, @thakis wrote:

Thoughts on making this affect the export lib creation as well?

Might be a good idea, but it's a completely separate tool (since llvm-ar/llvm-lib has no overlap with lld). I'd suggest doing it as a followup commit, rather than as part of this... especially since this one is not a change to the default behavior, whereas the export lib creation would be a change to the default behavior.

[thakis]

In D78894#2010852, @epastor wrote:

In D78894#2010440, @thakis wrote:

Thoughts on making this affect the export lib creation as well?

Might be a good idea, but it's a completely separate tool (since llvm-ar/llvm-lib has no overlap with lld). I'd suggest doing it as a followup commit, rather than as part of this... especially since this one is not a change to the default behavior, whereas the export lib creation would be a change to the default behavior.

I mean the .lib written by link.exe when you link a .dll (cf /IMPLIB: https://docs.microsoft.com/en-us/cpp/build/reference/implib-name-import-library?view=vs-2019)

[epastor]

In D78894#2013271, @thakis wrote:

In D78894#2010852, @epastor wrote:

In D78894#2010440, @thakis wrote:

Thoughts on making this affect the export lib creation as well?

Might be a good idea, but it's a completely separate tool (since llvm-ar/llvm-lib has no overlap with lld). I'd suggest doing it as a followup commit, rather than as part of this... especially since this one is not a change to the default behavior, whereas the export lib creation would be a change to the default behavior.

I mean the .lib written by link.exe when you link a .dll (cf /IMPLIB: https://docs.microsoft.com/en-us/cpp/build/reference/implib-name-import-library?view=vs-2019)

Right - but in these tools, that's written by lld-link, not llvm-lib... and there's no code overlap. And it would involve changing the default behavior of lld-link in a breaking fashion... so I'm suggesting that should go in a separate commit, probably with a quick RFC to the relevant lists.

[thakis]

Sorry, I missed that _this_ change is in llvm-lib. Makes sense, thanks for talking me through it.

[vvereschaka]

Hi Eric,

the updated no-inputs.test test gets crashed on llvm-clang-x86_64-expensive-checks-ubuntu builder.
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-ubuntu/builds/5166

• FAIL: LLVM::no-inputs.test

PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/build/bin/llvm-lib /llvmlibempty -out:/home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/build/test/tools/llvm-lib/Output/no-inputs.test.tmp.a 
 #0 0x0000558112e4fd13 llvm::sys::PrintStackTrace(llvm::raw_ostream&) /home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/llvm-project/llvm/lib/Support/Unix/Signals.inc:564:0
 #1 0x0000558112e4fda6 PrintStackTraceSignalHandler(void*) /home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/llvm-project/llvm/lib/Support/Unix/Signals.inc:625:0
 #2 0x0000558112e4da5d llvm::sys::RunSignalHandlers() /home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/llvm-project/llvm/lib/Support/Signals.cpp:68:0
 #3 0x0000558112e4f690 SignalHandler(int) /home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/llvm-project/llvm/lib/Support/Unix/Signals.inc:406:0
 #4 0x00007f691c9ba890 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12890)
 #5 0x00007f691ba99e97 raise (/lib/x86_64-linux-gnu/libc.so.6+0x3ee97)
 #6 0x00007f691ba9b801 abort (/lib/x86_64-linux-gnu/libc.so.6+0x40801)
 #7 0x000055811254936c operator new(unsigned long, void*) /usr/include/c++/7/new:169:0
 #8 0x0000558112c0ea23 std::vector<llvm::NewArchiveMember, std::allocator<llvm::NewArchiveMember> >::operator[](unsigned long) /usr/include/c++/7/bits/stl_vector.h:798:0
 #9 0x0000558112c0ccb4 llvm::libDriverMain(llvm::ArrayRef<char const*>) /home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/llvm-project/llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp:356:0
#10 0x0000558112547a72 main /home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/llvm-project/llvm/tools/llvm-ar/llvm-ar.cpp:1288:0
#11 0x00007f691ba7cb97 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b97)
#12 0x000055811254114a _start (/home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/build/bin/llvm-lib+0x4ca14a)
/home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/build/test/tools/llvm-lib/Output/no-inputs.test.script: line 4: 51780 Aborted                 (core dumped) /home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/build/bin/llvm-lib /llvmlibempty -out:/home/buildbot/as-builder-4/llvm-clang-x86_64-expensive-checks-ubuntu/build/test/tools/llvm-lib/Output/no-inputs.test.tmp.a

[epastor]

In D78894#2014398, @vvereschaka wrote:

Hi Eric,

the updated no-inputs.test test gets crashed on llvm-clang-x86_64-expensive-checks-ubuntu builder.
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-ubuntu/builds/5166

I believe this is already fixed in https://reviews.llvm.org/rG291d24838fcf

[epastor]

In D78894#2014406, @epastor wrote:

In D78894#2014398, @vvereschaka wrote:

Hi Eric,

the updated no-inputs.test test gets crashed on llvm-clang-x86_64-expensive-checks-ubuntu builder.
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-ubuntu/builds/5166

I believe this is already fixed in https://reviews.llvm.org/rG291d24838fcf

Hm. It doesn't seem to be fixed, but I don't see why not. I can't replicate this crash on my local machine. Can you give any more information that would help me reproduce this?

[vvereschaka]

This is the expensive checks builder and I see this crash on all that kind of builders

• http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-debian/builds/6242
• http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/23340

Probably adding -DLLVM_ENABLE_EXPENSIVE_CHECKS=ON to CMake command line will give a proper build.
Or you may try all the same CMake options as the builder uses
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-ubuntu/builds/5179/steps/cmake-configure/logs/stdio

[epastor]

In D78894#2014407, @epastor wrote:

In D78894#2014406, @epastor wrote:

In D78894#2014398, @vvereschaka wrote:

Hi Eric,

the updated no-inputs.test test gets crashed on llvm-clang-x86_64-expensive-checks-ubuntu builder.
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-ubuntu/builds/5166

I believe this is already fixed in https://reviews.llvm.org/rG291d24838fcf

Hm. It doesn't seem to be fixed, but I don't see why not. I can't replicate this crash on my local machine. Can you give any more information that would help me reproduce this?

Nevermind, I see a potential problem. If neither an output nor an input is provided, we can end up indexing into an uninitialized spot in the Members vector. I'll post a patch that fixes that ASAP, and send it to you and Nico for review - I'll submit it once I get approval.

However... that's not true with these tests, and they're passing on all the other buildbots. Is there maybe a problem with this buildbot and parameters like -out:<value>?

[vvereschaka]

Is there maybe a problem with this buildbot and parameters like -out:<value>?

Most likely there is no problems with this builder (in according of this situation). We got the same (similar) crash on three different systems: Ubuntu, Debian and Windows.

I'll post a patch that fixes that ASAP, and send it to you and Nico for review - I'll submit it once I get approval.

Thank you. I'll need some time to test this patch on the build system.

llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp

llvm::errs() << "ignoring unknown argument: " << Arg->getAsString(Args)
                  << "\n";
 
   // Handle /help
   if (Args.hasArg(OPT_help)) {
     Table.PrintHelp(outs(), "llvm-lib [options] file...", "LLVM Lib");
     return 0;
   }
 
-  // If no input files, silently do nothing to match lib.exe.
-  if (!Args.hasArgNoClaim(OPT_INPUT))
+  // If no input files and not told otherwise, silently do nothing to match
+  // lib.exe
+  if (!Args.hasArgNoClaim(OPT_INPUT) && !Args.hasArg(OPT_llvmlibempty))
     return 0;
 
   if (Args.hasArg(OPT_lst)) {
     doList(Args);
     return 0;
   }
 
   std::vector<StringRef> SearchPaths = getSearchPaths(&Args, Saver);

llvm/lib/ToolDrivers/llvm-lib/Options.td

 
 // Can't be called "list" since that's a keyword.
 def lst    : F<"list">, HelpText<"List contents of .lib file on stdout">;
 def out    : P<"out", "Path to file to write output">;
 
 def llvmlibthin : F<"llvmlibthin">,
     HelpText<"Make .lib point to .obj files instead of copying their contents">;
 
+def llvmlibempty : F<"llvmlibempty">,
+    HelpText<"When given no contents, produce an empty .lib file">;
+
 def machine: P<"machine", "Specify target platform">;
 
 def help : F<"help">;
 
 // /?? and -?? must be before /? and -? to not confuse lib/Options.
 def help_q : Flag<["/??", "-??", "/?", "-?"], "">, Alias<help>;
 
 //==============================================================================
 // The flags below do nothing. They are defined only for lib.exe compatibility.
 //==============================================================================
 
 class QF<string name> : Joined<["/", "-", "/?", "-?"], name#":">;
 
 def ignore : QF<"ignore">;
 def nologo : F<"nologo">;

llvm/test/tools/llvm-lib/no-inputs.test

 RUN: llvm-lib -out:%t.a
 RUN: test ! -e %t.a
+
+RUN: llvm-lib /llvmlibempty -out:%t.a
+RUN: FileCheck %s < %t.a
+CHECK: !<arch>