From b29eb4212e8d9077e0c01911c5448a5900210954 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Wed, 23 Oct 2019 22:22:24 +0200 Subject: [PATCH] Users can configure use of RPATH or RUNPATH (#9168) Add a new entry in `config.yaml`: config: shared_linking: 'rpath' If this variable is set to `rpath` (the default) Spack will set RPATH in ELF binaries. If set to `runpath` it will set RUNPATH. Details: * Spack cc wrapper explicitly adds `--disable-new-dtags` when linking * cc wrapper also strips `--enable-new-dtags` from the compile line when disabling (and vice versa) * We specifically do *not* add any dtags flags on macOS, which uses Mach-O binaries, not ELF, so there's no RUNPATH) --- etc/spack/defaults/config.yaml | 5 +++ lib/spack/docs/config_yaml.rst | 21 +++++++++++ lib/spack/env/cc | 34 ++++++++++++++++- lib/spack/spack/build_environment.py | 9 +++++ lib/spack/spack/compiler.py | 18 +++++++++ lib/spack/spack/compilers/nag.py | 4 ++ lib/spack/spack/schema/config.py | 4 ++ lib/spack/spack/test/build_environment.py | 33 ++++++++++++++++ lib/spack/spack/test/cc.py | 46 ++++++++++++++++++++++- 9 files changed, 172 insertions(+), 2 deletions(-) diff --git a/etc/spack/defaults/config.yaml b/etc/spack/defaults/config.yaml index d6c9551d93..6be1d9770b 100644 --- a/etc/spack/defaults/config.yaml +++ b/etc/spack/defaults/config.yaml @@ -138,3 +138,8 @@ config: # anticipates that a significant delay indicates that the lock attempt will # never succeed. package_lock_timeout: null + + # Control whether Spack embeds RPATH or RUNPATH attributes in ELF binaries. + # Has no effect on macOS. DO NOT MIX these within the same install tree. + # See the Spack documentation for details. + shared_linking: 'rpath' diff --git a/lib/spack/docs/config_yaml.rst b/lib/spack/docs/config_yaml.rst index 525f829c9e..d8d07b505a 100644 --- a/lib/spack/docs/config_yaml.rst +++ b/lib/spack/docs/config_yaml.rst @@ -226,3 +226,24 @@ ccache`` to learn more about the default settings and how to change them). Please note that we currently disable ccache's ``hash_dir`` feature to avoid an issue with the stage directory (see https://github.com/LLNL/spack/pull/3761#issuecomment-294352232). + +------------------ +``shared_linking`` +------------------ + +Control whether Spack embeds ``RPATH`` or ``RUNPATH`` attributes in ELF binaries +so that they can find their dependencies. Has no effect on macOS. +Two options are allowed: + + 1. ``rpath`` uses ``RPATH`` and forces the ``--disable-new-tags`` flag to be passed to the linker + 2. ``runpath`` uses ``RUNPATH`` and forces the ``--enable-new-tags`` flag to be passed to the linker + +``RPATH`` search paths have higher precedence than ``LD_LIBRARY_PATH`` +and ld.so will search for libraries in transitive ``RPATHs`` of +parent objects. + +``RUNPATH`` search paths have lower precedence than ``LD_LIBRARY_PATH``, +and ld.so will ONLY search for dependencies in the ``RUNPATH`` of +the loading object. + +DO NOT MIX the two options within the same install tree. diff --git a/lib/spack/env/cc b/lib/spack/env/cc index 8246539a00..2d6fe9998a 100755 --- a/lib/spack/env/cc +++ b/lib/spack/env/cc @@ -33,6 +33,9 @@ parameters=( SPACK_F77_RPATH_ARG SPACK_FC_RPATH_ARG SPACK_TARGET_ARGS + SPACK_DTAGS_TO_ADD + SPACK_DTAGS_TO_STRIP + SPACK_LINKER_ARG SPACK_SHORT_SPEC SPACK_SYSTEM_DIRS ) @@ -167,6 +170,25 @@ if [[ -z $mode ]]; then done fi +# This is needed to ensure we set RPATH instead of RUNPATH +# (or the opposite, depending on the configuration in config.yaml) +# +# Documentation on this mechanism is lacking at best. A few sources +# of information are (note that some of them take explicitly the +# opposite stance that Spack does): +# +# http://blog.qt.io/blog/2011/10/28/rpath-and-runpath/ +# https://wiki.debian.org/RpathIssue +# +# The only discussion I could find on enabling new dynamic tags by +# default on ld is the following: +# +# https://sourceware.org/ml/binutils/2013-01/msg00307.html +# +dtags_to_add="${SPACK_DTAGS_TO_ADD}" +dtags_to_strip="${SPACK_DTAGS_TO_STRIP}" +linker_arg="${SPACK_LINKER_ARG}" + # Set up rpath variable according to language. eval rpath=\$SPACK_${comp}_RPATH_ARG @@ -293,6 +315,8 @@ while [ -n "$1" ]; do die "-Wl,-rpath was not followed by -Wl,*" fi rp="${arg#-Wl,}" + elif [[ "$arg" = "$dtags_to_strip" ]] ; then + : # We want to remove explicitly this flag else other_args+=("-Wl,$arg") fi @@ -319,12 +343,18 @@ while [ -n "$1" ]; do fi shift 3; rp="$1" + elif [[ "$2" = "$dtags_to_strip" ]] ; then + shift # We want to remove explicitly this flag else other_args+=("$1") fi ;; *) - other_args+=("$1") + if [[ "$1" = "$dtags_to_strip" ]] ; then + : # We want to remove explicitly this flag + else + other_args+=("$1") + fi ;; esac @@ -462,10 +492,12 @@ for dir in "${system_libdirs[@]}"; do args+=("-L$dir"); done # RPATHs arguments case "$mode" in ccld) + if [ ! -z "$dtags_to_add" ] ; then args+=("$linker_arg$dtags_to_add") ; fi for dir in "${rpaths[@]}"; do args+=("$rpath$dir"); done for dir in "${system_rpaths[@]}"; do args+=("$rpath$dir"); done ;; ld) + if [ ! -z "$dtags_to_add" ] ; then args+=("$dtags_to_add") ; fi for dir in "${rpaths[@]}"; do args+=("-rpath" "$dir"); done for dir in "${system_rpaths[@]}"; do args+=("-rpath" "$dir"); done ;; diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index db99a7a4c1..84fc58587e 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -199,6 +199,15 @@ def set_compiler_environment_variables(pkg, env): env.set('SPACK_CXX_RPATH_ARG', compiler.cxx_rpath_arg) env.set('SPACK_F77_RPATH_ARG', compiler.f77_rpath_arg) env.set('SPACK_FC_RPATH_ARG', compiler.fc_rpath_arg) + env.set('SPACK_LINKER_ARG', compiler.linker_arg) + + # Check whether we want to force RPATH or RUNPATH + if spack.config.get('config:shared_linking') == 'rpath': + env.set('SPACK_DTAGS_TO_STRIP', compiler.enable_new_dtags) + env.set('SPACK_DTAGS_TO_ADD', compiler.disable_new_dtags) + else: + env.set('SPACK_DTAGS_TO_STRIP', compiler.disable_new_dtags) + env.set('SPACK_DTAGS_TO_ADD', compiler.enable_new_dtags) # Set the target parameters that the compiler will add isa_arg = spec.architecture.target.optimization_flags(compiler) diff --git a/lib/spack/spack/compiler.py b/lib/spack/spack/compiler.py index 2152152255..f2b62dc3f9 100644 --- a/lib/spack/spack/compiler.py +++ b/lib/spack/spack/compiler.py @@ -4,6 +4,7 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) import os +import platform import re import itertools import shutil @@ -222,6 +223,23 @@ def f77_rpath_arg(self): def fc_rpath_arg(self): return '-Wl,-rpath,' + @property + def linker_arg(self): + """Flag that need to be used to pass an argument to the linker.""" + return '-Wl,' + + @property + def disable_new_dtags(self): + if platform.system() == 'Darwin': + return '' + return '--disable-new-dtags' + + @property + def enable_new_dtags(self): + if platform.system() == 'Darwin': + return '' + return '--enable-new-dtags' + # Cray PrgEnv name that can be used to load this compiler PrgEnv = None # Name of module used to switch versions of this compiler diff --git a/lib/spack/spack/compilers/nag.py b/lib/spack/spack/compilers/nag.py index 5b0a79bcd9..bd0040c9a9 100644 --- a/lib/spack/spack/compilers/nag.py +++ b/lib/spack/spack/compilers/nag.py @@ -54,3 +54,7 @@ def f77_rpath_arg(self): @property def fc_rpath_arg(self): return '-Wl,-Wl,,-rpath,,' + + @property + def linker_arg(self): + return '-Wl,-Wl,,' diff --git a/lib/spack/spack/schema/config.py b/lib/spack/spack/schema/config.py index 4c49cbca6a..6eb127a359 100644 --- a/lib/spack/spack/schema/config.py +++ b/lib/spack/spack/schema/config.py @@ -16,6 +16,10 @@ 'type': 'object', 'default': {}, 'properties': { + 'shared_linking': { + 'type': 'string', + 'enum': ['rpath', 'runpath'] + }, 'install_tree': {'type': 'string'}, 'install_hash_length': {'type': 'integer', 'minimum': 1}, 'install_path_scheme': {'type': 'string'}, diff --git a/lib/spack/spack/test/build_environment.py b/lib/spack/spack/test/build_environment.py index e30e4a7c16..b6a0f4b441 100644 --- a/lib/spack/spack/test/build_environment.py +++ b/lib/spack/spack/test/build_environment.py @@ -4,9 +4,12 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) import os +import platform + import pytest import spack.build_environment +import spack.config import spack.spec from spack.paths import build_env_path from spack.build_environment import dso_suffix, _static_to_shared_library @@ -42,6 +45,9 @@ def build_environment(working_env): os.environ['SPACK_CXX_RPATH_ARG'] = "-Wl,-rpath," os.environ['SPACK_F77_RPATH_ARG'] = "-Wl,-rpath," os.environ['SPACK_FC_RPATH_ARG'] = "-Wl,-rpath," + os.environ['SPACK_LINKER_ARG'] = '-Wl,' + os.environ['SPACK_DTAGS_TO_ADD'] = '--disable-new-dtags' + os.environ['SPACK_DTAGS_TO_STRIP'] = '--enable-new-dtags' os.environ['SPACK_SYSTEM_DIRS'] = '/usr/include /usr/lib' os.environ['SPACK_TARGET_ARGS'] = '' @@ -64,9 +70,11 @@ def test_static_to_shared_library(build_environment): expected = { 'linux': ('/bin/mycc -shared' + ' -Wl,--disable-new-dtags' ' -Wl,-soname,{2} -Wl,--whole-archive {0}' ' -Wl,--no-whole-archive -o {1}'), 'darwin': ('/bin/mycc -dynamiclib' + ' -Wl,--disable-new-dtags' ' -install_name {1} -Wl,-force_load,{0} -o {1}') } @@ -304,3 +312,28 @@ class AttributeHolder(object): m = AttributeHolder() spack.build_environment._set_variables_for_single_module(s.package, m) assert m.make_jobs == expected_jobs + + +@pytest.mark.parametrize('config_setting,expected_flag', [ + ('runpath', '' if platform.system() == 'Darwin' else '--enable-new-dtags'), + ('rpath', '' if platform.system() == 'Darwin' else '--disable-new-dtags'), +]) +def test_setting_dtags_based_on_config( + config_setting, expected_flag, config, mock_packages +): + # Pick a random package to be able to set compiler's variables + s = spack.spec.Spec('cmake') + s.concretize() + pkg = s.package + + env = EnvironmentModifications() + with spack.config.override('config:shared_linking', config_setting): + spack.build_environment.set_compiler_environment_variables(pkg, env) + modifications = env.group_by_name() + assert 'SPACK_DTAGS_TO_STRIP' in modifications + assert 'SPACK_DTAGS_TO_ADD' in modifications + assert len(modifications['SPACK_DTAGS_TO_ADD']) == 1 + assert len(modifications['SPACK_DTAGS_TO_STRIP']) == 1 + + dtags_to_add = modifications['SPACK_DTAGS_TO_ADD'][0] + assert dtags_to_add.value == expected_flag diff --git a/lib/spack/spack/test/cc.py b/lib/spack/spack/test/cc.py index a24f2b7d4e..e3c962b9d7 100644 --- a/lib/spack/spack/test/cc.py +++ b/lib/spack/spack/test/cc.py @@ -103,7 +103,10 @@ def wrapper_environment(): SPACK_LINK_DIRS=None, SPACK_INCLUDE_DIRS=None, SPACK_RPATH_DIRS=None, - SPACK_TARGET_ARGS=''): + SPACK_TARGET_ARGS='', + SPACK_LINKER_ARG='-Wl,', + SPACK_DTAGS_TO_ADD='--disable-new-dtags', + SPACK_DTAGS_TO_STRIP='--enable-new-dtags'): yield @@ -180,6 +183,7 @@ def test_ld_flags(wrapper_flags): spack_ldflags + test_include_paths + test_library_paths + + ['--disable-new-dtags'] + test_rpaths + test_args_without_paths + spack_ldlibs) @@ -204,6 +208,7 @@ def test_cc_flags(wrapper_flags): spack_ldflags + test_include_paths + test_library_paths + + ['-Wl,--disable-new-dtags'] + test_wl_rpaths + test_args_without_paths + spack_ldlibs) @@ -218,6 +223,7 @@ def test_cxx_flags(wrapper_flags): spack_ldflags + test_include_paths + test_library_paths + + ['-Wl,--disable-new-dtags'] + test_wl_rpaths + test_args_without_paths + spack_ldlibs) @@ -232,6 +238,7 @@ def test_fc_flags(wrapper_flags): spack_ldflags + test_include_paths + test_library_paths + + ['-Wl,--disable-new-dtags'] + test_wl_rpaths + test_args_without_paths + spack_ldlibs) @@ -244,6 +251,7 @@ def test_dep_rpath(): [real_cc] + test_include_paths + test_library_paths + + ['-Wl,--disable-new-dtags'] + test_wl_rpaths + test_args_without_paths) @@ -257,6 +265,7 @@ def test_dep_include(): test_include_paths + ['-Ix'] + test_library_paths + + ['-Wl,--disable-new-dtags'] + test_wl_rpaths + test_args_without_paths) @@ -271,6 +280,7 @@ def test_dep_lib(): test_include_paths + test_library_paths + ['-Lx'] + + ['-Wl,--disable-new-dtags'] + test_wl_rpaths + ['-Wl,-rpath,x'] + test_args_without_paths) @@ -285,6 +295,7 @@ def test_dep_lib_no_rpath(): test_include_paths + test_library_paths + ['-Lx'] + + ['-Wl,--disable-new-dtags'] + test_wl_rpaths + test_args_without_paths) @@ -297,6 +308,7 @@ def test_dep_lib_no_lib(): [real_cc] + test_include_paths + test_library_paths + + ['-Wl,--disable-new-dtags'] + test_wl_rpaths + ['-Wl,-rpath,x'] + test_args_without_paths) @@ -318,6 +330,7 @@ def test_ccld_deps(): ['-Lxlib', '-Lylib', '-Lzlib'] + + ['-Wl,--disable-new-dtags'] + test_wl_rpaths + ['-Wl,-rpath,xlib', '-Wl,-rpath,ylib', @@ -368,6 +381,7 @@ def test_ccld_with_system_dirs(): '-Lzlib'] + ['-L/usr/local/lib', '-L/lib64/'] + + ['-Wl,--disable-new-dtags'] + test_wl_rpaths + ['-Wl,-rpath,xlib', '-Wl,-rpath,ylib', @@ -389,6 +403,7 @@ def test_ld_deps(): ['-Lxlib', '-Lylib', '-Lzlib'] + + ['--disable-new-dtags'] + test_rpaths + ['-rpath', 'xlib', '-rpath', 'ylib', @@ -408,6 +423,7 @@ def test_ld_deps_no_rpath(): ['-Lxlib', '-Lylib', '-Lzlib'] + + ['--disable-new-dtags'] + test_rpaths + test_args_without_paths) @@ -421,6 +437,7 @@ def test_ld_deps_no_link(): ['ld'] + test_include_paths + test_library_paths + + ['--disable-new-dtags'] + test_rpaths + ['-rpath', 'xlib', '-rpath', 'ylib', @@ -444,6 +461,7 @@ def test_ld_deps_partial(): test_include_paths + test_library_paths + ['-Lxlib'] + + ['--disable-new-dtags'] + test_rpaths + ['-rpath', 'xlib'] + ['-r'] + @@ -459,6 +477,7 @@ def test_ld_deps_partial(): test_include_paths + test_library_paths + ['-Lxlib'] + + ['--disable-new-dtags'] + test_rpaths + ['-r'] + test_args_without_paths) @@ -473,6 +492,7 @@ def test_ccache_prepend_for_cc(): [real_cc] + test_include_paths + test_library_paths + + ['-Wl,--disable-new-dtags'] + test_wl_rpaths + test_args_without_paths) os.environ['SPACK_SHORT_SPEC'] = "foo@1.2=darwin-x86_64" @@ -483,6 +503,7 @@ def test_ccache_prepend_for_cc(): lheaderpad + test_include_paths + test_library_paths + + ['-Wl,--disable-new-dtags'] + test_wl_rpaths + test_args_without_paths) @@ -495,6 +516,7 @@ def test_no_ccache_prepend_for_fc(): [real_cc] + test_include_paths + test_library_paths + + ['-Wl,--disable-new-dtags'] + test_wl_rpaths + test_args_without_paths) os.environ['SPACK_SHORT_SPEC'] = "foo@1.2=darwin-x86_64" @@ -505,5 +527,27 @@ def test_no_ccache_prepend_for_fc(): lheaderpad + test_include_paths + test_library_paths + + ['-Wl,--disable-new-dtags'] + test_wl_rpaths + test_args_without_paths) + + +@pytest.mark.regression('9160') +def test_disable_new_dtags(wrapper_flags): + with set_env(SPACK_TEST_COMMAND='dump-args'): + result = ld(*test_args, output=str).strip().split('\n') + assert '--disable-new-dtags' in result + result = cc(*test_args, output=str).strip().split('\n') + assert '-Wl,--disable-new-dtags' in result + + +@pytest.mark.regression('9160') +def test_filter_enable_new_dtags(wrapper_flags): + with set_env(SPACK_TEST_COMMAND='dump-args'): + result = ld(*(test_args + ['--enable-new-dtags']), output=str) + result = result.strip().split('\n') + assert '--enable-new-dtags' not in result + + result = cc(*(test_args + ['-Wl,--enable-new-dtags']), output=str) + result = result.strip().split('\n') + assert '-Wl,--enable-new-dtags' not in result