[llvm] r370435 - [WindowsResource] Remove use of global variables in WindowsResourceParser

Martin Storsjo via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 29 23:56:03 PDT 2019


Author: mstorsjo
Date: Thu Aug 29 23:56:02 2019
New Revision: 370435

URL: http://llvm.org/viewvc/llvm-project?rev=370435&view=rev
Log:
[WindowsResource] Remove use of global variables in WindowsResourceParser

Instead of updating a global variable counter for the next index of
strings and data blobs, pass along a reference to actual data/string
vectors and let the TreeNode insertion methods add their data/strings to
the vectors when a new entry is needed.

Additionally, if the resource tree had duplicates, that were ignored
with -force:multipleres in lld, we no longer store all versions of the
duplicated resource data, now we only keep the one that actually ends
up referenced.

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

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=370435&r1=370434&r2=370435&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/WindowsResource.h (original)
+++ llvm/trunk/include/llvm/Object/WindowsResource.h Thu Aug 29 23:56:02 2019
@@ -181,32 +181,37 @@ public:
   private:
     friend class WindowsResourceParser;
 
-    static uint32_t StringCount;
-    static uint32_t DataCount;
-
-    static std::unique_ptr<TreeNode> createStringNode();
+    // Index is the StringTable vector index for this node's name.
+    static std::unique_ptr<TreeNode> createStringNode(uint32_t Index);
     static std::unique_ptr<TreeNode> createIDNode();
+    // DataIndex is the Data vector index that the data node points at.
     static std::unique_ptr<TreeNode> createDataNode(uint16_t MajorVersion,
                                                     uint16_t MinorVersion,
                                                     uint32_t Characteristics,
-                                                    uint32_t Origin);
+                                                    uint32_t Origin,
+                                                    uint32_t DataIndex);
 
-    explicit TreeNode(bool IsStringNode);
+    explicit TreeNode(uint32_t StringIndex);
     TreeNode(uint16_t MajorVersion, uint16_t MinorVersion,
-             uint32_t Characteristics, uint32_t Origin);
+             uint32_t Characteristics, uint32_t Origin, uint32_t DataIndex);
 
     bool addEntry(const ResourceEntryRef &Entry, uint32_t Origin,
-                  bool &IsNewTypeString, bool &IsNewNameString,
+                  std::vector<std::vector<uint8_t>> &Data,
+                  std::vector<std::vector<UTF16>> &StringTable,
                   TreeNode *&Result);
-    TreeNode &addTypeNode(const ResourceEntryRef &Entry, bool &IsNewTypeString);
-    TreeNode &addNameNode(const ResourceEntryRef &Entry, bool &IsNewNameString);
+    TreeNode &addTypeNode(const ResourceEntryRef &Entry,
+                          std::vector<std::vector<UTF16>> &StringTable);
+    TreeNode &addNameNode(const ResourceEntryRef &Entry,
+                          std::vector<std::vector<UTF16>> &StringTable);
     bool addLanguageNode(const ResourceEntryRef &Entry, uint32_t Origin,
+                         std::vector<std::vector<uint8_t>> &Data,
                          TreeNode *&Result);
     bool addDataChild(uint32_t ID, uint16_t MajorVersion, uint16_t MinorVersion,
                       uint32_t Characteristics, uint32_t Origin,
-                      TreeNode *&Result);
+                      uint32_t DataIndex, TreeNode *&Result);
     TreeNode &addIDChild(uint32_t ID);
-    TreeNode &addNameChild(ArrayRef<UTF16> NameRef, bool &IsNewString);
+    TreeNode &addNameChild(ArrayRef<UTF16> NameRef,
+                           std::vector<std::vector<UTF16>> &StringTable);
 
     bool IsDataNode = false;
     uint32_t StringIndex;

Modified: llvm/trunk/lib/Object/WindowsResource.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/WindowsResource.cpp?rev=370435&r1=370434&r2=370435&view=diff
==============================================================================
--- llvm/trunk/lib/Object/WindowsResource.cpp (original)
+++ llvm/trunk/lib/Object/WindowsResource.cpp Thu Aug 29 23:56:02 2019
@@ -36,9 +36,6 @@ const uint32_t MIN_HEADER_SIZE = 7 * siz
 // 8-byte because it makes everyone happy.
 const uint32_t SECTION_ALIGNMENT = sizeof(uint64_t);
 
