[lld/COFF] Improve handling of the /manifestdependency: flag

If multiple /manifestdependency: flags are passed, they are
naively deduped, but after that each of them should have an
effect, instead of just the last one.

Also, /manifestdependency: flags are allowed in .drectve sections
(from `#pragma comment(linker, ...`). To make the interaction between
/manifestdependency: flags enabling manifest by default but
/manifest:no overriding this work, add an explict ManifestKind::Default
state to represent no explicit /manifest flag being passed.
To make /manifestdependency: flags from input file .drectve sections
work with /manifest:embed, delay embedded manifest emission until
after input files have been read.

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

Author: nico

lld/COFF/Config.h

@@ -10,6 +10,7 @@
 #define LLD_COFF_CONFIG_H
 
 #include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/COFF.h"
@@ -91,7 +92,7 @@ enum class ICFLevel {
 
 // Global configuration.
 struct Configuration {
-  enum ManifestKind { SideBySide, Embed, No };
+  enum ManifestKind { Default, SideBySide, Embed, No };
   bool is64() { return machine == AMD64 || machine == ARM64; }
 
   llvm::COFF::MachineTypes machine = IMAGE_FILE_MACHINE_UNKNOWN;
@@ -178,9 +179,9 @@ struct Configuration {
   std::map<StringRef, uint32_t> section;
 
   // Options for manifest files.
-  ManifestKind manifest = No;
+  ManifestKind manifest = Default;
   int manifestID = 1;
-  StringRef manifestDependency;
+  llvm::SetVector<StringRef> manifestDependencies;
   bool manifestUAC = true;
   std::vector<std::string> manifestInput;
   StringRef manifestLevel = "'asInvoker'";

lld/COFF/Driver.cpp

@@ -383,6 +383,7 @@ void LinkerDriver::parseDirectives(InputFile *file) {
   for (StringRef inc : directives.includes)
     addUndefined(inc);
 
+  // https://docs.microsoft.com/en-us/cpp/preprocessor/comment-c-cpp?view=msvc-160
   for (auto *arg : directives.args) {
     switch (arg->getOption().getID()) {
     case OPT_aligncomm:
@@ -404,6 +405,9 @@ void LinkerDriver::parseDirectives(InputFile *file) {
     case OPT_incl:
       addUndefined(arg->getValue());
       break;
+    case OPT_manifestdependency:
+      config->manifestDependencies.insert(arg->getValue());
+      break;
     case OPT_merge:
       parseMerge(arg->getValue());
       break;
@@ -651,15 +655,10 @@ static std::string createResponseFile(const opt::InputArgList &args,
     case OPT_INPUT:
     case OPT_defaultlib:
     case OPT_libpath:
-    case OPT_manifest:
-    case OPT_manifest_colon:
-    case OPT_manifestdependency:
-    case OPT_manifestfile:
-    case OPT_manifestinput:
-    case OPT_manifestuac:
       break;
     case OPT_call_graph_ordering_file:
     case OPT_deffile:
+    case OPT_manifestinput:
     case OPT_natvis:
       os << arg->getSpelling() << quote(rewritePath(arg->getValue())) << '\n';
       break;
@@ -677,6 +676,7 @@ static std::string createResponseFile(const opt::InputArgList &args,
       break;
     }
     case OPT_implib:
+    case OPT_manifestfile:
     case OPT_pdb:
     case OPT_pdbstripped:
     case OPT_out:
@@ -1705,12 +1705,9 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   for (auto *arg : args.filtered(OPT_aligncomm))
     parseAligncomm(arg->getValue());
 
-  // Handle /manifestdependency. This enables /manifest unless /manifest:no is
-  // also passed.
-  if (auto *arg = args.getLastArg(OPT_manifestdependency)) {
-    config->manifestDependency = arg->getValue();
-    config->manifest = Configuration::SideBySide;
-  }
+  // Handle /manifestdependency.
+  for (auto *arg : args.filtered(OPT_manifestdependency))
+    config->manifestDependencies.insert(arg->getValue());
 
   // Handle /manifest and /manifest:
   if (auto *arg = args.getLastArg(OPT_manifest, OPT_manifest_colon)) {
@@ -1873,10 +1870,6 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
     if (Optional<StringRef> path = findLib(arg->getValue()))
       enqueuePath(*path, false, false);
 
-  // Windows specific -- Create a resource file containing a manifest file.
-  if (config->manifest == Configuration::Embed)
-    addBuffer(createManifestRes(), false, false);
-
   // Read all input files given via the command line.
   run();
 
@@ -2230,8 +2223,14 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
     c->setAlignment(std::max(c->getAlignment(), alignment));
   }
 
-  // Windows specific -- Create a side-by-side manifest file.
-  if (config->manifest == Configuration::SideBySide)
+  // Windows specific -- Create an embedded or side-by-side manifest.
+  // /manifestdependency: enables /manifest unless an explicit /manifest:no is
+  // also passed.
+  if (config->manifest == Configuration::Embed)
+    addBuffer(createManifestRes(), false, false);
+  else if (config->manifest == Configuration::SideBySide ||
+           (config->manifest == Configuration::Default &&
+            !config->manifestDependencies.empty()))
     createSideBySideManifest();
 
   // Handle /order. We want to do this at this moment because we

lld/COFF/DriverUtils.cpp

@@ -385,10 +385,10 @@ static std::string createDefaultXml() {
        << "    </security>\n"
        << "  </trustInfo>\n";
   }
-  if (!config->manifestDependency.empty()) {
+  for (auto manifestDependency : config->manifestDependencies) {
     os << "  <dependency>\n"
        << "    <dependentAssembly>\n"
-       << "      <assemblyIdentity " << config->manifestDependency << " />\n"
+       << "      <assemblyIdentity " << manifestDependency << " />\n"
        << "    </dependentAssembly>\n"
        << "  </dependency>\n";
   }
@@ -408,7 +408,8 @@ static std::string createManifestXmlWithInternalMt(StringRef defaultXml) {
   for (StringRef filename : config->manifestInput) {
     std::unique_ptr<MemoryBuffer> manifest =
         check(MemoryBuffer::getFile(filename));
-    if (auto e = merger.merge(*manifest.get()))
+    // Call takeBuffer to include in /reproduce: output if applicable.
+    if (auto e = merger.merge(driver->takeBuffer(std::move(manifest))))
       fatal("internal manifest tool failed on file " + filename + ": " +
             toString(std::move(e)));
   }
@@ -436,6 +437,11 @@ static std::string createManifestXmlWithExternalMt(StringRef defaultXml) {
   for (StringRef filename : config->manifestInput) {
     e.add("/manifest");
     e.add(filename);
+
+    // Manually add the file to the /reproduce: tar if needed.
+    if (driver->tar)
+      if (auto mbOrErr = MemoryBuffer::getFile(filename))
+        driver->takeBuffer(std::move(*mbOrErr));
   }
   e.add("/nologo");
   e.add("/out:" + StringRef(user.path));

lld/test/COFF/Inputs/manifestdependency-drectve.yaml

@@ -0,0 +1,14 @@
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: []
+sections:
+  - Name:            .drectve
+    Characteristics: [ IMAGE_SCN_LNK_INFO, IMAGE_SCN_LNK_REMOVE ]
+    Alignment:       1
+    # "/manifestdependency:foo='bar'" "/manifestdependency:baz='quux'"
+    # (pipe into `xxd -p -r` to recover raw contents; pipe into `xxd -p` to put
+    # something new here.)
+    SectionData: 222f6d616e6966657374646570656e64656e63793a666f6f3d27626172272220222f6d616e6966657374646570656e64656e63793a62617a3d27717575782722
+symbols:
+...

lld/test/COFF/linkrepro-manifest.test

@@ -1,18 +1,15 @@
 REQUIRES: x86, gnutar, manifest_tool
 
-manifest-related files are compiled to a .res file and the .res file is
-added to the repro archive, instead of adding the inputs.
-
 RUN: rm -rf %t && mkdir %t && cd %t
 RUN: lld-link -entry:__ImageBase -nodefaultlib -linkrepro:%t \
 RUN:     -manifest:embed %p/Inputs/std32.lib -subsystem:console \
 RUN:     -manifestinput:%p/Inputs/manifestinput.test
 
 RUN: tar tf repro.tar | FileCheck --check-prefix=LIST %s
-RUN: tar xOf repro.tar repro/response.txt | FileCheck %s
+RUN: tar xOf repro.tar repro/response.txt \
+RUN:     | FileCheck --implicit-check-not=.manifest.res %s
 
-LIST: manifest.res
+LIST: {{.*}}manifestinput.test
 
-CHECK-NOT: -manifest:
-CHECK: .manifest.res
-CHECK-NOT: -manifest:
+CHECK-DAG: -manifest:embed
+CHECK-DAG: -manifestinput:{{.*}}manifestinput.test

lld/test/COFF/manifest.test

@@ -78,3 +78,92 @@ NOUACNODEP: <?xml version="1.0" standalone="yes"?>
 NOUACNODEP: <assembly xmlns="urn:schemas-microsoft-com:asm.v1"
 NOUACNODEP:           manifestVersion="1.0">
 NOUACNODEP: </assembly>
+
+# Several /manifestdependency: flags are naively dedup'd.
+# RUN: lld-link /out:%t.exe /entry:main \
+# RUN:   /manifestdependency:"foo='bar'" \
+# RUN:   /manifestdependency:"foo='bar'" \
+# RUN:   /manifestdependency:"baz='quux'" \
+# RUN:   %t.obj
+# RUN: FileCheck -check-prefix=SEVERALDEPS %s < %t.exe.manifest
+
+SEVERALDEPS: <?xml version="1.0" standalone="yes"?>
+SEVERALDEPS: <assembly xmlns="urn:schemas-microsoft-com:asm.v1"
+SEVERALDEPS:           manifestVersion="1.0">
+SEVERALDEPS:   <trustInfo>
+SEVERALDEPS:     <security>
+SEVERALDEPS:       <requestedPrivileges>
+SEVERALDEPS:          <requestedExecutionLevel level='asInvoker' uiAccess='false'/>
+SEVERALDEPS:       </requestedPrivileges>
+SEVERALDEPS:     </security>
+SEVERALDEPS:   </trustInfo>
+SEVERALDEPS:   <dependency>
+SEVERALDEPS:     <dependentAssembly>
+SEVERALDEPS:       <assemblyIdentity foo='bar' />
+SEVERALDEPS:     </dependentAssembly>
+SEVERALDEPS:   <dependency>
+SEVERALDEPS:     <dependentAssembly>
+SEVERALDEPS:       <assemblyIdentity baz='quux' />
+SEVERALDEPS:     </dependentAssembly>
+SEVERALDEPS:   </dependency>
+SEVERALDEPS: </assembly>
+
+# /manifestdependency: flags can be in .drectve sections.
+# RUN: yaml2obj %p/Inputs/manifestdependency-drectve.yaml -o %t.dir.obj
+# RUN: rm %t.exe.manifest
+# RUN: lld-link /out:%t.exe /entry:main \
+# RUN:   %t.obj %t.dir.obj
+# RUN: FileCheck -check-prefix=SEVERALDEPS %s < %t.exe.manifest
+
+# /manifestdependency: flags in .drectve sections are ignored with an
+# explicit /manifest:no.
+# RUN: rm %t.exe.manifest
+# RUN: lld-link /out:%t.exe /entry:main /manifest:no \
+# RUN:   %t.obj %t.dir.obj
+# RUN: test ! -e %t.exe.manifest
+
+# Test that /manifestdependency: flags in .drectve sections work
+# with /manifest:embed too.
+# RUN: lld-link /out:%t.exe /entry:main /manifest:embed \
+# RUN:   %t.obj %t.dir.obj
+# RUN: test ! -e %t.exe.manifest
+# RUN: llvm-readobj --coff-resources %t.exe \
+# RUN:     | FileCheck --check-prefix EMBED %s
+
+EMBED: Data (
+EMBED:   0000: 3C3F786D 6C207665 7273696F 6E3D2231  |<?xml version="1|
+EMBED:   0010: 2E302220 7374616E 64616C6F 6E653D22  |.0" standalone="|
+EMBED:   0020: 79657322 3F3E0A3C 61737365 6D626C79  |yes"?>.<assembly|
+EMBED:   0030: 20786D6C 6E733D22 75726E3A 73636865  | xmlns="urn:sche|
+EMBED:   0040: 6D61732D 6D696372 6F736F66 742D636F  |mas-microsoft-co|
+EMBED:   0050: 6D3A6173 6D2E7631 220A2020 20202020  |m:asm.v1".      |
+EMBED:   0060: 20202020 6D616E69 66657374 56657273  |    manifestVers|
+EMBED:   0070: 696F6E3D 22312E30 223E0A20 203C7472  |ion="1.0">.  <tr|
+EMBED:   0080: 75737449 6E666F3E 0A202020 203C7365  |ustInfo>.    <se|
+EMBED:   0090: 63757269 74793E0A 20202020 20203C72  |curity>.      <r|
+EMBED:   00A0: 65717565 73746564 50726976 696C6567  |equestedPrivileg|
+EMBED:   00B0: 65733E0A 20202020 20202020 203C7265  |es>.         <re|
+EMBED:   00C0: 71756573 74656445 78656375 74696F6E  |questedExecution|
+EMBED:   00D0: 4C657665 6C206C65 76656C3D 27617349  |Level level='asI|
+EMBED:   00E0: 6E766F6B 65722720 75694163 63657373  |nvoker' uiAccess|
+EMBED:   00F0: 3D276661 6C736527 2F3E0A20 20202020  |='false'/>.     |
+EMBED:   0100: 203C2F72 65717565 73746564 50726976  | </requestedPriv|
+EMBED:   0110: 696C6567 65733E0A 20202020 3C2F7365  |ileges>.    </se|
+EMBED:   0120: 63757269 74793E0A 20203C2F 74727573  |curity>.  </trus|
+EMBED:   0130: 74496E66 6F3E0A20 203C6465 70656E64  |tInfo>.  <depend|
+EMBED:   0140: 656E6379 3E0A2020 20203C64 6570656E  |ency>.    <depen|
+EMBED:   0150: 64656E74 41737365 6D626C79 3E0A2020  |dentAssembly>.  |
+EMBED:   0160: 20202020 3C617373 656D626C 79496465  |    <assemblyIde|
+EMBED:   0170: 6E746974 7920666F 6F3D2762 61722720  |ntity foo='bar' |
+EMBED:   0180: 2F3E0A20 2020203C 2F646570 656E6465  |/>.    </depende|
+EMBED:   0190: 6E744173 73656D62 6C793E0A 20203C2F  |ntAssembly>.  </|
+EMBED:   01A0: 64657065 6E64656E 63793E0A 20203C64  |dependency>.  <d|
+EMBED:   01B0: 6570656E 64656E63 793E0A20 2020203C  |ependency>.    <|
+EMBED:   01C0: 64657065 6E64656E 74417373 656D626C  |dependentAssembl|
+EMBED:   01D0: 793E0A20 20202020 203C6173 73656D62  |y>.      <assemb|
+EMBED:   01E0: 6C794964 656E7469 74792062 617A3D27  |lyIdentity baz='|
+EMBED:   01F0: 71757578 27202F3E 0A202020 203C2F64  |quux' />.    </d|
+EMBED:   0200: 6570656E 64656E74 41737365 6D626C79  |ependentAssembly|
+EMBED:   0210: 3E0A2020 3C2F6465 70656E64 656E6379  |>.  </dependency|
+EMBED:   0220: 3E0A3C2F 61737365 6D626C79 3E0A      |>.</assembly>.|
+EMBED: )