Integrate manifest merging library into LLD.

[ecbeckmann]

Summary

Now that the llvm-mt manifest merging libraries are complete, we may use them to merge manifests instead of needing to shell out to mt.exe.

Contingent on D36201.

[ecbeckmann]

Shared return.

[ecbeckmann]

diff to correct revision

[zturner]

lld/COFF/DriverUtils.cpp

316–319

Can you mark this function static?

352

Indentation is off here. Is this clang-formatted?

354

I wonder if all this should be an implementation detail of WindowsManifestMerger itself. Then this entire function would jsut become:

WindowsManifestMerger Merger;
for (StringRef Filename : Config->ManifestInput)
  Merger.mergeFromFile(Filename);
return Merger.getMergedOutput();

and the decision about whether to use libxml or shell out to mt.exe would be part of the WindowsManifestMerger class.

377

If we're not on Windows we should emit a warning that the manifest file was ignored since mt will not be available.

[ruiu]

lld/COFF/DriverUtils.cpp

317

Can you make it more lld-ish?

DefaultXml -> Ret

343–344

I believe raw_string_ostream is unbuffered, so you don't need this.

354

I agree with Zach. lld shouldn't know whether libxml is available or not. So, #if LLVM_LIBXML2_ENABLED in lld doesn't seem right.

[ecbeckmann]

some format and style changes, also do not check if libxml2 is installed but rather just check if WindowsManifestMerger is successful or not.

[ecbeckmann]

lld/COFF/DriverUtils.cpp

352

hmm seems it wasn't

[ecbeckmann]

Diff with the correction revision.

[ecbeckmann]

Correct format.

[ruiu]

lld/COFF/DriverUtils.cpp

347–348

Separate this function into two functions, createManifestXmlLibxml and createManifestXmlMt, and call them sequentially from createManifestXml.

363–365

Please follow the coding style in this file, so that the new code looks like they existed from beginning. Remove {}.

399

Define OutputBuffer here instead of at the binning of this function, as this is the first line you are using that variable.

[ecbeckmann]

Separate manifest merge functions, style fix.

[ecbeckmann]

lld/COFF/DriverUtils.cpp

399

now defined right before use

[ecbeckmann]

Diff with correct revision.

[ruiu]

lld/COFF/DriverUtils.cpp

343–345

It is more natural to return a value rather than assigning to a variable passed by reference.

415

I'm a little confused. Is this safe?

I mean, you are returning a pointer to a buffer owned by an object, which is in turn managed by a unique_ptr. But the unique_ptr is destructed as soon as you return from this function. What am I missing?

[ecbeckmann]

lld/COFF/DriverUtils.cpp

415

Function returns std::string, not a pointer. std::string copy constructor is implicitly called in the return.

[ecbeckmann]

Return mem buff pointer instead of pass by reference.

[zturner]

lld/test/lit.cfg

271

Can you line-break here? This goes past 80. Also, why are we checking for cvtres instead of mt? The comment makes sense, but why not just check for mt directly?

[ecbeckmann]

lld/test/lit.cfg

271

Checking for mt always seems to succeed, there are too many identically named tools I believe. Note that this was the standard check for finding Microsoft utils even before I started making changes, you could search for rc and cvtres but not for mt.

[ecbeckmann]

Instead of returning nullptr when libxml is not installed, it returns an error
which is more descriptive, and also doesn't require code outside the manifest
library to understand the cases for returning error/nullptr.

[ecbeckmann]

Rebase, diff with correction version.

[ecbeckmann]

Rebase and diff properly.

[ecbeckmann]

Pull out a lot of unnecessary changes.

[ecbeckmann]

lld/COFF/DriverUtils.cpp

377

I actually think we should continue to throw an error, because it is never valid to ignore the manifest when producing the executable.

[ecbeckmann]

Formatting change

[ecbeckmann]

Remove more unnecessary files.

[ruiu]

lld/COFF/DriverUtils.cpp

344

In general, we prefer StringRef over std::string & (or const std::string &). Can you replace?

351

Ditto

352

It is nice to explain why we don't want to execute mt command on non-Windows machines, because this is different from what we usually do (which is to try to execute a command and see if execution succeeded.)

355

std::string() -> ""

lld/test/lit.cfg

271

Yeah, mt is a command to operate magnetic tapes, and even though most Unix machines don't have tape drives anymore, it is still distributed and installed as part of GNU commands by default. So checking existence of cvtres makes sense.

[hans]

lld/trunk/COFF/DriverUtils.cpp

432 ↗

(On Diff #112108)

I'm not sure it makes sense to warn here. The user probably doesn't care which way the manifests were merged?

This breaks Chromium's build with LLD on Windows since libxml2 isn't availalbe and we treat warnings as errors. See https://bugs.chromium.org/p/chromium/issues/detail?id=760385

[hans]

lld/trunk/COFF/DriverUtils.cpp

432 ↗

(On Diff #112108)

I take it back, we don't treat it as an error and that's not why the build is broken.

But I'm still not sure the warning is actually helpful.

[orivej]

This broke building LLD with an installed LLVM: https://bugs.llvm.org/show_bug.cgi?id=34608

lld/COFF/CMakeLists.txt

add_lld_library(lldCOFF
   LTO
   LibDriver
   Object
   MC
   MCDisassembler
   Target
   Option
   Support
+  WindowsManifest
 
   LINK_LIBS
   lldCore
   ${LLVM_PTHREAD_LIB}
 
   DEPENDS
   COFFOptionsTableGen
   ${tablegen_deps}
   )

lld/COFF/DriverUtils.cpp

 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/WindowsManifest/WindowsManifestMerger.h"
 #include <memory>
 
 using namespace llvm::COFF;
 using namespace llvm;
 using llvm::cl::ExpandResponseFiles;
 using llvm::cl::TokenizeWindowsCommandLine;
 using llvm::sys::Process;
 
return check(MemoryBuffer::getFile(Path, /*FileSize=*/-1,
                                        /*IsVolatileSize=*/true),
                  "could not open " + Path);
   }
 
   std::string Path;
 };
 }
 
