Browse Source

cmInstallTargetGenerator: Introduce CMP0095

Escape coincidental CMake syntax in RPATH entries when generating the
intermediary cmake_install.cmake script.

Fixes #19225
Dennis Klein 6 years ago
parent
commit
9e84c7c5e8

+ 1 - 0
Help/manual/cmake-policies.7.rst

@@ -57,6 +57,7 @@ Policies Introduced by CMake 3.15
 .. toctree::
    :maxdepth: 1
 
+   CMP0095: RPATH entries are properly escaped in the intermediary CMake install script. </policy/CMP0095>
    CMP0094: FindPython3, FindPython2 and FindPython use LOCATION for lookup strategy. </policy/CMP0094>
    CMP0093: FindBoost reports Boost_VERSION in x.y.z format. </policy/CMP0093>
    CMP0092: MSVC warning flags are not in CMAKE_{C,CXX}_FLAGS by default. </policy/CMP0092>

+ 30 - 0
Help/policy/CMP0095.rst

@@ -0,0 +1,30 @@
+CMP0095
+-------
+
+``RPATH`` entries are properly escaped in the intermediary CMake install script.
+
+In CMake 3.15 and earlier, ``RPATH`` entries set via
+:variable:`CMAKE_INSTALL_RPATH` or via :prop_tgt:`INSTALL_RPATH` have not been
+escaped before being inserted into the ``cmake_install.cmake`` script. Dynamic
+linkers on ELF-based systems (e.g. Linux and FreeBSD) allow certain keywords in
+``RPATH`` entries, such as ``${ORIGIN}`` (More details are available in the
+``ld.so`` man pages on those systems). The syntax of these keywords can match
+CMake's variable syntax. In order to not be substituted (usually to an empty
+string) already by the intermediary ``cmake_install.cmake`` script, the user had
+to double-escape such ``RPATH`` keywords, e.g.
+``set(CMAKE_INSTALL_RPATH "\\\${ORIGIN}/../lib")``. Since the intermediary
+``cmake_install.cmake`` script is an implementation detail of CMake, CMake 3.16
+and later will make sure ``RPATH`` entries are inserted literally by escaping
+any coincidental CMake syntax.
+
+The ``OLD`` behavior of this policy is to not escape ``RPATH`` entries in the
+intermediary ``cmake_install.cmake`` script. The ``NEW`` behavior is to properly
+escape coincidental CMake syntax in ``RPATH`` entries when generating the
+intermediary ``cmake_install.cmake`` script.
+
+This policy was introduced in CMake version 3.16. CMake version |release| warns
+when the policy is not set and detected usage of CMake-like syntax and uses
+``OLD`` behavior. Use the :command:`cmake_policy` command to set it to ``OLD``
+or ``NEW`` explicitly.
+
+.. include:: DEPRECATED.txt

+ 5 - 0
Help/release/dev/CMP0095.rst

@@ -0,0 +1,5 @@
+CMP0095
+-------
+
+* ``RPATH`` entries are properly escaped in the intermediary CMake install script.
+  See policy :policy:`CMP0095`.

+ 67 - 9
Source/cmInstallTargetGenerator.cxx

@@ -16,6 +16,8 @@
 #include "cmLocalGenerator.h"
 #include "cmMakefile.h"
 #include "cmMessageType.h"
+#include "cmOutputConverter.h"
+#include "cmPolicies.h"
 #include "cmStateTypes.h"
 #include "cmSystemTools.h"
 #include "cmTarget.h"
@@ -632,17 +634,34 @@ void cmInstallTargetGenerator::AddRPathCheckRule(
     return;
   }
 
-  // Get the install RPATH from the link information.
-  std::string newRpath = cli->GetChrpathString();
-
   // Write a rule to remove the installed file if its rpath is not the
   // new rpath.  This is needed for existing build/install trees when
   // the installed rpath changes but the file is not rebuilt.
-  /* clang-format off */
   os << indent << "file(RPATH_CHECK\n"
