[COFF] Add libcall symbols to the link when LTO is being used.

[akhuang]

Summary

If a library function is defined in a bitcode file in an archive
member, add the archive members to the link before LTO, since LTO may
create references to the library functions.

Similar to the change for ELF (https://reviews.llvm.org/D50017).

This is a fix for https://bugs.llvm.org/show_bug.cgi?id=42626#c3.

[pcc]

lld/COFF/SymbolTable.cpp

588

The logic here should depend on the name. If it begins with \01 use find on name.substr(1), otherwise use findUnderscore.

595

In D50475 we limited this to symbols defined in bitcode. We should do the same for COFF.

lld/test/COFF/libcall-archive.ll

11

I think this isn't actually testing for what you expect here because you're looking at the LTO output file, which won't contain libcall-archive.s since that isn't a bitcode file. Maybe you could test this by running FileCheck on the map file (/lldmap: output).

[rnk]

lld/COFF/SymbolTable.cpp

588

So far as I can tell, none of the symbols from RuntimeLibcalls use the \01 escape prefix. Is that necessary? Can this be simplified to just one call to findUnderscore?

595

Yep, agreed.

lld/test/COFF/libcall-archive.ll

11

Right, you would have to look at %t.exe directly, and that usually doesn't have symbols usable with llvm-nm. The map file seems like a good solution.

[pcc]

lld/COFF/SymbolTable.cpp

588

I vaguely recalled that we had some, but yes, apparently we don't. Simplifying to findUnderscore sounds good then.

[akhuang]

• Check that file is bitcode
• Change test to look at lld map

[rnk]

lld/COFF/Driver.cpp

1780

I think this should be inside the do / while loop above in case one of the bitcode objects that provides a libcall introduces a new undefined symbol reference.

lld/COFF/SymbolTable.cpp

588

I think you should be able to simplify this down to just findUnderscore, no need to check for startswith("_"). I think the reason you have it is that in your test case, the .s file would normally use a "mangled" symbol for i686-windows-msvc, so it would provide the symbol __sync_val_compare_and_swap_8.

[akhuang]

• Move libcall check inside while loop
• Remove find()

[rnk]

lgtm with one change

lld/COFF/SymbolTable.cpp

594

I'd use isa<BitcodeFile>(l->getFile()) instead, rather than checking for the magic bytes again.

[akhuang]

lld/COFF/SymbolTable.cpp

594

It seems like isa<BitcodeFile>(l->getFile()) is always false and the file is an ArchiveFile?

[rnk]

lgtm

lld/COFF/SymbolTable.cpp

594

I see, you're right. I see this is also what is done for ELF. The whole idea of a lazy symbol is that we may not have loaded the object file yet.

[hans]

In D66355#1637842, @rnk wrote:

lgtm

You wrote lgtm, but it says changes required. And it looks like it landed in r369694.

Someone asked about merging this to the relaese_90 branch. What do you think?

[rnk]

In D66355#1642467, @hans wrote:

In D66355#1637842, @rnk wrote:

lgtm

You wrote lgtm, but it says changes required. And it looks like it landed in r369694.

Oops.

Someone asked about merging this to the relaese_90 branch. What do you think?

I think we should do it. Ultimately, this is porting some well-tested logic from ELF back to COFF.

[hans]

Someone asked about merging this to the relaese_90 branch. What do you think?

I think we should do it. Ultimately, this is porting some well-tested logic from ELF back to COFF.

Sounds good. Merged in r369883.

lld/COFF/Driver.cpp

symtab->forEachSymbol([&](Symbol *s) {
     e.sym = def;
     if (Chunk *c = def->getChunk())
       if (!(c->getOutputCharacteristics() & IMAGE_SCN_MEM_EXECUTE))
         e.data = true;
     config->exports.push_back(e);
   });
 }
 
