From: Nico Weber Date: Tue, 24 Aug 2021 14:19:21 +0000 (-0400) Subject: [lld/COFF] Improve handling of the /manifestdependency: flag X-Git-Tag: upstream/15.0.7~33030 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=400a1de3ac4583794797852aab49aaa821c95b7d;p=platform%2Fupstream%2Fllvm.git [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 --- diff --git a/lld/COFF/Config.h b/lld/COFF/Config.h index 002d128..feaad55 100644 --- a/lld/COFF/Config.h +++ b/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 section; // Options for manifest files. - ManifestKind manifest = No; + ManifestKind manifest = Default; int manifestID = 1; - StringRef manifestDependency; + llvm::SetVector manifestDependencies; bool manifestUAC = true; std::vector manifestInput; StringRef manifestLevel = "'asInvoker'"; diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index 5f6bc91..fd9ae7f 100644 --- a/lld/COFF/Driver.cpp +++ b/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 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 argsArr) { if (Optional 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 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 diff --git a/lld/COFF/DriverUtils.cpp b/lld/COFF/DriverUtils.cpp index b5abe8b..9775e37 100644 --- a/lld/COFF/DriverUtils.cpp +++ b/lld/COFF/DriverUtils.cpp @@ -385,10 +385,10 @@ static std::string createDefaultXml() { << " \n" << " \n"; } - if (!config->manifestDependency.empty()) { + for (auto manifestDependency : config->manifestDependencies) { os << " \n" << " \n" - << " manifestDependency << " />\n" + << " \n" << " \n" << " \n"; } @@ -408,7 +408,8 @@ static std::string createManifestXmlWithInternalMt(StringRef defaultXml) { for (StringRef filename : config->manifestInput) { std::unique_ptr 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)); diff --git a/lld/test/COFF/Inputs/manifestdependency-drectve.yaml b/lld/test/COFF/Inputs/manifestdependency-drectve.yaml new file mode 100644 index 0000000..7c6b447 --- /dev/null +++ b/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: +... diff --git a/lld/test/COFF/linkrepro-manifest.test b/lld/test/COFF/linkrepro-manifest.test index 2640588..ab198af 100644 --- a/lld/test/COFF/linkrepro-manifest.test +++ b/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 diff --git a/lld/test/COFF/manifest.test b/lld/test/COFF/manifest.test index f3a388f..4910600 100644 --- a/lld/test/COFF/manifest.test +++ b/lld/test/COFF/manifest.test @@ -78,3 +78,92 @@ NOUACNODEP: NOUACNODEP: NOUACNODEP: + +# 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: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: +SEVERALDEPS: + +# /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 |.. . . . . | +EMBED: 0100: 203C2F72 65717565 73746564 50726976 | . . . . . | +EMBED: 0160: 20202020 3C617373 656D626C 79496465 | . . . . <| +EMBED: 01C0: 64657065 6E64656E 74417373 656D626C |dependentAssembl| +EMBED: 01D0: 793E0A20 20202020 203C6173 73656D62 |y>. . . ..| +EMBED: )