From 3f1a773983e0bd5a14c454536ee60b3949752872 Mon Sep 17 00:00:00 2001 From: John Parent Date: Fri, 30 Jan 2026 19:02:26 -0500 Subject: [PATCH 1/2] Relocation: Change strategy Previously we were searching a set of prefixes for DLLs during relocation. If we found a dll that matched the dll we were looking for (based on file name) we performed relocation based on that path. This is both dangerous and extraneous. This is dangerous as mutli config layouts may have the same binary with the same name in mutliple different paths for different configs or variations. Since we were previously only checking the filename, this could lead to a false positive detection and bad relocation not detected until runtime. This is extraneous as we should never need to search. We have the dll locations before and after relocation, whether from the stage to install prefix or from buildcache to buildcache, so rather than a filesystem search, we can have a linear time operation where we search through a list of relocation old->new prefix mappings. Spack core will set an environment variable of the structure: old_prefix|new_prefix and the compiler wrapper now composes a map out of that list and then PE files looking to relocate their internal dll references get a constant time lookup. Signed-off-by: John Parent --- Makefile | 70 ++++++++++++++++++++--------------- src/utils.cxx | 63 ++++++++++++++++++++++++++++++- src/utils.h | 14 +++++++ test/setup_and_drive_test.bat | 1 - 4 files changed, 117 insertions(+), 31 deletions(-) diff --git a/Makefile b/Makefile index bc2f75f..b45889c 100644 --- a/Makefile +++ b/Makefile @@ -72,9 +72,10 @@ install : cl.exe mklink $(PREFIX)\relocate.exe $(PREFIX)\cl.exe setup_test: cl.exe - echo "-------------------" - echo "Running Test Setup" - echo "-------------------" + @echo \n + @echo ------------------- + @echo Running Test Setup + @echo ------------------- -@ if NOT EXIST "tmp\test" mkdir "tmp\test" cd tmp\test copy ..\..\cl.exe cl.exe @@ -86,9 +87,9 @@ setup_test: cl.exe # * space in a path - preserved by quoted arguments # * escaped quoted arguments build_and_check_test_sample : setup_test - echo "--------------------" - echo "Building Test Sample" - echo "--------------------" + @echo -------------------- + @echo Building Test Sample + @echo -------------------- cd tmp\test cl /c /EHsc "..\..\test\src file\calc.cxx" /DCALC_EXPORTS /DCALC_HEADER="\"calc header/calc.h\"" /I ..\..\test\include cl /c /EHsc ..\..\test\main.cxx /I ..\..\test\include @@ -100,9 +101,10 @@ build_and_check_test_sample : setup_test # Test basic wrapper behavior - did the absolute path to the DLL get injected # into the executable test_wrapper : build_and_check_test_sample - echo "--------------------" - echo "Running Wrapper Test" - echo "--------------------" + @echo \n + @echo -------------------- + @echo Running Wrapper Test + @echo -------------------- cd tmp move test\tester.exe .\tester.exe .\tester.exe @@ -116,23 +118,25 @@ test_wrapper : build_and_check_test_sample # Test relocating an executable - re-write internal paths to dlls test_relocate_exe: build_and_check_test_sample - echo "--------------------------" - echo "Running Relocate Exe Test" - echo "--------------------------" + @echo \n + @echo -------------------------- + @echo Running Relocate Exe Test + @echo -------------------------- cd tmp\test -@ if NOT EXIST "relocate.exe" mklink relocate.exe cl.exe move calc.dll ..\calc.dll - relocate.exe --pe tester.exe --deploy --full - relocate.exe --pe tester.exe --export --full + SET SPACK_RELOCATE_PATH=$(MAKEDIR)\tmp\test\calc.dll|$(MAKEDIR)\tmp\calc.dll + relocate.exe --pe tester.exe --full tester.exe move ..\calc.dll calc.dll cd ../.. # Test relocating a dll - re-write import library test_relocate_dll: build_and_check_test_sample - echo "--------------------------" - echo "Running Relocate DLL test" - echo "--------------------------" + @echo \n + @echo -------------------------- + @echo Running Relocate DLL test + @echo -------------------------- cd tmp/test -@ if NOT EXIST "relocate.exe" mklink relocate.exe cl.exe cd .. @@ -171,18 +175,20 @@ build_zerowrite_test: test\writezero.obj link $(LFLAGS) $** $(API_LIBS) /out:writezero.exe test_zerowrite: build_zerowrite_test - echo "-----------------------" - echo "Running zerowrite test" - echo "-----------------------" + @echo \n + @echo ----------------------- + @echo Running zerowrite test + @echo ----------------------- set SPACK_CC_TMP=%SPACK_CC% set SPACK_CC=$(MAKEDIR)\writezero.exe cl /c EHsc "test\src file\calc.cxx" set SPACK_CC=%SPACK_CC_TMP% test_long_paths: build_and_check_test_sample - echo "------------------------" - echo "Running long paths test" - echo "------------------------" + @echo \n + @echo ------------------------ + @echo Running long paths test + @echo ------------------------ mkdir tmp\tmp\verylongdirectoryname\evenlongersubdirectoryname xcopy /E test\include tmp\tmp\verylongdirectoryname\evenlongersubdirectoryname xcopy /E "test\src file" tmp\tmp\verylongdirectoryname\evenlongersubdirectoryname @@ -199,9 +205,10 @@ test_long_paths: build_and_check_test_sample cd ../../../.. test_relocate_long_paths: test_long_paths - echo "---------------------------------" - echo "Running relocate logn paths test" - echo "---------------------------------" + @echo \n + @echo --------------------------------- + @echo Running relocate logn paths test + @echo --------------------------------- cd tmp\tmp\verylongdirectoryname\evenlongersubdirectoryname -@ if NOT EXIST "relocate.exe" mklink relocate.exe cl.exe cd .. @@ -217,9 +224,10 @@ test_relocate_long_paths: test_long_paths cd ../../../.. test_exe_with_exports: - echo ------------------------------ - echo Running exe with exports test - echo ------------------------------ + @echo \n + @echo ------------------------------ + @echo Running exe with exports test + @echo ------------------------------ mkdir tmp\test\exe_with_exports xcopy /E test\include tmp\test\exe_with_exports xcopy /E "test\src file" tmp\test\exe_with_exports @@ -240,6 +248,10 @@ test_exe_with_exports: cd ../../.. test_def_file_name_override: + @echo + @echo ------------------------------------ + @echo Running Def file name override test + @echo ------------------------------------ mkdir tmp\test\def\def_override xcopy /E test\include tmp\test\def\def_override xcopy /E "test\src file" tmp\test\def\def_override diff --git a/src/utils.cxx b/src/utils.cxx index 25809b4..52f323d 100644 --- a/src/utils.cxx +++ b/src/utils.cxx @@ -875,13 +875,74 @@ std::string LibraryFinder::Finder(const std::string& pth, return std::string(); } +PathRelocator::PathRelocator() { + this->new_prefix_ = GetSpackEnv("SPACK_INSTALL_PREFIX"); + this->parseRelocate(); +} + +void PathRelocator::parseRelocate() { + const std::string relocations = GetSpackEnv("SPACK_RELOCATE_PATH"); + // relocations is a semi colon separated list of + // | separated pairs, of old_prefix|new_prefix + // where old prefix is either the stage or the + // old install root and new prefix is the dll location in the + // install tree or just the new install prefix + if (relocations.empty()) { + return; + } + const StrList mappings = split(relocations, ";"); + for (const auto& pair : mappings) { + const StrList old_new = split(pair, "|"); + const std::string& old = old_new[0]; + const std::string& new_ = old_new[1]; + this->old_new_map[old] = new_; + if (endswith(old, ".dll") || endswith(old, ".exe")) { + this->bc_ = false; + } + } +} + +std::string PathRelocator::getRelocation(std::string const& pe) { + if (this->bc_) { + return this->relocateBC(pe); + } + return this->relocateStage(pe); +} + +std::string PathRelocator::relocateBC(std::string const& pe) { + for (auto& root : this->old_new_map) { + if (startswith(pe, root.first)) { + std::array rel_root; + if (PathRelativePathToW( + &rel_root[0], ConvertASCIIToWide(root.first).c_str(), + FILE_ATTRIBUTE_DIRECTORY, ConvertASCIIToWide(pe).c_str(), + FILE_ATTRIBUTE_NORMAL) != 0) { + // we have the pe's relative root in the old + // prefix, slap the new prefix on it and return + std::string const real_rel( + ConvertWideToASCII(std::wstring(&rel_root[0]))); + return join({root.second, real_rel}, "\\"); + } + } + } + return std::string(); +} + +std::string PathRelocator::relocateStage(std::string const& pe) { + try { + std::string prefix_loc = this->old_new_map.at(pe); + return prefix_loc; + } catch (std::out_of_range& e) { + return std::string(); + } +} + namespace { std::vector system_locations = { "api-ms-", "ext-ms-", "ieshims", "emclient", "devicelock", "wpax", "vcruntime", "WINDOWS", "system32", "KERNEL32", "WS2_32", "dbghelp", "bcrypt", "ADVAPI32", "SHELL32", "CRYPT32", "USER32", "ole32", "OLEAUTH32"}; - } bool LibraryFinder::IsSystem(const std::string& pth) { diff --git a/src/utils.h b/src/utils.h index f0eb71b..f738662 100644 --- a/src/utils.h +++ b/src/utils.h @@ -293,6 +293,20 @@ class LibraryFinder { void EvalSearchPaths(); }; +class PathRelocator { + private: + bool bc_; + std::string new_prefix_; + std::map old_new_map; + std::string relocateBC(std::string const& pe); + std::string relocateStage(std::string const& pe); + void parseRelocate(); + + public: + PathRelocator(); + std::string getRelocation(std::string const& pe); +}; + using ScopedLocalInfo = std::unique_ptr; using ScopedSid = std::unique_ptr; diff --git a/test/setup_and_drive_test.bat b/test/setup_and_drive_test.bat index 88285fd..2ae5e83 100644 --- a/test/setup_and_drive_test.bat +++ b/test/setup_and_drive_test.bat @@ -17,6 +17,5 @@ SET SPACK_DEBUG_LOG_ID=TEST SET SPACK_SHORT_SPEC=test%msvc SET SPACK_SYSTEM_DIRS=%PATH% SET SPACK_MANAGED_DIRS=%CD%\tmp -SET SPACK_RELOCATE_PATH=%CD%\tmp nmake test \ No newline at end of file From 594c39f7dfbbe55f06be31b8dcf3d595eb6c347a Mon Sep 17 00:00:00 2001 From: John Parent Date: Thu, 9 Apr 2026 19:17:41 -0400 Subject: [PATCH 2/2] add array header Signed-off-by: John Parent --- src/utils.cxx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/utils.cxx b/src/utils.cxx index 52f323d..d2eac6c 100644 --- a/src/utils.cxx +++ b/src/utils.cxx @@ -39,6 +39,7 @@ #include #include #include +#include #include "shlwapi.h" #include "PathCch.h"