-uint32_t WindowsResourceParser::TreeNode::StringCount = 0;
-uint32_t WindowsResourceParser::TreeNode::DataCount = 0;
-
 WindowsResource::WindowsResource(MemoryBufferRef Source)
     : Binary(Binary::ID_WinRes, Source) {
   size_t LeadingSize = WIN_RES_MAGIC_SIZE + WIN_RES_NULL_ENTRY_SIZE;
@@ -223,25 +220,14 @@ Error WindowsResourceParser::parse(Windo
   InputFilenames.push_back(WR->getFileName());
   bool End = false;
   while (!End) {
-    Data.push_back(Entry.getData());
-
-    bool IsNewTypeString = false;
-    bool IsNewNameString = false;
 
     TreeNode *Node;
-    bool IsNewNode =
-        Root.addEntry(Entry, Origin, IsNewTypeString, IsNewNameString, Node);
+    bool IsNewNode = Root.addEntry(Entry, Origin, Data, StringTable, Node);
     if (!IsNewNode) {
       Duplicates.push_back(makeDuplicateResourceError(
           Entry, InputFilenames[Node->Origin], WR->getFileName()));
     }
 
-    if (IsNewTypeString)
-      StringTable.push_back(Entry.getTypeString());
-
-    if (IsNewNameString)
-      StringTable.push_back(Entry.getNameString());
-
     RETURN_IF_ERROR(Entry.moveNext(End));
   }
 
@@ -253,79 +239,81 @@ void WindowsResourceParser::printTree(ra
   Root.print(Writer, "Resource Tree");
 }
 
-bool WindowsResourceParser::TreeNode::addEntry(const ResourceEntryRef &Entry,
-                                               uint32_t Origin,
-                                               bool &IsNewTypeString,
-                                               bool &IsNewNameString,
-                                               TreeNode *&Result) {
-  TreeNode &TypeNode = addTypeNode(Entry, IsNewTypeString);
-  TreeNode &NameNode = TypeNode.addNameNode(Entry, IsNewNameString);
-  return NameNode.addLanguageNode(Entry, Origin, Result);
+bool WindowsResourceParser::TreeNode::addEntry(
+    const ResourceEntryRef &Entry, uint32_t Origin,
+    std::vector<std::vector<uint8_t>> &Data,
+    std::vector<std::vector<UTF16>> &StringTable, TreeNode *&Result) {
+  TreeNode &TypeNode = addTypeNode(Entry, StringTable);
+  TreeNode &NameNode = TypeNode.addNameNode(Entry, StringTable);
+  return NameNode.addLanguageNode(Entry, Origin, Data, Result);
 }
 
-WindowsResourceParser::TreeNode::TreeNode(bool IsStringNode) {
-  if (IsStringNode)
-    StringIndex = StringCount++;
-}
+WindowsResourceParser::TreeNode::TreeNode(uint32_t StringIndex)
+    : StringIndex(StringIndex) {}
 
 WindowsResourceParser::TreeNode::TreeNode(uint16_t MajorVersion,
                                           uint16_t MinorVersion,
                                           uint32_t Characteristics,
-                                          uint32_t Origin)
-    : IsDataNode(true), MajorVersion(MajorVersion), MinorVersion(MinorVersion),
-      Characteristics(Characteristics), Origin(Origin) {
-  DataIndex = DataCount++;
-}
+                                          uint32_t Origin, uint32_t DataIndex)
+    : IsDataNode(true), DataIndex(DataIndex), MajorVersion(MajorVersion),
+      MinorVersion(MinorVersion), Characteristics(Characteristics),
+      Origin(Origin) {}
 
 std::unique_ptr<WindowsResourceParser::TreeNode>
-WindowsResourceParser::TreeNode::createStringNode() {
-  return std::unique_ptr<TreeNode>(new TreeNode(true));
+WindowsResourceParser::TreeNode::createStringNode(uint32_t Index) {
+  return std::unique_ptr<TreeNode>(new TreeNode(Index));
 }
 
 std::unique_ptr<WindowsResourceParser::TreeNode>
 WindowsResourceParser::TreeNode::createIDNode() {
-  return std::unique_ptr<TreeNode>(new TreeNode(false));
+  return std::unique_ptr<TreeNode>(new TreeNode(0));
 }
 
 std::unique_ptr<WindowsResourceParser::TreeNode>
 WindowsResourceParser::TreeNode::createDataNode(uint16_t MajorVersion,
                                                 uint16_t MinorVersion,
                                                 uint32_t Characteristics,
-                                                uint32_t Origin) {
-  return std::unique_ptr<TreeNode>(
-      new TreeNode(MajorVersion, MinorVersion, Characteristics, Origin));
+                                                uint32_t Origin,
+                                                uint32_t DataIndex) {
+  return std::unique_ptr<TreeNode>(new TreeNode(
+      MajorVersion, MinorVersion, Characteristics, Origin, DataIndex));
 }
 
-WindowsResourceParser::TreeNode &
-WindowsResourceParser::TreeNode::addTypeNode(const ResourceEntryRef &Entry,
-                                             bool &IsNewTypeString) {
+WindowsResourceParser::TreeNode &WindowsResourceParser::TreeNode::addTypeNode(
+    const ResourceEntryRef &Entry,
+    std::vector<std::vector<UTF16>> &StringTable) {
   if (Entry.checkTypeString())
-    return addNameChild(Entry.getTypeString(), IsNewTypeString);
+    return addNameChild(Entry.getTypeString(), StringTable);
   else
     return addIDChild(Entry.getTypeID());
 }
 
-WindowsResourceParser::TreeNode &
-WindowsResourceParser::TreeNode::addNameNode(const ResourceEntryRef &Entry,
-                                             bool &IsNewNameString) {
+WindowsResourceParser::TreeNode &WindowsResourceParser::TreeNode::addNameNode(
+    const ResourceEntryRef &Entry,
+    std::vector<std::vector<UTF16>> &StringTable) {
   if (Entry.checkNameString())
-    return addNameChild(Entry.getNameString(), IsNewNameString);
+    return addNameChild(Entry.getNameString(), StringTable);
   else
     return addIDChild(Entry.getNameID());
 }
 
 bool WindowsResourceParser::TreeNode::addLanguageNode(
-    const ResourceEntryRef &Entry, uint32_t Origin, TreeNode *&Result) {
-  return addDataChild(Entry.getLanguage(), Entry.getMajorVersion(),
-                      Entry.getMinorVersion(), Entry.getCharacteristics(),
-                      Origin, Result);
+    const ResourceEntryRef &Entry, uint32_t Origin,
+    std::vector<std::vector<uint8_t>> &Data, TreeNode *&Result) {
+  bool Added = addDataChild(Entry.getLanguage(), Entry.getMajorVersion(),
+                            Entry.getMinorVersion(), Entry.getCharacteristics(),
+                            Origin, Data.size(), Result);
+  if (Added)
+    Data.push_back(Entry.getData());
+  return Added;
 }
 
 bool WindowsResourceParser::TreeNode::addDataChild(
     uint32_t ID, uint16_t MajorVersion, uint16_t MinorVersion,
-    uint32_t Characteristics, uint32_t Origin, TreeNode *&Result) {
-  auto NewChild =
-      createDataNode(MajorVersion, MinorVersion, Characteristics, Origin);
+    uint32_t Characteristics, uint32_t Origin, uint32_t DataIndex,
+    TreeNode *&Result) {
+  auto NewChild = createDataNode(MajorVersion, MinorVersion, Characteristics,
+                                 Origin, DataIndex);
   auto ElementInserted = IDChildren.emplace(ID, std::move(NewChild));
   Result = ElementInserted.first->second.get();
   return ElementInserted.second;
@@ -343,16 +331,15 @@ WindowsResourceParser::TreeNode &Windows
     return *(Child->second);
 }
 
-WindowsResourceParser::TreeNode &
-WindowsResourceParser::TreeNode::addNameChild(ArrayRef<UTF16> NameRef,
-                                              bool &IsNewString) {
+WindowsResourceParser::TreeNode &WindowsResourceParser::TreeNode::addNameChild(
+    ArrayRef<UTF16> NameRef, std::vector<std::vector<UTF16>> &StringTable) {
   std::string NameString;
   convertUTF16LEToUTF8String(NameRef, NameString);
 
   auto Child = StringChildren.find(NameString);
   if (Child == StringChildren.end()) {
-    auto NewChild = createStringNode();
-    IsNewString = true;
+    auto NewChild = createStringNode(StringTable.size());
+    StringTable.push_back(NameRef);
     WindowsResourceParser::TreeNode &Node = *NewChild;
     StringChildren.emplace(NameString, std::move(NewChild));
     return Node;




More information about the llvm-commits mailing list