+static const char *libcallRoutineNames[] = {
+#define HANDLE_LIBCALL(code, name) name,
+#include "llvm/IR/RuntimeLibcalls.def"
+#undef HANDLE_LIBCALL
+};
+
 void LinkerDriver::link(ArrayRef<const char *> argsArr) {
   // Needed for LTO.
   InitializeAllTargetInfos();
   InitializeAllTargets();
   InitializeAllTargetMCs();
   InitializeAllAsmParsers();
   InitializeAllAsmPrinters();
 
for (auto pair : config->alternateNames) {
       Symbol *sym = symtab->find(from);
       if (!sym)
         continue;
       if (auto *u = dyn_cast<Undefined>(sym))
         if (!u->weakAlias)
           u->weakAlias = symtab->addUndefined(to);
     }
 
+    // If any inputs are bitcode files, the LTO code generator may create
+    // references to library functions that are not explicit in the bitcode
+    // file's symbol table. If any of those library functions are defined in a
+    // bitcode file in an archive member, we need to arrange to use LTO to
+    // compile those archive members by adding them to the link beforehand.
+    if (!BitcodeFile::instances.empty())
+      for (const char *s : libcallRoutineNames)
+        symtab->addLibcall(s);
+
     // Windows specific -- if __load_config_used can be resolved, resolve it.
     if (symtab->findUnderscore("_load_config_used"))
       addUndefined(mangle("_load_config_used"));
   } while (run());
 
   if (args.hasArg(OPT_include_optional)) {
     // Handle /includeoptional
     for (auto *arg : args.filtered(OPT_include_optional))
       if (dyn_cast_or_null<Lazy>(symtab->find(arg->getValue())))
         addUndefined(arg->getValue());
     while (run());
   }
 
   if (config->mingw) {

lld/COFF/SymbolTable.h

public:
   addComdat(InputFile *f, StringRef n,
             const llvm::object::coff_symbol_generic *s = nullptr);
   Symbol *addCommon(InputFile *f, StringRef n, uint64_t size,
                     const llvm::object::coff_symbol_generic *s = nullptr,
                     CommonChunk *c = nullptr);
   Symbol *addImportData(StringRef n, ImportFile *f);
   Symbol *addImportThunk(StringRef name, DefinedImportData *s,
                          uint16_t machine);
+  void addLibcall(StringRef name);
 
   void reportDuplicate(Symbol *existing, InputFile *newFile);
 
   // A list of chunks which to be added to .rdata.
   std::vector<Chunk *> localImportChunks;
 
   // Iterates symbols in non-determinstic hash table order.
   template <typename T> void forEachSymbol(T callback) {

lld/COFF/SymbolTable.cpp

if (wasInserted || isa<Undefined>(s) || isa<Lazy>(s)) {
     replaceSymbol<DefinedImportThunk>(s, name, id, machine);
     return s;
   }
 
   reportDuplicate(s, id->file);
   return nullptr;
 }
 
+void SymbolTable::addLibcall(StringRef name) {
+  Symbol *sym = findUnderscore(name);
+  if (!sym)
+    return;
+
+  if (Lazy *l = dyn_cast<Lazy>(sym)) {
+    MemoryBufferRef mb = l->getMemberBuffer();
+    if (identify_magic(mb.getBuffer()) == llvm::file_magic::bitcode)
+      addUndefined(sym->getName());
+  }
+}
+
 std::vector<Chunk *> SymbolTable::getChunks() {
   std::vector<Chunk *> res;
   for (ObjFile *file : ObjFile::instances) {
     ArrayRef<Chunk *> v = file->getChunks();
     res.insert(res.end(), v.begin(), v.end());
   }
   return res;
 }

lld/COFF/Symbols.h

 // the same name, it will ask the Lazy to load a file.
 class Lazy : public Symbol {
 public:
   Lazy(ArchiveFile *f, const Archive::Symbol s)
       : Symbol(LazyKind, s.getName()), file(f), sym(s) {}
 
   static bool classof(const Symbol *s) { return s->kind() == LazyKind; }
 
+  MemoryBufferRef getMemberBuffer();
+
   ArchiveFile *file;
 
 private:
   friend SymbolTable;
 
 private:
   const Archive::Symbol sym;
 };

lld/COFF/Symbols.cpp

 
 Defined *Undefined::getWeakAlias() {
   // A weak alias may be a weak alias to another symbol, so check recursively.
   for (Symbol *a = weakAlias; a; a = cast<Undefined>(a)->weakAlias)
     if (auto *d = dyn_cast<Defined>(a))
       return d;
   return nullptr;
 }
+
+MemoryBufferRef Lazy::getMemberBuffer() {
+  Archive::Child c =
+    CHECK(sym.getMember(),
+          "could not get the member for symbol " + toCOFFString(sym));
+  return CHECK(c.getMemoryBufferRef(),
+      "could not get the buffer for the member defining symbol " +
+      toCOFFString(sym));
+}
 } // namespace coff
 } // namespace lld

lld/test/COFF/Inputs/libcall-archive.ll

+target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
+target triple = "i686-unknown-windows"
+
+define void @memcpy() {
+  ret void
+}

lld/test/COFF/Inputs/libcall-archive.s

+.globl ___sync_val_compare_and_swap_8
+___sync_val_compare_and_swap_8:

lld/test/COFF/libcall-archive.ll

+; REQUIRES: x86
+; RUN: rm -f %t.a
+; RUN: llvm-as -o %t.obj %s
+; RUN: llvm-as -o %t2.obj %S/Inputs/libcall-archive.ll
+; RUN: llvm-mc -filetype=obj -triple=i686-unknown-windows -o %t3.obj %S/Inputs/libcall-archive.s
+; RUN: llvm-ar rcs %t.a %t2.obj %t3.obj
+; RUN: lld-link -out:%t.exe -subsystem:console -entry:start -safeseh:no -lldmap:- %t.obj %t.a | FileCheck %s
+
+; CHECK-NOT: ___sync_val_compare_and_swap_8
+; CHECK: _start
+; CHECK: _memcpy
+
+target datalayout = "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
+target triple = "i686-unknown-windows"
+
+define void @start(i8* %a, i8* %b) {
+entry:
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %a, i8* %b, i64 1024, i1 false)
+  ret void
+}
+
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture, i64, i1)