cmCommonTargetGenerator: Improve launcher shell conversion

Encapsulate the shell escaping logic in `cmCommonTargetGenerator` and
represent launchers via a vector of strings (as is done with custom
commands, execute process commands, etc.) to make them easier to work
with.

This facilitates work in a future commit which will reuse this pathway.

Issue: #27598, #27402

Source/cmCommonTargetGenerator.cxx

@@ -8,6 +8,7 @@
 
 #include <cm/filesystem>
 #include <cm/string_view>
+#include <cmext/algorithm>
 #include <cmext/string_view>
 
 #include "cmComputeLinkInformation.h"
@@ -422,10 +423,10 @@ void cmCommonTargetGenerator::AppendOSXVerFlag(std::string& flags,
   }
 }
 
-std::string cmCommonTargetGenerator::GetCompilerLauncher(
+std::vector<std::string> cmCommonTargetGenerator::GetCompilerLauncher(
   std::string const& lang, std::string const& config)
 {
-  std::string compilerLauncher;
+  std::vector<std::string> compilerLauncher;
   if (lang == "C" || lang == "CXX" || lang == "Fortran" || lang == "CUDA" ||
       lang == "HIP" || lang == "ISPC" || lang == "OBJC" || lang == "OBJCXX") {
     std::string const propName = cmStrCat(lang, "_COMPILER_LAUNCHER");
@@ -434,14 +435,15 @@ std::string cmCommonTargetGenerator::GetCompilerLauncher(
       *cLauncherValue, this->GeneratorTarget->GetLocalGenerator(), config,
       this->GeneratorTarget, nullptr, this->GeneratorTarget, lang);
     if (!evaluatedCLauncher.empty()) {
-      compilerLauncher = evaluatedCLauncher;
+      cm::append(compilerLauncher,
+                 cmList{ evaluatedCLauncher, cmList::EmptyElements::Yes });
     }
   }
   return compilerLauncher;
 }
 
 std::string cmCommonTargetGenerator::GenerateCodeCheckRules(
-  cmSourceFile const& source, std::string& compilerLauncher,
+  cmSourceFile const& source, std::vector<std::string>& compilerLauncher,
   std::string const& cmakeCmd, std::string const& config,
   std::function<std::string(std::string const&)> const& pathConverter)
 {
@@ -488,11 +490,14 @@ std::string cmCommonTargetGenerator::GenerateCodeCheckRules(
     std::string code_check = cmakeCmd + " -E __run_co_compile";
     if (!compilerLauncher.empty()) {
       // In __run_co_compile case the launcher command is supplied
-      // via --launcher=<maybe-list> and consumed
+      // via --launcher=<list> and consumed
+      for (std::string& arg : compilerLauncher) {
+        cmSystemTools::ReplaceString(arg, ";", "\\;");
+      }
       code_check =
         cmStrCat(std::move(code_check), " --launcher=",
                  this->GeneratorTarget->GetLocalGenerator()->EscapeForShell(
-                   compilerLauncher));
+                   cmJoin(compilerLauncher, ";")));
       compilerLauncher.clear();
     }
     if (cmNonempty(iwyu)) {
@@ -639,32 +644,44 @@ std::string cmCommonTargetGenerator::GenerateCodeCheckRules(
   return "";
 }
 
-std::string cmCommonTargetGenerator::GetLinkerLauncher(
+std::vector<std::string> cmCommonTargetGenerator::GetLinkerLauncher(
   std::string const& config)
 {
-  std::string lang = this->GeneratorTarget->GetLinkerLanguage(config);
-  std::string propName = lang + "_LINKER_LAUNCHER";
+  std::vector<std::string> linkLauncher;
+  std::string const lang = this->GeneratorTarget->GetLinkerLanguage(config);
+  std::string const propName = cmStrCat(lang, "_LINKER_LAUNCHER");
   cmValue launcherProp = this->GeneratorTarget->GetProperty(propName);
   if (cmNonempty(launcherProp)) {
     cm::GenEx::Context context(this->LocalCommonGenerator, config, lang);
     cmGeneratorExpressionDAGChecker dagChecker{ this->GeneratorTarget,
                                                 propName, nullptr, nullptr,
                                                 context };
-    std::string evaluatedLinklauncher = cmGeneratorExpression::Evaluate(
+
+    std::string const evaluatedLinkLauncher = cmGeneratorExpression::Evaluate(
       *launcherProp, context.LG, context.Config, this->GeneratorTarget,
       &dagChecker, this->GeneratorTarget, context.Language);
-    // Convert ;-delimited list to single string
-    cmList args{ evaluatedLinklauncher, cmList::EmptyElements::Yes };
-    if (!args.empty()) {
-      args[0] = this->LocalCommonGenerator->ConvertToOutputFormat(
-        args[0], cmOutputConverter::SHELL);
-      for (std::string& i : cmMakeRange(args.begin() + 1, args.end())) {
-        i = this->LocalCommonGenerator->EscapeForShell(i);
-      }
-      return cmJoin(args, " ");
+    if (!evaluatedLinkLauncher.empty()) {
+      cm::append(linkLauncher,
+                 cmList{ evaluatedLinkLauncher, cmList::EmptyElements::Yes });
     }
   }
-  return std::string();
+  return linkLauncher;
+}
+
+std::string cmCommonTargetGenerator::ConvertLauncherToShell(
+  std::vector<std::string> const& launcher) const
+{
+  if (launcher.empty()) {
+    return std::string{};
+  }
+
+  std::vector<std::string> args = launcher;
+  args[0] = this->LocalCommonGenerator->ConvertToOutputFormat(
+    args[0], cmOutputConverter::SHELL);
+  for (std::string& arg : cmMakeRange(args.begin() + 1, args.end())) {
+    arg = this->LocalCommonGenerator->EscapeForShell(arg);
+  }
+  return cmJoin(args, " ");
 }
 
 bool cmCommonTargetGenerator::HaveRequiredLanguages(

Source/cmCommonTargetGenerator.h

@@ -68,12 +68,12 @@ protected:
   std::string GetManifests(std::string const& config);
   std::string GetAIXExports(std::string const& config);
   std::string GenerateCodeCheckRules(
-    cmSourceFile const& source, std::string& compilerLauncher,
+    cmSourceFile const& source, std::vector<std::string>& compilerLauncher,
     std::string const& cmakeCmd, std::string const& config,
     std::function<std::string(std::string const&)> const& pathConverter);
 
-  std::string GetCompilerLauncher(std::string const& lang,
-                                  std::string const& config);
+  std::vector<std::string> GetCompilerLauncher(std::string const& lang,
+                                               std::string const& config);
 
   struct LinkedTargetDirs
   {
@@ -85,7 +85,10 @@ protected:
                                               std::string const& config) const;
   std::string ComputeTargetCompilePDB(std::string const& config) const;
 
-  std::string GetLinkerLauncher(std::string const& config);
+  std::vector<std::string> GetLinkerLauncher(std::string const& config);
+
+  std::string ConvertLauncherToShell(
+    std::vector<std::string> const& launcher) const;
 
   bool HaveRequiredLanguages(std::vector<cmSourceFile const*> const& sources,
                              std::set<std::string>& languagesNeeded) const;

Source/cmFastbuildNormalTargetGenerator.cxx

@@ -822,11 +822,12 @@ void cmFastbuildNormalTargetGenerator::AddCompilerLaunchersForLanguages()
     this->GetGeneratorTarget(), "RULE_LAUNCH_COMPILE", Config);
   // See if we need to use a compiler launcher like ccache or distcc
   for (std::string const& language : Languages) {
-    std::string const compilerLauncher =
+    std::vector<std::string> expanded =
       cmCommonTargetGenerator::GetCompilerLauncher(language, Config);
-    LogMessage("compilerLauncher: " + compilerLauncher);
-    std::vector<std::string> expanded;
-    cmExpandList(compilerLauncher, expanded);
+    LogMessage("compilerLauncher: " + cmJoin(expanded, ";"));
+    // FIXME(#27402): Empty arguments are not supported here.
+    expanded.erase(std::remove(expanded.begin(), expanded.end(), ""),
+                   expanded.end());
 
     if (!expanded.empty()) {
       std::string const exe = expanded[0];
@@ -844,14 +845,8 @@ void cmFastbuildNormalTargetGenerator::AddCompilerLaunchersForLanguages()
 }
 void cmFastbuildNormalTargetGenerator::AddLinkerLauncher()
 {
-  std::string const linkerLauncher =
+  std::vector<std::string> args =
     cmCommonTargetGenerator::GetLinkerLauncher(Config);
-  std::vector<std::string> args;
-#ifdef _WIN32
-  cmSystemTools::ParseWindowsCommandLine(linkerLauncher.c_str(), args);
-#else
-  cmSystemTools::ParseUnixCommandLine(linkerLauncher.c_str(), args);
-#endif
   if (!args.empty()) {
     std::string const exe = std::move(args[0]);
     args.erase(args.begin());
@@ -1191,7 +1186,7 @@ std::string cmFastbuildNormalTargetGenerator::ComputeCodeCheckOptions(
   if (skipCodeCheck) {
     return {};
   }
-  std::string compilerLauncher;
+  std::vector<std::string> compilerLauncher;
   std::string staticCheckRule = this->GenerateCodeCheckRules(
     srcFile, compilerLauncher, "", Config, nullptr);
   LogMessage(cmStrCat("CodeCheck: ", staticCheckRule));

Source/cmMakefileExecutableTargetGenerator.cxx

@@ -592,8 +592,8 @@ void cmMakefileExecutableTargetGenerator::WriteExecutableRule(bool relink)
     vars.LinkFlags = linkFlags.c_str();
     vars.Manifests = manifests.c_str();
 
-    std::string linkerLauncher =
-      this->GetLinkerLauncher(this->GetConfigName());
+    std::string const linkerLauncher = this->ConvertLauncherToShell(
+      this->GetLinkerLauncher(this->GetConfigName()));
     if (cmNonempty(linkerLauncher)) {
       vars.Launcher = linkerLauncher.c_str();
     }

Source/cmMakefileLibraryTargetGenerator.cxx

@@ -876,8 +876,8 @@ void cmMakefileLibraryTargetGenerator::WriteLibraryRules(
 
     vars.LanguageCompileFlags = langFlags.c_str();
 
-    std::string linkerLauncher =
-      this->GetLinkerLauncher(this->GetConfigName());
+    std::string const linkerLauncher = this->ConvertLauncherToShell(
+      this->GetLinkerLauncher(this->GetConfigName()));
     if (cmNonempty(linkerLauncher)) {
       vars.Launcher = linkerLauncher.c_str();
     }

Source/cmMakefileTargetGenerator.cxx

@@ -1106,7 +1106,7 @@ void cmMakefileTargetGenerator::WriteObjectRuleFiles(
     }
 
     // See if we need to use a compiler launcher like ccache or distcc
-    std::string compilerLauncher;
+    std::vector<std::string> compilerLauncher;
     if (!compileCommands.empty()) {
       compilerLauncher = GetCompilerLauncher(lang, config);
     }
@@ -1131,15 +1131,8 @@ void cmMakefileTargetGenerator::WriteObjectRuleFiles(
     // If compiler launcher was specified and not consumed above, it
     // goes to the beginning of the command line.
     if (!compileCommands.empty() && !compilerLauncher.empty()) {
-      cmList args{ compilerLauncher, cmList::EmptyElements::Yes };
-      if (!args.empty()) {
-        args[0] = this->LocalGenerator->ConvertToOutputFormat(
-          args[0], cmOutputConverter::SHELL);
-        for (std::string& i : cmMakeRange(args.begin() + 1, args.end())) {
-          i = this->LocalGenerator->EscapeForShell(i);
-        }
-      }
-      compileCommands.front().insert(0, args.join(" ") + " ");
+      compileCommands.front().insert(
+        0, cmStrCat(this->ConvertLauncherToShell(compilerLauncher), ' '));
     }
 
     std::string launcher;

Source/cmNinjaNormalTargetGenerator.cxx

@@ -584,7 +584,8 @@ void cmNinjaNormalTargetGenerator::WriteLinkRule(
       vars.LanguageCompileFlags = langFlags.c_str();
     }
 
-    std::string linkerLauncher = this->GetLinkerLauncher(config);
+    std::string const linkerLauncher =
+      this->ConvertLauncherToShell(this->GetLinkerLauncher(config));
     if (cmNonempty(linkerLauncher)) {
       vars.Launcher = linkerLauncher.c_str();
     }

Source/cmNinjaTargetGenerator.cxx

@@ -47,7 +47,6 @@
 #include "cmNinjaUtilityTargetGenerator.h"
 #include "cmOutputConverter.h"
 #include "cmPolicies.h"
-#include "cmRange.h"
 #include "cmRulePlaceholderExpander.h"
 #include "cmSourceFile.h"
 #include "cmSourceFileLocationKind.h"
@@ -1598,15 +1597,8 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement(
   // If compiler launcher was specified and not consumed above, it
   // goes to the beginning of the command line.
   if (!compilerLauncher.empty()) {
-    cmList args{ compilerLauncher, cmList::EmptyElements::Yes };
-    if (!args.empty()) {
-      args[0] = this->LocalGenerator->ConvertToOutputFormat(
-        args[0], cmOutputConverter::SHELL);
-      for (std::string& i : cmMakeRange(args.begin() + 1, args.end())) {
-        i = this->LocalGenerator->EscapeForShell(i);
-      }
-      vars["LAUNCHER"] = args.join(" ") + " ";
-    }
+    vars["LAUNCHER"] =
+      cmStrCat(this->ConvertLauncherToShell(compilerLauncher), ' ');
   }
 
   if (this->GetMakefile()->GetSafeDefinition(

Tests/RunCMake/CompilerLauncher/C-special-args-Build-stdout.txt

@@ -0,0 +1 @@
+special-args launcher ok: compile

Tests/RunCMake/CompilerLauncher/C-special-args-cppcheck-Build-stdout.txt

@@ -0,0 +1 @@
+special-args launcher ok: compile

Tests/RunCMake/CompilerLauncher/C-special-args-cppcheck.cmake

@@ -0,0 +1,3 @@
+# Lint tools run the launcher through "cmake -E __run_co_compile --launcher=...".
+set(CMAKE_C_CPPCHECK "${CMAKE_COMMAND};-E;true")
+include(C-special-args.cmake)

Tests/RunCMake/CompilerLauncher/C-special-args.cmake

@@ -0,0 +1,4 @@
+set(check "${CMAKE_CURRENT_LIST_DIR}/check-special-args.cmake")
+set(CMAKE_C_COMPILER_LAUNCHER
+  "${CMAKE_COMMAND};-P;${check};--;compile;;semi\\;colon;with space")
+include(C-common.cmake)

Tests/RunCMake/CompilerLauncher/RunCMakeTest.cmake

@@ -51,3 +51,10 @@ foreach(lang ${langs})
     run_compiler_launcher_env(${lang}-launch-env)
   endif()
 endforeach()
+
+if(NOT RunCMake_GENERATOR MATCHES "FASTBuild")
+  # FIXME(#27402): FASTBuild builds its launcher command line without shell
+  # quoting, so it can't handle empty or special-character arguments.
+  run_compiler_launcher(C-special-args)
+  run_compiler_launcher(C-special-args-cppcheck)
+endif()

Tests/RunCMake/CompilerLauncher/check-special-args.cmake

@@ -0,0 +1,41 @@
+# Verifies that the launcher's own arguments arrive intact, then runs the
+# launched command.
+
+set(base 4) # CMAKE_ARGV0..3 are: cmake -P <script> --
+set(num_args 4) # <phase> plus the three arguments below
+
+set(expect_1 "")
+set(expect_2 "semi;colon")
+set(expect_3 "with space")
+
+math(EXPR min_argc "${base} + ${num_args} + 1")
+if(CMAKE_ARGC LESS min_argc)
+  message(FATAL_ERROR "too few arguments: ${CMAKE_ARGC}")
+endif()
+
+foreach(i RANGE 1 3)
+  math(EXPR idx "${base} + ${i}")
+  if(NOT "${CMAKE_ARGV${idx}}" STREQUAL "${expect_${i}}")
+    message(FATAL_ERROR
+      "launcher argument ${i}: expected [${expect_${i}}] but got [${CMAKE_ARGV${idx}}]")
+  endif()
+endforeach()
+message("special-args launcher ok: ${CMAKE_ARGV${base}}")
+
+# Build and invoke the launched command via cmake_language(EVAL CODE) using
+# bracket arguments, rather than a ";"-separated list. A list element ending
+# in a backslash (e.g. MSVC's "/Fd<dir>\") would otherwise merge with the
+# list's own separator when the list is later expanded, corrupting the
+# command line.
+set(cmd_code "execute_process(COMMAND")
+math(EXPR first "${base} + ${num_args}")
+math(EXPR last "${CMAKE_ARGC} - 1")
+foreach(i RANGE ${first} ${last})
+  string(APPEND cmd_code " [==[${CMAKE_ARGV${i}}]==]")
+endforeach()
+string(APPEND cmd_code " RESULT_VARIABLE result)")
+cmake_language(EVAL CODE "${cmd_code}")
+
+if(NOT result EQUAL 0)
+  message(FATAL_ERROR "launched command failed: ${result}")
+endif()

Tests/RunCMake/LinkerLauncher/C-special-args-build-check.cmake

@@ -0,0 +1,12 @@
+# Launcher invocations cannot be reliably verified via stdout, so it records to
+# a log file instead. Verify by checking its content.
+set(log_file "${RunCMake_TEST_BINARY_DIR}/special-args.log")
+if(NOT EXISTS "${log_file}")
+  set(RunCMake_TEST_FAILED "special-args.log was not created by the wrapper")
+else()
+  file(READ "${log_file}" log_content)
+  if(NOT log_content MATCHES "link")
+    set(RunCMake_TEST_FAILED
+      "special-args.log does not show link launcher ran:\n${log_content}")
+  endif()
+endif()

Tests/RunCMake/LinkerLauncher/C-special-args.cmake

@@ -0,0 +1,4 @@
+set(check "${CMAKE_CURRENT_LIST_DIR}/check-special-args.cmake")
+set(CMAKE_C_LINKER_LAUNCHER
+  "${CMAKE_COMMAND};-P;${check};--;link;;semi\\;colon;with space")
+include(C-common.cmake)

Tests/RunCMake/LinkerLauncher/RunCMakeTest.cmake

@@ -48,3 +48,9 @@ foreach(lang ${langs})
     run_linker_launcher_env(${lang}-launch-env)
   endif()
 endforeach()
+
+if(NOT RunCMake_GENERATOR MATCHES "FASTBuild")
+  # FIXME(#27402): FASTBuild builds its launcher command line without shell
+  # quoting, so it can't handle empty or special-character arguments.
+  run_linker_launcher(C-special-args)
+endif()

Tests/RunCMake/LinkerLauncher/check-special-args.cmake

@@ -0,0 +1,42 @@
+# Verifies that the launcher's own arguments arrive intact, then runs the
+# launched command.
+
+set(base 4) # CMAKE_ARGV0..3 are: cmake -P <script> --
+set(num_args 4) # <phase> plus the three arguments below
+
+set(expect_1 "")
+set(expect_2 "semi;colon")
+set(expect_3 "with space")
+
+math(EXPR min_argc "${base} + ${num_args} + 1")
+if(CMAKE_ARGC LESS min_argc)
+  message(FATAL_ERROR "too few arguments: ${CMAKE_ARGC}")
+endif()
+
+foreach(i RANGE 1 3)
+  math(EXPR idx "${base} + ${i}")
+  if(NOT "${CMAKE_ARGV${idx}}" STREQUAL "${expect_${i}}")
+    message(FATAL_ERROR
+      "launcher argument ${i}: expected [${expect_${i}}] but got [${CMAKE_ARGV${idx}}]")
+  endif()
+endforeach()
+
+file(APPEND "special-args.log" "${CMAKE_ARGV${base}}\n")
+
+# Build and invoke the launched command via cmake_language(EVAL CODE) using
+# bracket arguments, rather than a ";"-separated list. A list element ending
+# in a backslash (e.g. MSVC's "/Fd<dir>\") would otherwise merge with the
+# list's own separator when the list is later expanded, corrupting the
+# command line.
+set(cmd_code "execute_process(COMMAND")
+math(EXPR first "${base} + ${num_args}")
+math(EXPR last "${CMAKE_ARGC} - 1")
+foreach(i RANGE ${first} ${last})
+  string(APPEND cmd_code " [==[${CMAKE_ARGV${i}}]==]")
+endforeach()
+string(APPEND cmd_code " RESULT_VARIABLE result)")
+cmake_language(EVAL CODE "${cmd_code}")
+
+if(NOT result EQUAL 0)
+  message(FATAL_ERROR "launched command failed: ${result}")
+endif()