From e017ba046ccab62fcc67e7cf8fc858e991c1d3e7 Mon Sep 17 00:00:00 2001 From: Brad King Date: Tue, 2 Feb 2021 19:27:09 -0500 Subject: AIX: Enable XCOFF editing to replace RPATH on installation Avoid relinking before installation. --- Help/release/dev/aix-xcoff-edit.rst | 8 ++ Source/cmGeneratorTarget.cxx | 27 +++-- Source/cmSystemTools.cxx | 113 ++++++++++++++++++++- Tests/RunCMake/CMakeLists.txt | 4 +- Tests/RunCMake/file-RPATH/RunCMakeTest.cmake | 4 + Tests/RunCMake/file-RPATH/XCOFF.cmake | 7 ++ Tests/RunCMake/install/RunCMakeTest.cmake | 10 +- .../TARGETS-FILE_RPATH_CHANGE-check-common.cmake | 2 + 8 files changed, 157 insertions(+), 18 deletions(-) create mode 100644 Help/release/dev/aix-xcoff-edit.rst create mode 100644 Tests/RunCMake/file-RPATH/XCOFF.cmake diff --git a/Help/release/dev/aix-xcoff-edit.rst b/Help/release/dev/aix-xcoff-edit.rst new file mode 100644 index 0000000..be3143d --- /dev/null +++ b/Help/release/dev/aix-xcoff-edit.rst @@ -0,0 +1,8 @@ +aix-xcoff-edit +-------------- + +* On AIX, installation of XCOFF executables and shared libraries + no longer requires relinking to change the runtime search path + from the build-tree RPATH to the install-tree RPATH. CMake now + edits the XCOFF binaries directly during installation, as has + long been done on ELF platforms. diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 0171d8a..9235faa 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -2009,10 +2009,10 @@ bool cmGeneratorTarget::NeedRelinkBeforeInstall( std::ostringstream w; /* clang-format off */ w << - "The install of the " << this->GetName() << " target requires " - "changing an RPATH from the build tree, but this is not supported " - "with the Ninja generator unless on an ELF-based platform. The " - "CMAKE_BUILD_WITH_INSTALL_RPATH variable may be set to avoid this " + "The install of the " << this->GetName() << " target requires changing " + "an RPATH from the build tree, but this is not supported with the Ninja " + "generator unless on an ELF-based or XCOFF-based platform. " + "The CMAKE_BUILD_WITH_INSTALL_RPATH variable may be set to avoid this " "relinking step." ; /* clang-format on */ @@ -2058,20 +2058,29 @@ bool cmGeneratorTarget::IsChrpathUsed(const std::string& config) const return true; } -#if defined(CMake_USE_ELF_PARSER) - // Enable if the rpath flag uses a separator and the target uses ELF - // binaries. +#if defined(CMake_USE_ELF_PARSER) || defined(CMake_USE_XCOFF_PARSER) + // Enable if the rpath flag uses a separator and the target uses + // binaries we know how to edit. std::string ll = this->GetLinkerLanguage(config); if (!ll.empty()) { std::string sepVar = cmStrCat("CMAKE_SHARED_LIBRARY_RUNTIME_", ll, "_FLAG_SEP"); cmProp sep = this->Makefile->GetDefinition(sepVar); if (cmNonempty(sep)) { - // TODO: Add ELF check to ABI detection and get rid of + // TODO: Add binary format check to ABI detection and get rid of // CMAKE_EXECUTABLE_FORMAT. if (cmProp fmt = this->Makefile->GetDefinition("CMAKE_EXECUTABLE_FORMAT")) { - return (*fmt == "ELF"); +# if defined(CMake_USE_ELF_PARSER) + if (*fmt == "ELF") { + return true; + } +# endif +# if defined(CMake_USE_XCOFF_PARSER) + if (*fmt == "XCOFF") { + return true; + } +# endif } } } diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index 8c5a657..0807590 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -53,6 +53,10 @@ # include "cmMachO.h" #endif +#if defined(CMake_USE_XCOFF_PARSER) +# include "cmXCOFF.h" +#endif + #include #include #include @@ -2360,9 +2364,9 @@ bool cmSystemTools::GuessLibraryInstallName(std::string const& fullPath, return false; } -#if defined(CMake_USE_ELF_PARSER) -std::string::size_type cmSystemToolsFindRPath(std::string const& have, - std::string const& want) +#if defined(CMake_USE_ELF_PARSER) || defined(CMake_USE_XCOFF_PARSER) +std::string::size_type cmSystemToolsFindRPath(cm::string_view const& have, + cm::string_view const& want) { std::string::size_type pos = 0; while (pos < have.size()) { @@ -2404,6 +2408,7 @@ struct cmSystemToolsRPathInfo }; #endif +// FIXME: Dispatch if multiple formats are supported. #if defined(CMake_USE_ELF_PARSER) bool cmSystemTools::ChangeRPath(std::string const& file, std::string const& oldRPath, @@ -2576,6 +2581,75 @@ bool cmSystemTools::ChangeRPath(std::string const& file, } return true; } +#elif defined(CMake_USE_XCOFF_PARSER) +bool cmSystemTools::ChangeRPath(std::string const& file, + std::string const& oldRPath, + std::string const& newRPath, + bool removeEnvironmentRPath, std::string* emsg, + bool* changed) +{ + if (changed) { + *changed = false; + } + + bool chg = false; + cmXCOFF xcoff(file.c_str(), cmXCOFF::Mode::ReadWrite); + if (cm::optional maybeLibPath = xcoff.GetLibPath()) { + cm::string_view libPath = *maybeLibPath; + // Make sure the current rpath contains the old rpath. + std::string::size_type pos = cmSystemToolsFindRPath(libPath, oldRPath); + if (pos == std::string::npos) { + // If it contains the new rpath instead then it is okay. + if (cmSystemToolsFindRPath(libPath, newRPath) != std::string::npos) { + return true; + } + if (emsg) { + std::ostringstream e; + /* clang-format off */ + e << "The current RPATH is:\n" + << " " << libPath << "\n" + << "which does not contain:\n" + << " " << oldRPath << "\n" + << "as was expected."; + /* clang-format on */ + *emsg = e.str(); + } + return false; + } + + // The prefix is either empty or ends in a ':'. + cm::string_view prefix = libPath.substr(0, pos); + if (newRPath.empty() && !prefix.empty()) { + prefix.remove_suffix(1); + } + + // The suffix is either empty or starts in a ':'. + cm::string_view suffix = libPath.substr(pos + oldRPath.length()); + + // Construct the new value which preserves the part of the path + // not being changed. + std::string newLibPath; + if (!removeEnvironmentRPath) { + newLibPath = std::string(prefix); + } + newLibPath += newRPath; + newLibPath += suffix; + + chg = xcoff.SetLibPath(newLibPath); + } + if (!xcoff) { + if (emsg) { + *emsg = xcoff.GetErrorMessage(); + } + return false; + } + + // Everything was updated successfully. + if (changed) { + *changed = chg; + } + return true; +} #else bool cmSystemTools::ChangeRPath(std::string const& /*file*/, std::string const& /*oldRPath*/, @@ -2720,6 +2794,7 @@ int cmSystemTools::strverscmp(std::string const& lhs, std::string const& rhs) return cm_strverscmp(lhs.c_str(), rhs.c_str()); } +// FIXME: Dispatch if multiple formats are supported. #if defined(CMake_USE_ELF_PARSER) bool cmSystemTools::RemoveRPath(std::string const& file, std::string* emsg, bool* removed) @@ -2861,6 +2936,28 @@ bool cmSystemTools::RemoveRPath(std::string const& file, std::string* emsg, } return true; } +#elif defined(CMake_USE_XCOFF_PARSER) +bool cmSystemTools::RemoveRPath(std::string const& file, std::string* emsg, + bool* removed) +{ + if (removed) { + *removed = false; + } + + cmXCOFF xcoff(file.c_str(), cmXCOFF::Mode::ReadWrite); + bool rm = xcoff.RemoveLibPath(); + if (!xcoff) { + if (emsg) { + *emsg = xcoff.GetErrorMessage(); + } + return false; + } + + if (removed) { + *removed = rm; + } + return true; +} #else bool cmSystemTools::RemoveRPath(std::string const& /*file*/, std::string* /*emsg*/, bool* /*removed*/) @@ -2869,6 +2966,7 @@ bool cmSystemTools::RemoveRPath(std::string const& /*file*/, } #endif +// FIXME: Dispatch if multiple formats are supported. bool cmSystemTools::CheckRPath(std::string const& file, std::string const& newRPath) { @@ -2894,6 +2992,15 @@ bool cmSystemTools::CheckRPath(std::string const& file, } } return false; +#elif defined(CMake_USE_XCOFF_PARSER) + // Parse the XCOFF binary. + cmXCOFF xcoff(file.c_str()); + if (cm::optional libPath = xcoff.GetLibPath()) { + if (cmSystemToolsFindRPath(*libPath, newRPath) != std::string::npos) { + return true; + } + } + return false; #else (void)file; (void)newRPath; diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt index e0804d7..549d8eb 100644 --- a/Tests/RunCMake/CMakeLists.txt +++ b/Tests/RunCMake/CMakeLists.txt @@ -358,8 +358,8 @@ add_RunCMake_test(ctest_upload) add_RunCMake_test(ctest_fixtures) add_RunCMake_test(file) add_RunCMake_test(file-CHMOD) -if(HAVE_ELF_H) - add_RunCMake_test(file-RPATH -DHAVE_ELF_H=${HAVE_ELF_H}) +if(HAVE_ELF_H OR CMAKE_SYSTEM_NAME STREQUAL "AIX") + add_RunCMake_test(file-RPATH -DCMAKE_SYSTEM_NAME=${CMAKE_SYSTEM_NAME} -DHAVE_ELF_H=${HAVE_ELF_H}) endif() add_RunCMake_test(find_file) add_RunCMake_test(find_library -DCYGWIN=${CYGWIN}) diff --git a/Tests/RunCMake/file-RPATH/RunCMakeTest.cmake b/Tests/RunCMake/file-RPATH/RunCMakeTest.cmake index 11e90bb..eb7b497 100644 --- a/Tests/RunCMake/file-RPATH/RunCMakeTest.cmake +++ b/Tests/RunCMake/file-RPATH/RunCMakeTest.cmake @@ -3,3 +3,7 @@ include(RunCMake) if(HAVE_ELF_H) run_cmake_command(ELF ${CMAKE_COMMAND} -P ${RunCMake_SOURCE_DIR}/ELF.cmake) endif() + +if(CMAKE_SYSTEM_NAME STREQUAL "AIX") + run_cmake_command(XCOFF ${CMAKE_COMMAND} -P ${RunCMake_SOURCE_DIR}/XCOFF.cmake) +endif() diff --git a/Tests/RunCMake/file-RPATH/XCOFF.cmake b/Tests/RunCMake/file-RPATH/XCOFF.cmake new file mode 100644 index 0000000..b570920 --- /dev/null +++ b/Tests/RunCMake/file-RPATH/XCOFF.cmake @@ -0,0 +1,7 @@ +set(names + xcoff32.bin + xcoff64.bin + ) +set(format XCOFF) + +include(${CMAKE_CURRENT_LIST_DIR}/Common.cmake) diff --git a/Tests/RunCMake/install/RunCMakeTest.cmake b/Tests/RunCMake/install/RunCMakeTest.cmake index b067b3a..efd03df 100644 --- a/Tests/RunCMake/install/RunCMakeTest.cmake +++ b/Tests/RunCMake/install/RunCMakeTest.cmake @@ -48,14 +48,16 @@ in directory: endif() endfunction() -# Wrapper for run_cmake() that skips platforms that are non-ELF or have no RPATH support -function(run_cmake_ELFRPATH_only case) - if(UNIX AND CMAKE_SHARED_LIBRARY_RUNTIME_C_FLAG AND CMAKE_EXECUTABLE_FORMAT STREQUAL "ELF") +# Wrapper for run_cmake() that skips platforms on which we do not support editing the RPATH. +function(run_cmake_EDIT_RPATH_only case) + if(UNIX AND CMAKE_SHARED_LIBRARY_RUNTIME_C_FLAG AND CMAKE_EXECUTABLE_FORMAT MATCHES "^(ELF|XCOFF)$") run_cmake(${case}) else() # Sanity check against a platform known to be ELF-based if(CMAKE_SYSTEM_NAME STREQUAL "Linux") message(FATAL_ERROR "Expected platform Linux to advertize itself as ELF-based, but it did not.") + elseif(CMAKE_SYSTEM_NAME STREQUAL "AIX") + message(FATAL_ERROR "Expected platform AIX to advertize itself as XCOFF-based, but it did not.") else() message(STATUS "${case} - SKIPPED (No ELF-based platform found)") endif() @@ -63,7 +65,7 @@ function(run_cmake_ELFRPATH_only case) endfunction() run_cmake(TARGETS-FILE_RPATH_CHANGE-old_rpath) -run_cmake_ELFRPATH_only(TARGETS-FILE_RPATH_CHANGE-new_rpath) +run_cmake_EDIT_RPATH_only(TARGETS-FILE_RPATH_CHANGE-new_rpath) run_cmake(DIRECTORY-MESSAGE_NEVER) run_cmake(DIRECTORY-PATTERN-MESSAGE_NEVER) run_cmake(DIRECTORY-message) diff --git a/Tests/RunCMake/install/TARGETS-FILE_RPATH_CHANGE-check-common.cmake b/Tests/RunCMake/install/TARGETS-FILE_RPATH_CHANGE-check-common.cmake index 673fdde..6b2faa3 100644 --- a/Tests/RunCMake/install/TARGETS-FILE_RPATH_CHANGE-check-common.cmake +++ b/Tests/RunCMake/install/TARGETS-FILE_RPATH_CHANGE-check-common.cmake @@ -22,6 +22,8 @@ macro(skip_without_rpath_change_rule) # Sanity check against a platform known to generate a file(RPATH_CHANGE) rule if(CMAKE_SYSTEM_NAME STREQUAL "Linux") message(FATAL_ERROR "Expected generated file(RPATH_CHANGE) rule on platform Linux.") + elseif(CMAKE_SYSTEM_NAME STREQUAL "AIX") + message(FATAL_ERROR "Expected generated file(RPATH_CHANGE) rule on platform AIX.") else() message(STATUS "${test} - All checks skipped. No file(RPATH_CHANGE) rule found on this platform.") return() -- cgit v0.12