-     << indent << "     FILE \"" << toDestDirPath << "\"\n"
-     << indent << "     RPATH \"" << newRpath << "\")\n";
-  /* clang-format on */
+     << indent << "     FILE \"" << toDestDirPath << "\"\n";
+
+  // CMP0095: ``RPATH`` entries are properly escaped in the intermediary
+  // CMake install script.
+  switch (this->Target->GetPolicyStatusCMP0095()) {
+    case cmPolicies::WARN:
+      // No author warning needed here, we warn later in
+      // cmInstallTargetGenerator::AddChrpathPatchRule().
+      CM_FALLTHROUGH;
+    case cmPolicies::OLD: {
+      // Get the install RPATH from the link information.
+      std::string newRpath = cli->GetChrpathString();
+      os << indent << "     RPATH \"" << newRpath << "\")\n";
+      break;
+    }
+    default: {
+      // Get the install RPATH from the link information and
+      // escape any CMake syntax in the install RPATH.
+      std::string escapedNewRpath =
+        cmOutputConverter::EscapeForCMake(cli->GetChrpathString());
+      os << indent << "     RPATH " << escapedNewRpath << ")\n";
+      break;
+    }
+  }
 }
 
 void cmInstallTargetGenerator::AddChrpathPatchRule(
@@ -731,11 +750,27 @@ void cmInstallTargetGenerator::AddChrpathPatchRule(
       return;
     }
 
+    // Escape any CMake syntax in the install RPATH.
+    std::string escapedNewRpath = cmOutputConverter::EscapeForCMake(newRpath);
+
     // Write a rule to run chrpath to set the install-tree RPATH
     os << indent << "file(RPATH_CHANGE\n"
        << indent << "     FILE \"" << toDestDirPath << "\"\n"
-       << indent << "     OLD_RPATH \"" << oldRpath << "\"\n"
-       << indent << "     NEW_RPATH \"" << newRpath << "\")\n";
+       << indent << "     OLD_RPATH \"" << oldRpath << "\"\n";
+
+    // CMP0095: ``RPATH`` entries are properly escaped in the intermediary
+    // CMake install script.
+    switch (this->Target->GetPolicyStatusCMP0095()) {
+      case cmPolicies::WARN:
+        this->IssueCMP0095Warning(newRpath);
+        CM_FALLTHROUGH;
+      case cmPolicies::OLD:
+        os << indent << "     NEW_RPATH \"" << newRpath << "\")\n";
+        break;
+      default:
+        os << indent << "     NEW_RPATH " << escapedNewRpath << ")\n";
+        break;
+    }
   }
 }
 
@@ -838,3 +873,26 @@ void cmInstallTargetGenerator::AddUniversalInstallRule(
      << "\"" << this->Target->Target->GetName() << "\" "
      << "\"" << toDestDirPath << "\")\n";
 }
+
+void cmInstallTargetGenerator::IssueCMP0095Warning(
+  const std::string& unescapedRpath)
+{
+  // Reduce warning noise to cases where used RPATHs may actually be affected
+  // by CMP0095. This filter is meant to skip warnings in cases when
+  // non-curly-braces syntax (e.g. $ORIGIN) or no keyword is used which has
+  // worked already before CMP0095. We intend to issue a warning in all cases
+  // with curly-braces syntax, even if the workaround of double-escaping is in
+  // place, since we deprecate the need for it with CMP0095.
+  const bool potentially_affected(unescapedRpath.find("${") !=
+                                  std::string::npos);
+
+  if (potentially_affected) {
+    std::ostringstream w;
+    w << cmPolicies::GetPolicyWarning(cmPolicies::CMP0095) << "\n";
+    w << "RPATH entries for target '" << this->Target->GetName() << "' "
+      << "will not be escaped in the intermediary "
+      << "cmake_install.cmake script.";
+    this->Target->GetGlobalGenerator()->GetCMakeInstance()->IssueMessage(
+      MessageType::AUTHOR_WARNING, w.str(), this->GetBacktrace());
+  }
+}

+ 1 - 0
Source/cmInstallTargetGenerator.h

@@ -104,6 +104,7 @@ protected:
                      const std::string& toDestDirPath);
   void AddUniversalInstallRule(std::ostream& os, Indent indent,
                                const std::string& toDestDirPath);
+  void IssueCMP0095Warning(const std::string& unescapedRpath);
 
   std::string TargetName;
   cmGeneratorTarget* Target;

+ 7 - 2
Source/cmPolicies.h

