[09/10] [LLD] [COFF] Support merging resource object files

[mstorsjo]

Summary

Extend WindowsResourceParser to support using a ResourceSectionRef for loading resources from an object file.

@thakis - do you want to keep the error on multiple resource object files if not in mingw mode, or just do the merging if needed? This patch so far keeps that error unless the mingw flag is set.

If there only is one resource object file and no .res resources, don't mangle the .rsrc section, but just link it in without inspecting it. This allows users to produce any .rsrc section (outside of what the parser supports), just like before. (I don't have a specific need for this, but it reduces the risk of this new feature.)

Only parse the .rsrc section and recreate the resource tree if needed (more than one resource object file, or one resource object file and one or more .res files).

Separate out the .rsrc section chunks in InputFiles.cpp, and only include them in the list of section chunks to link if we've determined that there only was one single resource object. (We need to keep the rest of those object files, as they can legitimately contain other sections as well, in addition to .rsrc section chunks.)

[mstorsjo]

Factorized error/warning reporting of duplicates between resource files and object files.

[mstorsjo]

Changed new test object files to yaml form, added another test of a mixed object file with both code and resources, merged with another resource object.

[thakis]

I'd vote for not running complicated code unless we have to, so I'd keep this MinGW only.

Do you really need this feature in MinGW mode? IIRC you said this was for some "default res file" feature in gcc which adds a default res which is only supposed to be used if there's no user res file -- which doesn't require any merging.

[mstorsjo]

In D66824#1651587, @thakis wrote:

I'd vote for not running complicated code unless we have to, so I'd keep this MinGW only.

TBH it's not all that complicated, but in any case, none of that code should run in the cases that are currently supported anyway. It's just choosing between if you want to keep the error, to keep matching link.exe, or be more tolerant and support multiple resource objects even for non-MinGW setups.

Do you really need this feature in MinGW mode? IIRC you said this was for some "default res file" feature in gcc which adds a default res which is only supposed to be used if there's no user res file -- which doesn't require any merging.

The user resource object file could be other resources (icons and dialogs and whatnot), where the manifest is supposed to be merged with the rest. Only if the user resources happens to have a manifest should the implicit default one be dropped.

[mstorsjo]

lld/COFF/Driver.cpp

1012

@thakis - The question is only about whether to keep this explicit error here, or remove the whole condition.

[thakis]

In D66824#1651630, @mstorsjo wrote:

In D66824#1651587, @thakis wrote:

I'd vote for not running complicated code unless we have to, so I'd keep this MinGW only.

TBH it's not all that complicated, but in any case, none of that code should run in the cases that are currently supported anyway. It's just choosing between if you want to keep the error, to keep matching link.exe, or be more tolerant and support multiple resource objects even for non-MinGW setups.

To be honest, I don't have a strong opinion. I can see arguments for both behaviors and I'm not sure which of them are better, assuming we have to have this resource merging code in lld anyways. If there's a way to prevent having to have this merging code and making the error behavior work for mingw, I'd prefer that :)

Do you really need this feature in MinGW mode? IIRC you said this was for some "default res file" feature in gcc which adds a default res which is only supposed to be used if there's no user res file -- which doesn't require any merging.

The user resource object file could be other resources (icons and dialogs and whatnot), where the manifest is supposed to be merged with the rest. Only if the user resources happens to have a manifest should the implicit default one be dropped.

Wouldn't users pass .res files to the linker? If not, why not?

[mstorsjo]

In D66824#1651639, @thakis wrote:

Do you really need this feature in MinGW mode? IIRC you said this was for some "default res file" feature in gcc which adds a default res which is only supposed to be used if there's no user res file -- which doesn't require any merging.

The user resource object file could be other resources (icons and dialogs and whatnot), where the manifest is supposed to be merged with the rest. Only if the user resources happens to have a manifest should the implicit default one be dropped.

Wouldn't users pass .res files to the linker? If not, why not?

GNU ld doesn't support .res files at all, it only supports resource object files, so that's how it's always been done in MinGW project builds. (But these days, GNU ld _does_ have this whole resource object unpacking and merging logic, but still no support for plain .res files.)

[thakis]

lgtm, with a suggestion for an (I think?) missing test:

lld/COFF/Driver.cpp

