Do not pass archive files containing non-native object files.

[ruiu]

Summary

The MSVC linker doesn't like archive files containing non-native object
files.

When we are doing an LTO build, we may create archive files containing
both LLVM bitcode files and native object files. For example, if a
project contains assembly files and C++ files, we create native object
files for the assembly files and LLVM bitcode files for the C++ files.

With the /msvclto option, LLD passes archive files to the MSVC linker.
Previously, we didn't pass archive files if they contain at least one
bitcode files. That wasn't correct because the native object files that
weren't passed to the MSVC linker may be needed to complete linking.

In this patch, we create new temporary archive files to strip bitcode
files.

[pcc]

lld/COFF/Driver.cpp

466 ↗

(On Diff #92046)

I think you will need to rebuild the archive unconditionally if it is a thin archive because the MSVC linker won't understand thin archives.

[ruiu]

• Always rebuild an archive if it's a thin archive.

[pcc]

LGTM

lld/trunk/COFF/Driver.cpp

 #include "Memory.h"
 #include "SymbolTable.h"
 #include "Symbols.h"
 #include "Writer.h"
 #include "lld/Driver/Driver.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/LibDriver/LibDriver.h"
+#include "llvm/Object/ArchiveWriter.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/TarWriter.h"
 #include "llvm/Support/TargetSelect.h"
static std::string getMapFile(const opt::InputArgList &Args) {
   if (Arg->getOption().getID() == OPT_lldmap_file)
     return Arg->getValue();
 
   assert(Arg->getOption().getID() == OPT_lldmap);
   StringRef OutFile = Config->OutputFile;
   return (OutFile.substr(0, OutFile.rfind('.')) + ".map").str();
 }
 
-// Returns true if a given file is a LLVM bitcode file. If it is a
-// static library, this function returns true if all files in the
-// archive are bitcode files.
-static bool isBitcodeFile(StringRef Path) {
-  using namespace sys::fs;
-
-  std::unique_ptr<MemoryBuffer> MB = check(
-      MemoryBuffer::getFile(Path, -1, false, true), "could not open " + Path);
-  file_magic Magic = identify_magic(MB->getBuffer());
-
-  if (Magic == file_magic::bitcode)
-    return true;
-
-  if (Magic == file_magic::archive) {
-    std::unique_ptr<Archive> File =
-        check(Archive::create(MB->getMemBufferRef()));
+// Returns slices of MB by parsing MB as an archive file.
+// Each slice consists of a member file in the archive.
+std::vector<MemoryBufferRef> getArchiveMembers(MemoryBufferRef MB) {
+      check(Archive::create(MB),
+            MB.getBufferIdentifier() + ": failed to parse archive");
 
-    Error Err = Error::success();
-    for (const ErrorOr<Archive::Child> &COrErr : File->children(Err)) {
-      if (Err)
-        return false;
-      Archive::Child C = check(COrErr);
-      MemoryBufferRef MBRef = check(C.getMemoryBufferRef());
-      if (identify_magic(MBRef.getBuffer()) != file_magic::bitcode)
-        return false;
-    }
-    if (Err)
-      return false;
-    return true;
-  }
+  std::vector<MemoryBufferRef> V;
+    Archive::Child C =
+        check(COrErr, MB.getBufferIdentifier() +
+                          ": could not get the child of the archive");
+    MemoryBufferRef MBRef =
+        check(C.getMemoryBufferRef(),
+              MB.getBufferIdentifier() +
+                  ": could not get the buffer for a child of the archive");
+    V.push_back(MBRef);
+    fatal(MB.getBufferIdentifier() +
+          ": Archive::children failed: " + toString(std::move(Err)));
+  return V;
 
+// A helper function for filterBitcodeFiles.
+static bool needsRebuilding(MemoryBufferRef MB) {
+  // The MSVC linker doesn't support thin archives, so if it's a thin
+  // archive, we always need to rebuild it.
+  std::unique_ptr<Archive> File =
+      check(Archive::create(MB), "Failed to read " + MB.getBufferIdentifier());
+  if (File->isThin())
+    return true;
+
+  // Returns true if the archive contains at least one bitcode file.
+  for (MemoryBufferRef Member : getArchiveMembers(MB))
+    if (identify_magic(Member.getBuffer()) == file_magic::bitcode)
+      return true;
   return false;
 }
 
+// Opens a given path as an archive file and removes bitcode files
+// from them if exists. This function is to appease the MSVC linker as
+// their linker doesn't like archive files containing non-native
+// object files.
+//
+// If a given archive doesn't contain bitcode files, the archive path
+// is returned as-is. Otherwise, a new temporary file is created and
+// its path is returned.
+static Optional<std::string>
+filterBitcodeFiles(StringRef Path, std::vector<std::string> &TemporaryFiles) {
+  std::unique_ptr<MemoryBuffer> MB = check(
+      MemoryBuffer::getFile(Path, -1, false, true), "could not open " + Path);
+  MemoryBufferRef MBRef = MB->getMemBufferRef();
+  file_magic Magic = identify_magic(MBRef.getBuffer());
+
+  if (Magic == file_magic::bitcode)
+    return None;
+  if (Magic != file_magic::archive)
+    return Path.str();
+  if (!needsRebuilding(MBRef))
+    return Path.str();
+
+  log("Creating a temporary archive for " + Path +
+      " to remove bitcode files");
+
+  std::vector<NewArchiveMember> New;
+  for (MemoryBufferRef Member : getArchiveMembers(MBRef))
+    if (identify_magic(Member.getBuffer()) != file_magic::bitcode)
+      New.emplace_back(Member);
+
+  SmallString<128> S;
+  if (auto EC = sys::fs::createTemporaryFile("lld-" + sys::path::stem(Path),
+                                             ".lib", S))
+    fatal(EC, "cannot create a temporary file");
+  std::string Temp = S.str();
+  TemporaryFiles.push_back(Temp);
+
+  std::pair<StringRef, std::error_code> Ret =
+      llvm::writeArchive(Temp, New, /*WriteSymtab=*/true, Archive::Kind::K_GNU,
+                         /*Deterministics=*/true,
+                         /*Thin=*/false);
+  if (Ret.second)
+    error("failed to create a new archive " + S.str() + ": " + Ret.first);
+  return Temp;
+}
+
 // Create response file contents and invoke the MSVC linker.
 void LinkerDriver::invokeMSVC(opt::InputArgList &Args) {
   std::string Rsp = "/nologo ";
+  std::vector<std::string> Temps;
 
   for (auto *Arg : Args) {
     switch (Arg->getOption().getID()) {
     case OPT_linkrepro:
     case OPT_lldmap:
     case OPT_lldmap_file:
     case OPT_msvclto:
       // LLD-specific options are stripped.
       break;
     case OPT_opt:
       if (!StringRef(Arg->getValue()).startswith("lld"))
         Rsp += toString(Arg) + " ";
       break;
-    case OPT_INPUT:
-      // Bitcode files are stripped as they've been compiled to
-      // native object files.
-      if (Optional<StringRef> Path = doFindFile(Arg->getValue()))
-        if (isBitcodeFile(*Path))
-          break;
+    case OPT_INPUT: {
+      if (Optional<StringRef> Path = doFindFile(Arg->getValue())) {
+        if (Optional<std::string> S = filterBitcodeFiles(*Path, Temps))
+          Rsp += quote(*S) + " ";
+        continue;
+      }
       Rsp += quote(Arg->getValue()) + " ";
       break;
+    }
     default:
       Rsp += toString(Arg) + " ";
     }
   }
 
   std::vector<StringRef> ObjectFiles = Symtab.compileBitcodeFiles();
   runMSVCLinker(Rsp, ObjectFiles);
+
+  for (StringRef Path : Temps)
+    sys::fs::remove(Path);
 }
 
 void LinkerDriver::enqueueTask(std::function<void()> Task) {
   TaskQueue.push_back(std::move(Task));
 }
 
 bool LinkerDriver::run() {
   bool DidWork = !TaskQueue.empty();

lld/trunk/COFF/Error.h

 void log(const Twine &Msg);
 void message(const Twine &Msg);
 void warn(const Twine &Msg);
 void error(const Twine &Msg);
 LLVM_ATTRIBUTE_NORETURN void fatal(const Twine &Msg);
 LLVM_ATTRIBUTE_NORETURN void fatal(std::error_code EC, const Twine &Prefix);
 LLVM_ATTRIBUTE_NORETURN void fatal(llvm::Error &Err, const Twine &Prefix);
 
-template <class T> T check(ErrorOr<T> &&V, const Twine &Prefix) {
+template <class T> T check(ErrorOr<T> V, const Twine &Prefix) {
   if (auto EC = V.getError())
     fatal(EC, Prefix);
   return std::move(*V);
 }
 
 template <class T> T check(Expected<T> E, const Twine &Prefix) {
   if (llvm::Error Err = E.takeError())
     fatal(Err, Prefix);

lld/trunk/test/COFF/msvclto-archive.ll

-;; Make sure we do not pass archive files containing only bitcode files.
+;; Make sure we re-create archive files to strip bitcode files.
 
 ; RUN: llvm-as -o %t.obj %s
+; RUN: rm -f %t-main1.a
 ; RUN: llvm-ar cru %t-main1.a %t.obj
 ; RUN: mkdir -p %t.dir
 ; RUN: llvm-mc -triple=x86_64-pc-windows-msvc -filetype=obj -o %t.dir/bitcode.obj %p/Inputs/msvclto.s
 ; RUN: lld-link %t-main1.a %t.dir/bitcode.obj /msvclto /out:%t.exe /opt:lldlto=1 /opt:icf \
 ; RUN:   /entry:main /verbose > %t.log || true
 ; RUN: FileCheck -check-prefix=BC %s < %t.log
-; BC-NOT: link.exe {{.*}}-main1.a
+; BC: Creating a temporary archive for
 
-; RUN: llvm-ar cru %t-main2.a %t.obj %t.dir/bitcode.obj
-; RUN: lld-link %t.dir/bitcode.obj %t-main2.a /msvclto /out:%t.exe /opt:lldlto=1 /opt:icf \
+; RUN: rm -f %t-main2.a
+; RUN: llvm-ar cru %t-main2.a %t.dir/bitcode.obj
+; RUN: lld-link %t.obj %t-main2.a /msvclto /out:%t.exe /opt:lldlto=1 /opt:icf \
 ; RUN:   /entry:main /verbose > %t.log || true
 ; RUN: FileCheck -check-prefix=OBJ %s < %t.log
-; OBJ: link.exe {{.*}}-main2.a
+; OBJ-NOT: Creating a temporary archive
+
+;; Make sure that we always rebuild thin archives because
+;; the MSVC linker doesn't support thin archives.
+; RUN: rm -f %t-main3.a
+; RUN: llvm-ar cruT %t-main3.a %t.dir/bitcode.obj
+; RUN: lld-link %t.obj %t-main3.a /msvclto /out:%t.exe /opt:lldlto=1 /opt:icf \
+; RUN:   /entry:main /verbose > %t.log || true
+; RUN: FileCheck -check-prefix=THIN %s < %t.log
+; THIN: Creating a temporary archive
 
 target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-pc-windows-msvc"
 
 declare void @foo()
 
 define i32 @main() {
   call void @foo()
   ret i32 0
 }