@@ -279,7 +279,11 @@ class cmMakefile;
   SELECT(POLICY, CMP0094,                                                     \
          "FindPython3,  FindPython2 and FindPyton use "                       \
          "LOCATION for lookup strategy.",                                     \
-         3, 15, 0, cmPolicies::WARN)
+         3, 15, 0, cmPolicies::WARN)                                          \
+  SELECT(POLICY, CMP0095,                                                     \
+         "RPATH entries are properly escaped in the intermediary CMake "      \
+         "install script.",                                                   \
+         3, 16, 0, cmPolicies::WARN)
 
 #define CM_SELECT_ID(F, A1, A2, A3, A4, A5, A6) F(A1)
 #define CM_FOR_EACH_POLICY_ID(POLICY)                                         \
@@ -307,7 +311,8 @@ class cmMakefile;
   F(CMP0073)                                                                  \
   F(CMP0076)                                                                  \
   F(CMP0081)                                                                  \
-  F(CMP0083)
+  F(CMP0083)                                                                  \
+  F(CMP0095)
 
 /** \class cmPolicies
  * \brief Handles changes in CMake behavior and policies

+ 1 - 0
Tests/RunCMake/TargetPolicies/PolicyList-stderr.txt

@@ -27,6 +27,7 @@
    \* CMP0076
    \* CMP0081
    \* CMP0083
+   \* CMP0095
 
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)

+ 2 - 1
Tests/RunCMake/install/file-GET_RUNTIME_DEPENDENCIES-linux-notfile.cmake

@@ -1,4 +1,5 @@
 enable_language(C)
+cmake_policy(SET CMP0095 NEW)
 
 file(WRITE "${CMAKE_BINARY_DIR}/test.c" "void test(void) {}\n")
 file(WRITE "${CMAKE_BINARY_DIR}/main.c" [[extern void test(void);
@@ -13,7 +14,7 @@ int main(void)
 add_library(test SHARED "${CMAKE_BINARY_DIR}/test.c")
 add_executable(exe "${CMAKE_BINARY_DIR}/main.c")
 target_link_libraries(exe PRIVATE test)
-set_property(TARGET exe PROPERTY INSTALL_RPATH "\\\${ORIGIN}/../lib")
+set_property(TARGET exe PROPERTY INSTALL_RPATH "\${ORIGIN}/../lib")
 
 install(TARGETS exe DESTINATION bin)
 

+ 5 - 4
Tests/RunCMake/install/file-GET_RUNTIME_DEPENDENCIES-linux.cmake

@@ -1,4 +1,5 @@
 enable_language(C)
+cmake_policy(SET CMP0095 NEW)
 
 set(test_rpath_names
   preexcluded
@@ -52,8 +53,8 @@ target_link_libraries(test_rpath PRIVATE ${test_rpath_names})
 set_property(TARGET test_rpath PROPERTY INSTALL_RPATH
   "${CMAKE_BINARY_DIR}/root-all/lib/rpath_postexcluded"
   "${CMAKE_BINARY_DIR}/root-all/lib/rpath"
-  "\\\$ORIGIN/rpath_origin_postexcluded"
-  "\\\${ORIGIN}/rpath_origin" # This must be double-escaped because of issue #19225.
+  "\$ORIGIN/rpath_origin_postexcluded"
+  "\${ORIGIN}/rpath_origin"
   "${CMAKE_BINARY_DIR}/root-all/lib/conflict"
   )
 target_link_options(test_rpath PRIVATE -Wl,--disable-new-dtags)
@@ -88,8 +89,8 @@ set_property(TARGET test_runpath PROPERTY INSTALL_RPATH
   "${CMAKE_BINARY_DIR}/root-all/lib/runpath/../rpath" # Ensure that files that don't conflict are treated correctly
   "${CMAKE_BINARY_DIR}/root-all/lib/runpath_postexcluded"
   "${CMAKE_BINARY_DIR}/root-all/lib/runpath"
-  "\\\${ORIGIN}/runpath_origin_postexcluded" # This must be double-escaped because of issue #19225.
-  "\\\$ORIGIN/runpath_origin"
+  "\${ORIGIN}/runpath_origin_postexcluded"
+  "\$ORIGIN/runpath_origin"
   "${CMAKE_BINARY_DIR}/root-all/lib/conflict2"
   )
 target_link_options(test_runpath PRIVATE -Wl,--enable-new-dtags)