1012

Ack. I'd keep it, but I don't feel strongly.

lld/test/COFF/combined-resources.test

32

You could add a test where there's a dupe resource spread over two .res / res-.obj files.

[mstorsjo]

lld/COFF/Driver.cpp

1012

Ok, I can keep it. It allows multiple-resource-objs.test to keep testing that we don't leave the output file behind as well :-)

lld/test/COFF/combined-resources.test

32

Oh, good point, I'll add such a test as well, before committing it.

[peter.smith]

The test mixed-resource-obj.yaml is failing on our ARM/AArch64 buildbot http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/2131 as it doesn't have an X86 target. I think the test is missing a REQUIRES: x86

[mstorsjo]

In D66824#1652544, @peter.smith wrote:

The test mixed-resource-obj.yaml is failing on our ARM/AArch64 buildbot http://lab.llvm.org:8011/builders/clang-cmake-armv8-lld/builds/2131 as it doesn't have an X86 target. I think the test is missing a REQUIRES: x86

Yes, this seems to be the case. Sorry for the breakage, this should be fixed now.

lld/COFF/Driver.h

private:
   StringRef doFindLibMinGW(StringRef filename);
 
   // Parses LIB environment which contains a list of search paths.
   void addLibSearchPaths();
 
   // Library search path. The first element is always "" (current directory).
   std::vector<StringRef> searchPaths;
 
+  // Convert resource files and potentially merge input resource object
+  // trees into one resource tree.
+  void convertResources();
+
   void maybeExportMinGWSymbols(const llvm::opt::InputArgList &args);
 
   // We don't want to add the same file more than once.
   // Files are uniquified by their filesystem and file number.
   std::set<llvm::sys::fs::UniqueID> visitedFiles;
 
   std::set<std::string> visitedLibs;
 

 
 // Parses a string in the form of "key=value" and check
 // if value matches previous values for the key.
 // This feature used in the directive section to reject
 // incompatible objects.
 void checkFailIfMismatch(StringRef arg, InputFile *source);
 
 // Convert Windows resource files (.res files) to a .obj file.
-MemoryBufferRef convertResToCOFF(ArrayRef<MemoryBufferRef> mbs);
+MemoryBufferRef convertResToCOFF(ArrayRef<MemoryBufferRef> mbs,
+                                 ArrayRef<ObjFile *> objs);
 
 void runMSVCLinker(std::string rsp, ArrayRef<StringRef> objects);
 
 // Create enum with OPT_xxx values for each option in Options.td
 enum {
   OPT_INVALID = 0,
 #define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11, _12) OPT_##ID,
 #include "Options.inc"
 #undef OPTION
 };
 
 } // namespace coff
 } // namespace lld
 
 #endif

lld/COFF/Driver.cpp

while (cursor < altPath.size()) {
     }
 
     cursor = secondMark + 1;
   }
 
   config->pdbAltPath = buf;
 }
 
-/// Check that at most one resource obj file was used.
+/// Convert resource files and potentially merge input resource object
+/// trees into one resource tree.
 /// Call after ObjFile::Instances is complete.
