[llvm] r359083 - Let llvm-cvtres (and lld-link) report duplicate resources

Nico Weber via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 24 04:42:59 PDT 2019


Author: nico
Date: Wed Apr 24 04:42:59 2019
New Revision: 359083

URL: http://llvm.org/viewvc/llvm-project?rev=359083&view=rev
Log:
Let llvm-cvtres (and lld-link) report duplicate resources

If two .res files contain the same resource, cvtres.exe (and hence
link.exe) reject the input with this message:

    CVTRES : fatal error CVT1100: duplicate resource.  type:STRING, name:101, language:0x0409
    LINK : fatal error LNK1123: failure during conversion to COFF: file invalid or corrupt

llvm-cvtres (and lld-link) used to silently pick one of the duplicate
resources instead. This patch makes them report an error as well.
We slightly improve on cvtres by printing the name of two .res files
containing duplicate entries as well.

Differential Revision: https://reviews.llvm.org/D61049

Added:
    llvm/trunk/test/tools/llvm-cvtres/Inputs/id.rc
    llvm/trunk/test/tools/llvm-cvtres/Inputs/id.res   (with props)
    llvm/trunk/test/tools/llvm-cvtres/Inputs/name.rc
    llvm/trunk/test/tools/llvm-cvtres/Inputs/name.res   (with props)
    llvm/trunk/test/tools/llvm-cvtres/duplicate.test
Modified:
    llvm/trunk/include/llvm/Object/WindowsResource.h
    llvm/trunk/lib/Object/WindowsResource.cpp

Modified: llvm/trunk/include/llvm/Object/WindowsResource.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/WindowsResource.h?rev=359083&r1=359082&r2=359083&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/WindowsResource.h (original)
+++ llvm/trunk/include/llvm/Object/WindowsResource.h Wed Apr 24 04:42:59 2019
@@ -184,19 +184,23 @@ public:
     static std::unique_ptr<TreeNode> createIDNode();
     static std::unique_ptr<TreeNode> createDataNode(uint16_t MajorVersion,
                                                     uint16_t MinorVersion,
-                                                    uint32_t Characteristics);
+                                                    uint32_t Characteristics,
+                                                    uint32_t Origin);
 
     explicit TreeNode(bool IsStringNode);
     TreeNode(uint16_t MajorVersion, uint16_t MinorVersion,
-             uint32_t Characteristics);
+             uint32_t Characteristics, uint32_t Origin);
 
-    void addEntry(const ResourceEntryRef &Entry, bool &IsNewTypeString,
-                  bool &IsNewNameString);
+    bool addEntry(const ResourceEntryRef &Entry, uint32_t Origin,
+                  bool &IsNewTypeString, bool &IsNewNameString,
+                  TreeNode *&Result);
     TreeNode &addTypeNode(const ResourceEntryRef &Entry, bool &IsNewTypeString);
     TreeNode &addNameNode(const ResourceEntryRef &Entry, bool &IsNewNameString);
-    TreeNode &addLanguageNode(const ResourceEntryRef &Entry);
-    TreeNode &addDataChild(uint32_t ID, uint16_t MajorVersion,
-                           uint16_t MinorVersion, uint32_t Characteristics);
+    bool addLanguageNode(const ResourceEntryRef &Entry, uint32_t Origin,
+                         TreeNode *&Result);
+    bool addDataChild(uint32_t ID, uint16_t MajorVersion, uint16_t MinorVersion,
+                      uint32_t Characteristics, uint32_t Origin,
+                      TreeNode *&Result);
     TreeNode &addIDChild(uint32_t ID);
     TreeNode &addNameChild(ArrayRef<UTF16> NameRef, bool &IsNewString);
 
@@ -208,12 +212,18 @@ public:
     uint16_t MajorVersion = 0;
     uint16_t MinorVersion = 0;
     uint32_t Characteristics = 0;
+
+    // The .res file that defined this TreeNode, for diagnostics.
+    // Index into InputFilenames.
+    uint32_t Origin;
   };
 
 private:
   TreeNode Root;
   std::vector<std::vector<uint8_t>> Data;
   std::vector<std::vector<UTF16>> StringTable;
+
+  std::vector<std::string> InputFilenames;
 };
 
 Expected<std::unique_ptr<MemoryBuffer>>

