Address review comments especially i386 support

Author: aganea

lld/COFF/Driver.cpp

@@ -198,28 +198,8 @@ static bool compatibleMachineType(COFFLinkerContext &ctx, MachineTypes mt) {
 }
 
 void LinkerDriver::addFile(InputFile *file) {
-  Log(ctx) << "Reading " << toString(file);
-  if (file->lazy) {
-    if (auto *f = dyn_cast<BitcodeFile>(file))
-      f->parseLazy();
-    else
-      cast<ObjFile>(file)->parseLazy();
-  } else {
-    file->parse();
-    if (auto *f = dyn_cast<ObjFile>(file)) {
-      ctx.objFileInstances.push_back(f);
-    } else if (auto *f = dyn_cast<BitcodeFile>(file)) {
-      if (ltoCompilationDone) {
-        Err(ctx) << "LTO object file " << toString(file)
-                 << " linked in after "
-                    "doing LTO compilation.";
-      }
-      f->symtab.bitcodeFileInstances.push_back(f);
-    } else if (auto *f = dyn_cast<ImportFile>(file)) {
-      ctx.importFileInstances.push_back(f);
-    }
-  }
-
+  // We need to process the machine type early on, so that APIs such as
+  // `mangle()` won't fail.
   MachineTypes mt = file->getMachineType();
   // The ARM64EC target must be explicitly specified and cannot be inferred.
   if (mt == ARM64EC &&
@@ -242,6 +222,28 @@ void LinkerDriver::addFile(InputFile *file) {
     setMachine(mt);
   }
 
+  Log(ctx) << "Reading " << toString(file);
+  if (file->lazy) {
+    if (auto *f = dyn_cast<BitcodeFile>(file))
+      f->parseLazy();
+    else
+      cast<ObjFile>(file)->parseLazy();
+  } else {
+    file->parse();
+    if (auto *f = dyn_cast<ObjFile>(file)) {
+      ctx.objFileInstances.push_back(f);
+    } else if (auto *f = dyn_cast<BitcodeFile>(file)) {
+      if (ltoCompilationDone) {
+        Err(ctx) << "LTO object file " << toString(file)
+                 << " linked in after "
+                    "doing LTO compilation.";
+      }
+      f->symtab.bitcodeFileInstances.push_back(f);
+    } else if (auto *f = dyn_cast<ImportFile>(file)) {
+      ctx.importFileInstances.push_back(f);
+    }
+  }
+
   parseDirectives(file);
 }
 

lld/COFF/InputFiles.cpp

@@ -117,8 +117,6 @@ static coff_symbol_generic *cloneSymbol(COFFSymbolRef sym) {
 // Skip importing DllMain thunks from import libraries.
 static bool fixupDllMain(COFFLinkerContext &ctx, llvm::object::Archive *file,
                          const Archive::Symbol &sym, bool &skipDllMain) {
-  if (skipDllMain)
-    return true;
   const Archive::Child &c =
       CHECK(sym.getMember(), file->getFileName() +
                                  ": could not get the member for symbol " +
@@ -144,15 +142,15 @@ static bool fixupDllMain(COFFLinkerContext &ctx, llvm::object::Archive *file,
 }
 
 ArchiveFile::ArchiveFile(COFFLinkerContext &ctx, MemoryBufferRef m)
-    : InputFile(ctx.symtab, ArchiveKind, m) {}
+    : InputFile(ctx.symtab, ArchiveKind, m) {
+  // Parse a MemoryBufferRef as an archive file.
+  file = CHECK(Archive::create(mb), this);
+}
 
 void ArchiveFile::parse() {
   COFFLinkerContext &ctx = symtab.ctx;
   SymbolTable *archiveSymtab = &symtab;
 
-  // Parse a MemoryBufferRef as an archive file.
-  file = CHECK(Archive::create(mb), this);
-
   // Try to read symbols from ECSYMBOLS section on ARM64EC.
   if (ctx.symtab.isEC()) {
     iterator_range<Archive::symbol_iterator> symbols =
@@ -168,56 +166,79 @@ void ArchiveFile::parse() {
       // be either a native-only ARM64 or x86_64 archive. Check the machine type
       // of the object containing a symbol to determine which symbol table to
       // use.
-      Archive::symbol_iterator sym = file->symbol_begin();
-      if (sym != file->symbol_end()) {
-        MachineTypes machine = IMAGE_FILE_MACHINE_UNKNOWN;
-        Archive::Child child =
-            CHECK(sym->getMember(),
-                  file->getFileName() +
-                      ": could not get the buffer for a child of the archive");
-        MemoryBufferRef mb = CHECK(
-            child.getMemoryBufferRef(),
-            file->getFileName() +
-                ": could not get the buffer for a child buffer of the archive");
-        switch (identify_magic(mb.getBuffer())) {
-        case file_magic::coff_object: {
-          std::unique_ptr<COFFObjectFile> obj =
-              CHECK(COFFObjectFile::create(mb),
-                    check(child.getName()) + ":" + ": not a valid COFF file");
-          machine = MachineTypes(obj->getMachine());
-          break;
-        }
-        case file_magic::coff_import_library:
-          machine = MachineTypes(COFFImportFile(mb).getMachine());
-          break;
-        case file_magic::bitcode: {
-          std::unique_ptr<lto::InputFile> obj =
-              check(lto::InputFile::create(mb));
-          machine = BitcodeFile::getMachineType(obj.get());
-          break;
-        }
-        default:
-          break;
-        }
+      MachineTypes machine = getMachineType();
+      if (machine != IMAGE_FILE_MACHINE_UNKNOWN)
         archiveSymtab = &ctx.getSymtab(machine);
-      }
     }
   }
 
-  // Read the symbol table to construct Lazy objects.
   bool skipDllMain = false;
+  StringRef mangledDllMain, impMangledDllMain;
+
+  // The calls below will fail if we haven't set the machine type yet. Instead
+  // of failing, it is preferable to skip this "imported DllMain" check if we
+  // don't know the machine type at this point.
+  if (!file->isEmpty() && ctx.config.machine != IMAGE_FILE_MACHINE_UNKNOWN) {
+    mangledDllMain = archiveSymtab->mangle("DllMain");
+    impMangledDllMain = uniqueSaver().save("__imp_" + mangledDllMain);
+  }
+
+  // Read the symbol table to construct Lazy objects.
   for (const Archive::Symbol &sym : file->symbols()) {
     // If an import library provides the DllMain symbol, skip importing it, as
     // we should be using our own DllMain, not another DLL's DllMain.
-    if (sym.getName() == "__imp_DllMain" || sym.getName() == "DllMain" ||
-        sym.getName() == "_DllMain") {
-      if (fixupDllMain(ctx, file.get(), sym, skipDllMain))
+    if (!mangledDllMain.empty() && (sym.getName() == mangledDllMain ||
+                                    sym.getName() == impMangledDllMain)) {
+      if (skipDllMain || fixupDllMain(ctx, file.get(), sym, skipDllMain))
         continue;
     }
     archiveSymtab->addLazyArchive(this, sym);
   }
 }
 
+MachineTypes ArchiveFile::getMachineType() const {
+  if (!file)
+    return IMAGE_FILE_MACHINE_UNKNOWN;
+  if (file->isEmpty())
+    return IMAGE_FILE_MACHINE_UNKNOWN;
+  Archive::symbol_iterator sym = file->symbol_begin();
+  if (sym != file->symbol_end()) {
+    Expected<Archive::Child> child = sym->getMember();
+    if (!child) {
+      consumeError(child.takeError());
+      return IMAGE_FILE_MACHINE_UNKNOWN;
+    }
+    Expected<MemoryBufferRef> mb = child->getMemoryBufferRef();
+    if (!mb) {
+      consumeError(mb.takeError());
+      return IMAGE_FILE_MACHINE_UNKNOWN;
+    }
+    switch (identify_magic(mb->getBuffer())) {
+    case file_magic::coff_object: {
+      Expected<std::unique_ptr<COFFObjectFile>> obj =
+          COFFObjectFile::create(*mb);
+      if (!obj) {
+        consumeError(obj.takeError());
+        return IMAGE_FILE_MACHINE_UNKNOWN;
+      }
+      return MachineTypes((*obj)->getMachine());
+      break;
+    }
+    case file_magic::coff_import_library:
+      return MachineTypes(COFFImportFile(*mb).getMachine());
+      break;
+    case file_magic::bitcode: {
+      std::unique_ptr<lto::InputFile> obj = check(lto::InputFile::create(*mb));
+      return BitcodeFile::getMachineType(obj.get());
+      break;
+    }
+    default:
+      break;
+    }
+  }
+  return IMAGE_FILE_MACHINE_UNKNOWN;
+}
+
 // Returns a buffer pointing to a member file containing a given symbol.
 void ArchiveFile::addMember(const Archive::Symbol &sym) {
   const Archive::Child &c =

lld/COFF/InputFiles.h

@@ -121,6 +121,7 @@ class ArchiveFile : public InputFile {
   explicit ArchiveFile(COFFLinkerContext &ctx, MemoryBufferRef m);
   static bool classof(const InputFile *f) { return f->kind() == ArchiveKind; }
   void parse() override;
+  MachineTypes getMachineType() const override;
 
   // Enqueues an archive member load for the given symbol. If we've already
   // enqueued a load for the same archive member, this function does nothing,

lld/test/COFF/implib-machine.s

@@ -6,10 +6,10 @@
 # RUN: llvm-mc -triple x86_64-windows-msvc %t.dir/test.s -filetype=obj -o %t.dir/test64.obj
 
 # RUN: not lld-link -dll -noentry -out:%t32.dll %t.dir/test32.obj %t.dir/test64.lib 2>&1 | FileCheck --check-prefix=ERR32 %s
-# ERR32: error: test64.lib(test.dll): machine type x64 conflicts with x86
+# ERR32: error: {{.*[/\\]}}test64.lib: machine type x64 conflicts with x86
 
 # RUN: not lld-link -dll -noentry -out:%t64.dll %t.dir/test64.obj %t.dir/test32.lib 2>&1 | FileCheck --check-prefix=ERR64 %s
-# ERR64: error: test32.lib(test.dll): machine type x86 conflicts with x64
+# ERR64: error: {{.*[/\\]}}test32.lib: machine type x86 conflicts with x64
 
 #--- test.s
         .def     @feat.00;

lld/test/COFF/imported-dllmain-i386.test

@@ -25,8 +25,7 @@ DISASM: DLL Name: b.dll
 DISASM-NOT: DllMain
 DISASM: bar
 DISASM: Disassembly of section .text:
-DISASM-EMPTY:
-DISASM-NEXT: b0 01                         movb    $0x1, %al
+DISASM:      b0 01                         movb    $0x1, %al
 DISASM-NEXT: c3                            retl
 
 #--- a.s