LLD COFF: Add support for /force:multiple option

[troughton]

Summary

This patch adds support for linking with multiple definitions to LLD's COFF driver, in line with link.exe's /force:multiple option.

[troughton]

Could you please either take a look or suggest a reviewer?

[ruiu]

COFF/SymbolTable.cpp

264

Then you don't need to change the function name.

329–336

Looks like this is the only place where you are using this function. I'd inline it.

[troughton]

Address review comments by inlining errorOrWarnMultiple.

[ruiu]

LGTM

COFF/SymbolTable.cpp

329

We don't use auto unless an actual type is obvious:

auto -> std::string

Variable names should start with an uppercase letter and should be concise:

message -> Msg

[troughton]

Specify std::string over auto and change message to Msg.

[troughton]

If this looks fine, could you please merge it in? I’m not sure how the merge process is handled for LLVM.

[ruiu]

If you have a commit access, you can do that yourself, but since you don't have one, I'll do that for you.

[ruiu]

It needs a test, but I'll write that since the test should be small.

COFF/Config.h

struct Configuration {
   Symbol *Entry = nullptr;
   bool NoEntry = false;
   std::string OutputFile;
   std::string ImportName;
   bool DoGC = true;
   bool DoICF = true;
   bool TailMerge;
   bool Relocatable = true;
-  bool Force = false;
+  bool ForceMultiple = false;
+  bool ForceUnresolved = false;
   bool Debug = false;
   bool DebugDwarf = false;
   bool DebugGHashes = false;
   bool DebugSymtab = false;
   bool ShowTiming = false;
   unsigned DebugTypes = static_cast<unsigned>(DebugType::None);
   std::vector<std::string> NatvisFiles;
   llvm::SmallString<128> PDBAltPath;

COFF/Driver.cpp

void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
 
   // Handle /verbose
   if (Args.hasArg(OPT_verbose))
     Config->Verbose = true;
   errorHandler().Verbose = Config->Verbose;
 
   // Handle /force or /force:unresolved
   if (Args.hasArg(OPT_force, OPT_force_unresolved))
-    Config->Force = true;
+    Config->ForceUnresolved = true;
+
+  // Handle /force or /force:multiple
+  if (Args.hasArg(OPT_force, OPT_force_multiple))
+    Config->ForceMultiple = true;
 
   // Handle /debug
   if (Args.hasArg(OPT_debug, OPT_debug_dwarf, OPT_debug_ghash)) {
     Config->Debug = true;
     Config->Incremental = true;
     if (auto *Arg = Args.getLastArg(OPT_debugtype))
       Config->DebugTypes = parseDebugType(Arg->getValue());
     else

COFF/Options.td

 def profile : F<"profile">;
 def repro : F<"Brepro">, HelpText<"Use a hash of the executable as the PE header timestamp">;
 def swaprun_cd : F<"swaprun:cd">;
 def swaprun_net : F<"swaprun:net">;
 def verbose : F<"verbose">;
 def wholearchive_flag : F<"wholearchive">;
 
 def force : F<"force">,
+    HelpText<"Allow undefined and multiply defined symbols when creating executables">;
+def force_unresolved : F<"force:unresolved">,
     HelpText<"Allow undefined symbols when creating executables">;
-def force_unresolved : F<"force:unresolved">;
+def force_multiple : F<"force:multiple">,
+    HelpText<"Allow multiply defined symbols when creating executables">;
 defm WX : B<"WX", "Treat warnings as errors", "Don't treat warnings as errors">;
 
 defm allowbind : B<"allowbind", "Enable DLL binding (default)",
                    "Disable DLL binding">;
 defm allowisolation : B<"allowisolation", "Enable DLL isolation (default)",
                         "Disable DLL isolation">;
 defm appcontainer : B<"appcontainer",
                       "Image can only be run in an app container",

COFF/SymbolTable.cpp

void SymbolTable::addFile(InputFile *File) {
   if (S.empty())
     return;
 
   log("Directives: " + toString(File) + ": " + S);
   Driver->parseDirectives(S);
 }
 
 static void errorOrWarn(const Twine &S) {
-  if (Config->Force)
+  if (Config->ForceUnresolved)
     warn(S);
   else
     error(S);
 }
 
 // Returns the name of the symbol in SC whose value is <= Addr that is closest
 // to Addr. This is generally the name of the global variable or function whose
 // definition contains Addr.
if (Name.startswith("__imp_")) {
       }
     }
 
