From 0b552eb877b887638e8130bb6c982106a76827d8 Mon Sep 17 00:00:00 2001 From: Andrew Ng Date: Mon, 20 Feb 2023 18:00:44 +0000 Subject: MSVC: Embed manifests directly for non-incremental vs_link_exe links This avoids the need to separately execute `mt.exe` to perform the embedding of manifests into the output for non-incremental links. The primary motivation for this change is that this separate execution of `mt.exe` to embed manifests is known to cause intermittent failures due to AV/security scanning. The only change in behavior is that any linker generated manifest will no longer be output as a separate manifest file alongside the output file. Fixes: #24531 --- Source/cmcmd.cxx | 42 ++++++++++++++++----------------- Tests/MSManifest/Subdir/CMakeLists.txt | 5 ++++ Tests/MSManifest/Subdir2/CMakeLists.txt | 10 ++++++++ 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/Source/cmcmd.cxx b/Source/cmcmd.cxx index d57b78b..a2a9e09 100644 --- a/Source/cmcmd.cxx +++ b/Source/cmcmd.cxx @@ -2329,6 +2329,9 @@ bool cmVSLink::Parse(std::vector::const_iterator argBeg, cmSystemTools::Strucmp(arg->c_str(), "/INCREMENTAL") == 0 || cmSystemTools::Strucmp(arg->c_str(), "-INCREMENTAL") == 0) { this->Incremental = true; + } else if (cmSystemTools::Strucmp(arg->c_str(), "/INCREMENTAL:NO") == 0 || + cmSystemTools::Strucmp(arg->c_str(), "-INCREMENTAL:NO") == 0) { + this->Incremental = false; } else if (cmSystemTools::Strucmp(arg->c_str(), "/MANIFEST:NO") == 0 || cmSystemTools::Strucmp(arg->c_str(), "-MANIFEST:NO") == 0) { this->LinkGeneratesManifest = false; @@ -2353,17 +2356,11 @@ bool cmVSLink::Parse(std::vector::const_iterator argBeg, // pass it to the link command. this->ManifestFileRC = intDir + "/manifest.rc"; this->ManifestFileRes = intDir + "/manifest.res"; - } else if (this->UserManifests.empty()) { - // Prior to support for user-specified manifests CMake placed the - // linker-generated manifest next to the binary (as if it were not to be - // embedded) when not linking incrementally. Preserve this behavior. - this->ManifestFile = this->TargetFile + ".manifest"; - this->LinkerManifestFile = this->ManifestFile; - } - if (this->LinkGeneratesManifest) { - this->LinkCommand.emplace_back("/MANIFEST"); - this->LinkCommand.push_back("/MANIFESTFILE:" + this->LinkerManifestFile); + if (this->LinkGeneratesManifest) { + this->LinkCommand.emplace_back("/MANIFEST"); + this->LinkCommand.push_back("/MANIFESTFILE:" + this->LinkerManifestFile); + } } return true; @@ -2497,20 +2494,23 @@ int cmVSLink::LinkIncremental() int cmVSLink::LinkNonIncremental() { - // Run the link command (possibly generates intermediate manifest). - if (!RunCommand("LINK", this->LinkCommand, this->Verbose, FORMAT_DECIMAL)) { - return -1; - } + // Sort out any manifests. + if (this->LinkGeneratesManifest || !this->UserManifests.empty()) { + std::string opt = + std::string("/MANIFEST:EMBED,ID=") + (this->Type == 1 ? '1' : '2'); + this->LinkCommand.emplace_back(opt); - // If we have no manifest files we are done. - if (!this->LinkGeneratesManifest && this->UserManifests.empty()) { - return 0; + for (auto const& m : this->UserManifests) { + opt = "/MANIFESTINPUT:" + m; + this->LinkCommand.emplace_back(opt); + } } - // Run the manifest tool to embed the final manifest in the binary. - std::string mtOut = - "/outputresource:" + this->TargetFile + (this->Type == 1 ? ";#1" : ";#2"); - return this->RunMT(mtOut, false); + // Run the link command. + if (!RunCommand("LINK", this->LinkCommand, this->Verbose, FORMAT_DECIMAL)) { + return -1; + } + return 0; } int cmVSLink::RunMT(std::string const& out, bool notify) diff --git a/Tests/MSManifest/Subdir/CMakeLists.txt b/Tests/MSManifest/Subdir/CMakeLists.txt index 3b4fccc..68c66fe 100644 --- a/Tests/MSManifest/Subdir/CMakeLists.txt +++ b/Tests/MSManifest/Subdir/CMakeLists.txt @@ -5,6 +5,11 @@ if(MSVC AND NOT MSVC_VERSION LESS 1400) add_test(NAME MSManifest.Single COMMAND ${CMAKE_COMMAND} -Dexe=$ -P ${CMAKE_CURRENT_SOURCE_DIR}/check.cmake) + add_executable(MSManifestNonIncremental main.c ${CMAKE_CURRENT_BINARY_DIR}/test.manifest) + set_property(TARGET MSManifestNonIncremental PROPERTY LINK_FLAGS "/INCREMENTAL:NO") + add_test(NAME MSManifest.Single.NonIncremental COMMAND + ${CMAKE_COMMAND} -Dexe=$ + -P ${CMAKE_CURRENT_SOURCE_DIR}/check.cmake) add_executable(MSManifestNone main.c) set_property(TARGET MSManifestNone PROPERTY LINK_FLAGS "/MANIFEST:NO") elseif(WIN32 AND CMAKE_C_COMPILER_ID MATCHES "Clang") diff --git a/Tests/MSManifest/Subdir2/CMakeLists.txt b/Tests/MSManifest/Subdir2/CMakeLists.txt index 0d960ad..bbc70dc 100644 --- a/Tests/MSManifest/Subdir2/CMakeLists.txt +++ b/Tests/MSManifest/Subdir2/CMakeLists.txt @@ -10,4 +10,14 @@ if((MSVC AND NOT MSVC_VERSION LESS 1400) OR (WIN32 AND CMAKE_C_COMPILER_ID MATCH add_test(NAME MSManifest.Multiple COMMAND ${CMAKE_COMMAND} -Dexe=$ -P ${CMAKE_CURRENT_SOURCE_DIR}/check.cmake) + if(MSVC AND NOT MSVC_VERSION LESS 1400) + add_executable(MSMultipleManifestNonIncremental main.c + ${CMAKE_CURRENT_BINARY_DIR}/test_manifest1.manifest + ${CMAKE_CURRENT_BINARY_DIR}/test_manifest2.manifest + ${CMAKE_CURRENT_BINARY_DIR}/test_manifest3.manifest) + set_property(TARGET MSMultipleManifestNonIncremental PROPERTY LINK_FLAGS "/INCREMENTAL:NO") + add_test(NAME MSManifest.Multiple.NonIncremental COMMAND + ${CMAKE_COMMAND} -Dexe=$ + -P ${CMAKE_CURRENT_SOURCE_DIR}/check.cmake) + endif() endif() -- cgit v0.12