From daa37c31411d1d7623986798b7de8468ff341820 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 24 Apr 2019 11:42:59 +0000 Subject: [PATCH] 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 llvm-svn: 359083 --- include/llvm/Object/WindowsResource.h | 24 +++++-- lib/Object/WindowsResource.cpp | 86 ++++++++++++++++++------- test/tools/llvm-cvtres/Inputs/id.rc | 3 + test/tools/llvm-cvtres/Inputs/id.res | Bin 0 -> 100 bytes test/tools/llvm-cvtres/Inputs/name.rc | 1 + test/tools/llvm-cvtres/Inputs/name.res | Bin 0 -> 92 bytes test/tools/llvm-cvtres/duplicate.test | 19 ++++++ 7 files changed, 103 insertions(+), 30 deletions(-) create mode 100644 test/tools/llvm-cvtres/Inputs/id.rc create mode 100644 test/tools/llvm-cvtres/Inputs/id.res create mode 100644 test/tools/llvm-cvtres/Inputs/name.rc create mode 100644 test/tools/llvm-cvtres/Inputs/name.res create mode 100644 test/tools/llvm-cvtres/duplicate.test diff --git a/include/llvm/Object/WindowsResource.h b/include/llvm/Object/WindowsResource.h index d241da831b9..4700ae8cec9 100644 --- a/include/llvm/Object/WindowsResource.h +++ b/include/llvm/Object/WindowsResource.h @@ -184,19 +184,23 @@ public: static std::unique_ptr createIDNode(); static std::unique_ptr 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 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> Data; std::vector> StringTable; + + std::vector InputFilenames; }; Expected> diff --git a/lib/Object/WindowsResource.cpp b/lib/Object/WindowsResource.cpp index 2195a619605..813b4b66e3e 100644 --- a/lib/Object/WindowsResource.cpp +++ b/lib/Object/WindowsResource.cpp @@ -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(OS.str(), object_error::parse_failed); +} + Error WindowsResourceParser::parse(WindowsResource *WR) { auto EntryOrErr = WR->getHeadEntry(); if (!EntryOrErr) { @@ -152,7 +185,13 @@ Error WindowsResourceParser::parse(WindowsResource *WR) { 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(raw_ostream &OS) const { 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::TreeNode(bool IsStringNode) { 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 @@ -205,9 +247,10 @@ WindowsResourceParser::TreeNode::createIDNode() { std::unique_ptr WindowsResourceParser::TreeNode::createDataNode(uint16_t MajorVersion, uint16_t MinorVersion, - uint32_t Characteristics) { + uint32_t Characteristics, + uint32_t Origin) { return std::unique_ptr( - new TreeNode(MajorVersion, MinorVersion, Characteristics)); + new TreeNode(MajorVersion, MinorVersion, Characteristics, Origin)); } WindowsResourceParser::TreeNode & @@ -228,24 +271,21 @@ WindowsResourceParser::TreeNode::addNameNode(const ResourceEntryRef &Entry, 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( diff --git a/test/tools/llvm-cvtres/Inputs/id.rc b/test/tools/llvm-cvtres/Inputs/id.rc new file mode 100644 index 00000000000..141e9035ea0 --- /dev/null +++ b/test/tools/llvm-cvtres/Inputs/id.rc @@ -0,0 +1,3 @@ +stringtable begin + 42, "hi" +end diff --git a/test/tools/llvm-cvtres/Inputs/id.res b/test/tools/llvm-cvtres/Inputs/id.res new file mode 100644 index 0000000000000000000000000000000000000000..1d58013a91cc3c671301cef10a5696dcea965369 GIT binary patch literal 100 wcmZQzU|>)H;{X347|28cDnOnB5dZ(r2E@!@IRgPs7BB+`!NickkclJ*0D{5^h5!Hn literal 0 HcmV?d00001 diff --git a/test/tools/llvm-cvtres/Inputs/name.rc b/test/tools/llvm-cvtres/Inputs/name.rc new file mode 100644 index 00000000000..cb9ace74312 --- /dev/null +++ b/test/tools/llvm-cvtres/Inputs/name.rc @@ -0,0 +1 @@ +namebar typefoo { "data" } diff --git a/test/tools/llvm-cvtres/Inputs/name.res b/test/tools/llvm-cvtres/Inputs/name.res new file mode 100644 index 0000000000000000000000000000000000000000..d6bb2cb2df0e89bd39f112fa774e6d9f84538562 GIT binary patch literal 92 zcmZQzU|>)H;{X347|28cEI^(G5Qi{CG6XQVGPp7L10e&0AA=)Y{; L%E2Hdu_O@y0}={M literal 0 HcmV?d00001 diff --git a/test/tools/llvm-cvtres/duplicate.test b/test/tools/llvm-cvtres/duplicate.test new file mode 100644 index 00000000000..e8c4b8e95cd --- /dev/null +++ b/test/tools/llvm-cvtres/duplicate.test @@ -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