     if (Config->MinGW && handleMinGWAutomaticImport(Sym, Name))
       continue;
 
     // Remaining undefined symbols are not fatal if /force is specified.
     // They are replaced with dummy defined symbols.
-    if (Config->Force)
+    if (Config->ForceUnresolved)
       replaceSymbol<DefinedAbsolute>(Sym, Name, 0);
     Undefs.insert(Sym);
   }
 
   if (Undefs.empty() && LocalImports.empty())
     return;
 
   for (Symbol *B : Config->GCRoot) {
     if (Undefs.count(B))
       errorOrWarn("<root>: undefined symbol: " + B->getName());
     if (Config->WarnLocallyDefinedImported)
       if (Symbol *Imp = LocalImports.lookup(B))
         warn("<root>: locally defined symbol imported: " + Imp->getName() +
              " (defined in " + toString(Imp->getFile()) + ") [LNK4217]");
   }
 
   for (ObjFile *File : ObjFile::Instances) {
     size_t SymIndex = (size_t)-1;
     for (Symbol *Sym : File->getSymbols()) {
       ++SymIndex;
       if (!Sym)
         continue;
       if (Undefs.count(Sym))
         errorOrWarn("undefined symbol: " + Sym->getName() +
                     getSymbolLocations(File, SymIndex));
       if (Config->WarnLocallyDefinedImported)
         if (Symbol *Imp = LocalImports.lookup(Sym))
           warn(toString(File) + ": locally defined symbol imported: " +
                Imp->getName() + " (defined in " + toString(Imp->getFile()) +
                ") [LNK4217]");
     }
   }
void SymbolTable::addLazy(ArchiveFile *F, const Archive::Symbol Sym) {
   auto *U = dyn_cast<Undefined>(S);
   if (!U || U->WeakAlias || S->PendingArchiveLoad)
     return;
   S->PendingArchiveLoad = true;
   F->addMember(&Sym);
 }
 
 void SymbolTable::reportDuplicate(Symbol *Existing, InputFile *NewFile) {
-  error("duplicate symbol: " + toString(*Existing) + " in " +
-        toString(Existing->getFile()) + " and in " + toString(NewFile));
+  std::string Msg = "duplicate symbol: " + toString(*Existing) + " in " +
+                    toString(Existing->getFile()) + " and in " +
+                    toString(NewFile);
+
+  if (Config->ForceMultiple)
+    warn(Msg);
+  else
+    error(Msg);
 }
 
 Symbol *SymbolTable::addAbsolute(StringRef N, COFFSymbolRef Sym) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(N, nullptr);
   S->IsUsedInRegularObj = true;
   if (WasInserted || isa<Undefined>(S) || isa<Lazy>(S))

test/COFF/force-multiple.test

+# RUN: yaml2obj < %s > %t1.obj
+# RUN: yaml2obj < %s > %t2.obj
+
+# RUN: not lld-link /out:%t.exe /entry:main %t1.obj %t2.obj >& %t.log
+# RUN: FileCheck -check-prefix=ERROR %s < %t.log
+
+# RUN: lld-link /out:%t.exe /entry:main %t1.obj %t2.obj /force >& %t.log
+# RUN: FileCheck -check-prefix=WARN %s < %t.log
+
+# RUN: lld-link /out:%t.exe /entry:main %t1.obj %t2.obj /force:multiple >& %t.log
+# RUN: FileCheck -check-prefix=WARN %s < %t.log
+
+# ERROR: error: duplicate symbol: main
+
+# WARN: warning: duplicate symbol: main
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: []
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     000000000000
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          6
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          0
+  - Name:            main
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...