From 542c55eb9294756d44ee61ce0cdfc83f11a25322 Mon Sep 17 00:00:00 2001 From: John Parent Date: Wed, 6 May 2026 17:11:12 -0400 Subject: [PATCH 1/2] ld.cxx: res and rc variables were accidentally swapped. Unswap them Signed-off-by: John Parent --- src/ld.cxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ld.cxx b/src/ld.cxx index cb6d124..5df1c7c 100644 --- a/src/ld.cxx +++ b/src/ld.cxx @@ -51,12 +51,12 @@ DWORD LdInvocation::InvokeToolchain() { if (ret_code != 0) { return ret_code; } - + debug("hello!"); if(!DeleteFile2A(rc_file.c_str(), FILE_FLAG_DISALLOW_PATH_REDIRECTS)) { throw std::system_error(static_cast(::GetLastError()), std::system_category(), "Failed to remove intermediate rc file"); } - + debug("hello!"); // We're creating a PE, we need to create an appropriate import lib std::string const imp_lib_name = link_run.get_implib_name(); @@ -196,8 +196,8 @@ std::string LdInvocation::createRC(LinkerInvocation& link_run) { base_res_file_name = "spack-" + base_res_file_name; } - const std::string res_file_name = join({rc_tmp_dir, base_rc_file_name}, "\\"); - const std::string rc_file_name = join({rc_tmp_dir, base_res_file_name}, "\\"); + const std::string res_file_name = join({rc_tmp_dir, base_res_file_name}, "\\"); + const std::string rc_file_name = join({rc_tmp_dir, base_rc_file_name}, "\\"); ExecuteCommand rc_executor("rc", {"/fo" + res_file_name + " " + rc_file_name}); From 0a66548ce6cb8dfc6076b489871b050f0ad8b539 Mon Sep 17 00:00:00 2001 From: John Parent Date: Thu, 7 May 2026 17:18:25 -0400 Subject: [PATCH 2/2] use raii to cleanup the res file It seems like the linker is allowing the OS to cleanup the handle to the resource file we create to inject the Spack stage path into a given dll This means our cleanup is racey. Ensure it happens and is not racey using raii, falling back to a stderr warning message if we fail to clean vs an exception. Signed-off-by: John Parent --- src/ld.cxx | 48 +++++++++++++++++++++++++++++++++++------------- src/ld.h | 13 ++++++++++++- 2 files changed, 47 insertions(+), 14 deletions(-) diff --git a/src/ld.cxx b/src/ld.cxx index 5df1c7c..3f267c8 100644 --- a/src/ld.cxx +++ b/src/ld.cxx @@ -4,13 +4,22 @@ * SPDX-License-Identifier: (Apache-2.0 OR MIT) */ #include "ld.h" +#include +#include #include +#include +#include +#include #include #include #include #include #include #include +#include +#include +#include +#include #include "coff_parser.h" #include "coff_reader_writer.h" #include "linker_invocation.h" @@ -18,6 +27,7 @@ #include "toolchain.h" #include "utils.h" + void LdInvocation::LoadToolchainDependentSpackVars(SpackEnvState& spackenv) { this->command = spackenv.SpackLD; } @@ -29,7 +39,7 @@ DWORD LdInvocation::InvokeToolchain() { // understand what we'll be doing LinkerInvocation link_run(this->inputs); link_run.Parse(); - std::string rc_file; + std::unique_ptr rc_file; try { // Run resource compiler to create // Resource for id'ing binary when relocating its import library @@ -44,19 +54,13 @@ DWORD LdInvocation::InvokeToolchain() { // file the linker sees (or is referenced in the case of an rsp) // otherwise this resource file will dictate the binairies // name, which will break client expectations - this->inputs.push_back(rc_file); + this->inputs.push_back(rc_file->getRC()); // Run base linker invocation to produce initial // dll and import library DWORD const ret_code = ToolChainInvocation::InvokeToolchain(); if (ret_code != 0) { return ret_code; } - debug("hello!"); - if(!DeleteFile2A(rc_file.c_str(), FILE_FLAG_DISALLOW_PATH_REDIRECTS)) { - throw std::system_error(static_cast(::GetLastError()), - std::system_category(), "Failed to remove intermediate rc file"); - } - debug("hello!"); // We're creating a PE, we need to create an appropriate import lib std::string const imp_lib_name = link_run.get_implib_name(); @@ -166,7 +170,8 @@ DWORD LdInvocation::InvokeToolchain() { return ret_code; } -std::string LdInvocation::createRC(LinkerInvocation& link_run) { + +std::unique_ptr LdInvocation::createRC(LinkerInvocation& link_run) { const std::string pe_stage_name = link_run.get_out(); const std::string template_base = "spack SPACKRESOURCE\n" @@ -180,9 +185,9 @@ std::string LdInvocation::createRC(LinkerInvocation& link_run) { throw std::system_error(static_cast(::GetLastError()), std::system_category(), "Failed to get TEMP PATH"); } - std::string rc_tmp_dir = join({std::string(temp_dir_buffer.data()), std::to_string(_getpid())}, ""); - if(!CreateDirectoryA(rc_tmp_dir.c_str(), NULL)){ - DWORD err = ::GetLastError(); + const std::string rc_tmp_dir = join({std::string(temp_dir_buffer.data()), std::to_string(_getpid())}, ""); + if(!CreateDirectoryA(rc_tmp_dir.c_str(), nullptr)){ + const DWORD err = ::GetLastError(); if (err != ERROR_ALREADY_EXISTS) { throw std::system_error(static_cast(err), std::system_category(), "Failed to make directory"); @@ -229,5 +234,22 @@ std::string LdInvocation::createRC(LinkerInvocation& link_run) { throw std::system_error(static_cast(::GetLastError()), std::system_category(), "Failed to remove intermediate rc file"); } - return res_file_name; + std::unique_ptr res_file_ptr = std::make_unique(res_file_name); + return res_file_ptr; +} + + +RCFileManager::RCFileManager(std::string file) { + this->rc_file_ = std::move(file); +} + +RCFileManager::~RCFileManager(){ + if(!DeleteFile2A(this->rc_file_.c_str(), FILE_FLAG_DISALLOW_PATH_REDIRECTS)) { + std::cerr << std::system_error(static_cast(::GetLastError()), + std::system_category(), "Failed to remove intermediate rc file").what() << "\n"; + } +} + +const std::string& RCFileManager::getRC() { + return this->rc_file_; } diff --git a/src/ld.h b/src/ld.h index 191aaba..93abfd3 100644 --- a/src/ld.h +++ b/src/ld.h @@ -9,6 +9,17 @@ #include "toolchain.h" + +class RCFileManager { +public: + explicit RCFileManager(std::string file); + ~RCFileManager(); + const std::string& getRC(); +private: + std::string rc_file_; +}; + + /** * @brief ClInvocation exposes an interface driving invocations of * link.exe and defines the parameters of the call to said executable @@ -22,5 +33,5 @@ class LdInvocation : public ToolChainInvocation { void LoadToolchainDependentSpackVars(SpackEnvState& spackenv); std::string lang = "link"; ExecuteCommand rpath_executor; - static std::string createRC(LinkerInvocation& link_run); + static std::unique_ptr createRC(LinkerInvocation& link_run); };