From 726e6454b50613d95dda83f329c452293b75d3c6 Mon Sep 17 00:00:00 2001 From: Florent Castelli Date: Mon, 21 Sep 2026 18:07:48 +0200 Subject: [PATCH] Ninja,Makefile: Do not convert separators in the install-name directory The Ninja and Makefile generators passed the install-name directory of a shared library (`@rpath/` by default, or `INSTALL_NAME_DIR`) through `ConvertToOutputFormat(SHELL)`. That converts directory separators for the host shell, so a Windows host cross-compiling for macOS wrote `-install_name @rpath\libfoo.dylib`. The install name is not a host path: it is embedded in the Mach-O load command and read by dyld on the target, where a backslash never matches anything. Escape the value for the shell without converting separators, in both generators. Add a test to each that fakes an install-name platform and checks the generated build files keep the forward slashes. Fixes: #28104 --- Source/cmMakefileLibraryTargetGenerator.cxx | 7 ++++--- Source/cmNinjaNormalTargetGenerator.cxx | 10 ++++++---- Tests/RunCMake/Make/InstallNameDir-check.cmake | 17 +++++++++++++++++ Tests/RunCMake/Make/InstallNameDir.cmake | 17 +++++++++++++++++ Tests/RunCMake/Make/RunCMakeTest.cmake | 2 ++ Tests/RunCMake/Ninja/InstallNameDir-check.cmake | 12 ++++++++++++ Tests/RunCMake/Ninja/InstallNameDir.cmake | 15 +++++++++++++++ Tests/RunCMake/Ninja/RunCMakeTest.cmake | 2 ++ 8 files changed, 75 insertions(+), 7 deletions(-) create mode 100644 Tests/RunCMake/Make/InstallNameDir-check.cmake create mode 100644 Tests/RunCMake/Make/InstallNameDir.cmake create mode 100644 Tests/RunCMake/Ninja/InstallNameDir-check.cmake create mode 100644 Tests/RunCMake/Ninja/InstallNameDir.cmake diff --git a/Source/cmMakefileLibraryTargetGenerator.cxx b/Source/cmMakefileLibraryTargetGenerator.cxx index 9991868351..01adabe89f 100644 --- a/Source/cmMakefileLibraryTargetGenerator.cxx +++ b/Source/cmMakefileLibraryTargetGenerator.cxx @@ -859,9 +859,10 @@ void cmMakefileLibraryTargetGenerator::WriteLibraryRules( if (install_name_dir.empty()) { vars.TargetInstallNameDir = ""; } else { - // Convert to a path for the native build tool. - install_name_dir = this->LocalGenerator->ConvertToOutputFormat( - install_name_dir, cmOutputConverter::SHELL); + // The install name is target data embedded in the binary, not a + // host path: escape it for the shell without converting separators. + install_name_dir = + this->LocalGenerator->EscapeForShell(install_name_dir, true); vars.TargetInstallNameDir = install_name_dir.c_str(); } } diff --git a/Source/cmNinjaNormalTargetGenerator.cxx b/Source/cmNinjaNormalTargetGenerator.cxx index 8a569188a3..67d2f25c11 100644 --- a/Source/cmNinjaNormalTargetGenerator.cxx +++ b/Source/cmNinjaNormalTargetGenerator.cxx @@ -1073,8 +1073,9 @@ void cmNinjaNormalTargetGenerator::WriteNvidiaDeviceLinkStatement( std::string install_dir = this->GetGeneratorTarget()->GetInstallNameDirForBuildTree(config); if (!install_dir.empty()) { - vars["INSTALLNAME_DIR"] = localGen.ConvertToOutputFormat( - install_dir, cmOutputConverter::SHELL); + // The install name is target data embedded in the binary, + // not a host path: escape it without converting separators. + vars["INSTALLNAME_DIR"] = localGen.EscapeForShell(install_dir, true); } } } @@ -1436,8 +1437,9 @@ void cmNinjaNormalTargetGenerator::WriteLinkStatement( if (targetType == cm::TargetType::SHARED_LIBRARY) { std::string install_dir = gt->GetInstallNameDirForBuildTree(config); if (!install_dir.empty()) { - vars["INSTALLNAME_DIR"] = localGen.ConvertToOutputFormat( - install_dir, cmOutputConverter::SHELL); + // The install name is target data embedded in the binary, + // not a host path: escape it without converting separators. + vars["INSTALLNAME_DIR"] = localGen.EscapeForShell(install_dir, true); } } } diff --git a/Tests/RunCMake/Make/InstallNameDir-check.cmake b/Tests/RunCMake/Make/InstallNameDir-check.cmake new file mode 100644 index 0000000000..561103008b --- /dev/null +++ b/Tests/RunCMake/Make/InstallNameDir-check.cmake @@ -0,0 +1,17 @@ +# The install-name directory is target data embedded in the binary, so it +# must keep its forward slashes on every host. +foreach(case "rpath_dir;@rpath/" "custom_dir;/custom/dir/") + list(GET case 0 target) + list(GET case 1 dir) + # Generators without link scripts inline the link command in build.make. + set(link_txt "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/${target}.dir/link.txt") + if(NOT EXISTS "${link_txt}") + set(link_txt "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/${target}.dir/build.make") + endif() + file(READ "${link_txt}" content) + string(FIND "${content}" "-install_name ${dir}" pos) + if(pos EQUAL -1) + string(APPEND RunCMake_TEST_FAILED + "${link_txt} does not contain '-install_name ${dir}':\n${content}") + endif() +endforeach() diff --git a/Tests/RunCMake/Make/InstallNameDir.cmake b/Tests/RunCMake/Make/InstallNameDir.cmake new file mode 100644 index 0000000000..c4f879b9c6 --- /dev/null +++ b/Tests/RunCMake/Make/InstallNameDir.cmake @@ -0,0 +1,17 @@ +enable_language(C) + +# Pretend the platform supports install names, as Darwin does, so that the +# generator writes the install-name directory on every host. +set(CMAKE_PLATFORM_HAS_INSTALLNAME 1) +set(CMAKE_SHARED_LIBRARY_SONAME_C_FLAG "-install_name ") +set(CMAKE_SHARED_LIBRARY_RUNTIME_C_FLAG "-Wl,-rpath,") +set(CMAKE_C_CREATE_SHARED_LIBRARY + " -o ") + +add_library(rpath_dir SHARED hello.c) +set_property(TARGET rpath_dir PROPERTY AIX_SHARED_LIBRARY_ARCHIVE OFF) + +add_library(custom_dir SHARED hello.c) +set_property(TARGET custom_dir PROPERTY INSTALL_NAME_DIR "/custom/dir") +set_property(TARGET custom_dir PROPERTY BUILD_WITH_INSTALL_NAME_DIR ON) +set_property(TARGET custom_dir PROPERTY AIX_SHARED_LIBRARY_ARCHIVE OFF) diff --git a/Tests/RunCMake/Make/RunCMakeTest.cmake b/Tests/RunCMake/Make/RunCMakeTest.cmake index 5b8e839945..a03814d7ca 100644 --- a/Tests/RunCMake/Make/RunCMakeTest.cmake +++ b/Tests/RunCMake/Make/RunCMakeTest.cmake @@ -262,3 +262,5 @@ if(FAKE_MAKE AND RunCMake_GENERATOR MATCHES "Unix Makefiles|MinGW Makefiles|MSYS endfunction() run_OutputSyncUsesTerminal() endif() + +run_cmake(InstallNameDir) diff --git a/Tests/RunCMake/Ninja/InstallNameDir-check.cmake b/Tests/RunCMake/Ninja/InstallNameDir-check.cmake new file mode 100644 index 0000000000..c77d521898 --- /dev/null +++ b/Tests/RunCMake/Ninja/InstallNameDir-check.cmake @@ -0,0 +1,12 @@ +# The install-name directory is target data embedded in the binary, so it +# must keep its forward slashes on every host. +set(build_ninja "${RunCMake_TEST_BINARY_DIR}/build.ninja") +file(READ "${build_ninja}" content) +foreach(expected "INSTALLNAME_DIR = @rpath/\n" "INSTALLNAME_DIR = /custom/dir/\n") + string(FIND "${content}" "${expected}" pos) + if(pos EQUAL -1) + string(STRIP "${expected}" expected) + string(APPEND RunCMake_TEST_FAILED + "${build_ninja} does not contain the line:\n ${expected}\n") + endif() +endforeach() diff --git a/Tests/RunCMake/Ninja/InstallNameDir.cmake b/Tests/RunCMake/Ninja/InstallNameDir.cmake new file mode 100644 index 0000000000..17320a6b7d --- /dev/null +++ b/Tests/RunCMake/Ninja/InstallNameDir.cmake @@ -0,0 +1,15 @@ +enable_language(C) + +# Pretend the platform supports install names, as Darwin does, so that the +# generator writes the install-name directory on every host. +set(CMAKE_PLATFORM_HAS_INSTALLNAME 1) +set(CMAKE_SHARED_LIBRARY_SONAME_C_FLAG "-install_name ") +set(CMAKE_SHARED_LIBRARY_RUNTIME_C_FLAG "-Wl,-rpath,") +set(CMAKE_C_CREATE_SHARED_LIBRARY + " -o ") + +add_library(rpath_dir SHARED hello.c) + +add_library(custom_dir SHARED hello.c) +set_property(TARGET custom_dir PROPERTY INSTALL_NAME_DIR "/custom/dir") +set_property(TARGET custom_dir PROPERTY BUILD_WITH_INSTALL_NAME_DIR ON) diff --git a/Tests/RunCMake/Ninja/RunCMakeTest.cmake b/Tests/RunCMake/Ninja/RunCMakeTest.cmake index 65202f77ab..029b42569d 100644 --- a/Tests/RunCMake/Ninja/RunCMakeTest.cmake +++ b/Tests/RunCMake/Ninja/RunCMakeTest.cmake @@ -465,3 +465,5 @@ endif() run_cmake(LINK_OPTIONSWithNewlines) run_cmake(StaticLibShort) + +run_cmake(InstallNameDir)