Modified: llvm/trunk/lib/Object/WindowsResource.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/WindowsResource.cpp?rev=359083&r1=359082&r2=359083&view=diff
==============================================================================
--- llvm/trunk/lib/Object/WindowsResource.cpp (original)
+++ llvm/trunk/lib/Object/WindowsResource.cpp Wed Apr 24 04:42:59 2019
@@ -127,6 +127,39 @@ Error ResourceEntryRef::loadNext() {
 
 WindowsResourceParser::WindowsResourceParser() : Root(false) {}
 
+static Error duplicateResourceError(const ResourceEntryRef& Entry,
+                            StringRef File1, StringRef File2) {
+  std::string Ret;
+  raw_string_ostream OS(Ret);
+
+  OS << "duplicate resource:";
+
+  OS << " type ";
+  if (Entry.checkTypeString()) {
+    std::string UTF8;
+    if (!convertUTF16ToUTF8String(Entry.getTypeString(), UTF8))
+      UTF8 = "(failed conversion from UTF16)";
+    OS << '\"' << UTF8 << '\"';
+  } else {
+    OS << "ID " << Entry.getTypeID();
+  }
+
+  OS << "/name ";
+  if (Entry.checkNameString()) {
+    std::string UTF8;
+    if (!convertUTF16ToUTF8String(Entry.getNameString(), UTF8))
+      UTF8 = "(failed conversion from UTF16)";
+    OS << '\"' << UTF8 << '\"';
+  } else {
+    OS << "ID " << Entry.getNameID();
+  }
+
+  OS << "/language " << Entry.getLanguage() << ", in " << File1 << " and in "
+     << File2;
+
+  return make_error<GenericBinaryError>(OS.str(), object_error::parse_failed);
+}
+
 Error WindowsResourceParser::parse(WindowsResource *WR) {
   auto EntryOrErr = WR->getHeadEntry();
   if (!EntryOrErr) {
@@ -152,7 +185,13 @@ Error WindowsResourceParser::parse(Windo
     bool IsNewTypeString = false;
     bool IsNewNameString = false;
 
-    Root.addEntry(Entry, IsNewTypeString, IsNewNameString);
+    TreeNode* Node;
+    bool IsNewNode = Root.addEntry(Entry, InputFilenames.size(),
+                                   IsNewTypeString, IsNewNameString, Node);
+    InputFilenames.push_back(WR->getFileName());
+    if (!IsNewNode)
+      return duplicateResourceError(Entry, InputFilenames[Node->Origin],
+                                    WR->getFileName());
 
     if (IsNewTypeString)
       StringTable.push_back(Entry.getTypeString());
@@ -171,12 +210,14 @@ void WindowsResourceParser::printTree(ra
   Root.print(Writer, "Resource Tree");
 }
 
-void WindowsResourceParser::TreeNode::addEntry(const ResourceEntryRef &Entry,
+bool WindowsResourceParser::TreeNode::addEntry(const ResourceEntryRef &Entry,
+                                               uint32_t Origin,
                                                bool &IsNewTypeString,
-                                               bool &IsNewNameString) {
+                                               bool &IsNewNameString,
+                                               TreeNode *&Result) {
   TreeNode &TypeNode = addTypeNode(Entry, IsNewTypeString);
   TreeNode &NameNode = TypeNode.addNameNode(Entry, IsNewNameString);
-  NameNode.addLanguageNode(Entry);
+  return NameNode.addLanguageNode(Entry, Origin, Result);
 }
 
 WindowsResourceParser::TreeNode::TreeNode(bool IsStringNode) {
@@ -186,10 +227,11 @@ WindowsResourceParser::TreeNode::TreeNod
 
 WindowsResourceParser::TreeNode::TreeNode(uint16_t MajorVersion,
                                           uint16_t MinorVersion,
-                                          uint32_t Characteristics)
+                                          uint32_t Characteristics,
+                                          uint32_t Origin)
     : IsDataNode(true), MajorVersion(MajorVersion), MinorVersion(MinorVersion),
-      Characteristics(Characteristics) {
-    DataIndex = DataCount++;
+      Characteristics(Characteristics), Origin(Origin) {
+  DataIndex = DataCount++;
 }
 
 std::unique_ptr<WindowsResourceParser::TreeNode>
@@ -205,9 +247,10 @@ WindowsResourceParser::TreeNode::createI
 std::unique_ptr<WindowsResourceParser::TreeNode>
 WindowsResourceParser::TreeNode::createDataNode(uint16_t MajorVersion,
                                                 uint16_t MinorVersion,
-                                                uint32_t Characteristics) {
+                                                uint32_t Characteristics,
+                                                uint32_t Origin) {
   return std::unique_ptr<TreeNode>(
-      new TreeNode(MajorVersion, MinorVersion, Characteristics));
+      new TreeNode(MajorVersion, MinorVersion, Characteristics, Origin));
 }
 
 WindowsResourceParser::TreeNode &
@@ -228,24 +271,21 @@ WindowsResourceParser::TreeNode::addName
     return addIDChild(Entry.getNameID());
 }
 
-WindowsResourceParser::TreeNode &
-WindowsResourceParser::TreeNode::addLanguageNode(
-    const ResourceEntryRef &Entry) {
+bool WindowsResourceParser::TreeNode::addLanguageNode(
+    const ResourceEntryRef &Entry, uint32_t Origin, TreeNode *&Result) {
   return addDataChild(Entry.getLanguage(), Entry.getMajorVersion(),
-                      Entry.getMinorVersion(), Entry.getCharacteristics());
+                      Entry.getMinorVersion(), Entry.getCharacteristics(),
+                      Origin, Result);
 }
 
-WindowsResourceParser::TreeNode &WindowsResourceParser::TreeNode::addDataChild(
+bool WindowsResourceParser::TreeNode::addDataChild(
     uint32_t ID, uint16_t MajorVersion, uint16_t MinorVersion,
-    uint32_t Characteristics) {
-  auto Child = IDChildren.find(ID);
-  if (Child == IDChildren.end()) {
-    auto NewChild = createDataNode(MajorVersion, MinorVersion, Characteristics);
-    WindowsResourceParser::TreeNode &Node = *NewChild;
-    IDChildren.emplace(ID, std::move(NewChild));
-    return Node;
-  } else
-    return *(Child->second);
+    uint32_t Characteristics, uint32_t Origin, TreeNode *&Result) {
+  auto NewChild =
+      createDataNode(MajorVersion, MinorVersion, Characteristics, Origin);
+  auto ElementInserted = IDChildren.emplace(ID, std::move(NewChild));
+  Result = ElementInserted.first->second.get();
+  return ElementInserted.second;
 }
 
 WindowsResourceParser::TreeNode &WindowsResourceParser::TreeNode::addIDChild(

Added: llvm/trunk/test/tools/llvm-cvtres/Inputs/id.rc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cvtres/Inputs/id.rc?rev=359083&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-cvtres/Inputs/id.rc (added)
+++ llvm/trunk/test/tools/llvm-cvtres/Inputs/id.rc Wed Apr 24 04:42:59 2019
@@ -0,0 +1,3 @@
+stringtable begin
+  42, "hi"
+end

Added: llvm/trunk/test/tools/llvm-cvtres/Inputs/id.res
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cvtres/Inputs/id.res?rev=359083&view=auto
==============================================================================
Binary file - no diff available.

Propchange: llvm/trunk/test/tools/llvm-cvtres/Inputs/id.res
------------------------------------------------------------------------------
    svn:mime-type = application/octet-stream

Added: llvm/trunk/test/tools/llvm-cvtres/Inputs/name.rc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cvtres/Inputs/name.rc?rev=359083&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-cvtres/Inputs/name.rc (added)
+++ llvm/trunk/test/tools/llvm-cvtres/Inputs/name.rc Wed Apr 24 04:42:59 2019
@@ -0,0 +1 @@
+namebar typefoo { "data" }

Added: llvm/trunk/test/tools/llvm-cvtres/Inputs/name.res
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cvtres/Inputs/name.res?rev=359083&view=auto
==============================================================================
Binary file - no diff available.

Propchange: llvm/trunk/test/tools/llvm-cvtres/Inputs/name.res
------------------------------------------------------------------------------
    svn:mime-type = application/octet-stream

Added: llvm/trunk/test/tools/llvm-cvtres/duplicate.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-cvtres/duplicate.test?rev=359083&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-cvtres/duplicate.test (added)
+++ llvm/trunk/test/tools/llvm-cvtres/duplicate.test Wed Apr 24 04:42:59 2019
@@ -0,0 +1,19 @@
+// Check that cvtres rejects duplicate resources.
+// 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 llvm-cvtres /machine:X86 %t.dir/id1.res %t.dir/id2.res 2>&1 | \
+RUN:     FileCheck -check-prefix=ID %s
+ID: duplicate resource: type ID 6/name ID 3/language 1033, in {{.*}}id1.res and in {{.*}}id2.res
+
+RUN: cp %S/Inputs/name.res %t.dir/name1.res
+RUN: cp %S/Inputs/name.res %t.dir/name2.res
+RUN: not llvm-cvtres /machine:X86 %t.dir/name1.res %t.dir/name2.res 2>&1 | \
+RUN:     FileCheck -check-prefix=NAME %s
+NAME: duplicate resource: type "TYPEFOO"/name "NAMEBAR"/language 1033, in {{.*}}name1.res and in {{.*}}name2.res




More information about the llvm-commits mailing list