-static void diagnoseMultipleResourceObjFiles() {
-  // The .rsrc$01 section in a resource obj file contains a tree description
-  // of resources.  Merging multiple resource obj files would require merging
-  // the trees instead of using usual linker section merging semantics.
-  // Since link.exe disallows linking more than one resource obj file with
-  // LNK4078, mirror that.  The normal use of resource files is to give the
-  // linker many .res files, which are then converted to a single resource obj
-  // file internally, so this is not a big restriction in practice.
-  ObjFile *resourceObjFile = nullptr;
-  for (ObjFile *f : ObjFile::instances) {
-    if (!f->isResourceObjFile)
-      continue;
+void LinkerDriver::convertResources() {
+  std::vector<ObjFile *> resourceObjFiles;
 
-    if (!resourceObjFile) {
-      resourceObjFile = f;
-      continue;
-    }
+  for (ObjFile *f : ObjFile::instances) {
+    if (f->isResourceObjFile())
+      resourceObjFiles.push_back(f);
 
-    error(toString(f) +
+  if (!config->mingw &&
+      (resourceObjFiles.size() > 1 ||
+       (resourceObjFiles.size() == 1 && !resources.empty()))) {
+    error((!resources.empty() ? "internal .obj file created from .res files"
+                              : toString(resourceObjFiles[1])) +
           ": more than one resource obj file not allowed, already got " +
-          toString(resourceObjFile));
+          toString(resourceObjFiles.front()));
+    return;
   }
+
+  if (resources.empty() && resourceObjFiles.size() <= 1) {
+    // No resources to convert, and max one resource object file in
+    // the input. Keep that preconverted resource section as is.
+    for (ObjFile *f : resourceObjFiles)
+      f->includeResourceChunks();
+    return;
+  }
+  ObjFile *f = make<ObjFile>(convertResToCOFF(resources, resourceObjFiles));
+  symtab->addFile(f);
+  f->includeResourceChunks();
 }
 
 // In MinGW, if no symbols are chosen to be exported, then all symbols are
 // automatically exported by default. This behavior can be forced by the
 // -export-all-symbols option, so that it happens even when exports are
 // explicitly specified. The automatic behavior can be disabled using the
 // -exclude-all-symbols option, so that lld-link behaves like link.exe rather
 // than MinGW in the case that nothing is explicitly exported.
void LinkerDriver::link(ArrayRef<const char *> argsArr) {
   if (config->machine == I386 &&
       args.hasFlag(OPT_safeseh, OPT_safeseh_no, !config->mingw))
     config->safeSEH = true;
 
   // Handle /functionpadmin
   for (auto *arg : args.filtered(OPT_functionpadmin, OPT_functionpadmin_opt))
     parseFunctionPadMin(arg, config->machine);
 
-  // Input files can be Windows resource files (.res files). We use
-  // WindowsResource to convert resource files to a regular COFF file,
-  // then link the resulting file normally.
-  if (!resources.empty())
-    symtab->addFile(make<ObjFile>(convertResToCOFF(resources)));
-
   if (tar)
     tar->append("response.txt",
                 createResponseFile(args, filePaths,
                                    ArrayRef<StringRef>(searchPaths).slice(1)));
 
   // Handle /largeaddressaware
   config->largeAddressAware = args.hasFlag(
       OPT_largeaddressaware, OPT_largeaddressaware_no, config->is64());
void LinkerDriver::link(ArrayRef<const char *> argsArr) {
   if (auto *arg = args.getLastArg(OPT_order))
     parseOrderFile(arg->getValue());
 
   // Identify unreferenced COMDAT sections.
   if (config->doGC)
     markLive(symtab->getChunks());
 
   // Needs to happen after the last call to addFile().
-  diagnoseMultipleResourceObjFiles();
+  convertResources();
 
   // Identify identical COMDAT sections to merge them.
   if (config->doICF) {
     findKeepUniqueSections();
     doICF(symtab->getChunks());
   }
 
   // Write the result.

lld/COFF/DriverUtils.cpp

fatal("/failifmismatch: mismatch detected for '" + k + "':\n>>> " +
           existingStr + " has value " + existing.first + "\n>>> " + sourceStr +
           " has value " + v);
   }
   config->mustMatch[k] = {v, source};
 }
 
 // Convert Windows resource files (.res files) to a .obj file.
 // Does what cvtres.exe does, but in-process and cross-platform.
-MemoryBufferRef convertResToCOFF(ArrayRef<MemoryBufferRef> mbs) {
+MemoryBufferRef convertResToCOFF(ArrayRef<MemoryBufferRef> mbs,
+                                 ArrayRef<ObjFile *> objs) {
   object::WindowsResourceParser parser;
 
+  std::vector<std::string> duplicates;
   for (MemoryBufferRef mb : mbs) {
     std::unique_ptr<object::Binary> bin = check(object::createBinary(mb));
     object::WindowsResource *rf = dyn_cast<object::WindowsResource>(bin.get());
     if (!rf)
       fatal("cannot compile non-resource file as resource");
 
-    std::vector<std::string> duplicates;
     if (auto ec = parser.parse(rf, duplicates))
       fatal(toString(std::move(ec)));
+  }
+
+  // Note: This processes all .res files before all objs. Ideally they'd be
+  // handled in the same order they were linked (to keep the right one, if
+  // there are duplicates that are tolerated due to forceMultipleRes).
+  for (ObjFile *f : objs) {
+    object::ResourceSectionRef rsf;
+    if (auto ec = rsf.load(f->getCOFFObj()))
+      fatal(toString(f) + ": " + toString(std::move(ec)));
+
+    if (auto ec = parser.parse(rsf, f->getName(), 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 =
       llvm::object::writeWindowsResourceCOFF(config->machine, parser,
                                              config->timestamp);
   if (!e)
     fatal("failed to write .res to COFF: " + toString(e.takeError()));
 
   MemoryBufferRef mbref = **e;

lld/COFF/InputFiles.h

public:
 
   // Add a symbol for a range extension thunk. Return the new symbol table
   // index. This index can be used to modify a relocation.
   uint32_t addRangeThunkSymbol(Symbol *thunk) {
     symbols.push_back(thunk);
     return symbols.size() - 1;
   }
 
+  void includeResourceChunks();
+
+  bool isResourceObjFile() const { return !resourceChunks.empty(); }
+
   static std::vector<ObjFile *> instances;
 
   // Flags in the absolute @feat.00 symbol if it is present. These usually
   // indicate if an object was compiled with certain security features enabled
   // like stack guard, safeseh, /guard:cf, or other things.
   uint32_t feat00Flags = 0;
 
   // True if this object file is compatible with SEH.  COFF-specific and
public:
 
   const coff_section *addrsigSec = nullptr;
 
   // When using Microsoft precompiled headers, this is the PCH's key.
   // The same key is used by both the precompiled object, and objects using the
   // precompiled object. Any difference indicates out-of-date objects.
   llvm::Optional<uint32_t> pchSignature;
 
-  // Whether this is an object file created from .res files.
-  bool isResourceObjFile = false;
-
   // Whether this file was compiled with /hotpatch.
   bool hotPatchable = false;
 
   // Whether the object was already merged into the final PDB.
   bool mergedIntoPDB = false;
 
   // If the OBJ has a .debug$T stream, this tells how it will be handled.
   TpiSource *debugTypesObj = nullptr;
private:
   Symbol *createUndefined(COFFSymbolRef sym);
 
   std::unique_ptr<COFFObjectFile> coffObj;
 
   // List of all chunks defined by this file. This includes both section
   // chunks and non-section chunks for common symbols.
   std::vector<Chunk *> chunks;
 
+  std::vector<SectionChunk *> resourceChunks;
+
   // CodeView debug info sections.
   std::vector<SectionChunk *> debugChunks;
 
   // Chunks containing symbol table indices of exception handlers. Only used for
   // 32-bit x86.
   std::vector<SectionChunk *> sXDataChunks;
 
   // Chunks containing symbol table indices of address taken symbols and longjmp

lld/COFF/InputFiles.cpp

if (!config->debug && name.startswith(".debug_"))
     return nullptr;
 
   if (sec->Characteristics & llvm::COFF::IMAGE_SCN_LNK_REMOVE)
     return nullptr;
   auto *c = make<SectionChunk>(this, sec);
   if (def)
     c->checksum = def->CheckSum;
 
-  // link.exe uses the presence of .rsrc$01 for LNK4078, so match that.
-  if (name == ".rsrc$01")
-    isResourceObjFile = true;
-
   // CodeView sections are stored to a different vector because they are not
   // linked in the regular manner.
   if (c->isCodeView())
     debugChunks.push_back(c);
   else if (name == ".gfids$y")
     guardFidChunks.push_back(c);
   else if (name == ".gljmp$y")
     guardLJmpChunks.push_back(c);
   else if (name == ".sxdata")
     sXDataChunks.push_back(c);
   else if (config->tailMerge && sec->NumberOfRelocations == 0 &&
            name == ".rdata" && leaderName.startswith("??_C@"))
     // COFF sections that look like string literal sections (i.e. no
     // relocations, in .rdata, leader symbol name matches the MSVC name mangling
     // for string literals) are subject to string tail merging.
     MergeChunk::addSection(c);
+  else if (name == ".rsrc" || name.startswith(".rsrc$"))
+    resourceChunks.push_back(c);
   else
     chunks.push_back(c);
 
   return c;
 }
 
+void ObjFile::includeResourceChunks() {
+  chunks.insert(chunks.end(), resourceChunks.begin(), resourceChunks.end());
+}
+
 void ObjFile::readAssociativeDefinition(
     COFFSymbolRef sym, const coff_aux_section_definition *def) {
   readAssociativeDefinition(sym, def, def->getNumber(sym.isBigObj()));
 }
 
 void ObjFile::readAssociativeDefinition(COFFSymbolRef sym,
                                         const coff_aux_section_definition *def,
                                         uint32_t parentIndex) {

lld/test/COFF/combined-resources.test

 // Check that lld properly handles merging multiple .res files.
 // The inputs were generated with the following commands, using the original
 // Windows rc.exe
 // > rc /fo combined-resources.res /nologo combined-resources.rc
 // > rc /fo combined-resources-2.res /nologo combined-resources-2.rc
 
+// The object files were generated with GNU windres and MS cvtres.exe,
+// > x86_64-w64-mingw32-windres combined-resources.res combined-resources.o
+// > cvtres -machine:x64 -out:combined-resources-2.o combined-resources.res
+
 # RUN: yaml2obj < %p/Inputs/ret42.yaml > %t.obj
 # RUN: lld-link /out:%t.exe /entry:main %t.obj %p/Inputs/resource.res \
 # RUN:   %p/Inputs/combined-resources.res %p/Inputs/combined-resources-2.res
 
 # RUN: llvm-readobj --coff-resources --file-headers --section-data %t.exe | \
 # RUN:   FileCheck %s
 
+# RUN: lld-link /lldmingw /out:%t-resobj.exe /entry:main %t.obj %p/Inputs/resource.res \
+# RUN:   %p/Inputs/combined-resources.o %p/Inputs/combined-resources-2.o
+
+// As input resources are traversed in a slightly different order, the
+// RVAs of data blobs will end up slightly different, even if they are
+// equivalent. Filter out such addresses from llvm-readobj's output,
+// and compare the rest to make sure it is equivalent.
+
+# RUN: llvm-readobj --coff-resources %t.exe | sed -E 's/(RVA|Address|File): .*//' > %t-orig.txt
+# RUN: llvm-readobj --coff-resources %t-resobj.exe | sed -E 's/(RVA|Address|File): .*//' > %t-resobj.txt
+# RUN: cmp %t-orig.txt %t-resobj.txt
+
 CHECK:      ResourceTableRVA: 0x2000
 CHECK-NEXT: ResourceTableSize: 0xC20
 CHECK-DAG:  Resources [
 CHECK-NEXT:   Total Number of Resources: 13
 CHECK-DAG:  .rsrc Data (
 CHECK-NEXT: 0000: 00000000 00000000 00000000 01000600  |................|
 CHECK-NEXT: 0010: 38030080 48000080 02000000 60000080  |8...H.......`...|
 CHECK-NEXT: 0020: 04000000 80000080 05000000 A0000080  |................|
 CHECK-NEXT: 0030: 06000000 B8000080 09000000 D0000080  |................|
 CHECK-NEXT: 0040: 0A000000 F0000080 00000000 00000000  |................|
 CHECK-NEXT: 0050: 00000000 01000000 50030080 08010080  |........P.......|

llvm/include/llvm/Object/WindowsResource.h

 //===---------------------------------------------------------------------===//
 
 #ifndef LLVM_INCLUDE_LLVM_OBJECT_RESFILE_H
 #define LLVM_INCLUDE_LLVM_OBJECT_RESFILE_H
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/Object/Binary.h"
+#include "llvm/Object/COFF.h"
 #include "llvm/Object/Error.h"
 #include "llvm/Support/BinaryByteStream.h"
 #include "llvm/Support/BinaryStreamReader.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 
 #include <map>
 
 namespace llvm {
 
 class raw_ostream;
 class ScopedPrinter;
 
 namespace object {
 
 class WindowsResource;
+class ResourceSectionRef;
 
 const size_t WIN_RES_MAGIC_SIZE = 16;
 const size_t WIN_RES_NULL_ENTRY_SIZE = 16;
 const uint32_t WIN_RES_HEADER_ALIGNMENT = 4;
 const uint32_t WIN_RES_DATA_ALIGNMENT = 4;
 const uint16_t WIN_RES_PURE_MOVEABLE = 0x0030;
 
 struct WinResHeaderPrefix {
private:
   BinaryByteStream BBS;
 };
 
 class WindowsResourceParser {
 public:
   class TreeNode;
   WindowsResourceParser();
   Error parse(WindowsResource *WR, std::vector<std::string> &Duplicates);
+  Error parse(ResourceSectionRef &RSR, StringRef Filename,
+              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; }
   const ArrayRef<std::vector<UTF16>> getStringTable() const {
     return StringTable;
   }
 
   class TreeNode {
private:
     uint16_t MinorVersion = 0;
     uint32_t Characteristics = 0;
 
     // The .res file that defined this TreeNode, for diagnostics.
     // Index into InputFilenames.
     uint32_t Origin;
   };
 
+  struct StringOrID {
+    bool IsString;
+    ArrayRef<UTF16> String;
+    uint32_t ID;
+
+    StringOrID(uint32_t ID) : IsString(false), ID(ID) {}
+    StringOrID(ArrayRef<UTF16> String) : IsString(true), String(String) {}
+  };
+
 private:
+  Error addChildren(TreeNode &Node, ResourceSectionRef &RSR,
+                    const coff_resource_dir_table &Table, uint32_t Origin,
+                    std::vector<StringOrID> &Context,
+                    std::vector<std::string> &Duplicates);
+
   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>>

llvm/lib/Object/WindowsResource.cpp

 
 namespace llvm {
 namespace object {
 
 #define RETURN_IF_ERROR(X)                                                     \
   if (auto EC = X)                                                             \
     return EC;
 
+#define UNWRAP_REF_OR_RETURN(Name, Expr)                                       \
+  auto Name##OrErr = Expr;                                                     \
+  if (!Name##OrErr)                                                            \
+    return Name##OrErr.takeError();                                            \
+  const auto &Name = *Name##OrErr;
+
+#define UNWRAP_OR_RETURN(Name, Expr)                                           \
+  auto Name##OrErr = Expr;                                                     \
+  if (!Name##OrErr)                                                            \
+    return Name##OrErr.takeError();                                            \
+  auto Name = *Name##OrErr;
+
 const uint32_t MIN_HEADER_SIZE = 7 * sizeof(uint32_t) + 2 * sizeof(uint16_t);
 
 // COFF files seem to be inconsistent with alignment between sections, just use
 // 8-byte because it makes everyone happy.
 const uint32_t SECTION_ALIGNMENT = sizeof(uint64_t);
 
 WindowsResource::WindowsResource(MemoryBufferRef Source)
     : Binary(Binary::ID_WinRes, Source) {
static std::string makeDuplicateResourceError(
   }
 
   OS << "/language " << Entry.getLanguage() << ", in " << File1 << " and in "
      << File2;
 
   return OS.str();
 }
 
+static void printStringOrID(const WindowsResourceParser::StringOrID &S,
+                            raw_string_ostream &OS, bool IsType, bool IsID) {
+  if (S.IsString) {
+    std::string UTF8;
+    if (!convertUTF16LEToUTF8String(S.String, UTF8))
+      UTF8 = "(failed conversion from UTF16)";
+    OS << '\"' << UTF8 << '\"';
+  } else if (IsType)
+    printResourceTypeName(S.ID, OS);
+  else if (IsID)
+    OS << "ID " << S.ID;
+  else
+    OS << S.ID;
+}
+
+static std::string makeDuplicateResourceError(
+    const std::vector<WindowsResourceParser::StringOrID> &Context,
+    StringRef File1, StringRef File2) {
+  std::string Ret;
+  raw_string_ostream OS(Ret);
+
+  OS << "duplicate resource:";
+
+  if (Context.size() >= 1) {
+    OS << " type ";
+    printStringOrID(Context[0], OS, /* IsType */ true, /* IsID */ true);
+  }
+
+  if (Context.size() >= 2) {
+    OS << "/name ";
+    printStringOrID(Context[1], OS, /* IsType */ false, /* IsID */ true);
+  }
+
+  if (Context.size() >= 3) {
+    OS << "/language ";
+    printStringOrID(Context[2], OS, /* IsType */ false, /* IsID */ false);
+  }
+  OS << ", in " << File1 << " and in " << File2;
+
+  return OS.str();
+}
+
 Error WindowsResourceParser::parse(WindowsResource *WR,
                                    std::vector<std::string> &Duplicates) {
   auto EntryOrErr = WR->getHeadEntry();
   if (!EntryOrErr) {
     auto E = EntryOrErr.takeError();
     if (E.isA<EmptyResError>()) {
       // Check if the .res file contains no entries.  In this case we don't have
       // to throw an error but can rather just return without parsing anything.
while (!End) {
     }
 
     RETURN_IF_ERROR(Entry.moveNext(End));
   }
 
   return Error::success();
 }
 
+Error WindowsResourceParser::parse(ResourceSectionRef &RSR, StringRef Filename,
+                                   std::vector<std::string> &Duplicates) {
+  UNWRAP_REF_OR_RETURN(BaseTable, RSR.getBaseTable());
+  uint32_t Origin = InputFilenames.size();
+  InputFilenames.push_back(Filename);
+  std::vector<StringOrID> Context;
+  return addChildren(Root, RSR, BaseTable, Origin, Context, Duplicates);
+}
+
 void WindowsResourceParser::printTree(raw_ostream &OS) const {
   ScopedPrinter Writer(OS);
   Root.print(Writer, "Resource Tree");
 }
 
 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);
 }
 
+Error WindowsResourceParser::addChildren(TreeNode &Node,
+                                         ResourceSectionRef &RSR,
+                                         const coff_resource_dir_table &Table,
+                                         uint32_t Origin,
+                                         std::vector<StringOrID> &Context,
+                                         std::vector<std::string> &Duplicates) {
+
+  for (int i = 0; i < Table.NumberOfNameEntries + Table.NumberOfIDEntries;
+       i++) {
+    UNWRAP_REF_OR_RETURN(Entry, RSR.getTableEntry(Table, i));
+    TreeNode *Child;
+    if (Entry.Offset.isSubDir()) {
+      if (i < Table.NumberOfNameEntries) {
+        UNWRAP_OR_RETURN(NameString, RSR.getEntryNameString(Entry));
+        Child = &Node.addNameChild(NameString, StringTable);
+        Context.push_back(StringOrID(NameString));
+      } else {
+        Child = &Node.addIDChild(Entry.Identifier.ID);
+        Context.push_back(StringOrID(Entry.Identifier.ID));
+      }
+      UNWRAP_REF_OR_RETURN(NextTable, RSR.getEntrySubDir(Entry));
+      Error E =
+          addChildren(*Child, RSR, NextTable, Origin, Context, Duplicates);
+      if (E)
+        return E;
+      Context.pop_back();
+    } else {
+      if (Table.NumberOfNameEntries > 0)
+        return createStringError(object_error::parse_failed,
+                                 "unexpected string key for data object");
+      UNWRAP_REF_OR_RETURN(DataEntry, RSR.getEntryData(Entry));
+      TreeNode *Child;
+      Context.push_back(StringOrID(Entry.Identifier.ID));
+      bool Added = Node.addDataChild(Entry.Identifier.ID, Table.MajorVersion,
+                                     Table.MinorVersion, Table.Characteristics,
+                                     Origin, Data.size(), Child);
+      if (Added) {
+        UNWRAP_OR_RETURN(Contents, RSR.getContents(DataEntry));
+        Data.push_back(ArrayRef<uint8_t>(
+            reinterpret_cast<const uint8_t *>(Contents.data()),
+            Contents.size()));
+      } else {
+        Duplicates.push_back(makeDuplicateResourceError(
+            Context, InputFilenames[Child->Origin], InputFilenames.back()));
+      }
+      Context.pop_back();
+    }
+  }
+  return Error::success();
+}
+
 WindowsResourceParser::TreeNode::TreeNode(uint32_t StringIndex)
     : StringIndex(StringIndex) {}
 
 WindowsResourceParser::TreeNode::TreeNode(uint16_t MajorVersion,
                                           uint16_t MinorVersion,
                                           uint32_t Characteristics,
                                           uint32_t Origin, uint32_t DataIndex)
     : IsDataNode(true), DataIndex(DataIndex), MajorVersion(MajorVersion),