From e3895f4a8bcce3d9caae7d0e5559e6f057725fad Mon Sep 17 00:00:00 2001 From: Gergely Meszaros Date: Tue, 17 Sep 2024 01:29:23 -0700 Subject: Linking: Preserve nested LINKER: prefixes as written Previously LINKER:,-Xlinker and -Wl, options nested inside LINKER: prefixes would be transformed to separate prefixed options. This is confusing and undocumented behavior, instead preserve these as written. Fixes: #26298 --- Source/cmGeneratorTarget_Options.cxx | 46 +++++++++++++++------- .../LINKER_expansion-LINKER-check.cmake | 1 + .../LINKER_expansion-LINKER_NESTED-check.cmake | 4 ++ ...INKER_expansion-LINKER_NESTED_SHELL-check.cmake | 4 ++ .../LINKER_expansion-LINKER_SHELL-check.cmake | 1 + .../LINKER_expansion-validation.cmake | 8 ++-- .../target_link_options/LINKER_expansion.cmake | 18 +++++++++ .../target_link_options/RunCMakeTest.cmake | 2 + 8 files changed, 65 insertions(+), 19 deletions(-) create mode 100644 Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED-check.cmake create mode 100644 Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED_SHELL-check.cmake diff --git a/Source/cmGeneratorTarget_Options.cxx b/Source/cmGeneratorTarget_Options.cxx index df22b90..d8b3eb3 100644 --- a/Source/cmGeneratorTarget_Options.cxx +++ b/Source/cmGeneratorTarget_Options.cxx @@ -7,6 +7,7 @@ #include "cmConfigure.h" #include +#include #include #include #include @@ -91,10 +92,16 @@ void processOptions(cmGeneratorTarget const* tgt, } } +enum class NestedLinkerFlags +{ + PreserveAsSpelled, + Normalize, +}; + std::vector> wrapOptions( std::vector& options, const cmListFileBacktrace& bt, const std::vector& wrapperFlag, const std::string& wrapperSep, - bool concatFlagAndArgs) + bool concatFlagAndArgs, NestedLinkerFlags nestedLinkerFlags) { std::vector> result; @@ -148,6 +155,11 @@ std::vector> wrapOptions( } }; + if (nestedLinkerFlags == NestedLinkerFlags::PreserveAsSpelled) { + insertWrapped(options); + return result; + } + for (std::vector::size_type index = 0; index < options.size(); index++) { if (cmHasLiteralPrefix(options[index], "LINKER:")) { @@ -488,8 +500,9 @@ std::vector> cmGeneratorTarget::GetLinkOptions( // host link options must be wrapped std::vector options; cmSystemTools::ParseUnixCommandLine(it->Value.c_str(), options); - auto hostOptions = wrapOptions(options, it->Backtrace, wrapperFlag, - wrapperSep, concatFlagAndArgs); + auto hostOptions = + wrapOptions(options, it->Backtrace, wrapperFlag, wrapperSep, + concatFlagAndArgs, NestedLinkerFlags::Normalize); it = result.erase(it); // some compilers (like gcc 4.8 or Intel 19.0 or XLC 16) do not respect // C++11 standard: 'std::vector::insert()' do not returns an iterator, @@ -538,12 +551,11 @@ std::vector>& cmGeneratorTarget::ResolveLinkerWrapper( const std::string SHELL{ "SHELL:" }; const std::string LINKER_SHELL = LINKER + SHELL; - std::vector>::iterator entry; - while ((entry = std::find_if(result.begin(), result.end(), - [&LINKER](BT const& item) -> bool { - return item.Value.compare(0, LINKER.length(), - LINKER) == 0; - })) != result.end()) { + for (auto entry = result.begin(); entry != result.end(); ++entry) { + if (entry->Value.compare(0, LINKER.length(), LINKER) != 0) { + continue; + } + std::string value = std::move(entry->Value); cmListFileBacktrace bt = std::move(entry->Backtrace); entry = result.erase(entry); @@ -573,15 +585,19 @@ std::vector>& cmGeneratorTarget::ResolveLinkerWrapper( return result; } - std::vector> options = wrapOptions( - linkerOptions, bt, wrapperFlag, wrapperSep, concatFlagAndArgs); + // Very old versions of the C++ standard library return void for insert, so + // can't use it to get the new iterator + const auto index = entry - result.begin(); + std::vector> options = + wrapOptions(linkerOptions, bt, wrapperFlag, wrapperSep, + concatFlagAndArgs, NestedLinkerFlags::PreserveAsSpelled); if (joinItems) { - result.insert(entry, - cmJoin(cmRange( - options.cbegin(), options.cend()), - " "_s)); + result.insert( + entry, cmJoin(cmMakeRange(options.begin(), options.end()), " "_s)); + entry = std::next(result.begin(), index); } else { result.insert(entry, options.begin(), options.end()); + entry = std::next(result.begin(), index + options.size() - 1); } } return result; diff --git a/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER-check.cmake b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER-check.cmake index d0ef8de..99f5aa3 100644 --- a/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER-check.cmake +++ b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER-check.cmake @@ -1,2 +1,3 @@ +set(reference_file "LINKER.txt") include ("${CMAKE_CURRENT_LIST_DIR}/LINKER_expansion-validation.cmake") diff --git a/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED-check.cmake b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED-check.cmake new file mode 100644 index 0000000..522fe86 --- /dev/null +++ b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED-check.cmake @@ -0,0 +1,4 @@ + +set(reference_file "LINKER_NESTED.txt") +set(linker_prefix_expected YES) +include ("${CMAKE_CURRENT_LIST_DIR}/LINKER_expansion-validation.cmake") diff --git a/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED_SHELL-check.cmake b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED_SHELL-check.cmake new file mode 100644 index 0000000..917455c --- /dev/null +++ b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_NESTED_SHELL-check.cmake @@ -0,0 +1,4 @@ + +set(reference_file "LINKER_NESTED_SHELL.txt") +set(linker_prefix_expected YES) +include ("${CMAKE_CURRENT_LIST_DIR}/LINKER_expansion-validation.cmake") diff --git a/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_SHELL-check.cmake b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_SHELL-check.cmake index d0ef8de..99f5aa3 100644 --- a/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_SHELL-check.cmake +++ b/Tests/RunCMake/target_link_options/LINKER_expansion-LINKER_SHELL-check.cmake @@ -1,2 +1,3 @@ +set(reference_file "LINKER.txt") include ("${CMAKE_CURRENT_LIST_DIR}/LINKER_expansion-validation.cmake") diff --git a/Tests/RunCMake/target_link_options/LINKER_expansion-validation.cmake b/Tests/RunCMake/target_link_options/LINKER_expansion-validation.cmake index 1af8f13..27adde6 100644 --- a/Tests/RunCMake/target_link_options/LINKER_expansion-validation.cmake +++ b/Tests/RunCMake/target_link_options/LINKER_expansion-validation.cmake @@ -1,14 +1,14 @@ -if (actual_stdout MATCHES "LINKER:") +if (actual_stdout MATCHES "LINKER:" AND NOT linker_prefix_expected) set (RunCMake_TEST_FAILED "LINKER: prefix was not expanded.") return() endif() -if (NOT EXISTS "${RunCMake_TEST_BINARY_DIR}/LINKER.txt") - set (RunCMake_TEST_FAILED "${RunCMake_TEST_BINARY_DIR}/LINKER.txt: Reference file not found.") +if (NOT EXISTS "${RunCMake_TEST_BINARY_DIR}/${reference_file}") + set (RunCMake_TEST_FAILED "${RunCMake_TEST_BINARY_DIR}/${reference_file}: Reference file not found.") return() endif() -file(READ "${RunCMake_TEST_BINARY_DIR}/LINKER.txt" linker_flag) +file(READ "${RunCMake_TEST_BINARY_DIR}/${reference_file}" linker_flag) if (NOT actual_stdout MATCHES "${linker_flag}") set (RunCMake_TEST_FAILED "LINKER: was not expanded correctly.") diff --git a/Tests/RunCMake/target_link_options/LINKER_expansion.cmake b/Tests/RunCMake/target_link_options/LINKER_expansion.cmake index 8be6cef..2219218 100644 --- a/Tests/RunCMake/target_link_options/LINKER_expansion.cmake +++ b/Tests/RunCMake/target_link_options/LINKER_expansion.cmake @@ -31,6 +31,14 @@ target_link_options(linker PRIVATE "LINKER:-foo,bar") add_test_library(linker_shell) target_link_options(linker_shell PRIVATE "LINKER:SHELL:-foo bar") +# Nested LINKER: prefixes should be preserved as written, only the outermost LINKER: prefix removed +add_test_library(linker_nested) +target_link_options(linker_nested PRIVATE "LINKER:LINKER:-foo,-Xlinker=bar,-Xlinker,--baz,-Wl,/qux") + +# Same with LINKER:SHELL: +add_test_library(linker_nested_shell SHARED LinkOptionsLib.c) +target_link_options(linker_nested_shell PRIVATE "LINKER:SHELL:LINKER:-foo -Xlinker=bar -Xlinker --baz -Wl,/qux") + # generate reference for LINKER flag if (CMAKE_C_LINKER_WRAPPER_FLAG) set(linker_flag ${CMAKE_C_LINKER_WRAPPER_FLAG}) @@ -43,12 +51,22 @@ if (CMAKE_C_LINKER_WRAPPER_FLAG) list (JOIN linker_flag " " linker_flag) if (CMAKE_C_LINKER_WRAPPER_FLAG_SEP) set(linker_sep "${CMAKE_C_LINKER_WRAPPER_FLAG_SEP}") + + set(linker_flag_nested "${linker_flag}${linker_space}LINKER:-foo${linker_sep}-Xlinker=bar${linker_sep}-Xlinker${linker_sep}--baz${linker_sep}-Wl${linker_sep}/qux") + set(linker_flag_nested_shell "${linker_flag}${linker_space}LINKER:-foo${linker_sep}-Xlinker=bar${linker_sep}-Xlinker${linker_sep}--baz${linker_sep}-Wl,/qux") string (APPEND linker_flag "${linker_space}" "-foo${linker_sep}bar") else() set(linker_prefix "${linker_flag}${linker_space}") + + set(linker_flag_nested "${linker_prefix}LINKER:-foo ${linker_prefix}-Xlinker=bar ${linker_prefix}-Xlinker ${linker_prefix}--baz ${linker_prefix}-Wl ${linker_prefix}/qux") + set(linker_flag_nested_shell "${linker_prefix}LINKER:-foo ${linker_prefix}-Xlinker=bar ${linker_prefix}-Xlinker ${linker_prefix}--baz ${linker_prefix}-Wl,/qux") set (linker_flag "${linker_prefix}-foo ${linker_prefix}bar") endif() else() + set(linker_flag_nested "LINKER:-foo -Xlinker=bar -Xlinker --baz -Wl /qux") + set(linker_flag_nested_shell "LINKER:-foo -Xlinker=bar -Xlinker --baz -Wl,/qux") set(linker_flag "-foo bar") endif() file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/LINKER.txt" "${linker_flag}") +file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/LINKER_NESTED.txt" "${linker_flag_nested}") +file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/LINKER_NESTED_SHELL.txt" "${linker_flag_nested_shell}") diff --git a/Tests/RunCMake/target_link_options/RunCMakeTest.cmake b/Tests/RunCMake/target_link_options/RunCMakeTest.cmake index ff0c5a8..5562c9e 100644 --- a/Tests/RunCMake/target_link_options/RunCMakeTest.cmake +++ b/Tests/RunCMake/target_link_options/RunCMakeTest.cmake @@ -76,6 +76,8 @@ if(RunCMake_GENERATOR MATCHES "(Ninja|Makefile)") run_cmake_target(LINKER_expansion LINKER linker) run_cmake_target(LINKER_expansion LINKER_SHELL linker_shell) + run_cmake_target(LINKER_expansion LINKER_NESTED linker_nested) + run_cmake_target(LINKER_expansion LINKER_NESTED_SHELL linker_nested_shell) endif() run_cmake(empty_keyword_args) -- cgit v0.12