COFF: Change symbol resolution order for entry and /include.

We were resolving entry symbols and /include'd symbols after all other
symbols are resolved. But looks like it's too late. I found that it
causes some program to fail to link.

Let's say we have an object file A which defines symbols X and Y in an
archive. We also have another file B after A which defines X, Y and
_DLLMainCRTStartup in another archive. They conflict each other, so
either A or B can be linked.

If we have _DLLMainCRTStartup as an undefined symbol, file B is always
chosen. If not, there's a chance that A is chosen. If the linker
find it needs _DllMainCRTStartup after that, it's too late.

This patch adds undefined symbols to the symbol table as soon as
possible to fix the issue.

llvm-svn: 240757

lld/COFF/Config.h

@@ -53,9 +53,6 @@ struct Configuration {
   std::set<StringRef> NoDefaultLibs;
   bool NoDefaultLibAll = false;
 
-  // Used for /include.
-  std::set<StringRef> Includes;
-
   // True if we are creating a DLL.
   bool DLL = false;
   StringRef Implib;

lld/COFF/Driver.cpp

@@ -117,7 +117,7 @@ LinkerDriver::parseDirectives(StringRef S) {
         return EC;
       break;
     case OPT_incl:
-      Config->Includes.insert(Arg->getValue());
+      addUndefined(Arg->getValue());
       break;
     case OPT_merge:
       // Ignore /merge for now.
@@ -201,6 +201,11 @@ void LinkerDriver::addLibSearchPaths() {
   }
 }
 
+void LinkerDriver::addUndefined(StringRef Sym) {
+  Symtab.addUndefined(Sym);
+  Config->GCRoots.insert(Sym);
+}
+
 static WindowsSubsystem inferSubsystem() {
   if (Config->DLL)
     return IMAGE_SUBSYSTEM_WINDOWS_GUI;
@@ -259,16 +264,22 @@ bool LinkerDriver::link(llvm::ArrayRef<const char *> ArgsArr) {
   if (Args.hasArg(OPT_verbose))
     Config->Verbose = true;
 
+  // Handle /entry
+  if (auto *Arg = Args.getLastArg(OPT_entry)) {
+    Config->EntryName = Arg->getValue();
+    addUndefined(Config->EntryName);
+  }
+
   // Handle /dll
   if (Args.hasArg(OPT_dll)) {
     Config->DLL = true;
     Config->ManifestID = 2;
+    if (Config->EntryName.empty()) {
+      Config->EntryName = "_DllMainCRTStartup";
+      addUndefined("_DllMainCRTStartup");
+    }
   }
 
-  // Handle /entry
-  if (auto *Arg = Args.getLastArg(OPT_entry))
-    Config->EntryName = Arg->getValue();
-
   // Handle /fixed
   if (Args.hasArg(OPT_fixed)) {
     if (Args.hasArg(OPT_dynamicbase)) {
@@ -347,7 +358,7 @@ bool LinkerDriver::link(llvm::ArrayRef<const char *> ArgsArr) {
 
   // Handle /include
   for (auto *Arg : Args.filtered(OPT_incl))
-    Config->Includes.insert(Arg->getValue());
+    addUndefined(Arg->getValue());
 
   // Handle /implib
   if (auto *Arg = Args.getLastArg(OPT_implib))
@@ -383,7 +394,7 @@ bool LinkerDriver::link(llvm::ArrayRef<const char *> ArgsArr) {
   // Handle /delayload
   for (auto *Arg : Args.filtered(OPT_delayload)) {
     Config->DelayLoads.insert(Arg->getValue());
-    Config->Includes.insert("__delayLoadHelper2");
+    addUndefined("__delayLoadHelper2");
   }
 
   // Handle /failifmismatch
@@ -504,13 +515,9 @@ bool LinkerDriver::link(llvm::ArrayRef<const char *> ArgsArr) {
   for (;;) {
     size_t Ver = Symtab.getVersion();
 
-    // Add undefined symbols specified by /include.
-    for (StringRef Sym : Config->Includes)
-      Symtab.addUndefined(Sym);
-
     // Windows specific -- Make sure we resolve all dllexported symbols.
     for (Export &E : Config->Exports)
-      Symtab.addUndefined(E.Name);
+      addUndefined(E.Name);
 
     // Add weak aliases. Weak aliases is a mechanism to give remaining
     // undefined symbols final chance to be resolved successfully.
@@ -535,7 +542,7 @@ bool LinkerDriver::link(llvm::ArrayRef<const char *> ArgsArr) {
       }
       Config->EntryName = EntryOrErr.get();
     }
-    Symtab.addUndefined(Config->EntryName);
+    addUndefined(Config->EntryName);
 
     if (auto EC = Symtab.run()) {
       llvm::errs() << EC.message() << "\n";
@@ -549,13 +556,6 @@ bool LinkerDriver::link(llvm::ArrayRef<const char *> ArgsArr) {
   if (Symtab.reportRemainingUndefines())
     return false;
 
-  // Initialize a list of GC root.
-  for (StringRef Sym : Config->Includes)
-    Config->GCRoots.insert(Sym);
-  for (Export &E : Config->Exports)
-    Config->GCRoots.insert(E.Name);
-  Config->GCRoots.insert(Config->EntryName);
-
   // Do LTO by compiling bitcode input files to a native COFF file
   // then link that file.
   if (auto EC = Symtab.addCombinedLTOObject()) {

lld/COFF/Driver.h

@@ -88,9 +88,10 @@ private:
 
   // Library search path. The first element is always "" (current directory).
   std::vector<StringRef> SearchPaths;
-
   std::set<std::string> VisitedFiles;
 
+  void addUndefined(StringRef Sym);
+
   // Driver is the owner of all opened files.
   // InputFiles have MemoryBufferRefs to them.
   std::vector<std::unique_ptr<MemoryBuffer>> OwningMBs;

lld/COFF/SymbolTable.cpp

@@ -184,14 +184,6 @@ std::error_code SymbolTable::resolveLazy(StringRef Name) {
 
 // Windows specific -- Link default entry point name.
 ErrorOr<StringRef> SymbolTable::findDefaultEntry() {
-  // If it's DLL, the rule is easy.
-  if (Config->DLL) {
-    StringRef Sym = "_DllMainCRTStartup";
-    if (auto EC = resolve(new (Alloc) Undefined(Sym)))
-      return EC;
-    return Sym;
-  }
-
   // User-defined main functions and their corresponding entry points.
   static const char *Entries[][2] = {
       {"main", "mainCRTStartup"},

lld/test/COFF/include2.test

@@ -9,6 +9,6 @@
 
 CHECK: include2.test.tmp1.obj
 CHECK: include2.test.tmp2.lib
-CHECK: include2.test.tmp3.lib
 CHECK: include2.test.tmp2.lib(include2.test.tmp2.obj) for foo
+CHECK: include2.test.tmp3.lib
 CHECK: include2.test.tmp3.lib(include2.test.tmp3.obj) for bar

lld/test/COFF/order.test

@@ -9,6 +9,6 @@
 
 CHECK: order.test.tmp1.obj
 CHECK: order.test.tmp2.lib
+CHECK: order.test.tmp2.lib(order.test.tmp2.obj) for foo
 CHECK: order.test.tmp3.obj
 CHECK: order.test.tmp3.lib
-CHECK: order.test.tmp2.lib(order.test.tmp2.obj) for foo