From 32f8e1cb4e1975f4bba05af655df79af00665f8d Mon Sep 17 00:00:00 2001 From: Rui Ueyama Date: Fri, 26 Jun 2015 03:44:00 +0000 Subject: [PATCH] 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 | 3 --- lld/COFF/Driver.cpp | 40 ++++++++++++++++++++-------------------- lld/COFF/Driver.h | 3 ++- lld/COFF/SymbolTable.cpp | 8 -------- lld/test/COFF/include2.test | 2 +- lld/test/COFF/order.test | 2 +- 6 files changed, 24 insertions(+), 34 deletions(-) diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h index d3881b8..8233852 100644 --- a/lld/COFF/Config.h +++ b/lld/COFF/Config.h @@ -53,9 +53,6 @@ struct Configuration { std::set NoDefaultLibs; bool NoDefaultLibAll = false; - // Used for /include. - std::set Includes; - // True if we are creating a DLL. bool DLL = false; StringRef Implib; diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index 1eb1a95..7c1612c 100644 --- a/lld/COFF/Driver.cpp +++ b/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 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 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 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 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 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 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()) { diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h index ab10d6b..4258cca 100644 --- a/lld/COFF/Driver.h +++ b/lld/COFF/Driver.h @@ -88,9 +88,10 @@ private: // Library search path. The first element is always "" (current directory). std::vector SearchPaths; - std::set VisitedFiles; + void addUndefined(StringRef Sym); + // Driver is the owner of all opened files. // InputFiles have MemoryBufferRefs to them. std::vector> OwningMBs; diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp index b010c4c..112ffb3 100644 --- a/lld/COFF/SymbolTable.cpp +++ b/lld/COFF/SymbolTable.cpp @@ -184,14 +184,6 @@ std::error_code SymbolTable::resolveLazy(StringRef Name) { // Windows specific -- Link default entry point name. ErrorOr 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"}, diff --git a/lld/test/COFF/include2.test b/lld/test/COFF/include2.test index 117942a..afcc934 100644 --- a/lld/test/COFF/include2.test +++ b/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 diff --git a/lld/test/COFF/order.test b/lld/test/COFF/order.test index 37e16f1..30e384f 100644 --- a/lld/test/COFF/order.test +++ b/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 -- 2.7.4