lld-link: Add /force:multipleres extension to make dupe resource diag non-fatal

nico · nico · commit 81862f82ee3d · 2019-05-02T21:21:55.000Z
As a side benefit, lld-link now reports more than one duplicate resource
entry before exiting with an error even if the new flag is not passed.

llvm-svn: 359829
diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h
@@ -97,6 +97,7 @@ struct Configuration {
   bool TailMerge;
   bool Relocatable = true;
   bool ForceMultiple = false;
+  bool ForceMultipleRes = false;
   bool ForceUnresolved = false;
   bool Debug = false;
   bool DebugDwarf = false;
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
@@ -1096,6 +1096,10 @@ void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   if (Args.hasArg(OPT_force, OPT_force_multiple))
     Config->ForceMultiple = true;
 
+  // Handle /force or /force:multipleres
+  if (Args.hasArg(OPT_force, OPT_force_multipleres))
+    Config->ForceMultipleRes = true;
+
   // Handle /debug
   DebugKind Debug = parseDebugKind(Args);
   if (Debug == DebugKind::Full || Debug == DebugKind::Dwarf ||
diff --git a/lld/COFF/DriverUtils.cpp b/lld/COFF/DriverUtils.cpp
@@ -745,8 +745,16 @@ MemoryBufferRef convertResToCOFF(ArrayRef<MemoryBufferRef> MBs) {
     object::WindowsResource *RF = dyn_cast<object::WindowsResource>(Bin.get());
     if (!RF)
       fatal("cannot compile non-resource file as resource");
-    if (auto EC = Parser.parse(RF))
+
+    std::vector<std::string> Duplicates;
+    if (auto EC = Parser.parse(RF, Duplicates))
       fatal(toString(std::move(EC)));
+
+    for (const auto &DupeDiag : Duplicates)
+      if (Config->ForceMultipleRes)
+        warn(DupeDiag);
+      else
+        error(DupeDiag);
   }
 
   Expected<std::unique_ptr<MemoryBuffer>> E =
diff --git a/lld/COFF/Options.td b/lld/COFF/Options.td
@@ -118,6 +118,8 @@ def force_unresolved : F<"force:unresolved">,
     HelpText<"Allow undefined symbols when creating executables">;
 def force_multiple : F<"force:multiple">,
     HelpText<"Allow multiply defined symbols when creating executables">;
+def force_multipleres : F<"force:multipleres">,
+    HelpText<"Allow multiply defined resources 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)",
diff --git a/lld/docs/ReleaseNotes.rst b/lld/docs/ReleaseNotes.rst
@@ -34,6 +34,7 @@ COFF Improvements
 
 * lld-link now correctly reports duplicate symbol errors when several res
   input files define resources with the same type, name, and language.
+  This can be demoted to a warning using ``/force:multipleres``.
 
 * Having more than two ``/natvis:`` now works correctly; it used to not
   work for larger binaries before.
diff --git a/lld/test/COFF/Inputs/id.res b/lld/test/COFF/Inputs/id.res
diff --git a/lld/test/COFF/force-multipleres.test b/lld/test/COFF/force-multipleres.test
@@ -0,0 +1,21 @@
+// Check that lld-link rejects duplicate resources, unless
+// /force or /force:multipleres is passed.
+// The input was generated with the following command, using the original Windows
+// rc.exe:
+// > rc /fo id.res /nologo id.rc
+// > rc /fo name.res /nologo name.rc
+
+RUN: rm -rf %t.dir
+RUN: mkdir %t.dir
+RUN: cp %S/Inputs/id.res %t.dir/id1.res
+RUN: cp %S/Inputs/id.res %t.dir/id2.res
+
+RUN: not lld-link /machine:x64 /nodefaultlib /noentry /dll %t.dir/id1.res %t.dir/id2.res 2>&1 | \
+RUN:     FileCheck -check-prefix=ERR %s
+ERR: error: duplicate resource: type STRINGTABLE (ID 6)/name ID 3/language 1033, in {{.*}}id1.res and in {{.*}}id2.res
+
+RUN: lld-link /force /machine:x64 /nodefaultlib /noentry /dll %t.dir/id1.res %t.dir/id2.res 2>&1 | \
+RUN:     FileCheck -check-prefix=WARN %s
+RUN: lld-link /force:multipleres /machine:x64 /nodefaultlib /noentry /dll %t.dir/id1.res %t.dir/id2.res 2>&1 | \
+RUN:     FileCheck -check-prefix=WARN %s
+WARN: warning: duplicate resource: type STRINGTABLE (ID 6)/name ID 3/language 1033, in {{.*}}id1.res and in {{.*}}id2.res
diff --git a/llvm/include/llvm/Object/WindowsResource.h b/llvm/include/llvm/Object/WindowsResource.h
@@ -152,7 +152,7 @@ class WindowsResourceParser {
 public:
   class TreeNode;
   WindowsResourceParser();
-  Error parse(WindowsResource *WR);
+  Error parse(WindowsResource *WR, std::vector<std::string> &Duplicates);
   void printTree(raw_ostream &OS) const;
   const TreeNode &getTree() const { return Root; }
   const ArrayRef<std::vector<uint8_t>> getData() const { return Data; }
diff --git a/llvm/lib/Object/WindowsResource.cpp b/llvm/lib/Object/WindowsResource.cpp
@@ -168,8 +168,8 @@ static bool convertUTF16LEToUTF8String(ArrayRef<UTF16> Src, std::string &Out) {
   return convertUTF16ToUTF8String(makeArrayRef(EndianCorrectedSrc), Out);
 }
 
-static Error makeDuplicateResourceError(const ResourceEntryRef &Entry,
-                                        StringRef File1, StringRef File2) {
+static std::string makeDuplicateResourceError(
+    const ResourceEntryRef &Entry, StringRef File1, StringRef File2) {
   std::string Ret;
   raw_string_ostream OS(Ret);
 
@@ -197,10 +197,11 @@ static Error makeDuplicateResourceError(const ResourceEntryRef &Entry,
   OS << "/language " << Entry.getLanguage() << ", in " << File1 << " and in "
      << File2;
 
-  return make_error<GenericBinaryError>(OS.str(), object_error::parse_failed);
+  return OS.str();
 }
 
-Error WindowsResourceParser::parse(WindowsResource *WR) {
+Error WindowsResourceParser::parse(WindowsResource *WR,
+                                   std::vector<std::string> &Duplicates) {
   auto EntryOrErr = WR->getHeadEntry();
   if (!EntryOrErr) {
     auto E = EntryOrErr.takeError();
@@ -229,9 +230,10 @@ Error WindowsResourceParser::parse(WindowsResource *WR) {
     bool IsNewNode = Root.addEntry(Entry, InputFilenames.size(),
                                    IsNewTypeString, IsNewNameString, Node);
     InputFilenames.push_back(WR->getFileName());
-    if (!IsNewNode)
-      return makeDuplicateResourceError(Entry, InputFilenames[Node->Origin],
-                                        WR->getFileName());
+    if (!IsNewNode) {
+      Duplicates.push_back(makeDuplicateResourceError(
+          Entry, InputFilenames[Node->Origin], WR->getFileName()));
+    }
 
     if (IsNewTypeString)
       StringTable.push_back(Entry.getTypeString());
diff --git a/llvm/tools/llvm-cvtres/llvm-cvtres.cpp b/llvm/tools/llvm-cvtres/llvm-cvtres.cpp
@@ -183,7 +183,10 @@ int main(int Argc, const char **Argv) {
       outs() << "Number of resources: " << EntryNumber << "\n";
     }
 
-    error(Parser.parse(RF));
+    std::vector<std::string> Duplicates;
+    error(Parser.parse(RF, Duplicates));
+    for (const auto& DupeDiag : Duplicates)
+      reportError(DupeDiag);
   }
 
   if (Verbose) {

Original file line number	Diff line number	Diff line change
`@@ -183,7 +183,10 @@ int main(int Argc, const char **Argv) {`
`183`	`183`	`outs() << "Number of resources: " << EntryNumber << "\n";`
`184`	`184`	`}`
`185`	`185`
`186`		`- error(Parser.parse(RF));`
	`186`	`+ std::vector<std::string> Duplicates;`
	`187`	`+ error(Parser.parse(RF, Duplicates));`
	`188`	`+ for (const auto& DupeDiag : Duplicates)`
	`189`	`+ reportError(DupeDiag);`
`187`	`190`	`}`
`188`	`191`
`189`	`192`	`if (Verbose) {`