From 101e2e231c934a2b579ea1a5181708442640906f Mon Sep 17 00:00:00 2001 From: John Parent Date: Fri, 30 Jan 2026 18:33:44 -0500 Subject: [PATCH 01/10] Utils: Expand api Adds support for * Path canonicalization * Making paths absolute * Add path padding character to padding signature * Stripping quotes * Backslash escaping * Corrects regex "match" bug * Adds error handling to SFN name processing * Adds file creation to sfn processing so sfns can be generated for "non existent" files * Adds method to validate a path is the proper length * Updates path padding to leverage the above api additions * Corrects a missing return statement * Adds new error types for current and future use Signed-off-by: John Parent --- src/utils.cxx | 135 +++++++++++++++++++++++++++++++++++++++++++------- src/utils.h | 32 +++++++++++- 2 files changed, 148 insertions(+), 19 deletions(-) diff --git a/src/utils.cxx b/src/utils.cxx index 6bb8c6c..5d7c319 100644 --- a/src/utils.cxx +++ b/src/utils.cxx @@ -6,12 +6,14 @@ #include "utils.h" #include #include +#include #include #include #include #include #include #include +#include #include #include #include @@ -27,12 +29,17 @@ #include #include #include +#include #include +#include #include #include #include +#include #include +#include #include "shlwapi.h" +#include "PathCch.h" ////////////////////////////////////////////////////////// // String helper methods adding cxx20 features to cxx14 // @@ -181,6 +188,17 @@ std::string lstrip(const std::string& str, const std::string& substr) { return str.substr(substr.size(), str.size()); } +/** + * Strips double quotes from front and back of string + * + * Returns str with no leading or trailing quotes + * + * Note: removes only one set of quotes + */ +std::string stripquotes(const std::string& str) { + return strip(lstrip(str, "\""), "\""); +} + /** * combines list of strings into one string joined on join_char */ @@ -337,7 +355,7 @@ std::string regexMatch( if (!std::regex_match(searchDomain, match, reg, flag)) { result_str = std::string(); } else { - result_str = match.str(); + result_str = match.str(1); } return result_str; } @@ -529,7 +547,8 @@ void replace_path_characters(char* path, size_t len) { * null terminators. * \param bsize the lengh of the padding to add */ -char* pad_path(const char* pth, DWORD str_size, DWORD bsize) { +char* pad_path(const char* pth, DWORD str_size, char padding_char, + DWORD bsize) { // If str_size > bsize we get inappropriate conversion // from signed to unsigned if (str_size > bsize) { @@ -543,13 +562,26 @@ char* pad_path(const char* pth, DWORD str_size, DWORD bsize) { padded_path[i] = pth[j]; ++j; } else { - padded_path[i] = '|'; + padded_path[i] = padding_char; } } padded_path[bsize] = '\0'; return padded_path; } +std::string escape_backslash(const std::string& path) { + std::string escaped; + escaped.reserve(path.length() * 2); + for (char const c : path) { + if (c == '\\') { + escaped += "\\\\"; + } else { + escaped += c; + } + } + return escaped; +} + /** * Given a padded library path, return how much the path * has been padded @@ -591,11 +623,37 @@ std::string getSFN(const std::string& path) { // Use "disable string parsing" prefix in case // the path is too long std::string const escaped = R"(\\?\)" + path; + // We cannot get the sfn for a path that doesn't exist + // if we find that the sfn we're looking for doesn't exist + // create a stub of the file, and allow the subsequent + // commands to overwrite it + if (!PathFileExistsA(path.c_str())) { + HANDLE h_file = CreateFileA(path.c_str(), GENERIC_WRITE, 0, nullptr, + CREATE_NEW, FILE_ATTRIBUTE_NORMAL, nullptr); + if (h_file == INVALID_HANDLE_VALUE) { + debug("File " + path + + " does not exist, nor can it be created, unable to " + "compute SFN\n"); + CloseHandle(h_file); + return std::string(); + } + CloseHandle(h_file); + } // Get SFN length so we can create buffer DWORD const sfn_size = GetShortPathNameA(escaped.c_str(), NULL, 0); //NOLINT char* sfn = new char[sfn_size + 1]; - GetShortPathNameA(escaped.c_str(), sfn, escaped.length()); + DWORD const res = GetShortPathNameA(escaped.c_str(), sfn, escaped.length()); + if (!res) { + + std::cerr << "Failed to process short name for " << path + << " Error: " << reportLastError() << "\n"; + } + if (!sfn && res) { + // buffer was too small + debug("buffer too small; had: " + std::to_string(sfn_size) + + " needed: " + std::to_string(res)); + } // sfn is null terminated per win32 api // Ensure we strip out the disable string parsing prefix std::string s_sfn = lstrip(sfn, R"(\\?\)"); @@ -623,6 +681,44 @@ std::string short_name(const std::string& path) { return new_abs_out; } +std::string MakePathAbsolute(const std::string& path) { + if (IsPathAbsolute(path)) { + return path; + } + // relative paths, assume they're relative to the CWD of the linker (as they have to be) + return join({GetCWD(), path}, "\\"); +} + +std::string CannonicalizePath(const std::string& path) { + std::wstring const wpath = ConvertASCIIToWide(path); + wchar_t canonicalized_path[PATHCCH_MAX_CCH]; + const size_t buffer_size = ARRAYSIZE(canonicalized_path); + + HRESULT const status = PathCchCanonicalizeEx( + canonicalized_path, buffer_size, wpath.c_str(), + PATHCCH_ALLOW_LONG_PATHS // Flags for long path support + ); + + if (!SUCCEEDED(status)) { + std::stringstream status_report; + status_report << "Cannot canonicalize path " + path + " error: " + << std::hex << status; + throw NameTooLongError(status_report.str().c_str()); + } + return ConvertWideToASCII(canonicalized_path); +} + +std::string EnsureValidLengthPath(const std::string& path) { + std::string proper_length_path = path; + if (path.length() > MAX_NAME_LEN) { + // Name is too long we need to attempt to shorten + std::string const short_path = short_name(path); + // If new, shortened path is too long, bail + proper_length_path = short_path; + } + return proper_length_path; +} + /** * Mangles a string representing a path to have no path characters * instead path characters (i.e. \\, :, etc) are replaced with @@ -633,19 +729,10 @@ std::string short_name(const std::string& path) { std::string mangle_name(const std::string& name) { std::string abs_out; std::string mangled_abs_out; - if (IsPathAbsolute(name)) { - abs_out = name; - } else { - // relative paths, assume they're relative to the CWD of the linker (as they have to be) - abs_out = join({GetCWD(), name}, "\\"); - } + abs_out = MakePathAbsolute(name); + abs_out = CannonicalizePath(abs_out); // Now that we have the full path, check size - if (abs_out.length() > MAX_NAME_LEN) { - // Name is too long we need to attempt to shorten - std::string const new_abs_out = short_name(abs_out); - // If new, shortened path is too long, bail - abs_out = new_abs_out; - } + abs_out = EnsureValidLengthPath(abs_out); char* chr_abs_out = new char[abs_out.length() + 1]; strcpy(chr_abs_out, abs_out.c_str()); replace_path_characters(chr_abs_out, abs_out.length()); @@ -688,7 +775,7 @@ bool SpackInstalledLib(const std::string& lib) { return false; } std::string const stripped_lib = strip_padding(lib); - startswith(stripped_lib, prefix); + return startswith(stripped_lib, prefix); } LibraryFinder::LibraryFinder() : search_vars{"SPACK_RELOCATE_PATH"} {} @@ -846,4 +933,18 @@ NameTooLongError::NameTooLongError(char const* const message) char const* NameTooLongError::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) {} + +char const* FileIOError::what() const { + return exception::what(); } \ No newline at end of file diff --git a/src/utils.h b/src/utils.h index b04d929..377ee91 100644 --- a/src/utils.h +++ b/src/utils.h @@ -16,6 +16,9 @@ #include #include #include +#include +#include +#include #include "version.h" @@ -52,7 +55,8 @@ enum ExitConditions { LIB_REMOVE_FAILURE, NORMALIZE_NAME_FAILURE, COFF_PARSE_FAILURE, - FILE_RENAME_FAILURE + FILE_RENAME_FAILURE, + CANNOT_OPEN_FILE_FAILURE }; typedef std::vector StrList; @@ -95,6 +99,9 @@ std::string strip(const std::string& s, const std::string& substr); //Strips substr of LHS of the larger string std::string lstrip(const std::string& s, const std::string& substr); +//Strips off leading and trailing quotes +std::string stripquotes(const std::string& str); + // Joins vector of strings by join character std::string join(const StrList& args, const std::string& join_char = " "); @@ -186,9 +193,14 @@ std::string short_name(const std::string& path); std::string mangle_name(const std::string& name); +std::string CannonicalizePath(const std::string& path); + int get_padding_length(const std::string& name); -char* pad_path(const char* pth, DWORD str_size, DWORD bsize = MAX_NAME_LEN); +char* pad_path(const char* pth, DWORD str_size, char padding_char = '|', + DWORD bsize = MAX_NAME_LEN); + +std::string escape_backslash(const std::string& path); void replace_path_characters(char* path, size_t len); @@ -196,6 +208,10 @@ void replace_special_characters(char* mangled, size_t len); bool SpackInstalledLib(const std::string& lib); +std::string MakePathAbsolute(const std::string& path); + +std::string EnsureValidLengthPath(const std::string& path); + // File and File handle helpers // /** * @brief Returns boolean indicating whether @@ -274,4 +290,16 @@ class NameTooLongError : 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 634fd1f44bdaad4865d456a6a591684c623b5e1b Mon Sep 17 00:00:00 2001 From: John Parent Date: Fri, 30 Jan 2026 18:43:22 -0500 Subject: [PATCH 02/10] misc updates Signed-off-by: John Parent --- .github/workflows/ci.yaml | 5 ++--- .gitignore | 7 ++++++- Makefile | 7 +++++-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 9979ea1..7c106b1 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -6,7 +6,6 @@ name: ci on: [pull_request, push] - concurrency: group: ci-${{github.ref}}-${{github.event.pull_request.number || github.run_number}} cancel-in-progress: true @@ -35,5 +34,5 @@ jobs: - uses: actions/upload-artifact@v4 if: always() with: - name: tester - path: tmp/test/tester.exe + name: tests + path: tmp diff --git a/.gitignore b/.gitignore index 6a74ea7..846f9c0 100644 --- a/.gitignore +++ b/.gitignore @@ -14,9 +14,14 @@ *.exp *.hex *.def +*.res +*.rc # Vscode /.vscode/ # ignore patch files -*.patch \ No newline at end of file +*.patch + +.clang-tidy +.clang-format \ No newline at end of file diff --git a/Makefile b/Makefile index 54646df..a2ee6ba 100644 --- a/Makefile +++ b/Makefile @@ -29,6 +29,9 @@ BUILD_LINK = /DEBUG BASE_CFLAGS = /EHsc CFLAGS = $(BASE_CFLAGS) $(BUILD_CFLAGS) $(CLFLAGS) LFLAGS = $(BUILD_LINK) $(LINKFLAGS) +API_LIBS = Shlwapi.lib \ +Pathcch.lib \ +Advapi32.lib SRCS = cl.obj \ execute.obj \ @@ -55,7 +58,7 @@ linker_invocation.obj all : install test cl.exe : $(SRCS) - link $(LFLAGS) $** Shlwapi.lib /out:cl.exe + link $(LFLAGS) $** $(API_LIBS) /out:cl.exe install : cl.exe mkdir $(PREFIX) @@ -151,7 +154,7 @@ test_pipe_overflow: build_and_check_test_sample set SPACK_CC=%SPACK_CC_TMP% build_zerowrite_test: test\writezero.obj - link $(LFLAGS) $** Shlwapi.lib /out:writezero.exe + link $(LFLAGS) $** $(API_LIBS) /out:writezero.exe test_zerowrite: build_zerowrite_test echo "-----------------------" From 21e925f7f7dd3fa9b352809b95b51139cbc8d5c4 Mon Sep 17 00:00:00 2001 From: John Parent Date: Fri, 30 Jan 2026 18:48:13 -0500 Subject: [PATCH 03/10] Stderr: overhaul error piping from child Ensures child stderr pipes are handled gracefully Corrects an issue where the error pipes were not copied on assign or copy or included in the process start info or closed. Signed-off-by: John Parent --- Makefile | 21 ++++++++++++++++----- src/execute.cxx | 10 +++++++++- test/lots-of-error.bat | 3 +++ 3 files changed, 28 insertions(+), 6 deletions(-) create mode 100644 test/lots-of-error.bat diff --git a/Makefile b/Makefile index a2ee6ba..759eae9 100644 --- a/Makefile +++ b/Makefile @@ -144,15 +144,26 @@ test_relocate_dll: build_and_check_test_sample .\tester.exe cd ../.. -test_pipe_overflow: build_and_check_test_sample - echo "--------------------" - echo " Pipe overflow test" - echo "--------------------" +test_pipe_out_overflow: build_and_check_test_sample + @echo \n + @echo --------------------------- + @echo Pipe stdout overflow test + @echo --------------------------- set SPACK_CC_TMP=%SPACK_CC% set SPACK_CC=$(MAKEDIR)\test\lots-of-output.bat cl /c /EHsc "test\src file\calc.cxx" set SPACK_CC=%SPACK_CC_TMP% +test_pipe_error_overflow: build_and_check_test_sample + @echo \n + @echo --------------------------- + @echo Pipe stderr overflow test + @echo --------------------------- + set SPACK_CC_TMP=%SPACK_CC% + set SPACK_CC=$(MAKEDIR)\test\lots-of-error.bat + cl /c /EHsc "test\src file\calc.cxx" + set SPACK_CC=%SPACK_CC_TMP% + build_zerowrite_test: test\writezero.obj link $(LFLAGS) $** $(API_LIBS) /out:writezero.exe @@ -244,7 +255,7 @@ test_def_file_name_override: test_and_cleanup: test clean-test -test: test_wrapper test_relocate_exe test_relocate_dll test_def_file_name_override test_exe_with_exports test_long_paths test_pipe_overflow +test: test_wrapper test_relocate_exe test_relocate_dll test_def_file_name_override test_exe_with_exports test_long_paths test_pipe_out_overflow test_pipe_error_overflow clean : clean-test clean-cl diff --git a/src/execute.cxx b/src/execute.cxx index b00bf1d..b4f8376 100644 --- a/src/execute.cxx +++ b/src/execute.cxx @@ -31,6 +31,8 @@ enum : std::uint16_t { InvalidExitCode = 999 }; ExecuteCommand::ExecuteCommand(std::string command) : ChildStdOut_Rd(nullptr), ChildStdOut_Wd(nullptr), + ChildStdErr_Rd(nullptr), + ChildStdErr_Wd(nullptr), base_command(std::move(command)) { this->CreateChildPipes(); this->SetupExecute(); @@ -39,6 +41,8 @@ ExecuteCommand::ExecuteCommand(std::string command) ExecuteCommand::ExecuteCommand(std::string arg, const StrList& args) : ChildStdOut_Rd(nullptr), ChildStdOut_Wd(nullptr), + ChildStdErr_Rd(nullptr), + ChildStdErr_Wd(nullptr), base_command(std::move(arg)) { for (const auto& argp : args) { this->command_args.push_back(argp); @@ -51,6 +55,8 @@ ExecuteCommand& ExecuteCommand::operator=( ExecuteCommand&& execute_command) noexcept { this->ChildStdOut_Rd = std::move(execute_command.ChildStdOut_Rd); this->ChildStdOut_Wd = std::move(execute_command.ChildStdOut_Wd); + this->ChildStdErr_Rd = std::move(execute_command.ChildStdErr_Rd); + this->ChildStdErr_Wd = std::move(execute_command.ChildStdErr_Wd); this->procInfo = std::move(execute_command.procInfo); this->startInfo = std::move(execute_command.startInfo); this->saAttr = std::move(execute_command.saAttr); @@ -59,6 +65,7 @@ ExecuteCommand& ExecuteCommand::operator=( this->base_command = std::move(execute_command.base_command); this->command_args = std::move(execute_command.command_args); this->child_out_future = std::move(execute_command.child_out_future); + this->child_err_future = std::move(execute_command.child_err_future); this->exit_code_future = std::move(exit_code_future); return *this; } @@ -76,7 +83,7 @@ void ExecuteCommand::SetupExecute() { // This structure specifies the STDIN and STDOUT handles for redirection. ZeroMemory(&si_start_info, sizeof(STARTUPINFOW)); si_start_info.cb = sizeof(STARTUPINFOW); - si_start_info.hStdError = this->ChildStdOut_Wd; + si_start_info.hStdError = this->ChildStdErr_Wd; si_start_info.hStdOutput = this->ChildStdOut_Wd; si_start_info.dwFlags |= STARTF_USESTDHANDLES; this->procInfo = pi_proc_info; @@ -162,6 +169,7 @@ bool ExecuteCommand::ExecuteToolChainChild() { // determine when child proc is done free(nc_command_line); CloseHandle(this->ChildStdOut_Wd); + CloseHandle(this->ChildStdErr_Wd); return true; } diff --git a/test/lots-of-error.bat b/test/lots-of-error.bat new file mode 100644 index 0000000..c1028b7 --- /dev/null +++ b/test/lots-of-error.bat @@ -0,0 +1,3 @@ +@echo OFF + +for /l %%x in (1, 1, 1250) do echo Test boilerplate output to fill stderr line number %%x 1>&2 From dfd67a988d44f23b6ed5a29fca52a9f268a40ed8 Mon Sep 17 00:00:00 2001 From: John Parent Date: Fri, 30 Jan 2026 18:55:38 -0500 Subject: [PATCH 04/10] Utils: Add file permissions Some packages install themselves as readonly, or with other file permissions that conflict with what we're doing with this compiler wrapper. Thus, we need the ability to override (and restore) those settings. For this compiler wrapper to perform it's duties, it needs to be able to read and write the various binaries artifacts from a build. We thus need to be able to: 1. Obtain and store current permissions 2. Grant ourselves the required permissions 3. Do our reading/writing 4. Restore previous permissions On Windows, file access is handled by the read/write bit, as on other platforms, but also by Access Control Lists (ACLs) which are populated by Discretionary Access Control Lists (DACLs) which are the atomic units responsible for granting rights. This PR adds an interface to inspect, store, update, and restore the read/write bits and DACLs of a given file. This functionality is wrapped up in a RAII based class that essentially provides scoped permissions to a file for scope of the permissions object. Note: this assumes the user creating the permissions is the user driving this wrapper, or this user has admin rights, otherwise this will fail (gracefully, but still a failure) Signed-off-by: John Parent --- src/utils.cxx | 211 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/utils.h | 66 ++++++++++++++++ 2 files changed, 277 insertions(+) diff --git a/src/utils.cxx b/src/utils.cxx index 5d7c319..1468ed8 100644 --- a/src/utils.cxx +++ b/src/utils.cxx @@ -4,6 +4,8 @@ * SPDX-License-Identifier: (Apache-2.0 OR MIT) */ #include "utils.h" +#include +#include #include #include #include @@ -12,6 +14,8 @@ #include #include #include +#include +#include #include #include #include @@ -928,6 +932,213 @@ char* findstr(char* search_str, const char* substr, size_t size) { return nullptr; } +ScopedSid FileSecurity::GetCurrentUserSid() { + HANDLE token_handle = nullptr; + if (!::OpenProcessToken(::GetCurrentProcess(), TOKEN_QUERY, + &token_handle)) { + return nullptr; + } + std::unique_ptr const scoped_token( + token_handle, &::CloseHandle); + + DWORD buffer_size = 0; + ::GetTokenInformation(token_handle, TokenUser, nullptr, 0, &buffer_size); + + std::vector buffer(buffer_size); + auto* token_user = reinterpret_cast(buffer.data()); + + if (!::GetTokenInformation(token_handle, TokenUser, token_user, buffer_size, + &buffer_size)) { + return nullptr; + } + + DWORD const sid_len = ::GetLengthSid(token_user->User.Sid); + void* sid_copy = std::malloc(sid_len); + if (sid_copy) { + ::CopySid(sid_len, sid_copy, token_user->User.Sid); + return ScopedSid(sid_copy); + } + return nullptr; +} + +bool FileSecurity::HasPermission(const std::wstring& file_path, + DWORD access_mask, PSID sid) { + PACL dacl = nullptr; + PSECURITY_DESCRIPTOR sd_raw = nullptr; + DWORD result = ::GetNamedSecurityInfoW(file_path.c_str(), SE_FILE_OBJECT, + DACL_SECURITY_INFORMATION, nullptr, + nullptr, &dacl, nullptr, &sd_raw); + + if (result != ERROR_SUCCESS) + return false; + ScopedLocalInfo const scoped_sd(sd_raw); + + TRUSTEE_W trustee = {nullptr}; + trustee.TrusteeForm = TRUSTEE_IS_SID; + trustee.TrusteeType = TRUSTEE_IS_USER; + trustee.ptstrName = static_cast(sid); + + ACCESS_MASK effective_rights = 0; + result = ::GetEffectiveRightsFromAclW(dacl, &trustee, &effective_rights); + + if (result != ERROR_SUCCESS) + return false; + + if ((access_mask & GENERIC_WRITE) && (effective_rights & FILE_WRITE_DATA)) + return true; + if ((access_mask & GENERIC_READ) && (effective_rights & FILE_READ_DATA)) + return true; + if ((access_mask & GENERIC_ALL) && (effective_rights & FILE_ALL_ACCESS)) + return true; + + return (effective_rights & access_mask) == access_mask; +} + +bool FileSecurity::GrantPermission(const std::wstring& file_path, + DWORD access_mask, PSID sid, + PSECURITY_DESCRIPTOR* out_old_sd) { + PACL old_dacl = nullptr; + PSECURITY_DESCRIPTOR sd_raw = nullptr; + + DWORD result = ::GetNamedSecurityInfoW( + file_path.c_str(), SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, nullptr, + nullptr, &old_dacl, nullptr, &sd_raw); + + if (result != ERROR_SUCCESS) + return false; + + if (out_old_sd) + *out_old_sd = sd_raw; + ScopedLocalInfo const temp_sd_wrapper(out_old_sd ? nullptr : sd_raw); + + EXPLICIT_ACCESS_W ea = {0}; + ea.grfAccessPermissions = access_mask; + ea.grfAccessMode = GRANT_ACCESS; + ea.grfInheritance = SUB_CONTAINERS_AND_OBJECTS_INHERIT; + ea.Trustee.TrusteeForm = TRUSTEE_IS_SID; + ea.Trustee.TrusteeType = TRUSTEE_IS_USER; + ea.Trustee.ptstrName = static_cast(sid); + + PACL new_dacl = nullptr; + result = ::SetEntriesInAclW(1, &ea, old_dacl, &new_dacl); + if (result != ERROR_SUCCESS) + return false; + + ScopedLocalInfo const scoped_new_dacl(new_dacl); + result = ::SetNamedSecurityInfoW(const_cast(file_path.c_str()), + SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, + nullptr, nullptr, new_dacl, nullptr); + return (result == ERROR_SUCCESS); +} + +bool FileSecurity::ApplyDescriptor(const std::wstring& file_path, + PSECURITY_DESCRIPTOR sd) { + if (!sd) + return false; + BOOL present = FALSE; + BOOL defaulted = FALSE; + PACL dacl = nullptr; + if (!::GetSecurityDescriptorDacl(sd, &present, &dacl, &defaulted) || + !present) + return false; + + return ::SetNamedSecurityInfoW(const_cast(file_path.c_str()), + SE_FILE_OBJECT, DACL_SECURITY_INFORMATION, + nullptr, nullptr, dacl, + nullptr) == ERROR_SUCCESS; +} + +bool FileSecurity::GetAttributes(const std::wstring& file_path, + DWORD* out_attr) { + DWORD const attr = ::GetFileAttributesW(file_path.c_str()); + if (attr == INVALID_FILE_ATTRIBUTES) + return false; + if (out_attr) + *out_attr = attr; + return true; +} + +bool FileSecurity::SetAttributes(const std::wstring& file_path, DWORD attr) { + return ::SetFileAttributesW(file_path.c_str(), attr) != 0; +} + +ScopedFileAccess::ScopedFileAccess(std::wstring file_path, DWORD desired_access) + : file_path_(std::move(file_path)), + desired_access_(desired_access), + original_sd_(nullptr), + current_user_sid_(nullptr), + acl_needs_revert_(false), + original_attributes_(0), + attributes_changed_(false) { + + // We must ensure we have permissions *first* before we try to + // change the file attributes in Phase 2. + + current_user_sid_ = FileSecurity::GetCurrentUserSid(); + if (!current_user_sid_) { + throw std::system_error(static_cast(::GetLastError()), + std::system_category(), "Failed to get SID"); + } + + // Check if we need to modify ACLs + if (!FileSecurity::HasPermission(file_path_, desired_access_, + current_user_sid_.get())) { + if (!FileSecurity::GrantPermission(file_path_, desired_access_, + current_user_sid_.get(), + &original_sd_)) { + throw std::system_error(static_cast(::GetLastError()), + std::system_category(), + "Failed to grant ACL"); + } + acl_needs_revert_ = true; + } + + if (FileSecurity::GetAttributes(file_path_, &original_attributes_)) { + if (original_attributes_ & FILE_ATTRIBUTE_READONLY) { + // Remove the Read-Only bit + DWORD const new_attributes = + original_attributes_ & ~FILE_ATTRIBUTE_READONLY; + + if (FileSecurity::SetAttributes(file_path_, new_attributes)) { + attributes_changed_ = true; + } else { + // If we fail to remove Read-Only, we might still fail to write later. + // We throw here to be safe and consistent. + throw std::system_error(static_cast(::GetLastError()), + std::system_category(), + "Failed to remove Read-Only attribute"); + } + } + } else { + throw std::system_error(static_cast(::GetLastError()), + std::system_category(), + "Failed to get file attributes"); + } +} + +ScopedFileAccess::~ScopedFileAccess() { + // We must restore attributes *before* we revert ACLs, because reverting ACLs + // might remove our permission to write attributes. + if (attributes_changed_) { + // We ignore errors in destructors to prevent termination + FileSecurity::SetAttributes(file_path_, original_attributes_); + } + + if (acl_needs_revert_ && original_sd_) { + FileSecurity::ApplyDescriptor(file_path_, original_sd_); + ::LocalFree(original_sd_); + } +} + +bool ScopedFileAccess::IsAccessGranted() const { + // If we had to change anything, we assume success (constructor would throw otherwise) + if (acl_needs_revert_ || attributes_changed_) + return true; + + return FileSecurity::HasPermission(file_path_, desired_access_, + current_user_sid_.get()); +} + NameTooLongError::NameTooLongError(char const* const message) : std::runtime_error(message) {} diff --git a/src/utils.h b/src/utils.h index 377ee91..db89a96 100644 --- a/src/utils.h +++ b/src/utils.h @@ -234,6 +234,21 @@ int SafeHandleCleanup(HANDLE& handle); // System Helpers // std::string reportLastError(); +struct LocalFreeDeleter { + void operator()(void* p) const { + if (p) + ::LocalFree(p); + } +}; + +// Custom deleter for Standard C pointers. +struct FreeDeleter { + void operator()(void* p) const { + if (p) + std::free(p); + } +}; + // Data helpers // // Converts big endian data to little endian form @@ -278,6 +293,57 @@ class LibraryFinder { void EvalSearchPaths(); }; +using ScopedLocalInfo = std::unique_ptr; + +using ScopedSid = std::unique_ptr; + +class FileSecurity { + public: + FileSecurity() = delete; + + static ScopedSid GetCurrentUserSid(); + + static bool HasPermission(const std::wstring& file_path, DWORD access_mask, + PSID sid); + + static bool GrantPermission(const std::wstring& file_path, + DWORD access_mask, PSID sid, + PSECURITY_DESCRIPTOR* out_old_sd); + + static bool ApplyDescriptor(const std::wstring& file_path, + PSECURITY_DESCRIPTOR sd); + + // Retrieves file attributes (e.g., ReadOnly, Hidden). + // Returns false if the file cannot be accessed. + static bool GetAttributes(const std::wstring& file_path, DWORD* out_attr); + + // Sets file attributes. + // Returns false if the operation fails. + static bool SetAttributes(const std::wstring& file_path, DWORD attr); +}; + +class ScopedFileAccess { + public: + explicit ScopedFileAccess(std::wstring file_path, + DWORD desired_access = GENERIC_WRITE); + ~ScopedFileAccess(); + + bool IsAccessGranted() const; + + private: + std::wstring file_path_; + DWORD desired_access_; + + // ACL State + PSECURITY_DESCRIPTOR original_sd_; + ScopedSid current_user_sid_; + bool acl_needs_revert_; + + // Attribute State + DWORD original_attributes_; + bool attributes_changed_; +}; + const std::map special_character_to_path{{'|', '\\'}, {';', ':'}}; const std::map path_to_special_characters{{'\\', '|'}, From fb2dfa3e984418100206ce724e48ee52fc2f575f Mon Sep 17 00:00:00 2001 From: John Parent Date: Fri, 30 Jan 2026 19:02:26 -0500 Subject: [PATCH 05/10] 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 759eae9..8ffd3da 100644 --- a/Makefile +++ b/Makefile @@ -69,9 +69,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 @@ -83,9 +84,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 @@ -97,9 +98,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 @@ -113,23 +115,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 .. @@ -168,18 +172,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 @@ -196,9 +202,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 .. @@ -214,9 +221,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 @@ -237,6 +245,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 1468ed8..7d2f0f0 100644 --- a/src/utils.cxx +++ b/src/utils.cxx @@ -877,13 +877,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 db89a96..8ded476 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 b49d918dbed1dbf9f27792f3a4c293b3f39feea4 Mon Sep 17 00:00:00 2001 From: John Parent Date: Fri, 30 Jan 2026 19:05:30 -0500 Subject: [PATCH 06/10] Coff Parser Improvements * Adds api calls for obtaining the PE names stored in coff files Supports both short and long names, or detecting both. Typically only one is defined in an import library, so whichever is returned first. * Adds access guards to coff reading Signed-off-by: John Parent --- src/coff_parser.cxx | 39 ++++++++++++++++++++++++++++++++++++-- src/coff_parser.h | 3 +++ src/coff_reader_writer.cxx | 22 ++++++++++++++++++--- 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/src/coff_parser.cxx b/src/coff_parser.cxx index 25f4b28..4cee1f0 100644 --- a/src/coff_parser.cxx +++ b/src/coff_parser.cxx @@ -46,8 +46,9 @@ CoffParser::CoffParser(CoffReaderWriter* coff_reader) */ bool CoffParser::Parse() { if (!this->coffStream_->Open()) { - std::cerr << "Unable to open coff file for reading: " - << reportLastError() << "\n"; + std::cerr << "Unable to open coff file: " + << this->coffStream_->get_file() + << " for reading: " << reportLastError() << "\n"; return false; } int const invalid_valid_sig = @@ -578,6 +579,40 @@ void CoffParser::ReportLongName(const char* data) { std::cout << "DLL: " << data << "\n"; } +std::string CoffParser::GetLongName() const { + // TODO(johnwparent): I think we can access the + // 2nd index of the members vec to get the long + // name + for (auto mem : this->coff_.members) { + if (mem.member->is_longname) { + return std::string(mem.member->data); + } + } + return std::string(); +} + +std::string CoffParser::GetShortName() const { + for (auto mem : this->coff_.members) { + if (mem.header->Name[0] != '/') { + int i = 0; + while (mem.header->Name[i] != '/') { + ++i; + } + return std::string(reinterpret_cast(mem.header->Name), + i); + } + } + return std::string(); +} + +std::string CoffParser::GetName() const { + std::string maybe_name = this->GetLongName(); + if (maybe_name.empty()) { + maybe_name = this->GetShortName(); + } + return maybe_name; +} + void CoffParser::Report() { for (auto mem : this->coff_.members) { if (mem.member->is_longname) { diff --git a/src/coff_parser.h b/src/coff_parser.h index 6cf7728..e485af0 100644 --- a/src/coff_parser.h +++ b/src/coff_parser.h @@ -43,6 +43,9 @@ class CoffParser { bool NormalizeName(std::string& name); void Report(); int Verify(); + std::string GetLongName() const; + std::string GetShortName() const; + std::string GetName() const; static int Validate(std::string& coff); }; diff --git a/src/coff_reader_writer.cxx b/src/coff_reader_writer.cxx index cbab86f..6e34736 100644 --- a/src/coff_reader_writer.cxx +++ b/src/coff_reader_writer.cxx @@ -6,6 +6,7 @@ #include "coff_reader_writer.h" #include "coff.h" +#include "utils.h" #include #include @@ -13,16 +14,31 @@ #include #include #include +#include #include +#include #include CoffReaderWriter::CoffReaderWriter(std::string const& file) : file_(std::move(file)) {} bool CoffReaderWriter::Open() { - this->pe_stream_.open(this->file_, - std::ios::in | std::ios::out | std::ios::binary); - return this->pe_stream_.is_open(); + std::wstring coff_file; + try { + coff_file = ConvertASCIIToWide(this->file_); + } catch (std::overflow_error& e) { + return false; + } + try { + ScopedFileAccess const obtain_write(coff_file, GENERIC_ALL); + this->pe_stream_.open(this->file_, + std::ios::in | std::ios::out | std::ios::binary); + return this->pe_stream_.is_open(); + } catch (std::system_error& e) { + std::cerr << "Could not obtain write access: " << e.what() + << " (Error Code: " << e.code().value() << ")" << '\n'; + return false; + } } bool CoffReaderWriter::Close() { From a669a6539c4be24da046c05e5138796afaca38a9 Mon Sep 17 00:00:00 2001 From: John Parent Date: Fri, 30 Jan 2026 19:12:13 -0500 Subject: [PATCH 07/10] CLI: Remove deploy We're no longer modifying binaries on BC push Signed-off-by: John Parent --- Makefile | 4 ++-- src/commandline.cxx | 26 -------------------------- src/main.cxx | 13 +++++-------- src/winrpath.cxx | 11 +++++------ src/winrpath.h | 6 ++---- 5 files changed, 14 insertions(+), 46 deletions(-) diff --git a/Makefile b/Makefile index 8ffd3da..e5111b4 100644 --- a/Makefile +++ b/Makefile @@ -141,7 +141,7 @@ test_relocate_dll: build_and_check_test_sample mkdir tmp_lib move test\calc.dll tmp_bin\calc.dll move test\calc.lib tmp_lib\calc.lib - test\relocate.exe --pe tmp_bin\calc.dll --coff tmp_lib\calc.lib --export + test\relocate.exe --pe tmp_bin\calc.dll --coff tmp_lib\calc.lib cd test del tester.exe link main.obj ..\tmp_lib\calc.lib /out:tester.exe @@ -213,7 +213,7 @@ test_relocate_long_paths: test_long_paths mkdir tmp_lib move evenlongersubdirectoryname\verylongfilepathnamethatwilldefinitelybegreaterthanonehundredandfourtyfourcharacters.dll tmp_bin\verylongfilepathnamethatwilldefinitelybegreaterthanonehundredandfourtyfourcharacters.dll move evenlongersubdirectoryname\verylongfilepathnamethatwilldefinitelybegreaterthanonehundredandfourtyfourcharacters.lib tmp_lib\verylongfilepathnamethatwilldefinitelybegreaterthanonehundredandfourtyfourcharacters.lib - evenlongersubdirectoryname\relocate.exe --pe tmp_bin\verylongfilepathnamethatwilldefinitelybegreaterthanonehundredandfourtyfourcharacters.dll --coff tmp_lib\verylongfilepathnamethatwilldefinitelybegreaterthanonehundredandfourtyfourcharacters.lib --export + evenlongersubdirectoryname\relocate.exe --pe tmp_bin\verylongfilepathnamethatwilldefinitelybegreaterthanonehundredandfourtyfourcharacters.dll --coff tmp_lib\verylongfilepathnamethatwilldefinitelybegreaterthanonehundredandfourtyfourcharacters.lib cd evenlongersubdirectoryname del tester.exe link main.obj ..\tmp_lib\verylongfilepathnamethatwilldefinitelybegreaterthanonehundredandfourtyfourcharacters.lib /out:tester.exe diff --git a/src/commandline.cxx b/src/commandline.cxx index 38bd760..df2bff1 100644 --- a/src/commandline.cxx +++ b/src/commandline.cxx @@ -86,14 +86,6 @@ bool print_help() { "said library is regenerated and the old imp lib\n"; std::cout << " " "replaced.\n"; - std::cout << " --export|--deploy = " - "Mutually exclusive command modifier.\n"; - std::cout << " " - "Instructs relocate to either prepare the\n"; - std::cout << " " - "dynamic library for exporting to build cache\n"; - std::cout << " " - "or for extraction from bc onto new host system\n"; std::cout << " --report = " "Report information about the parsed PE/Coff files\n"; std::cout << " --debug|-d = " @@ -149,24 +141,6 @@ std::map ParseRelocate(const char** args, int argc) { return opts; } opts.insert(std::pair("full", "full")); - } else if (!strcmp(args[i], "--export")) { - // export and deploy are mutually exclusive, if one is defined - // the other cannot be - if (redefinedArgCheck(opts, "export", "--export") || - redefinedArgCheck(opts, "deploy", "--deploy")) { - opts.clear(); - return opts; - } - opts.insert(std::pair("cmd", "export")); - } else if (!strcmp(args[i], "--deploy")) { - // export and deploy are mutually exclusive, if one is defined - // the other cannot be - if (redefinedArgCheck(opts, "export", "--export") || - redefinedArgCheck(opts, "deploy", "--deploy")) { - opts.clear(); - return opts; - } - opts.insert(std::pair("cmd", "deploy")); } else if (!strcmp(args[i], "--debug") || !strcmp(args[i], "-d")) { opts.insert(std::pair("debug", "on")); } else if (!strcmp(args[i], "--verify")) { diff --git a/src/main.cxx b/src/main.cxx index 1fb0b63..7fa7708 100644 --- a/src/main.cxx +++ b/src/main.cxx @@ -33,8 +33,6 @@ int main(int argc, const char* argv[]) { return -1; } bool const full = !(patch_args.find("full") == patch_args.end()); - bool const deploy = !(patch_args.find("cmd") == patch_args.end()) && - patch_args.at("cmd") == "deploy"; bool const report = !(patch_args.find("report") == patch_args.end()); bool const has_pe = !(patch_args.find("pe") == patch_args.end()); bool const debug = !(patch_args.find("debug") == patch_args.end()); @@ -77,12 +75,11 @@ int main(int argc, const char* argv[]) { std::unique_ptr rpath_lib; try { if (has_coff) { - rpath_lib = std::make_unique(patch_args.at("pe"), - patch_args.at("coff"), - full, deploy, true); + rpath_lib = std::make_unique( + patch_args.at("pe"), patch_args.at("coff"), full, true); } else { rpath_lib = std::make_unique(patch_args.at("pe"), - full, deploy, true); + full, true); } } catch (const NameTooLongError& e) { std::cerr << "Cannot Rename PE file " << patch_args.at("pe") @@ -104,8 +101,8 @@ int main(int argc, const char* argv[]) { } if (report_args.find("pe") != report_args.end()) { try { - LibRename portable_executable( - report_args.at("pe"), std::string(), false, false, true); + LibRename portable_executable(report_args.at("pe"), + std::string(), false, true); portable_executable.ExecuteRename(); } catch (const NameTooLongError& e) { std::cerr diff --git a/src/winrpath.cxx b/src/winrpath.cxx index f98e063..2a60e2e 100644 --- a/src/winrpath.cxx +++ b/src/winrpath.cxx @@ -4,7 +4,6 @@ * SPDX-License-Identifier: (Apache-2.0 OR MIT) */ #include -#include #include // NOLINT #include "winrpath.h" #include @@ -244,15 +243,15 @@ bool LibRename::FindDllAndRename(HANDLE& pe_in) { * \param replace a flag indicating if we're replacing the renamed import lib or making a copy with absolute dll names * \param report a flag indicating if we should be reporting the contents of the PE/COFF file we're parsing to stdout */ -LibRename::LibRename(std::string p_exe, bool full, bool deploy, bool replace) - : replace(replace), full(full), pe(std::move(p_exe)), deploy(deploy) {} +LibRename::LibRename(std::string p_exe, bool full, bool replace) + : replace(replace), full(full), pe(std::move(p_exe)) { +} LibRename::LibRename(std::string p_exe, std::string coff, bool full, - bool deploy, bool replace) + bool replace) : replace(replace), full(full), pe(std::move(p_exe)), - deploy(deploy), coff(std::move(coff)) { std::string const coff_path = stem(this->coff); this->tmp_def_file = coff_path + "-tmp.def"; @@ -357,7 +356,7 @@ bool LibRename::ExecuteRename() { // exes // We do not bother with defs for things that don't have // import libraries - if (!this->deploy && !this->coff.empty()) { + if (!this->coff.empty()) { // Extract DLL if (!this->ComputeDefFile()) { debug("Failed to compute def file"); diff --git a/src/winrpath.h b/src/winrpath.h index e166465..0e5061f 100644 --- a/src/winrpath.h +++ b/src/winrpath.h @@ -31,9 +31,8 @@ class LibRename { public: - LibRename(std::string p_exe, std::string coff, bool full, bool deploy, - bool replace); - LibRename(std::string p_exe, bool full, bool deploy, bool replace); + LibRename(std::string p_exe, std::string coff, bool full, bool replace); + LibRename(std::string p_exe, bool full, bool replace); bool ExecuteRename(); bool ExecuteLibRename(); bool ExecutePERename(); @@ -53,6 +52,5 @@ class LibRename { std::string def_file; std::string tmp_def_file; bool full; - bool deploy; bool replace; }; From 29674a23b34e7a4318bf0dc0150d4a0ffec08ecd Mon Sep 17 00:00:00 2001 From: John Parent Date: Fri, 30 Jan 2026 19:14:53 -0500 Subject: [PATCH 08/10] RPathing Logic Overhaul * Makes all PE paths absolute * Removes rename logic that dealt with the spack sigil and BC pushes * Uses new relocation logic to avoid FS search and instead use Spack env variable and util support added in prior PR * Adds access rights scoping for read and write operations * General code cleanup Signed-off-by: John Parent --- src/winrpath.cxx | 128 ++++++++++++++++++++--------------------------- src/winrpath.h | 6 +-- 2 files changed, 56 insertions(+), 78 deletions(-) diff --git a/src/winrpath.cxx b/src/winrpath.cxx index 2a60e2e..24198a9 100644 --- a/src/winrpath.cxx +++ b/src/winrpath.cxx @@ -22,6 +22,7 @@ #include #include #include +#include #include /* @@ -36,21 +37,8 @@ * \param name The dll name to check for sigils or special path characters * */ -bool LibRename::SpackCheckForDll(const std::string& dll_path) const { - if (this->deploy) { - return hasPathCharacters(dll_path); - } - // First check for the case we're relocating out of a buildcache - bool reloc_spack = false; - if (!(dll_path.find("") == std::string::npos) || - !(dll_path.find("") == std::string::npos)) { - reloc_spack = true; - } - // If not, maybe we're just relocating a binary on the same system - if (!reloc_spack) { - reloc_spack = hasPathCharacters(dll_path); - } - return reloc_spack; +bool LibRename::SpackCheckForDll(const std::string& dll_path) { + return hasPathCharacters(dll_path); } /* @@ -65,58 +53,36 @@ bool LibRename::SpackCheckForDll(const std::string& dll_path) const { * the dll name found at `name_loc` to the absolute path of * */ -bool LibRename::RenameDll(char* name_loc, const std::string& dll_path) const { - if (this->deploy) { - int const padding_len = get_padding_length(dll_path); - if (padding_len < MIN_PADDING_THRESHOLD) { - // path is too long to mark as a Spack path - // use shorter sigil - char short_sigil[] = ""; - // use _snprintf as it does not null terminate and we're writing into the middle - // of a null terminated string we want to later read from properly - _snprintf(name_loc, sizeof(short_sigil) - 1, "%s", short_sigil); - } else { - char long_sigil[] = ""; - // See _snprintf comment above for use context - _snprintf(name_loc, sizeof(long_sigil) - 1, "%s", long_sigil); - } - } else { - if (SpackInstalledLib(dll_path)) { - return true; - } - std::string const file_name = basename(dll_path); - if (file_name.empty()) { - std::cerr << "Unable to extract filename from dll for relocation" - << "\n"; - return false; - } - LibraryFinder lib_finder; - std::string new_library_loc = - lib_finder.FindLibrary(file_name, dll_path); - if (new_library_loc.empty()) { - std::cerr << "Unable to find library " << file_name << " from " - << dll_path << " for relocation" << "\n"; - return false; - } - if (new_library_loc.length() > MAX_NAME_LEN) { - try { - new_library_loc = short_name(new_library_loc); - } catch (NameTooLongError& e) { - return false; - } - } - char* new_lib_pth = - pad_path(new_library_loc.c_str(), - static_cast(new_library_loc.size())); - if (!new_lib_pth) { - return false; - } - replace_special_characters(new_lib_pth, MAX_NAME_LEN); +bool LibRename::RenameDll(char* name_loc, const std::string& dll_path) { + if (SpackInstalledLib(dll_path)) { + return true; + } + PathRelocator relocator; + std::string new_loc = relocator.getRelocation(dll_path); + if (new_loc.empty()) { + std::cerr << "Cannot find relocation mapping for library " << dll_path + << "\n"; + return false; + } + try { + new_loc = + EnsureValidLengthPath(CannonicalizePath(MakePathAbsolute(new_loc))); + } catch (NameTooLongError& e) { + std::cerr << "Cannot relocate path " << new_loc + << "it is too long to be relocated safely.\n"; + return false; + } - // c_str returns a proper (i.e. null terminated) value, so we dont need to worry about - // size differences w.r.t the path to the new library - snprintf(name_loc, MAX_NAME_LEN + 1, "%s", new_lib_pth); + char* new_lib_pth = + pad_path(new_loc.c_str(), static_cast(new_loc.size())); + if (!new_lib_pth) { + return false; } + replace_special_characters(new_lib_pth, MAX_NAME_LEN); + + // c_str returns a proper (i.e. null terminated) value, so we dont need to worry about + // size differences w.r.t the path to the new library + snprintf(name_loc, MAX_NAME_LEN + 1, "%s", new_lib_pth); return true; } @@ -205,8 +171,8 @@ bool LibRename::FindDllAndRename(HANDLE& pe_in) { import_table_offset + (import_image_descriptor->Name - rva_import_directory); std::string const str_dll_name = std::string(imported_dll); - if (this->SpackCheckForDll(str_dll_name)) { - if (!this->RenameDll(imported_dll, str_dll_name)) { + if (LibRename::SpackCheckForDll(str_dll_name)) { + if (!LibRename::RenameDll(imported_dll, str_dll_name)) { std::cerr << "Unable to relocate DLL reference: " << str_dll_name << "\n"; return false; @@ -245,6 +211,7 @@ bool LibRename::FindDllAndRename(HANDLE& pe_in) { */ LibRename::LibRename(std::string p_exe, bool full, bool replace) : replace(replace), full(full), pe(std::move(p_exe)) { + this->pe = MakePathAbsolute(this->pe); } LibRename::LibRename(std::string p_exe, std::string coff, bool full, @@ -253,6 +220,7 @@ LibRename::LibRename(std::string p_exe, std::string coff, bool full, full(full), pe(std::move(p_exe)), coff(std::move(coff)) { + this->pe = MakePathAbsolute(this->pe); std::string const coff_path = stem(this->coff); this->tmp_def_file = coff_path + "-tmp.def"; this->def_file = coff_path + ".def"; @@ -321,7 +289,9 @@ bool LibRename::ComputeDefFile() { npos) { // Skip header in export block if still present break; } - output_file << " " << regexReplace(line, R"(\s+)", "") << '\n'; + output_file << " " + << regexMatch(line, R"(^.*?(\S+)(?:\s+\(.*\))?\s*$)") + << '\n'; } input_file.close(); output_file.close(); @@ -438,15 +408,23 @@ bool LibRename::ExecutePERename() { std::cerr << e.what() << "\n"; return false; } - HANDLE pe_handle = CreateFileW( - pe_path.c_str(), (GENERIC_READ | GENERIC_WRITE), FILE_SHARE_READ, - nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); - if (!pe_handle || pe_handle == INVALID_HANDLE_VALUE) { - std::cerr << "Unable to acquire file handle to " << pe_path.c_str() - << ": " << reportLastError() << "\n"; + try { + ScopedFileAccess const obtain_write(pe_path, GENERIC_ALL); + HANDLE pe_handle = CreateFileW( + pe_path.c_str(), (GENERIC_READ | GENERIC_WRITE), FILE_SHARE_WRITE, + nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); + if (!pe_handle || pe_handle == INVALID_HANDLE_VALUE) { + std::cerr << "Unable to acquire file handle to " + << ConvertWideToASCII(pe_path) << ": " + << reportLastError() << "\n"; + return false; + } + return LibRename::FindDllAndRename(pe_handle); + } catch (const std::system_error& e) { + std::cerr << "Could not obtain write access: " << e.what() + << " (Error Code: " << e.code().value() << ")" << '\n'; return false; } - return this->FindDllAndRename(pe_handle); } /* Construct the line needed to produce a new import library diff --git a/src/winrpath.h b/src/winrpath.h index 0e5061f..16aeff6 100644 --- a/src/winrpath.h +++ b/src/winrpath.h @@ -41,9 +41,9 @@ class LibRename { std::string ComputeDefLine(); private: - bool FindDllAndRename(HANDLE& pe_in); - bool SpackCheckForDll(const std::string& dll_path) const; - bool RenameDll(char* name_loc, const std::string& dll_path) const; + static bool FindDllAndRename(HANDLE& pe_in); + static bool SpackCheckForDll(const std::string& dll_path) ; + static bool RenameDll(char* name_loc, const std::string& dll_path) ; ExecuteCommand def_executor; ExecuteCommand lib_executor; std::string pe; From 3f3f601f745942b39b60af5f9cadeca15e01e0fd Mon Sep 17 00:00:00 2001 From: John Parent Date: Fri, 30 Jan 2026 19:20:57 -0500 Subject: [PATCH 09/10] Toolchain: reduce CLI processed Reduce the degree to which we decompose an incoming command line in the toolchain. The odering and context of CLI arguments is highly fragile and important to the contents and naming of build artifacts, in particular w.r.t. maintaining user expectations and logic regarding name reasoning. Instead toolchain is now a thin wrapper to pass the toolchain commands through the compiler wrapper itself, preserving inputs and order and injecting Spack libs, includes, etc safely. We leave command line parsing to specialized tools designed for the linker/compiler/etc. Signed-off-by: John Parent --- src/toolchain.cxx | 46 +++++++--------------------------------------- src/toolchain.h | 7 ++----- 2 files changed, 9 insertions(+), 44 deletions(-) diff --git a/src/toolchain.cxx b/src/toolchain.cxx index a1c103f..1d3318e 100644 --- a/src/toolchain.cxx +++ b/src/toolchain.cxx @@ -15,7 +15,7 @@ ToolChainInvocation::ToolChainInvocation(std::string command, char const* const* cli) - : command(std::move(std::move(command))) { + : command(std::move(command)) { this->ParseCommandArgs(cli); } @@ -23,10 +23,10 @@ void ToolChainInvocation::InterpolateSpackEnv(SpackEnvState& spackenv) { // inject Spack includes before the default includes for (auto& include : spackenv.SpackIncludeDirs) { auto inc_arg = ToolChainInvocation::ComposeIncludeArg(include); - this->include_args.insert(this->include_args.begin(), inc_arg); + this->inputs.push_back(inc_arg); } for (auto& lib : spackenv.SpackLdLibs) { - this->lib_args.push_back(lib); + this->inputs.push_back(lib); } this->AddExtraLibPaths(spackenv.SpackLinkDirs); this->AddExtraLibPaths(spackenv.SpackRPathDirs); @@ -36,10 +36,8 @@ void ToolChainInvocation::InterpolateSpackEnv(SpackEnvState& spackenv) { } DWORD ToolChainInvocation::InvokeToolchain() { - StrList const command_line(ToolChainInvocation::ComposeCommandLists( - {this->command_args, this->include_args, this->lib_args, - this->lib_dir_args, this->obj_args})); - this->executor = ExecuteCommand(this->command, command_line); + quoteList(this->inputs); + this->executor = ExecuteCommand(this->command, this->inputs); debug("Setting up executor for " + std::string(typeid(*this).name()) + "toolchain"); debug("Toolchain: " + this->command); @@ -53,38 +51,9 @@ DWORD ToolChainInvocation::InvokeToolchain() { } void ToolChainInvocation::ParseCommandArgs(char const* const* cli) { - // Collect include args as we need to ensure Spack - // Includes come first for (char const* const* co = cli; *co; co++) { - std::string norm_arg = std::string(*co); const std::string arg = std::string(*co); - lower(norm_arg); - if (isCommandArg(norm_arg, "i")) { - // We have an include arg - // can have an optional space - // check if there are characters after - // "/I" and if not we consider the next - // argument to be the include - if (arg.size() > 2) - this->include_args.push_back(arg); - else { - this->include_args.push_back(arg); - this->include_args.emplace_back(*(++co)); - } - } else if (endswith(norm_arg, ".lib") && - (norm_arg.find("implib:") == std::string::npos)) - // Lib args are just libraries - // provided like system32.lib on the - // command line. - // lib specification order does not matter - // on MSVC but this is useful for filtering system libs - // and adding all libs - this->lib_args.push_back(arg); - else if (endswith(norm_arg, ".obj")) - this->obj_args.push_back(arg); - else { - this->command_args.push_back(arg); - } + this->inputs.push_back(arg); } } @@ -98,8 +67,7 @@ std::string ToolChainInvocation::ComposeLibPathArg(std::string& libPath) { void ToolChainInvocation::AddExtraLibPaths(StrList paths) { for (auto& lib_dir : paths) { - this->lib_dir_args.push_back( - ToolChainInvocation::ComposeLibPathArg(lib_dir)); + this->inputs.push_back(ToolChainInvocation::ComposeLibPathArg(lib_dir)); } } diff --git a/src/toolchain.h b/src/toolchain.h index ec80e2b..c702420 100644 --- a/src/toolchain.h +++ b/src/toolchain.h @@ -32,10 +32,7 @@ class ToolChainInvocation { std::string command; std::string lang; - StrList command_args; - StrList include_args; - StrList lib_dir_args; - StrList lib_args; - StrList obj_args; + StrList inputs; + ExecuteCommand executor; }; From 73cc11108496ada2f59882be9c902218c86a6094 Mon Sep 17 00:00:00 2001 From: John Parent Date: Fri, 30 Jan 2026 19:34:35 -0500 Subject: [PATCH 10/10] Linker Parsing: Overhaul linker CLI processing * Stop disabiguating between obj, lib, and lo files, they're treated the same by the linker and we don't need extra logic for each, and we need to be able to process them in order. * Carefully parses the command line and .def file to determine the intended output name and ensures the internal modeling of that name in the compiler wrapper is consistent * Adds behavior to allow for extremely long command lines (Paraviews are 75kb) by composing arguments into an RSP file * Adds logic to expand the contents of an RSP file so we can inspect the full command line to better reason about naming and eventual length * Small bug fixes of QOL improvements Signed-off-by: John Parent --- src/linker_invocation.cxx | 176 +++++++++++++++++++++++++++++++------- src/linker_invocation.h | 28 +++--- 2 files changed, 161 insertions(+), 43 deletions(-) diff --git a/src/linker_invocation.cxx b/src/linker_invocation.cxx index 4ed82be..1933591 100644 --- a/src/linker_invocation.cxx +++ b/src/linker_invocation.cxx @@ -3,14 +3,19 @@ * * SPDX-License-Identifier: (Apache-2.0 OR MIT) */ +#include #include #include #include #include #include +#include #include "linker_invocation.h" +#include #include "utils.h" +enum { MaxProcessCommandLength = 32767 }; + /** * Parses the command line of a given linker invocation and stores information * about that command line and its associated behavior @@ -46,21 +51,31 @@ void LinkerInvocation::Parse() { // len 2 StrList implib_line = split(*token, ":"); this->implibname_ = implib_line[1]; - } else if (endswith(normal_token, ".lib")) { - this->libs_.push_back(*token); } else if (normal_token == "dll") { this->is_exe_ = false; } else if (startswith(normal_token, "out")) { this->output_ = split(*token, ":")[1]; - } else if (endswith(normal_token, ".obj")) { - this->objs_.push_back(*token); - } else if (startswith(normal_token, "@") && - endswith(normal_token, ".rsp")) { + } else if (endswith(normal_token, ".obj") || + endswith(normal_token, ".lib") || + endswith(normal_token, ".lo")) { + this->input_files_.push_back(*token); + } else if (startswith(normal_token, "@")) { // RSP files are used to describe object files, libraries, other CLI // Switches relevant to the tool the rsp file is being passed to // Primarily utilized by CMake and MSBuild projects to bypass // Command line length limits - this->rsp_file_ = *token; + this->rsp_files_.push_back(*token); + // Since rsp files are essentially expanded in place on the command line + // i.e objA rspA objC + // where rspA defines objB the cli would then be + // objA objB objC + // so we also need to track them in binary_files_ since the order + // of their expansion has implications for naming, i.e + // if rspA was the first input file, the dll/imp name would be objB + this->input_files_.push_back(*token); + } else if (endswith(normal_token, ".res")) { + this->rc_files_.push_back(*token); + this->input_files_.push_back(*token); } else if (startswith(normal_token, "def")) { this->def_file_ = strip(split(*token, ":", 1)[1], "\""); } else if (this->piped_args_.find(normal_token) != @@ -68,29 +83,109 @@ void LinkerInvocation::Parse() { this->piped_args_.at(normal_token).emplace_back(*token); } } - // If we have a def file and no name, attempt to - // scrape the def file for a name to be sure - // we respect the intended project name - // vs overriding via the CLI - if (!this->def_file_.empty() && this->output_.empty()) { - this->processDefFile(); - } + + // Note for the below: name will never be specified so we only have + // /out, .def files, and input files + // To determine internal dll name + // If a def file was not specified: + // /name is used + // if no /name /out is used + // if no /out or /name use first input file + + // If a def file was specified: + // LIBRARY + // otherwise fallback to previous + + // To determine output name + // /OUT is always overriding + // If not /OUT and .def file: + // LIBRARY + // if no def or no LIBRARY + // /NAME + // if no /NAME + // first input file (post rc expanion) + + this->processDefFile(); + this->processInputFiles(); std::string const ext = this->is_exe_ ? ".exe" : ".dll"; - // If output wasn't defined on the command line - // or the def file - // compute it based on the same logic as the linker - // i.e. first obj file name if (this->output_.empty()) { // with no "out" argument, the linker // will place the file in the CWD - std::string const name_obj = this->objs_.front(); - std::string const filename = split(name_obj, "\\").back(); - this->output_ = join({GetCWD(), strip(filename, ".obj")}, "\\") + ext; + std::string const name_component = this->input_files_.front(); + std::string const filename = split(name_component, "\\").back(); + this->output_ = join({GetCWD(), stripLastExt(filename)}, "\\") + ext; } if (this->implibname_.empty()) { std::string const name = strip(this->output_, ext); this->implibname_ = name + ".lib"; } + this->makeRsp(); +} + +void LinkerInvocation::processInputFiles() { + StrList new_input_files; + for (auto input = this->input_files_.begin(); + input != this->input_files_.end(); ++input) { + if (startswith(*input, "@")) { + // rsp file - expand contents in input files + // list in place and remove self + StrList rsp_inputs = LinkerInvocation::processRSPFile(*input); + new_input_files.insert(new_input_files.end(), rsp_inputs.begin(), + rsp_inputs.end()); + } else { + new_input_files.push_back(*input); + } + } + this->input_files_ = new_input_files; +} + +StrList LinkerInvocation::processRSPFile(std::string const& rsp_file) { + std::string const rsp_file_in = lstrip(rsp_file, "@"); + std::ifstream rsp_stream(rsp_file_in); + if (!rsp_stream) { + std::cerr << "Error: Could not open input rsp file: " << rsp_file_in + << "\n"; + throw FileIOError("Cannot open rsp input file: " + GetLastError()); + } + StrList inputs; + std::string line; + while (std::getline(rsp_stream, line)) { + std::stringstream rsp_line(line); + std::string input_file; + rsp_line >> input_file; + inputs.push_back(input_file); + } + return inputs; +} + +/** + * \brief Ensure command line given to lib.exe is of appropriate length + * max windows createProcess command line length is 32,767, so if we exceed + * that, compose all input file args into an rsp. + * + * Writes an rsp file named spack-build.rsp and sets it to be the only + * input file for the lib tool + */ +bool LinkerInvocation::makeRsp() { + int const total_length = std::accumulate( + this->input_files_.begin(), this->input_files_.end(), 0, + [](size_t sum, const std::string& s) { return sum + s.size(); }); + if (total_length > MaxProcessCommandLength) { + std::string const rsp_name = "spack-build.rsp"; + std::ofstream rsp_out(rsp_name); + if (!rsp_out) { + std::cerr << "Unable to open rsp out file: spack-build.rsp\n"; + throw FileIOError("Unable to open lib rsp file"); + } + for (const auto& line : this->input_files_) { + rsp_out << escape_backslash(line) << "\n"; + } + rsp_out.close(); + this->input_files_ = {"@" + rsp_name}; + this->rsp_files_ = {"@" + rsp_name}; + return true; + } + return false; } /** @@ -104,11 +199,15 @@ void LinkerInvocation::Parse() { */ void LinkerInvocation::processDefFile() { + if (this->def_file_.empty()) { + return; + } // Def from link line std::ifstream def_in(this->def_file_); if (!def_in) { std::cerr << "Error: Could not open input def file: " << this->def_file_ << "\n"; + throw FileIOError("Cannot open def input file: " + GetLastError()); } std::string line; @@ -131,18 +230,22 @@ void LinkerInvocation::processDefFile() { if (keyword == "NAME") { this->is_exe_ = true; def_line >> this->pe_name_; - this->pe_name_ = this->pe_name_ + ".exe"; + this->pe_name_ = stripquotes(this->pe_name_) + ".exe"; def_file_export_name = true; } else if (keyword == "LIBRARY") { this->is_exe_ = false; def_line >> this->pe_name_; - this->pe_name_ = this->pe_name_ + ".dll"; + this->pe_name_ = stripquotes(this->pe_name_) + ".dll"; def_file_export_name = true; } else { exports.push_back(line); } } if (def_file_export_name) { + // if output is not specified on the command line, this defines the output name + if (this->output_.empty()) { + this->output_ = join({GetCWD(), this->pe_name_}, "\\"); + } const std::string def_name = stem(this->def_file_); const std::string def_path = this->def_file_.substr(0, this->def_file_.find(def_name)); @@ -152,6 +255,7 @@ void LinkerInvocation::processDefFile() { if (!def_out) { std::cerr << "Error: could not open output def file: " << rename_def << "\n"; + throw FileIOError("Cannot open def output file: " + GetLastError()); } for (const auto& line : exports) { def_out << line << "\n"; @@ -162,11 +266,11 @@ void LinkerInvocation::processDefFile() { def_in.close(); } -std::string LinkerInvocation::get_implib_name() { +std::string LinkerInvocation::get_implib_name() const { return this->implibname_; } -std::string LinkerInvocation::get_lib_link_args() { +std::string LinkerInvocation::get_lib_link_args() const { std::string lib_link_line; for (const auto& var_args : this->piped_args_) { // Most of these should be single arguments @@ -182,22 +286,30 @@ std::string LinkerInvocation::get_lib_link_args() { return lib_link_line; } -std::string LinkerInvocation::get_def_file() { +std::string LinkerInvocation::get_def_file() const { return this->def_file_; } -std::string LinkerInvocation::get_rsp_file() { - return this->rsp_file_; +StrList LinkerInvocation::get_rsp_files() const { + return this->rsp_files_; +} + +StrList LinkerInvocation::get_rc_files() const { + return this->rc_files_; +} + +StrList LinkerInvocation::get_input_files() const { + return this->input_files_; } -std::string LinkerInvocation::get_out() { - return this->pe_name_.empty() ? this->output_ : this->pe_name_; +std::string LinkerInvocation::get_out() const { + return this->output_; } -std::string LinkerInvocation::get_mangled_out() { +std::string LinkerInvocation::get_mangled_out() const { return mangle_name(this->get_out()); } -bool LinkerInvocation::IsExeLink() { +bool LinkerInvocation::IsExeLink() const { return this->is_exe_ || endswith(this->get_out(), ".exe"); } diff --git a/src/linker_invocation.h b/src/linker_invocation.h index a92a538..207feb0 100644 --- a/src/linker_invocation.h +++ b/src/linker_invocation.h @@ -15,25 +15,31 @@ class LinkerInvocation { explicit LinkerInvocation(const StrList& linkline); ~LinkerInvocation() = default; void Parse(); - bool IsExeLink(); - std::string get_out(); - std::string get_mangled_out(); - std::string get_implib_name(); - std::string get_def_file(); - std::string get_rsp_file(); - std::string get_lib_link_args(); + bool IsExeLink() const; + std::string get_out() const; + std::string get_mangled_out() const; + std::string get_implib_name() const; + std::string get_def_file() const; + StrList get_rsp_files() const; + StrList get_rc_files() const; + StrList get_input_files() const; + std::string get_lib_link_args() const; + bool makeRsp(); private: void processDefFile(); + void processInputFiles(); + static StrList processRSPFile(std::string const& rsp_file); std::string line_; - StrList tokens_; std::string pe_name_; std::string implibname_; std::string def_file_; - std::string rsp_file_; std::string output_; - StrList libs_; - StrList objs_; + StrList rsp_files_; + StrList rc_files_; + StrList command_files_; + StrList input_files_; + StrList tokens_; bool is_exe_; std::map piped_args_ = { {"export", {}}, {"include", {}}, {"libpath", {}},