-// Create the default manifest file as a temporary file.
-TemporaryFile createDefaultXml() {
-  // Create a temporary file.
-  TemporaryFile File("defaultxml", "manifest");
-
-  // Open the temporary file for writing.
-  std::error_code EC;
-  raw_fd_ostream OS(File.Path, EC, sys::fs::F_Text);
-  if (EC)
-    fatal(EC, "failed to open " + File.Path);
+std::string createDefaultXml() {
+  std::string DefaultXml;
+  raw_string_ostream OS(DefaultXml);
   
   // Emit the XML. Note that we do *not* verify that the XML attributes are
   // syntactically correct. This is intentional for link.exe compatibility.
   OS << "<?xml version=\"1.0\" standalone=\"yes\"?>\n"
      << "<assembly xmlns=\"urn:schemas-microsoft-com:asm.v1\"\n"
      << "          manifestVersion=\"1.0\">\n";
   if (Config->ManifestUAC) {
     OS << "  <trustInfo>\n"
        << "    <security>\n"
        << "      <requestedPrivileges>\n"
        << "         <requestedExecutionLevel level=" << Config->ManifestLevel
        << " uiAccess=" << Config->ManifestUIAccess << "/>\n"
        << "      </requestedPrivileges>\n"
        << "    </security>\n"
        << "  </trustInfo>\n";
   }
   if (!Config->ManifestDependency.empty()) {
     OS << "  <dependency>\n"
        << "    <dependentAssembly>\n"
        << "      <assemblyIdentity " << Config->ManifestDependency << " />\n"
        << "    </dependentAssembly>\n"
        << "  </dependency>\n";
   }
   OS << "</assembly>\n";
-  OS.close();
-  return File;
-}
-
-static std::string readFile(StringRef Path) {
-  std::unique_ptr<MemoryBuffer> MB =
-      check(MemoryBuffer::getFile(Path), "could not open " + Path);
-  return MB->getBuffer();
+  OS.flush();
+  return DefaultXml;
 }
 
 static std::string createManifestXml() {
-  // Create the default manifest file.
-  TemporaryFile File1 = createDefaultXml();
+  std::string DefaultXml = createDefaultXml();
   if (Config->ManifestInput.empty())
-    return readFile(File1.Path);
+    return DefaultXml;
+
+std::unique_ptr<MemoryBuffer> OutputBuffer;
 
+#if LLVM_LIBXML2_ENABLED
   // If manifest files are supplied by the user using /MANIFESTINPUT
-  // option, we need to merge them with the default manifest.
-  TemporaryFile File2("user", "manifest");
+  // option, we need to merge them with the default manifest. If libxml2
+  // is enabled, we may merge them with LLVM's own library.
+
+  std::unique_ptr<MemoryBuffer> DefaultXmlCopy =
+      MemoryBuffer::getMemBufferCopy(DefaultXml);
+
+  windows_manifest::WindowsManifestMerger Merger;
+  if (auto E = Merger.merge(*DefaultXmlCopy.get())) {
+    fatal(E, "failed to merge default manifest");
+  }
+
+  for (StringRef Filename : Config->ManifestInput) {
+    std::unique_ptr<MemoryBuffer> Manifest =
+        check(MemoryBuffer::getFile(Filename));
+    if (auto E = Merger.merge(*Manifest.get()))
+      fatal(E, "failed to merge " + Filename);
+  }
+
+  OutputBuffer = Merger.getMergedManifest();
+#else
+  // Create the default manifest file as a temporary file.
+  TemporaryFile Default("defaultxml", "manifest");
+  std::error_code EC;
+  raw_fd_ostream OS(Default.Path, EC, sys::fs::F_Text);
+  if (EC)
+    fatal(EC, "failed to open " + Default.Path);
+  OS << DefaultXml;
+  OS.close();
+
+  // Merge user-supplied manifests if they are given.  Since libxml2 is not
+  // enabled, we must shell out to Microsoft's mt.exe tool.
+  TemporaryFile User("user", "manifest");
 
   Executor E("mt.exe");
   E.add("/manifest");
-  E.add(File1.Path);
+  E.add(Default.Path);
   for (StringRef Filename : Config->ManifestInput) {
     E.add("/manifest");
     E.add(Filename);
   }
   E.add("/nologo");
-  E.add("/out:" + StringRef(File2.Path));
+  E.add("/out:" + StringRef(User.Path));
   E.run();
-  return readFile(File2.Path);
+
+  OutputBuffer =
+      check(MemoryBuffer::getFile(User.Path), "could not open " + User.Path);
+#endif
+  return OutputBuffer->getBuffer();
 }
 
 static std::unique_ptr<MemoryBuffer>
 createMemoryBufferForManifestRes(size_t ManifestSize) {
   size_t ResSize = alignTo(
       object::WIN_RES_MAGIC_SIZE + object::WIN_RES_NULL_ENTRY_SIZE +
           sizeof(object::WinResHeaderPrefix) + sizeof(object::WinResIDs) +
           sizeof(object::WinResHeaderSuffix) + ManifestSize,
       object::WIN_RES_DATA_ALIGNMENT);
   return MemoryBuffer::getNewMemBuffer(ResSize);
 }
 
 static void writeResFileHeader(char *&Buf) {
   memcpy(Buf, COFF::WinResMagic, sizeof(COFF::WinResMagic));
   Buf += sizeof(COFF::WinResMagic);
   memset(Buf, 0, object::WIN_RES_NULL_ENTRY_SIZE);
   Buf += object::WIN_RES_NULL_ENTRY_SIZE;
 }
 
 static void writeResEntryHeader(char *&Buf, size_t ManifestSize) {

lld/test/COFF/manifestinput.test

-# REQUIRES: win_mt
+# REQUIRES: manifest_tool
 
 # RUN: yaml2obj %p/Inputs/ret42.yaml > %t.obj
 # RUN: lld-link /out:%t.exe /entry:main \
 # RUN:   /manifest:embed \
 # RUN:   /manifestuac:"level='requireAdministrator'" \
 # RUN:   /manifestinput:%p/Inputs/manifestinput.test %t.obj
 # RUN: llvm-readobj -coff-resources -file-headers %t.exe | FileCheck %s \
 # RUN:   -check-prefix TEST_EMBED

lld/test/lit.cfg

if re.search(r'X86', archs):
     config.available_features.add('x86')
 llvm_config_cmd.wait()
 
 # Set a fake constant version so that we get consitent output.
 config.environment['LLD_VERSION'] = 'LLD 1.0'
 
 # Indirectly check if the mt.exe Microsoft utility exists by searching for
 # cvtres, which always accompanies it.
-if lit.util.which('cvtres', config.environment['PATH']):
-    config.available_features.add('win_mt')
+if (lit.util.which('cvtres', config.environment['PATH'])) or (config.llvm_libxml2_enabled == "1"):
+    config.available_features.add('manifest_tool')
 No newline at end of file

lld/test/lit.site.cfg.in

 @LIT_SITE_CFG_IN_HEADER@
 
 config.llvm_src_root = "@LLVM_SOURCE_DIR@"
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
 config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
 config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_libxml2_enabled = "@LLVM_LIBXML2_ENABLED@"
 config.lit_tools_dir = "@LLVM_LIT_TOOLS_DIR@"
 config.lld_obj_root = "@LLD_BINARY_DIR@"
 config.lld_libs_dir = "@LLVM_LIBRARY_OUTPUT_INTDIR@"
 config.lld_tools_dir = "@LLVM_RUNTIME_OUTPUT_INTDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @HAVE_LIBZ@
 

llvm/include/llvm/WindowsManifest/WindowsManifestMerger.h

 //===---------------------------------------------------------------------===//
 
 #ifndef LLVM_INCLUDE_LLVM_SUPPORT_WINDOWS_MANIFEST_MERGER_H
 #define LLVM_INCLUDE_LLVM_SUPPORT_WINDOWS_MANIFEST_MERGER_H
 
 #include "llvm/Config/config.h"
 #include "llvm/Support/Error.h"
 
-#if LLVM_LIBXML2_ENABLED
-#include <libxml/xmlreader.h>
-#endif
-
 namespace llvm {
 
 class MemoryBuffer;
 
-#if LLVM_LIBXML2_ENABLED
-typedef xmlDocPtr XMLDocumentImpl;
-typedef xmlNodePtr XMLNodeImpl;
-#else
-typedef void *XMLDocumentImpl;
-typedef void *XMLNodeImpl;
-#endif
+namespace windows_manifest {
 
 class WindowsManifestError : public ErrorInfo<WindowsManifestError, ECError> {
 public:
   static char ID;
   WindowsManifestError(const Twine &Msg);
   void log(raw_ostream &OS) const override;
 
 private:
   std::string Msg;
 };
 
 class WindowsManifestMerger {
+private:
+  class WindowsManifestMergerImpl;
+  std::unique_ptr<WindowsManifestMergerImpl> Impl;
 public:
+  WindowsManifestMerger();
   ~WindowsManifestMerger();
-
   Error merge(const MemoryBuffer &Manifest);
 
   // Returns vector containing merged xml manifest, or uninitialized vector for
   // empty manifest.
   std::unique_ptr<MemoryBuffer> getMergedManifest();
-
-private:
-  static void errorCallback(void *Ctx, const char *Format, ...);
-  Error getParseError();
-
-#if LLVM_LIBXML2_ENABLED
-  XMLDocumentImpl CombinedDoc = nullptr;
-  std::vector<XMLDocumentImpl> MergedDocs;
-#endif
-  bool ParseErrorOccurred = false;
 };
 
+} // namespace windows_manifest
 } // namespace llvm
 #endif

llvm/lib/WindowsManifest/WindowsManifestMerger.cpp

 //===-- WindowsManifestMerger.cpp ------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===---------------------------------------------------------------------===//
 //
 // This file implements the .manifest merger class.
 //
 //===---------------------------------------------------------------------===//
 
 #include "llvm/WindowsManifest/WindowsManifestMerger.h"
 #include "llvm/Support/MemoryBuffer.h"
 
-#include <stdarg.h>
+#if LLVM_LIBXML2_ENABLED
+#include <libxml/xmlreader.h>
+#endif
 
 #define TO_XML_CHAR(X) reinterpret_cast<const unsigned char *>(X)
 #define FROM_XML_CHAR(X) reinterpret_cast<const char *>(X)
 
 using namespace llvm;
-
-namespace llvm {
+using namespace windows_manifest;
 
 char WindowsManifestError::ID = 0;
 
 WindowsManifestError::WindowsManifestError(const Twine &Msg) : Msg(Msg.str()) {}
 
 void WindowsManifestError::log(raw_ostream &OS) const { OS << Msg; }
 
+class WindowsManifestMerger::WindowsManifestMergerImpl {
+public:
+  ~WindowsManifestMergerImpl();
+  Error merge(const MemoryBuffer &Manifest);
+  std::unique_ptr<MemoryBuffer> getMergedManifest();
+private:
+  static void errorCallback(void *Ctx, const char *Format, ...);
+  Error getParseError();
 #if LLVM_LIBXML2_ENABLED
+  xmlDocPtr CombinedDoc = nullptr;
+  std::vector<xmlDocPtr> MergedDocs;
+#endif
+  bool ParseErrorOccurred = false;
+};
+
+#if LLVM_LIBXML2_ENABLED
+
+static const std::pair<std::string, std::string>
+    MtNsHrefsPrefixes[] = {
+        {"urn:schemas-microsoft-com:asm.v1", "ms_asmv1"},
+        {"urn:schemas-microsoft-com:asm.v2", "ms_asmv2"},
+        {"urn:schemas-microsoft-com:asm.v3", "ms_asmv3"},
+        {"http://schemas.microsoft.com/SMI/2005/WindowsSettings",
+         "ms_windowsSettings"},
+        {"urn:schemas-microsoft-com:compatibility.v1", "ms_compatibilityv1"}};
+
 static bool xmlStringsEqual(const unsigned char *A, const unsigned char *B) {
+  // Handle null pointers.  Comparison of 2 null pointers returns true because
+  // this indicates the prefix of a default namespace.
+  if (!A || !B)
+    return A == B;
+
   return strcmp(FROM_XML_CHAR(A), FROM_XML_CHAR(B)) == 0;
 }
-#endif
 
-bool isMergeableElement(const unsigned char *ElementName) {
+static bool isMergeableElement(const unsigned char *ElementName) {
   for (StringRef S : {"application", "assembly", "assemblyIdentity",
                       "compatibility", "noInherit", "requestedExecutionLevel",
                       "requestedPrivileges", "security", "trustInfo"}) {
     if (S == FROM_XML_CHAR(ElementName))
       return true;
   }
   return false;
 }
 
-XMLNodeImpl getChildWithName(XMLNodeImpl Parent,
+static xmlNodePtr getChildWithName(xmlNodePtr Parent,
                              const unsigned char *ElementName) {
-#if LLVM_LIBXML2_ENABLED
-  for (XMLNodeImpl Child = Parent->children; Child; Child = Child->next)
+  for (xmlNodePtr Child = Parent->children; Child; Child = Child->next)
     if (xmlStringsEqual(Child->name, ElementName)) {
       return Child;
     }
-#endif
   return nullptr;
 }
 
-const unsigned char *getAttribute(XMLNodeImpl Node,
-                                  const unsigned char *AttributeName) {
-#if LLVM_LIBXML2_ENABLED
+static xmlAttrPtr getAttribute(xmlNodePtr Node, const unsigned char *AttributeName) {
   for (xmlAttrPtr Attribute = Node->properties; Attribute != nullptr;
        Attribute = Attribute->next) {
     if (xmlStringsEqual(Attribute->name, AttributeName))
-      return Attribute->children->content;
+      return Attribute;
   }
-#endif
   return nullptr;
 }
 
-Error mergeAttributes(XMLNodeImpl OriginalNode, XMLNodeImpl AdditionalNode) {
-#if LLVM_LIBXML2_ENABLED
-  for (xmlAttrPtr Attribute = AdditionalNode->properties; Attribute != nullptr;
+// Check if namespace specified by HRef1 overrides that of HRef2.
+static bool namespaceOverrides(const unsigned char *HRef1,
+                        const unsigned char *HRef2) {
+  auto HRef1Position = std::find_if(
+      std::begin(MtNsHrefsPrefixes), std::end(MtNsHrefsPrefixes),
+      [HRef1](const std::pair<std::string, std::string> &element) {
+        return xmlStringsEqual(HRef1, TO_XML_CHAR(element.first.c_str()));
+      });
+  if (HRef1Position == std::end(MtNsHrefsPrefixes))
+    return false;
+  auto HRef2Position = std::find_if(
+      std::begin(MtNsHrefsPrefixes), std::end(MtNsHrefsPrefixes),
+      [HRef2](const std::pair<std::string, std::string> &element) {
+        return xmlStringsEqual(HRef2, TO_XML_CHAR(element.first.c_str()));
+      });
+  if (HRef2Position == std::end(MtNsHrefsPrefixes))
+    return true;
+  int HRef1Priority = std::distance(HRef1Position, std::end(MtNsHrefsPrefixes));
+  int HRef2Priority = std::distance(HRef2Position, std::end(MtNsHrefsPrefixes));
+  if (HRef1Priority > HRef2Priority)
+    return true;
+  return false;
+}
+
+// Search for prefix-defined namespace specified by HRef, starting on Node and
+// continuing recurisvely upwards.  Returns the namespace or nullptr if not
+// found.
+static xmlNsPtr search(const unsigned char *HRef, xmlNodePtr Node) {
+  for (xmlNsPtr Def = Node->nsDef; Def; Def = Def->next) {
+    if (Def->prefix && xmlStringsEqual(Def->href, HRef))
+      return Def;
+  }
+  if (Node->parent) {
+    return search(HRef, Node->parent);
+  }
+  return nullptr;
+}
+
+// Return the prefix that corresponds to the HRef.  If HRef is not a recognized
+// URI, then just return the HRef itself to use as the prefix.
+static const unsigned char *getPrefixForHref(const unsigned char *HRef) {
+  for (auto &Ns : MtNsHrefsPrefixes) {
+    if (xmlStringsEqual(HRef, TO_XML_CHAR(Ns.first.c_str()))) {
+      return TO_XML_CHAR(Ns.second.c_str());
+    }
+  }
+  return HRef;
+}
+
+// Search for prefix-defined namespace specified by HRef, starting on Node and
+// continuing recurisvely upwards.  If it is found, then return it.  If it is
+// not found, then prefix-define that namespace on the node and return a
+// reference to it.
+static Expected<xmlNsPtr> searchOrDefine(const unsigned char *HRef,
+                                   xmlNodePtr Node) {
+  xmlNsPtr Def = search(HRef, Node);
+  if (Def)
+    return Def;
+  Def = xmlNewNs(Node, HRef, getPrefixForHref(HRef));
+  if (!Def)
+    return make_error<WindowsManifestError>("failed to create new namespace");
+  return Def;
+}
+
+// Set the namespace of OrigionalAttribute on OriginalNode to be that of
+// AdditionalAttribute's.
+static Error copyAttributeNamespace(xmlAttrPtr OriginalAttribute,
+                             xmlNodePtr OriginalNode,
+                             xmlAttrPtr AdditionalAttribute) {
+
+  Expected<xmlNsPtr> ExplicitOrError =
+      searchOrDefine(AdditionalAttribute->ns->href, OriginalNode);
+  if (!ExplicitOrError)
+    return ExplicitOrError.takeError();
+  OriginalAttribute->ns = std::move(ExplicitOrError.get());
+  return Error::success();
+}
+
+// Return the corresponding namespace definition for the prefix, defined on the
+// given Node.  Returns nullptr if there is no such definition.
+static xmlNsPtr getNamespaceWithPrefix(const unsigned char *Prefix,
+                                 xmlNodePtr Node) {
+
+  if (Node == nullptr)
+    return nullptr;
+  for (xmlNsPtr Def = Node->nsDef; Def; Def = Def->next) {
+    if (xmlStringsEqual(Def->prefix, Prefix))
+      return Def;
+  }
+  return nullptr;
+}
+
+// Search for the closest inheritable default namespace, starting on (and
+// including) the Node and traveling upwards through parent nodes.  Returns
+// nullptr if there are no inheritable default namespaces.
+static xmlNsPtr getClosestDefault(xmlNodePtr Node) {
+  if (xmlNsPtr Ret = getNamespaceWithPrefix(nullptr, Node))
+    return Ret;
+  if (Node->parent == nullptr)
+    return nullptr;
+  return getClosestDefault(Node->parent);
+}
+
+// Merge the attributes of AdditionalNode into OriginalNode.  If attributes
+// with identical types are present, they are not duplicated but rather if
+// their values are not consistent and error is thrown.  In addition, the
+// higher priority namespace is used for each attribute, EXCEPT in the case
+// of merging two default namespaces and the lower priority namespace
+// definition occurs closer than the higher priority one.
+static Error mergeAttributes(xmlNodePtr OriginalNode, xmlNodePtr AdditionalNode) {
+  xmlNsPtr ClosestDefault = getClosestDefault(OriginalNode);
+  for (xmlAttrPtr Attribute = AdditionalNode->properties; Attribute;
        Attribute = Attribute->next) {
-    if (const unsigned char *OriginalValue =
+    if (xmlAttrPtr OriginalAttribute =
             getAttribute(OriginalNode, Attribute->name)) {
-      // Attributes of the same name must also have the same value.  Otherwise
-      // an error is thrown.
-      if (!xmlStringsEqual(OriginalValue, Attribute->children->content))
+      if (!xmlStringsEqual(OriginalAttribute->children->content,
+                           Attribute->children->content))
         return make_error<WindowsManifestError>(
             Twine("conflicting attributes for ") +
             FROM_XML_CHAR(OriginalNode->name));
+      if (!Attribute->ns) {
+        continue;
+      }
+      if (!OriginalAttribute->ns) {
+        if (auto E = copyAttributeNamespace(OriginalAttribute, OriginalNode,
+                                            Attribute))
+          return E;
+        // In this case, the original attribute has a higher priority namespace
+        // than the incomiing attribute, however the namespace definition of
+        // the lower priority namespace occurs first traveling upwards in the
+        // tree.  Therefore the lower priority namespace is applied.
+      } else if (namespaceOverrides(OriginalAttribute->ns->href,
+                                    Attribute->ns->href)) {
+        if (!OriginalAttribute->ns->prefix && !Attribute->ns->prefix &&
+            ClosestDefault &&
+            xmlStringsEqual(Attribute->ns->href, ClosestDefault->href)) {
+          if (auto E = copyAttributeNamespace(OriginalAttribute, OriginalNode,
+                                              Attribute))
+            return E;
+        }
+        // This covers the case where the incoming attribute has the higher
+        // priority.  The higher priority namespace is applied in all cases
+        // EXCEPT when both of the namespaces are default inherited, and the
+        // closest inherited default is the lower priority one.
+      } else if (Attribute->ns->prefix || OriginalAttribute->ns->prefix ||
+                 (ClosestDefault &&
+                  !xmlStringsEqual(OriginalAttribute->ns->href,
+                                   ClosestDefault->href))) {
+        if (auto E = copyAttributeNamespace(OriginalAttribute, OriginalNode,
+                                            Attribute))
+          return E;
+      }
+      // If the incoming attribute is not already found on the node, append it
+      // to the end of the properties list.  Also explicitly apply its
+      // namespace as a prefix because it might be contained in a separate
+      // namespace that doesn't use the attribute.
     } else {
-      char *NameCopy = strdup(FROM_XML_CHAR(Attribute->name));
-      char *ContentCopy = strdup(FROM_XML_CHAR(Attribute->children->content));
-      xmlNewProp(OriginalNode, TO_XML_CHAR(NameCopy), TO_XML_CHAR(ContentCopy));
+      xmlAttrPtr NewProp = xmlNewProp(OriginalNode, Attribute->name,
+                                       Attribute->children->content);
+      Expected<xmlNsPtr> ExplicitOrError =
+          searchOrDefine(Attribute->ns->href, OriginalNode);
+      if (!ExplicitOrError)
+        return ExplicitOrError.takeError();
+      NewProp->ns = std::move(ExplicitOrError.get());
     }
   }
-#endif
   return Error::success();
 }
 
-Error treeMerge(XMLNodeImpl OriginalRoot, XMLNodeImpl AdditionalRoot) {
-#if LLVM_LIBXML2_ENABLED
-  XMLNodeImpl AdditionalFirstChild = AdditionalRoot->children;
+// Given two nodes, return the one with the higher priority namespace.
+static xmlNodePtr getDominantNode(xmlNodePtr Node1, xmlNodePtr Node2) {
+
+  if (!Node1 || !Node1->ns)
+    return Node2;
+  else if (!Node2 || !Node2->ns)
+    return Node1;
+  else if (namespaceOverrides(Node1->ns->href, Node2->ns->href))
+    return Node1;
+  else
+    return Node2;
+}
+
+// Checks if this Node's namespace is inherited or one it defined itself.
+static bool hasInheritedNs(xmlNodePtr Node) {
+  if (Node->ns && Node->ns != getNamespaceWithPrefix(Node->ns->prefix, Node))
+    return true;
+  else
+    return false;
+}
+
+// Check if this Node's namespace is a default namespace that it inherited, as
+// opposed to defining itself.
+static bool hasInheritedDefaultNs(xmlNodePtr Node) {
+  if (hasInheritedNs(Node) && Node->ns->prefix == nullptr)
+    return true;
+  else
+    return false;
+}
+
+// Check if this Node's namespace is a default namespace it defined itself.
+static bool hasDefinedDefaultNamespace(xmlNodePtr Node) {
+  if (Node->ns && (Node->ns == getNamespaceWithPrefix(nullptr, Node)))
+    return true;
+  else
+    return false;
+}
+
+// For the given explicit prefix-definition of a namespace, travel downwards
+// from a node recursively, and for every implicit, inherited default usage of
+// that namespace replace it with that explicit prefix use.  This is important
+// when namespace overriding occurs when merging, so that elements unique to a
+// namespace will still stay in that namespace.
+static void explicateNamespace(xmlNsPtr PrefixDef, xmlNodePtr Node) {
+  // If a node as its own default namespace definition it clearly cannot have
+  // inherited the given default namespace, and neither will any of its
+  // children.
+  if (hasDefinedDefaultNamespace(Node))
+    return;
+  if (Node->ns && xmlStringsEqual(Node->ns->href, PrefixDef->href) &&
+      hasInheritedDefaultNs(Node))
+    Node->ns = PrefixDef;
+  for (xmlAttrPtr Attribute = Node->properties; Attribute;
+       Attribute = Attribute->next) {
+    if (Attribute->ns &&
+        xmlStringsEqual(Attribute->ns->href, PrefixDef->href)) {
+      Attribute->ns = PrefixDef;
+    }
+  }
+  for (xmlNodePtr Child = Node->children; Child; Child = Child->next) {
+    explicateNamespace(PrefixDef, Child);
+  }
+}
+
+// Perform the namespace merge between two nodes.
+static Error mergeNamespaces(xmlNodePtr OriginalNode, xmlNodePtr AdditionalNode) {
+  // Save the original default namespace definition in case the incoming node
+  // overrides it.
+  const unsigned char *OriginalDefinedDefaultHref = nullptr;
+  if (xmlNsPtr OriginalDefinedDefaultNs =
+          getNamespaceWithPrefix(nullptr, OriginalNode)) {
+    OriginalDefinedDefaultHref = xmlStrdup(OriginalDefinedDefaultNs->href);
+  }
+  const unsigned char *NewDefinedDefaultHref = nullptr;
+  // Copy all namespace definitions.  There can only be one default namespace
+  // definition per node, so the higher priority one takes precedence in the
+  // case of collision.
+  for (xmlNsPtr Def = AdditionalNode->nsDef; Def; Def = Def->next) {
+    if (xmlNsPtr OriginalNsDef =
+            getNamespaceWithPrefix(Def->prefix, OriginalNode)) {
+      if (!Def->prefix) {
+        if (namespaceOverrides(Def->href, OriginalNsDef->href)) {
+          NewDefinedDefaultHref = TO_XML_CHAR(strdup(FROM_XML_CHAR(Def->href)));
+        }
+      } else if (!xmlStringsEqual(OriginalNsDef->href, Def->href)) {
+        return make_error<WindowsManifestError>(
+            Twine("conflicting namespace definitions for ") +
+            FROM_XML_CHAR(Def->prefix));
+      }
+    } else {
+      xmlNsPtr NewDef = xmlCopyNamespace(Def);
+      NewDef->next = OriginalNode->nsDef;
+      OriginalNode->nsDef = NewDef;
+    }
+  }
+
+  // Check whether the original node or the incoming node has the higher
+  // priority namespace.  Depending on which one is dominant, we will have
+  // to recursively apply namespace changes down to children of the original
+  // node.
+  xmlNodePtr DominantNode = getDominantNode(OriginalNode, AdditionalNode);
+  xmlNodePtr NonDominantNode =
+      DominantNode == OriginalNode ? AdditionalNode : OriginalNode;
+  if (DominantNode == OriginalNode) {
+    if (OriginalDefinedDefaultHref) {
+      xmlNsPtr NonDominantDefinedDefault =
+          getNamespaceWithPrefix(nullptr, NonDominantNode);
+      // In this case, both the nodes defined a default namespace.  However
+      // the lower priority node ended up having a higher priority default
+      // definition.  This can occur if the higher priority node is prefix
+      // namespace defined.  In this case we have to define an explicit
+      // prefix for the overridden definition and apply it to all children
+      // who relied on that definition.
+      if (NonDominantDefinedDefault &&
+          namespaceOverrides(NonDominantDefinedDefault->href,
+                             OriginalDefinedDefaultHref)) {
+        Expected<xmlNsPtr> EC =
+            searchOrDefine(OriginalDefinedDefaultHref, DominantNode);
+        if (!EC)
+          return EC.takeError();
+        xmlNsPtr PrefixDominantDefinedDefault = std::move(EC.get());
+        explicateNamespace(PrefixDominantDefinedDefault, DominantNode);
+      }
+      // In this case the node with a higher priority namespace did not have a
+      // default namespace definition, but the lower priority node did.  In this
+      // case the new default namespace definition is copied.  A side effect of
+      // this is that all children will suddenly find themselves in a different
+      // default namespace.  To maintain correctness we need to ensure that all
+      // children now explicitly refer to the namespace that they had previously
+      // implicitly inherited.
+    } else if (getNamespaceWithPrefix(nullptr, NonDominantNode)) {
+      if (DominantNode->parent) {
+        xmlNsPtr ClosestDefault = getClosestDefault(DominantNode->parent);
+        Expected<xmlNsPtr> EC =
+            searchOrDefine(ClosestDefault->href, DominantNode);
+        if (!EC)
+          return EC.takeError();
+        xmlNsPtr ExplicitDefault = std::move(EC.get());
+        explicateNamespace(ExplicitDefault, DominantNode);
+      }
+    }
+  } else {
+    // Covers case where the incoming node has a default namespace definition
+    // that overrides the original node's namespace.  This always leads to
+    // the original node receiving that new default namespace.
+    if (hasDefinedDefaultNamespace(DominantNode)) {
+      NonDominantNode->ns = getNamespaceWithPrefix(nullptr, NonDominantNode);
+    } else {
+      // This covers the case where the incoming node either has a prefix
+      // namespace, or an inherited default namespace.  Since the namespace
+      // may not yet be defined in the original tree we do a searchOrDefine
+      // for it, and then set the namespace equal to it.
+      Expected<xmlNsPtr> EC =
+          searchOrDefine(DominantNode->ns->href, NonDominantNode);
+      if (!EC)
+        return EC.takeError();
+      xmlNsPtr Explicit = std::move(EC.get());
+      NonDominantNode->ns = Explicit;
+    }
+    // This covers cases where the incoming dominant node HAS a default
+    // namespace definition, but MIGHT NOT NECESSARILY be in that namespace.
+    if (xmlNsPtr DominantDefaultDefined =
+            getNamespaceWithPrefix(nullptr, DominantNode)) {
+      if (OriginalDefinedDefaultHref) {
+        if (namespaceOverrides(DominantDefaultDefined->href,
+                               OriginalDefinedDefaultHref)) {
+          // In this case, the incoming node's default definition overrides
+          // the original default definition, all children who relied on that
+          // definition must be updated accordingly.
+          Expected<xmlNsPtr> EC =
+              searchOrDefine(OriginalDefinedDefaultHref, NonDominantNode);
+          if (!EC)
+            return EC.takeError();
+          xmlNsPtr ExplicitDefault = std::move(EC.get());
+          explicateNamespace(ExplicitDefault, NonDominantNode);
+        }
+      } else {
+        // The original did not define a default definition, however the new
+        // default definition still applies to all children, so they must be
+        // updated to explicitly refer to the namespace they had previously
+        // been inheriting implicitly.
+        xmlNsPtr ClosestDefault = getClosestDefault(NonDominantNode);
+        Expected<xmlNsPtr> EC =
+            searchOrDefine(ClosestDefault->href, NonDominantNode);
+        if (!EC)
+          return EC.takeError();
+        xmlNsPtr ExplicitDefault = std::move(EC.get());
+        explicateNamespace(ExplicitDefault, NonDominantNode);
+      }
+    }
+  }
+  if (NewDefinedDefaultHref) {
+    xmlNsPtr OriginalNsDef = getNamespaceWithPrefix(nullptr, OriginalNode);
+    xmlFree(const_cast<unsigned char *>(OriginalNsDef->href));
+    OriginalNsDef->href = NewDefinedDefaultHref;
+  }
+  xmlFree(const_cast<unsigned char *>(OriginalDefinedDefaultHref));
+  return Error::success();
+}
+
+static bool isRecognizedNamespace(const unsigned char *NsHref) {
+  for (auto &Ns : MtNsHrefsPrefixes) {
+    if (xmlStringsEqual(NsHref, TO_XML_CHAR(Ns.first.c_str())))
+      return true;
+  }
+  return false;
+}
+
+static bool hasRecognizedNamespace(xmlNodePtr Node) {
+  return isRecognizedNamespace(Node->ns->href);
+}
+
+// Ensure a node's inherited namespace is actually defined in the tree it
+// resides in.
+static Error reconcileNamespaces(xmlNodePtr Node) {
+  if (Node == nullptr)
+    return Error::success();
+  if (hasInheritedNs(Node)) {
+    Expected<xmlNsPtr> ExplicitOrError = searchOrDefine(Node->ns->href, Node);
+    if (!ExplicitOrError)
+      return ExplicitOrError.takeError();
+    xmlNsPtr Explicit = std::move(ExplicitOrError.get());
+    Node->ns = Explicit;
+  }
+  for (xmlNodePtr Child = Node->children; Child; Child = Child->next) {
+    if (auto E = reconcileNamespaces(Child))
+      return E;
+  }
+  return Error::success();
+}
+
+// Recursively merge the two given manifest trees, depending on which elements
+// are of a mergeable type, and choose namespaces according to which have
+// higher priority.
+static Error treeMerge(xmlNodePtr OriginalRoot, xmlNodePtr AdditionalRoot) {
+  if (auto E = mergeAttributes(OriginalRoot, AdditionalRoot))
+    return E;
+  if (auto E = mergeNamespaces(OriginalRoot, AdditionalRoot))
+    return E;
+  xmlNodePtr AdditionalFirstChild = AdditionalRoot->children;
   xmlNode StoreNext;
-  for (XMLNodeImpl Child = AdditionalFirstChild; Child; Child = Child->next) {
-    XMLNodeImpl OriginalChildWithName;
+  for (xmlNodePtr Child = AdditionalFirstChild; Child; Child = Child->next) {
+    xmlNodePtr OriginalChildWithName;
     if (!isMergeableElement(Child->name) ||
         !(OriginalChildWithName =
-              getChildWithName(OriginalRoot, Child->name))) {
+              getChildWithName(OriginalRoot, Child->name)) ||
+        !hasRecognizedNamespace(Child)) {
       StoreNext.next = Child->next;
       xmlUnlinkNode(Child);
       if (!xmlAddChild(OriginalRoot, Child))
         return make_error<WindowsManifestError>(Twine("could not merge ") +
                                                 FROM_XML_CHAR(Child->name));
+      if (auto E = reconcileNamespaces(Child))
+        return E;
       Child = &StoreNext;
     } else if (auto E = treeMerge(OriginalChildWithName, Child)) {
       return E;
     }
   }
-  if (auto E = mergeAttributes(OriginalRoot, AdditionalRoot))
-    return E;
-#endif
   return Error::success();
 }
 
-void stripCommentsAndText(XMLNodeImpl Root) {
-#if LLVM_LIBXML2_ENABLED
+static void stripComments(xmlNodePtr Root) {
   xmlNode StoreNext;
-  for (XMLNodeImpl Child = Root->children; Child; Child = Child->next) {
-    if (!xmlStringsEqual(Child->name, TO_XML_CHAR("text")) &&
-        !xmlStringsEqual(Child->name, TO_XML_CHAR("comment"))) {
-      stripCommentsAndText(Child);
+  for (xmlNodePtr Child = Root->children; Child; Child = Child->next) {
+    if (!xmlStringsEqual(Child->name, TO_XML_CHAR("comment"))) {
+      stripComments(Child);
     } else {
       StoreNext.next = Child->next;
-      XMLNodeImpl Remove = Child;
+      xmlNodePtr Remove = Child;
       Child = &StoreNext;
       xmlUnlinkNode(Remove);
       xmlFreeNode(Remove);
     }
   }
-#endif
 }
 
-WindowsManifestMerger::~WindowsManifestMerger() {
-#if LLVM_LIBXML2_ENABLED
+// The libxml2 assumes that attributes do not inherit default namespaces,
+// whereas the original  mt.exe does make this assumption.  This function
+// reconciles this by setting all attributes to have the inherited default
+// namespace.
+static void setAttributeNamespaces(xmlNodePtr Node) {
+  for (xmlAttrPtr Attribute = Node->properties; Attribute;
+       Attribute = Attribute->next) {
+    if (!Attribute->ns) {
+      Attribute->ns = getClosestDefault(Node);
+    }
+  }
+  for (xmlNodePtr Child = Node->children; Child; Child = Child->next) {
+    setAttributeNamespaces(Child);
+  }
+}
+
+// The merging process may create too many prefix defined namespaces.  This
+// function removes all unnecessary ones from the tree.
+static void checkAndStripPrefixes(xmlNodePtr Node,
+                           std::vector<xmlNsPtr> &RequiredPrefixes) {
+  for (xmlNodePtr Child = Node->children; Child; Child = Child->next) {
+    checkAndStripPrefixes(Child, RequiredPrefixes);
+  }
+  if (Node->ns && Node->ns->prefix != nullptr) {
+    xmlNsPtr ClosestDefault = getClosestDefault(Node);
+    if (ClosestDefault &&
+        xmlStringsEqual(ClosestDefault->href, Node->ns->href)) {
+      Node->ns = ClosestDefault;
+    } else if (std::find(RequiredPrefixes.begin(), RequiredPrefixes.end(),
+                         Node->ns) == RequiredPrefixes.end()) {
+      RequiredPrefixes.push_back(Node->ns);
+    }
+  }
+  for (xmlAttrPtr Attribute = Node->properties; Attribute;
+       Attribute = Attribute->next) {
+    if (Attribute->ns && Attribute->ns->prefix != nullptr) {
+      xmlNsPtr ClosestDefault = getClosestDefault(Node);
+      if (ClosestDefault &&
+          xmlStringsEqual(ClosestDefault->href, Attribute->ns->href)) {
+        Attribute->ns = ClosestDefault;
+      } else if (std::find(RequiredPrefixes.begin(), RequiredPrefixes.end(),
+                           Node->ns) == RequiredPrefixes.end()) {
+        RequiredPrefixes.push_back(Attribute->ns);
+      }
+    }
+  }
+  xmlNsPtr Prev;
+  xmlNs Temp;
+  for (xmlNsPtr Def = Node->nsDef; Def; Def = Def->next) {
+    if (Def->prefix &&
+        (std::find(RequiredPrefixes.begin(), RequiredPrefixes.end(), Def) ==
+         RequiredPrefixes.end())) {
+      if (Def == Node->nsDef)
+        Node->nsDef = Def->next;
+      else
+        Prev->next = Def->next;
+      Temp.next = Def->next;
+      xmlFreeNs(Def);
+      Def = &Temp;
+    } else {
+      Prev = Def;
+    }
+  }
+}
+
+WindowsManifestMerger::WindowsManifestMergerImpl::~WindowsManifestMergerImpl() {
   for (auto &Doc : MergedDocs)
     xmlFreeDoc(Doc);
-#endif
 }
 
-Error WindowsManifestMerger::merge(const MemoryBuffer &Manifest) {
-#if LLVM_LIBXML2_ENABLED
+Error WindowsManifestMerger::WindowsManifestMergerImpl::merge(const MemoryBuffer &Manifest) {
   if (Manifest.getBufferSize() == 0)
     return make_error<WindowsManifestError>(
         "attempted to merge empty manifest");
-  xmlSetGenericErrorFunc((void *)this, WindowsManifestMerger::errorCallback);
-  XMLDocumentImpl ManifestXML =
+  xmlSetGenericErrorFunc((void *)this, WindowsManifestMergerImpl::errorCallback);
+  xmlDocPtr ManifestXML =
       xmlReadMemory(Manifest.getBufferStart(), Manifest.getBufferSize(),
                     "manifest.xml", nullptr, XML_PARSE_NOBLANKS);
   xmlSetGenericErrorFunc(nullptr, nullptr);
   if (auto E = getParseError())
     return E;
-  XMLNodeImpl AdditionalRoot = xmlDocGetRootElement(ManifestXML);
-  stripCommentsAndText(AdditionalRoot);
+  xmlNodePtr AdditionalRoot = xmlDocGetRootElement(ManifestXML);
+  stripComments(AdditionalRoot);
+  setAttributeNamespaces(AdditionalRoot);
   if (CombinedDoc == nullptr) {
     CombinedDoc = ManifestXML;
   } else {
-    XMLNodeImpl CombinedRoot = xmlDocGetRootElement(CombinedDoc);
+    xmlNodePtr CombinedRoot = xmlDocGetRootElement(CombinedDoc);
     if (xmlStringsEqual(CombinedRoot->name, AdditionalRoot->name) &&
-        isMergeableElement(AdditionalRoot->name)) {
+        isMergeableElement(AdditionalRoot->name) &&
+        hasRecognizedNamespace(AdditionalRoot)) {
       if (auto E = treeMerge(CombinedRoot, AdditionalRoot)) {
         return E;
       }
     } else {
       return make_error<WindowsManifestError>("multiple root nodes");
     }
   }
   MergedDocs.push_back(ManifestXML);
-#endif
   return Error::success();
 }
 
-std::unique_ptr<MemoryBuffer> WindowsManifestMerger::getMergedManifest() {
-#if LLVM_LIBXML2_ENABLED
+std::unique_ptr<MemoryBuffer> WindowsManifestMerger::WindowsManifestMergerImpl::getMergedManifest() {
   unsigned char *XmlBuff;
   int BufferSize = 0;
   if (CombinedDoc) {
+    xmlNodePtr CombinedRoot = xmlDocGetRootElement(CombinedDoc);
+    std::vector<xmlNsPtr> RequiredPrefixes;
+    checkAndStripPrefixes(CombinedRoot, RequiredPrefixes);
     std::unique_ptr<xmlDoc> OutputDoc(xmlNewDoc((const unsigned char *)"1.0"));
-    xmlDocSetRootElement(OutputDoc.get(), xmlDocGetRootElement(CombinedDoc));
+    xmlDocSetRootElement(OutputDoc.get(), CombinedRoot);
     xmlKeepBlanksDefault(0);
-    xmlDocDumpFormatMemory(OutputDoc.get(), &XmlBuff, &BufferSize, 1);
+    xmlDocDumpFormatMemoryEnc(OutputDoc.get(), &XmlBuff, &BufferSize, "UTF-8",
+                              1);
   }
   if (BufferSize == 0)
     return nullptr;
   return MemoryBuffer::getMemBuffer(
       StringRef(FROM_XML_CHAR(XmlBuff), (size_t)BufferSize));
+}
+
 #else
+
+WindowsManifestMerger::WindowsManifestMergerImpl::~WindowsManifestMergerImpl() {}
+
+Error WindowsManifestMerger::WindowsManifestMergerImpl::merge(const MemoryBuffer &Manifest) {
+  return Error::success();
+}
+
+std::unique_ptr<MemoryBuffer> WindowsManifestMerger::WindowsManifestMergerImpl::getMergedManifest() {
   return nullptr;
-#endif
 }
 
-void WindowsManifestMerger::errorCallback(void *Ctx, const char *Format, ...) {
-  auto *Merger = (WindowsManifestMerger *)Ctx;
+#endif 
+
+WindowsManifestMerger::WindowsManifestMerger() : Impl(make_unique<WindowsManifestMergerImpl>()) {}
+
+WindowsManifestMerger::~WindowsManifestMerger() {}
+
+Error WindowsManifestMerger::merge(const MemoryBuffer &Manifest) { return Impl->merge(Manifest); }
+
+std::unique_ptr<MemoryBuffer> WindowsManifestMerger::getMergedManifest() { return Impl->getMergedManifest(); }
+
+void WindowsManifestMerger::WindowsManifestMergerImpl::errorCallback(void *Ctx, const char *Format, ...) {
+  auto *Merger = (WindowsManifestMergerImpl *)Ctx;
   Merger->ParseErrorOccurred = true;
 }
 
-Error WindowsManifestMerger::getParseError() {
+Error WindowsManifestMerger::WindowsManifestMergerImpl::getParseError() {
   if (!ParseErrorOccurred)
     return Error::success();
   return make_error<WindowsManifestError>("invalid xml document");
 }
-
-} // namespace llvm

llvm/test/tools/llvm-mt/Inputs/assembly_identity.manifest

+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<assembly xmlns="urn:schemas-microsoft-com:asm.v2" manifestVersion="1.0">
+  <dependency>
+    <dependentAssembly>
+      <assemblyIdentity foo="bar" name="identity1" version="1.2.3.4"></assemblyIdentity>
+    </dependentAssembly>
+  </dependency>
+</assembly>

llvm/test/tools/llvm-mt/Inputs/compatibility.manifest

+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<assembly manifestVersion="1.0" xmlns="urn:schemas-microsoft-com:asm.v1">
+  <application>
+    <compatibility xmlns="urn:schemas-microsoft-com:compatibility.v1">
+      <supportedOS Id="FooOS"/>
+      <supportedOS Id="BarOS"/>
+    </compatibility>
+  </application>
+</assembly>

llvm/test/tools/llvm-mt/Inputs/expected_big.manifest

+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
+<ms_asmv1:trustInfo xmlns="urn:schemas-microsoft-com:asm.v3" xmlns:ms_asmv1="urn:schemas-microsoft-com:asm.v1">
+  <ms_asmv1:security>
+    <ms_asmv1:requestedPrivileges>
+      <ms_asmv1:requestedExecutionLevel level="trust1" ms_asmv1:access="false"></ms_asmv1:requestedExecutionLevel>
+    </ms_asmv1:requestedPrivileges>
+  </ms_asmv1:security>
+</ms_asmv1:trustInfo>
+<ms_asmv2:dependency xmlns:ms_asmv2="urn:schemas-microsoft-com:asm.v2">
+  <ms_asmv2:dependentAssembly>
+    <ms_asmv2:assemblyIdentity foo="bar" name="identity1" version="1.2.3.4"></assemblyIdentity>
+  </ms_asmv2:dependentAssembly>
+</ms_asmv2:dependency>
+<dependency>
+  <dependentAssembly>
+    <assemblyIdentity bar="foo" name="my assembly" version="5"></assemblyIdentity>
+  </dependentAssembly>
+</dependency>
+<application xmlns:asmv3="urn:schemas-microsoft-com:asm.v3">
+  <compatibility xmlns="urn:schemas-microsoft-com:compatibility.v1">
+    <supportedOS Id="FooOS"/>
+    <supportedOS Id="BarOS"/>
+  </compatibility>
+  <asmv3:windowsSettings xmlns="http://schemas.microsoft.com/SMI/2005/WindowsSettings">
+    <dpiAware>true/pm</dpiAware>
+  </asmv3:windowsSettings>
+</application>
+</assembly>

llvm/test/tools/llvm-mt/Inputs/test_manifest.manifest

-<?xml version="1.0"?>
+<?xml version="1.0" encoding="UTF-8"?>
 <assembly xmlns="urn:schemas-microsoft-com:asm.v1">
   <trustInfo>
     <security>
       <requestedPrivileges>
         <requestedExecutionLevel level="3" uiAccess="1"/>
       </requestedPrivileges>
     </security>
   </trustInfo>

llvm/test/tools/llvm-mt/Inputs/trust_and_identity.manifest

+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
+
+  <!--random comment -->
+
+  <dependency>
+    <dependentAssembly>
+      <assemblyIdentity bar="foo" name="my assembly" version="5"></assemblyIdentity>
+    </dependentAssembly>
+  </dependency>
+
+  <trustInfo>
+    <security>
+      <requestedPrivileges>
+        <requestedExecutionLevel level="trust1" access="false"/>
+      </requestedPrivileges>
+    </security>
+  </trustInfo>
+
+</assembly>

llvm/test/tools/llvm-mt/Inputs/trust_info.manifest

+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
+<trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
+  <security>
+    <requestedPrivileges>
+      <requestedExecutionLevel level="trust1"></requestedExecutionLevel>
+    </requestedPrivileges>
+  </security>
+</trustInfo></assembly>

llvm/test/tools/llvm-mt/Inputs/windows_settings.manifest

+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
+ <asmv3:application xmlns:asmv3="urn:schemas-microsoft-com:asm.v3">
+    <asmv3:windowsSettings xmlns="http://schemas.microsoft.com/SMI/2005/WindowsSettings">
+      <dpiAware>true/pm</dpiAware>
+    </asmv3:windowsSettings>
+  </asmv3:application>
+</assembly>

llvm/test/tools/llvm-mt/big_merge.test

+REQUIRES: libxml2
+UNSUPPORTED: windows
+
+RUN: llvm-mt /manifest %p/Inputs/trust_info.manifest /manifest \
+RUN:   %p/Inputs/assembly_identity.manifest /manifest \
+RUN:   %p/Inputs/trust_and_identity.manifest /manifest \
+RUN:   %p/Inputs/compatibility.manifest /manifest \
+RUN:   %p/Inputs/windows_settings.manifest /out:%t
+RUN: FileCheck %s -input-file=%t
+
+CHECK:      <?xml version="1.0" encoding="UTF-8"?>
+CHECK-NEXT: <assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
+CHECK-NEXT: <ms_asmv1:trustInfo xmlns="urn:schemas-microsoft-com:asm.v3" xmlns:ms_asmv1="urn:schemas-microsoft-com:asm.v1">
+CHECK-NEXT:   <ms_asmv1:security>
+CHECK-NEXT:     <ms_asmv1:requestedPrivileges>
+CHECK-NEXT:       <ms_asmv1:requestedExecutionLevel level="trust1" ms_asmv1:access="false"/>
+CHECK-NEXT:     </ms_asmv1:requestedPrivileges>
+CHECK-NEXT:   </ms_asmv1:security>
+CHECK-NEXT: </ms_asmv1:trustInfo>
+CHECK-NEXT: <ms_asmv2:dependency xmlns:ms_asmv2="urn:schemas-microsoft-com:asm.v2">
+CHECK-NEXT:   <ms_asmv2:dependentAssembly>
+CHECK-NEXT:     <ms_asmv2:assemblyIdentity foo="bar" name="identity1" version="1.2.3.4"/>
+CHECK-NEXT:   </ms_asmv2:dependentAssembly>
+CHECK-NEXT: </ms_asmv2:dependency>
+CHECK-NEXT: <dependency>
+CHECK-NEXT:   <dependentAssembly>
+CHECK-NEXT:     <assemblyIdentity bar="foo" name="my assembly" version="5"/>
+CHECK-NEXT:   </dependentAssembly>
+CHECK-NEXT: </dependency>
+CHECK-NEXT: <application xmlns:asmv3="urn:schemas-microsoft-com:asm.v3">
+CHECK-NEXT:   <compatibility xmlns="urn:schemas-microsoft-com:compatibility.v1">
+CHECK-NEXT:     <supportedOS Id="FooOS"/>
+CHECK-NEXT:     <supportedOS Id="BarOS"/>
+CHECK-NEXT:   </compatibility>
+CHECK-NEXT:   <asmv3:windowsSettings xmlns="http://schemas.microsoft.com/SMI/2005/WindowsSettings">
+CHECK-NEXT:     <dpiAware>true/pm</dpiAware>
+CHECK-NEXT:   </asmv3:windowsSettings>
+CHECK-NEXT: </application>
+CHECK-NEXT: </assembly>

llvm/test/tools/llvm-mt/simple_merge.test

 REQUIRES: libxml2
 UNSUPPORTED: windows
 
 RUN: llvm-mt /manifest %p/Inputs/test_manifest.manifest /manifest \
 RUN:   %p/Inputs/additional.manifest /out:%t
 RUN: FileCheck %s -input-file=%t
 
-CHECK:      <?xml version="1.0"?>
+CHECK:      <?xml version="1.0" encoding="UTF-8"?>
 CHECK-NEXT: <assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
 CHECK-NEXT:   <trustInfo>
 CHECK-NEXT:     <security>
 CHECK-NEXT:       <requestedPrivileges>
 CHECK-NEXT:         <requestedExecutionLevel level="3" uiAccess="1" admin="false"/>
 CHECK-NEXT:       </requestedPrivileges>
 CHECK-NEXT:     </security>
 CHECK-NEXT:   </trustInfo>

llvm/test/tools/llvm-mt/single_file.test

 REQUIRES: libxml2
 UNSUPPORTED: windows
 
 RUN: llvm-mt /manifest %p/Inputs/test_manifest.manifest /out:%t
 RUN: FileCheck %s --input-file=%t
 
-CHECK:      <?xml version="1.0"?>
+CHECK:      <?xml version="1.0" encoding="UTF-8"?>
 CHECK-NEXT: <assembly xmlns="urn:schemas-microsoft-com:asm.v1">
 CHECK-NEXT:   <trustInfo>
 CHECK-NEXT:     <security>
 CHECK-NEXT:       <requestedPrivileges>
 CHECK-NEXT:         <requestedExecutionLevel level="3" uiAccess="1"/>
 CHECK-NEXT:       </requestedPrivileges>
 CHECK-NEXT:     </security>
 CHECK-NEXT:   </trustInfo>

llvm/tools/llvm-mt/Opts.td

 include "llvm/Option/OptParser.td"
 
+def supported : OptionGroup<"supported">;
 def unsupported : OptionGroup<"unsupported">;
-def manifest : Separate<["/", "-"], "manifest">, HelpText<"Used to specify each manifest that need to be processed">, MetaVarName<"manifest">;
+def manifest : Separate<["/", "-"], "manifest">, HelpText<"Used to specify each manifest that need to be processed">, MetaVarName<"manifest">, Group<supported>;
 def identity : Joined<["/", "-"], "identity:">, HelpText<"Not supported">, MetaVarName<"identity">, Group<unsupported>;
 def rgs : Joined<["/", "-"], "rgs:">, HelpText<"Not supported">, MetaVarName<"script">, Group<unsupported>;
 def tlb : Joined<["/", "-"], "tlb:">, HelpText<"Not supported">, MetaVarName<"file">, Group<unsupported>;
 def dll : Joined<["/", "-"], "dll:">, HelpText<"Not supported">, MetaVarName<"dll">, Group<unsupported>;
 def replacements : Joined<["/", "-"], "replacements:">, HelpText<"Not supported">, MetaVarName<"file">, Group<unsupported>;
 def managed_assembly_name : Joined<["/", "-"], "managedassemblyname:">, HelpText<"Not supported">, MetaVarName<"assembly">, Group<unsupported>;
 def no_dependency : Flag<["/", "-"], "nodependency">, HelpText<"Not supported">, Group<unsupported>;
 def category : Flag<["/", "-"], "category">, HelpText<"Not supported">, Group<unsupported>;
-def no_logo : Flag<["/", "-"], "nologo">, HelpText<"No effect as this tool never writes copyright data.  Included for parity">;
-def out : Joined<["/", "-"], "out:">, HelpText<"Name of the output manifest.  If this is skipped and only one manifest is being operated upon by the tool, that manifest is modified in place">, MetaVarName<"manifest">;
+def no_logo : Flag<["/", "-"], "nologo">, HelpText<"No effect as this tool never writes copyright data.  Included for parity">, Group<supported>;
+def out : Joined<["/", "-"], "out:">, HelpText<"Name of the output manifest.  If this is skipped and only one manifest is being operated upon by the tool, that manifest is modified in place">, MetaVarName<"manifest">, Group<supported>;
 def input_resource : Joined<["/", "-"], "inputresource:">, HelpText<"Not supported">, MetaVarName<"file">, Group<unsupported>;
 def output_resource : Joined<["/", "-"], "outputresource:">, HelpText<"Not supported">, MetaVarName<"file">, Group<unsupported>;
 def output_resource_flag : Flag<["/", "-"], "outputresource">, Alias<output_resource>, HelpText<"Not supported">, Group<unsupported>;
 def update_resource : Joined<["/", "-"], "updateresource:">, HelpText<"Not supported">, MetaVarName<"file">, Group<unsupported>;
 def hash_update : Joined<["/", "-"], "hashupdate:">, HelpText<"Not supported">, MetaVarName<"file">, Group<unsupported>;
 def hash_update_flag : Flag<["/", "-"], "hashupdate">, Alias<hash_update>, HelpText<"Not supported">, Group<unsupported>;
 def validate_manifest : Flag<["/", "-"], "validate_manifest">, HelpText<"Not supported">, Group<unsupported>;
 def validate_file_hashes : Joined<["/", "-"], "validate_file_hashes:">, HelpText<"Not supported">, MetaVarName<"">, Group<unsupported>;
 def canonicalize : Flag<["/", "-"], "canonicalize:">, HelpText<"Not supported">, Group<unsupported>;
 def check_for_duplicates : Flag<["/", "-"], "check_for_duplicates:">, HelpText<"Not supported">, Group<unsupported>;
 def make_cdfs : Flag<["/", "-"], "makecdfs:">, HelpText<"Not supported">, Group<unsupported>;
 def verbose : Flag<["/", "-"], "verbose">, HelpText<"Not supported">, Group<unsupported>;
-def help : Flag<["/", "-"], "?">;
-def help_long : Flag<["/", "-"], "help">, Alias<help>;
-def h : Flag<["/", "-"], "h">, Alias<help>;
+def help : Flag<["/", "-"], "?">, Group<supported>;
+def help_long : Flag<["/", "-"], "help">, Alias<help>, Group<supported>;
+def h : Flag<["/", "-"], "h">, Alias<help>, Group<supported>;

llvm/tools/llvm-mt/llvm-mt.cpp

int main(int argc, const char **argv) {
   llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.
 
   CvtResOptTable T;
   unsigned MAI, MAC;
   ArrayRef<const char *> ArgsArr = makeArrayRef(argv + 1, argc);
   opt::InputArgList InputArgs = T.ParseArgs(ArgsArr, MAI, MAC);
 
   for (auto &Arg : InputArgs) {
+    if (!(Arg->getOption().matches(OPT_unsupported) ||
+          Arg->getOption().matches(OPT_supported))) {
+      reportError(Twine("invalid option ") + Arg->getSpelling());
+    }
+  }
+
+  for (auto &Arg : InputArgs) {
     if (Arg->getOption().matches(OPT_unsupported)) {
       outs() << "llvm-mt: ignoring unsupported '" << Arg->getOption().getName()
              << "' option\n";
     }
   }
 
   if (InputArgs.hasArg(OPT_help)) {
     T.PrintHelp(outs(), "mt", "Manifest Tool", false);
int main(int argc, const char **argv) {
   if (InputArgs.hasArg(OPT_out)) {
     OutputFile = InputArgs.getLastArgValue(OPT_out);
   } else if (InputFiles.size() == 1) {
     OutputFile = InputFiles[0];
   } else {
     reportError("no output file specified");
   }
 
-  WindowsManifestMerger Merger;
+  windows_manifest::WindowsManifestMerger Merger;
 
   for (const auto &File : InputFiles) {
     ErrorOr<std::unique_ptr<MemoryBuffer>> ManifestOrErr =
         MemoryBuffer::getFile(File);
     if (!ManifestOrErr)
       reportError(File, ManifestOrErr.getError());
     MemoryBuffer &Manifest = *ManifestOrErr.get();
     error(Merger.merge(Manifest));