From 59fa3de4341a5d92db1ad754d4dcea1c9a023888 Mon Sep 17 00:00:00 2001 From: John Parent Date: Fri, 30 Jan 2026 18:33:44 -0500 Subject: [PATCH 1/2] Relocating DLL and their corresponding libs from the stage currently has a pitfall, where post installation, the paths inside import libraries to their corresponding DLLs is no longer valid, as it points to the DLL as it existed at link time, which is the stage. We cannot presume where the DLL will end up in the install tree vs the stage, and we need valid "rpaths" at test time, so we are forced to marshall libs and their dlls post install. Since we cannot 1:1 map a location in an install tree to a location in the stage, we need some other mechanism for aligning these two binaries. Previously we simply used the symbols exported by each, but since import libraries and DLLs can export different sets of symbols, despite the fact they are a import lib/dll pair, we cannot use that approach. Instead, inject the stage path inside the dll via a resource file, so when we go to do relocation post install, we can simply query the stage dll path from the import library, search for a dll with a corresponding entry in the string table, and know we've identified a pair. This PR: Creates a resource descriptor file (res) and injects the current path to the dll (stage time) into the string table with the identifier "59673" (randomly selected number, it just needs to be any consistent identifier) it into the first pass link line that creates the initial dll. The identifier needs to be a numeric, and this is a random one I chose. Signed-off-by: John Parent --- src/ld.cxx | 137 ++++++++++++++++++++++++++++++++++++++++++++------ src/ld.h | 3 ++ src/utils.cxx | 9 ++++ src/utils.h | 8 ++- 4 files changed, 141 insertions(+), 16 deletions(-) diff --git a/src/ld.cxx b/src/ld.cxx index 871e857..a9f45ae 100644 --- a/src/ld.cxx +++ b/src/ld.cxx @@ -6,6 +6,9 @@ #include "ld.h" #include #include +#include +#include +#include #include #include "coff_parser.h" #include "coff_reader_writer.h" @@ -19,20 +22,51 @@ void LdInvocation::LoadToolchainDependentSpackVars(SpackEnvState& spackenv) { } DWORD LdInvocation::InvokeToolchain() { + // Run a pass of the linker + + // First parse the linker command line to + // understand what we'll be doing + LinkerInvocation link_run(this->inputs); + link_run.Parse(); + std::string rc_file; + try { + // Run resource compiler to create + // Resource for id'ing binary when relocating its import library + rc_file = LdInvocation::createRC(link_run); + } catch (const RCCompilerFailure& e) { + return ExitConditions::TOOLCHAIN_FAILURE; + } + + // Add produced RC file to linker CLI to inject ID + // This needs to be at the end of either libs or objs or rsp files + // so long as this RC file is not the first binary + // 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); // Run base linker invocation to produce initial // dll and import library DWORD const ret_code = ToolChainInvocation::InvokeToolchain(); if (ret_code != 0) { return ret_code; } + + // We're creating a PE, we need to create an appropriate import lib + std::string const imp_lib_name = link_run.get_implib_name(); + // Next we want to construct the proper commmand line to // recreate the import library from the same set of obj files // and libs - LinkerInvocation link_run(LdInvocation::ComposeCommandLists( - {this->inputs})); - link_run.Parse(); - // We're creating a PE, we need to create an appropriate import lib - std::string const imp_lib_name = link_run.get_implib_name(); + + // first determine if this link run created the import library + // check if the import library that *might* be produced + // by this run (given input argument construction) + // exists. Multiple link runs could in theory produce the name + // imp lib (or at least with the same name) + // i.e. link /out:perl.exe perl.lib + // and link /out:perl.dll perl.lib /DLL could both in theory + // produce the same import library + // If there is no implib, we don't need to bother // trying to rename if (!fileExists(imp_lib_name)) { @@ -42,6 +76,35 @@ DWORD LdInvocation::InvokeToolchain() { // the concern of this wrapper return 0; } + // There is an imp lib, so + // Check if the imp lib is associated with the link command + // we just ran, if we cannot process the coff file + // we should exit with failure since something is unexpected + { + // Create temp scope to ensure all handles are appropriately deallocated + // since the Coff readers use RAII + CoffReaderWriter existing_coff_reader(imp_lib_name); + CoffParser existing_coff(&existing_coff_reader); + if (!existing_coff.Parse()) { + std::cerr << "Unable to parse coff file: " << imp_lib_name + << " unable to determine import library provenance\n"; + return ExitConditions::COFF_PARSE_FAILURE; + } + std::string const shorter_name = existing_coff.GetName(); + std::string const link_name = basename(link_run.get_out()); + if (shorter_name.empty() || link_name.empty()) { + debug("Cannot determine either PE or COFF names (Pe: " + link_name + + "; Coff: " + shorter_name + ") skipping absolute rename\n"); + } + + if (shorter_name != link_name) { + debug("internal lib name: " + shorter_name + + " Pe name: " + link_name + " are not equivalent"); + return 0; + } + existing_coff_reader.Close(); + } + std::string pe_name; try { pe_name = link_run.get_mangled_out(); @@ -51,17 +114,15 @@ DWORD LdInvocation::InvokeToolchain() { return ExitConditions::NORMALIZE_NAME_FAILURE; } std::string const abs_out_imp_lib_name = imp_lib_name + ".pe-abs.lib"; - std::string const def_file = - link_run.get_def_file().empty() ? " " : ":" + link_run.get_def_file(); - std::string const def = "-def" + def_file; + std::string const def_file = link_run.get_def_file(); + std::string const def = "-def" + (def_file.empty() ? " " : ":" + def_file); std::string piped_args = link_run.get_lib_link_args(); // create command line to generate new import lib - this->rpath_executor = - ExecuteCommand("lib.exe", LdInvocation::ComposeCommandLists({ - {def, piped_args, "-name:" + pe_name, - "-out:" + abs_out_imp_lib_name}, - link_run.get_input_files(), - })); + this->rpath_executor = ExecuteCommand( + "lib.exe", + LdInvocation::ComposeCommandLists({{def, piped_args, "-name:" + pe_name, + "-out:" + abs_out_imp_lib_name}, + link_run.get_input_files()})); this->rpath_executor.Execute(); DWORD const err_code = this->rpath_executor.Join(); if (err_code != 0) { @@ -69,10 +130,13 @@ DWORD LdInvocation::InvokeToolchain() { } CoffReaderWriter coff_reader(abs_out_imp_lib_name); CoffParser coff(&coff_reader); + debug("Parsing COFF file: " + abs_out_imp_lib_name); if (!coff.Parse()) { debug("Failed to parse COFF file: " + abs_out_imp_lib_name); return ExitConditions::COFF_PARSE_FAILURE; } + debug("COFF file parsed"); + debug("Normalizing coff file for name: " + pe_name); if (!coff.NormalizeName(pe_name)) { debug("Failed to normalize name for COFF file: " + abs_out_imp_lib_name); @@ -95,3 +159,48 @@ DWORD LdInvocation::InvokeToolchain() { } return ret_code; } + +std::string LdInvocation::createRC(LinkerInvocation& link_run) { + const std::string pe_stage_name = link_run.get_out(); + const std::string template_base = + "spack SPACKRESOURCE\n" + "BEGIN\n"; + const std::string template_end = "END\n"; + const std::string pe_name = stripLastExt(basename(pe_stage_name)); + const std::string rc_file_name = "spack-" + pe_name + ".rc"; + // This res file name needs to mirror the PE name _exactly_ + // Otherwise the RC file will override the default + // or user set name, violating user expectation + std::string res_file_name = pe_name + ".res"; + if (!link_run.get_rc_files().empty()) { + res_file_name = "spack-" + res_file_name; + } + + ExecuteCommand rc_executor("rc", + {"/fo" + res_file_name + " " + rc_file_name}); + std::ofstream rc_out(rc_file_name); + if (!rc_out) { + std::cerr << "Error: could not open rc file for creation: " + << rc_file_name << "\n"; + throw RCCompilerFailure("Could not open RC file"); + } + std::string abs_out = EnsureValidLengthPath( + CannonicalizePath(MakePathAbsolute(pe_stage_name))); + char* chr_abs_out = new char[abs_out.length() + 1]; + strcpy(chr_abs_out, abs_out.c_str()); + char* padded_path = + pad_path(chr_abs_out, static_cast(abs_out.length()), '\\'); + abs_out = std::string(padded_path, MAX_NAME_LEN); + free(chr_abs_out); + free(padded_path); + abs_out = escape_backslash(abs_out); + rc_out << template_base << " " << '"' << abs_out << '"' << "\n" + << template_end; + rc_out.close(); + rc_executor.Execute(); + DWORD const err_code = rc_executor.Join(); + if (err_code != 0) { + throw RCCompilerFailure("Could not compile RC file"); + } + return res_file_name; +} diff --git a/src/ld.h b/src/ld.h index a9f3a82..191aaba 100644 --- a/src/ld.h +++ b/src/ld.h @@ -5,6 +5,8 @@ */ #pragma once +#include "linker_invocation.h" + #include "toolchain.h" /** @@ -20,4 +22,5 @@ class LdInvocation : public ToolChainInvocation { void LoadToolchainDependentSpackVars(SpackEnvState& spackenv); std::string lang = "link"; ExecuteCommand rpath_executor; + static std::string createRC(LinkerInvocation& link_run); }; diff --git a/src/utils.cxx b/src/utils.cxx index a6ba900..39772fc 100644 --- a/src/utils.cxx +++ b/src/utils.cxx @@ -33,11 +33,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include "shlwapi.h" @@ -1262,6 +1264,13 @@ char const* SFNProcessingError::what() const { return exception::what(); } +RCCompilerFailure::RCCompilerFailure(char const* const message) + : std::runtime_error(message) {} + +char const* RCCompilerFailure::what() const { + return exception::what(); +} + FileIOError::FileIOError(char const* const message) : std::runtime_error(message) {} diff --git a/src/utils.h b/src/utils.h index 853a4ad..a868e87 100644 --- a/src/utils.h +++ b/src/utils.h @@ -383,12 +383,16 @@ class SFNProcessingError : public std::runtime_error { virtual char const* what() const; }; +class RCCompilerFailure : public std::runtime_error { + public: + RCCompilerFailure(char const* const message); + virtual char const* what() const; +}; + class FileIOError : public std::runtime_error { public: FileIOError(char const* const message); virtual char const* what() const; }; - - static bool DEBUG = false; From 8dcfe60f265ac318ae62c5276f1cfb1d4619c504 Mon Sep 17 00:00:00 2001 From: John Parent Date: Tue, 28 Apr 2026 15:00:50 -0400 Subject: [PATCH 2/2] remaining typo Signed-off-by: John Parent --- src/ld.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ld.cxx b/src/ld.cxx index a9f45ae..13856c7 100644 --- a/src/ld.cxx +++ b/src/ld.cxx @@ -185,7 +185,7 @@ std::string LdInvocation::createRC(LinkerInvocation& link_run) { throw RCCompilerFailure("Could not open RC file"); } std::string abs_out = EnsureValidLengthPath( - CannonicalizePath(MakePathAbsolute(pe_stage_name))); + CanonicalizePath(MakePathAbsolute(pe_stage_name))); char* chr_abs_out = new char[abs_out.length() + 1]; strcpy(chr_abs_out, abs_out.c_str()); char* padded_path =