From 141a1648e682bebff3c705f8b60678c00fa2ff11 Mon Sep 17 00:00:00 2001 From: Peter Scheibel Date: Tue, 17 Sep 2019 15:45:21 -0700 Subject: implicit rpaths filtering (#12789) * implicit_rpaths are now removed from compilers.yaml config and are always instantiated dynamically, this occurs one time in the build_environment module * per-compiler list required libraries (e.g. libstdc++, libgfortran) and whitelist directories from rpaths including those libraries. Remove non-whitelisted implicit rpaths. Some libraries default for all compilers. * reintroduce 'implicit_rpaths' as a config variable that can be used to disable Spack insertion of compiler RPATHs generated at build time. --- lib/spack/llnl/util/filesystem.py | 27 ++++++++ lib/spack/spack/build_environment.py | 6 +- lib/spack/spack/compiler.py | 98 +++++++++++++++++----------- lib/spack/spack/compilers/__init__.py | 20 ++++-- lib/spack/spack/compilers/arm.py | 2 + lib/spack/spack/compilers/clang.py | 2 + lib/spack/spack/compilers/gcc.py | 2 + lib/spack/spack/compilers/intel.py | 2 + lib/spack/spack/compilers/pgi.py | 2 + lib/spack/spack/schema/compilers.py | 7 +- lib/spack/spack/test/compilers.py | 18 +++++ lib/spack/spack/test/conftest.py | 43 +++++++++++- lib/spack/spack/test/link_paths.py | 4 +- lib/spack/spack/test/llnl/util/filesystem.py | 10 +++ 14 files changed, 191 insertions(+), 52 deletions(-) diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index 752aa0138e..762d851ec5 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -9,6 +9,7 @@ import hashlib import fileinput import glob import grp +import itertools import numbers import os import pwd @@ -68,6 +69,32 @@ def path_contains_subdirectory(path, root): return norm_path.startswith(norm_root) +def possible_library_filenames(library_names): + """Given a collection of library names like 'libfoo', generate the set of + library filenames that may be found on the system (e.g. libfoo.so). This + generates the library filenames that may appear on any OS. + """ + lib_extensions = ['a', 'la', 'so', 'tbd', 'dylib'] + return set( + '.'.join((lib, extension)) for lib, extension in + itertools.product(library_names, lib_extensions)) + + +def paths_containing_libs(paths, library_names): + """Given a collection of filesystem paths, return the list of paths that + which include one or more of the specified libraries. + """ + required_lib_fnames = possible_library_filenames(library_names) + + rpaths_to_include = [] + for path in paths: + fnames = set(os.listdir(path)) + if fnames & required_lib_fnames: + rpaths_to_include.append(path) + + return rpaths_to_include + + def same_path(path1, path2): norm1 = os.path.abspath(path1).rstrip(os.path.sep) norm2 = os.path.abspath(path2).rstrip(os.path.sep) diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index d37e139e3c..877ae09ab5 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -348,9 +348,9 @@ def set_build_environment_variables(pkg, env, dirty): extra_rpaths = ':'.join(compiler.extra_rpaths) env.set('SPACK_COMPILER_EXTRA_RPATHS', extra_rpaths) - if compiler.implicit_rpaths: - implicit_rpaths = ':'.join(compiler.implicit_rpaths) - env.set('SPACK_COMPILER_IMPLICIT_RPATHS', implicit_rpaths) + implicit_rpaths = compiler.implicit_rpaths() + if implicit_rpaths: + env.set('SPACK_COMPILER_IMPLICIT_RPATHS', ':'.join(implicit_rpaths)) # Add bin directories from dependencies to the PATH for the build. for prefix in build_prefixes: diff --git a/lib/spack/spack/compiler.py b/lib/spack/spack/compiler.py index 3748110470..b9f68f80f1 100644 --- a/lib/spack/spack/compiler.py +++ b/lib/spack/spack/compiler.py @@ -9,7 +9,9 @@ import itertools import shutil import tempfile -import llnl.util.filesystem +import llnl.util.lang +from llnl.util.filesystem import ( + path_contains_subdirectory, paths_containing_libs) import llnl.util.tty as tty import spack.error @@ -79,13 +81,7 @@ _LINK_DIR_ARG = re.compile(r'^-L(.:)?(?P[/\\].*)') _LIBPATH_ARG = re.compile(r'^[-/](LIBPATH|libpath):(?P.*)') -def is_subdirectory(path, prefix): - path = os.path.abspath(path) - prefix = os.path.abspath(prefix) + os.path.sep - return path.startswith(prefix) - - -def _parse_implicit_rpaths(string): +def _parse_link_paths(string): """Parse implicit link paths from compiler debug output. This gives the compiler runtime library paths that we need to add to @@ -142,18 +138,36 @@ def _parse_implicit_rpaths(string): if normalized_path not in visited: implicit_link_dirs.append(normalized_path) visited.add(normalized_path) - implicit_link_dirs = filter_system_paths(implicit_link_dirs) - - # Additional filtering: we also want to exclude paths that are - # subdirectories of /usr/lib/ and /lib/ - implicit_link_dirs = list( - path for path in implicit_link_dirs - if not any(is_subdirectory(path, d) for d in ['/lib/', '/usr/lib/'])) tty.debug('found link dirs: %s' % ', '.join(implicit_link_dirs)) return implicit_link_dirs +def _parse_non_system_link_dirs(string): + """Parses link paths out of compiler debug output. + + Args: + string (str): compiler debug output as a string + + Returns: + (list of str): implicit link paths parsed from the compiler output + """ + link_dirs = _parse_link_paths(string) + + # Return set of directories containing needed compiler libs, minus + # system paths. Note that 'filter_system_paths' only checks for an + # exact match, while 'in_system_subdirectory' checks if a path contains + # a system directory as a subdirectory + link_dirs = filter_system_paths(link_dirs) + return list(p for p in link_dirs if not in_system_subdirectory(p)) + + +def in_system_subdirectory(path): + system_dirs = ['/lib/', '/lib64/', '/usr/lib/', '/usr/lib64/', + '/usr/local/lib/', '/usr/local/lib64/'] + return any(path_contains_subdirectory(path, x) for x in system_dirs) + + class Compiler(object): """This class encapsulates a Spack "compiler", which includes C, C++, and Fortran compilers. Subclasses should implement @@ -187,6 +201,10 @@ class Compiler(object): #: Regex used to extract version from compiler's output version_regex = '(.*)' + # These libraries are anticipated to be required by all executables built + # by any compiler + _all_compiler_rpath_libraries = ['libc', 'libc++', 'libstdc++'] + # Default flags used by a compiler to set an rpath @property def cc_rpath_arg(self): @@ -210,7 +228,7 @@ class Compiler(object): def __init__(self, cspec, operating_system, target, paths, modules=[], alias=None, environment=None, - extra_rpaths=None, implicit_rpaths=None, + extra_rpaths=None, enable_implicit_rpaths=None, **kwargs): self.spec = cspec self.operating_system = str(operating_system) @@ -218,7 +236,7 @@ class Compiler(object): self.modules = modules self.alias = alias self.extra_rpaths = extra_rpaths - self.implicit_rpaths = implicit_rpaths + self.enable_implicit_rpaths = enable_implicit_rpaths def check(exe): if exe is None: @@ -251,31 +269,28 @@ class Compiler(object): def version(self): return self.spec.version - @classmethod - def verbose_flag(cls): - """ - This property should be overridden in the compiler subclass if a - verbose flag is available. - - If it is not overridden, it is assumed to not be supported. - """ - - @classmethod - def parse_implicit_rpaths(cls, string): - """Parses link paths out of compiler debug output. + def implicit_rpaths(self): + if self.enable_implicit_rpaths is False: + return [] - Args: - string (str): compiler debug output as a string + exe_paths = [ + x for x in [self.cc, self.cxx, self.fc, self.f77] if x] + link_dirs = self._get_compiler_link_paths(exe_paths) - Returns: - (list of str): implicit link paths parsed from the compiler output + all_required_libs = ( + list(self.required_libs) + Compiler._all_compiler_rpath_libraries) + return list(paths_containing_libs(link_dirs, all_required_libs)) - Subclasses can override this to customize. + @property + def required_libs(self): + """For executables created with this compiler, the compiler libraries + that would be generally required to run it. """ - return _parse_implicit_rpaths(string) + # By default every compiler returns the empty list + return [] @classmethod - def determine_implicit_rpaths(cls, paths): + def _get_compiler_link_paths(cls, paths): first_compiler = next((c for c in paths if c), None) if not first_compiler: return [] @@ -293,7 +308,7 @@ class Compiler(object): output = str(compiler_exe(cls.verbose_flag(), fin, '-o', fout, output=str, error=str)) # str for py2 - return cls.parse_implicit_rpaths(output) + return _parse_non_system_link_dirs(output) except spack.util.executable.ProcessError as pe: tty.debug('ProcessError: Command exited with non-zero status: ' + pe.long_message) @@ -301,6 +316,15 @@ class Compiler(object): finally: shutil.rmtree(tmpdir, ignore_errors=True) + @classmethod + def verbose_flag(cls): + """ + This property should be overridden in the compiler subclass if a + verbose flag is available. + + If it is not overridden, it is assumed to not be supported. + """ + # This property should be overridden in the compiler subclass if # OpenMP is supported by that compiler @property diff --git a/lib/spack/spack/compilers/__init__.py b/lib/spack/spack/compilers/__init__.py index b4c548365e..929cd07ae9 100644 --- a/lib/spack/spack/compilers/__init__.py +++ b/lib/spack/spack/compilers/__init__.py @@ -30,7 +30,7 @@ _imported_compilers_module = 'spack.compilers' _path_instance_vars = ['cc', 'cxx', 'f77', 'fc'] _flags_instance_vars = ['cflags', 'cppflags', 'cxxflags', 'fflags'] _other_instance_vars = ['modules', 'operating_system', 'environment', - 'extra_rpaths', 'implicit_rpaths'] + 'implicit_rpaths', 'extra_rpaths'] _cache_config_file = [] # TODO: Caches at module level make it difficult to mock configurations in @@ -73,7 +73,8 @@ def _to_dict(compiler): d['modules'] = compiler.modules or [] d['environment'] = compiler.environment or {} d['extra_rpaths'] = compiler.extra_rpaths or [] - d['implicit_rpaths'] = compiler.implicit_rpaths or [] + if compiler.enable_implicit_rpaths is not None: + d['implicit_rpaths'] = compiler.enable_implicit_rpaths if compiler.alias: d['alias'] = compiler.alias @@ -350,10 +351,17 @@ def compiler_from_dict(items): compiler_flags = items.get('flags', {}) environment = items.get('environment', {}) extra_rpaths = items.get('extra_rpaths', []) - implicit_rpaths = items.get('implicit_rpaths') + implicit_rpaths = items.get('implicit_rpaths', None) + + # Starting with c22a145, 'implicit_rpaths' was a list. Now it is a + # boolean which can be set by the user to disable all automatic + # RPATH insertion of compiler libraries + if implicit_rpaths is not None and not isinstance(implicit_rpaths, bool): + implicit_rpaths = None return cls(cspec, os, target, compiler_paths, mods, alias, - environment, extra_rpaths, implicit_rpaths, + environment, extra_rpaths, + enable_implicit_rpaths=implicit_rpaths, **compiler_flags) @@ -637,10 +645,8 @@ def make_compiler_list(detected_versions): compiler_cls = spack.compilers.class_for_compiler_name(compiler_name) spec = spack.spec.CompilerSpec(compiler_cls.name, version) paths = [paths.get(l, None) for l in ('cc', 'cxx', 'f77', 'fc')] - implicit_rpaths = compiler_cls.determine_implicit_rpaths(paths) compiler = compiler_cls( - spec, operating_system, py_platform.machine(), paths, - implicit_rpaths=implicit_rpaths + spec, operating_system, py_platform.machine(), paths ) return [compiler] diff --git a/lib/spack/spack/compilers/arm.py b/lib/spack/spack/compilers/arm.py index 2f9ee002c5..ff4c208531 100644 --- a/lib/spack/spack/compilers/arm.py +++ b/lib/spack/spack/compilers/arm.py @@ -69,6 +69,8 @@ class Arm(spack.compiler.Compiler): def pic_flag(self): return "-fPIC" + required_libs = ['libclang', 'libflang'] + @classmethod def fc_version(cls, fc): return cls.default_version(fc) diff --git a/lib/spack/spack/compilers/clang.py b/lib/spack/spack/compilers/clang.py index e6970d4f49..209759e7be 100644 --- a/lib/spack/spack/compilers/clang.py +++ b/lib/spack/spack/compilers/clang.py @@ -177,6 +177,8 @@ class Clang(Compiler): def pic_flag(self): return "-fPIC" + required_libs = ['libclang'] + @classmethod @llnl.util.lang.memoized def default_version(cls, comp): diff --git a/lib/spack/spack/compilers/gcc.py b/lib/spack/spack/compilers/gcc.py index fd1d3446fb..3ac78ccccf 100644 --- a/lib/spack/spack/compilers/gcc.py +++ b/lib/spack/spack/compilers/gcc.py @@ -113,6 +113,8 @@ class Gcc(Compiler): def pic_flag(self): return "-fPIC" + required_libs = ['libgcc', 'libgfortran'] + @classmethod def default_version(cls, cc): """Older versions of gcc use the ``-dumpversion`` option. diff --git a/lib/spack/spack/compilers/intel.py b/lib/spack/spack/compilers/intel.py index c3cbea4751..f06f5b07cb 100644 --- a/lib/spack/spack/compilers/intel.py +++ b/lib/spack/spack/compilers/intel.py @@ -36,6 +36,8 @@ class Intel(Compiler): def verbose_flag(cls): return "-v" + required_libs = ['libirc', 'libifcore', 'libifcoremt', 'libirng'] + @property def openmp_flag(self): if self.version < ver('16.0'): diff --git a/lib/spack/spack/compilers/pgi.py b/lib/spack/spack/compilers/pgi.py index 0f7585bc40..90560f7c63 100644 --- a/lib/spack/spack/compilers/pgi.py +++ b/lib/spack/spack/compilers/pgi.py @@ -48,6 +48,8 @@ class Pgi(Compiler): def pic_flag(self): return "-fpic" + required_libs = ['libpgc', 'libpgf90'] + @property def c99_flag(self): if self.version >= ver('12.10'): diff --git a/lib/spack/spack/schema/compilers.py b/lib/spack/spack/schema/compilers.py index 934c3cad3d..c2f72c89d1 100644 --- a/lib/spack/spack/schema/compilers.py +++ b/lib/spack/spack/schema/compilers.py @@ -62,8 +62,11 @@ properties = { {'type': 'null'}, {'type': 'array'}]}, 'implicit_rpaths': { - 'type': 'array', - 'items': {'type': 'string'} + 'anyOf': [ + {'type': 'array', + 'items': {'type': 'string'}}, + {'type': 'boolean'} + ] }, 'environment': { 'type': 'object', diff --git a/lib/spack/spack/test/compilers.py b/lib/spack/spack/test/compilers.py index b9e2465e25..0728d57699 100644 --- a/lib/spack/spack/test/compilers.py +++ b/lib/spack/spack/test/compilers.py @@ -170,6 +170,24 @@ class MockCompiler(Compiler): def version(self): return "1.0.0" + required_libs = ['libgfortran'] + + +def test_implicit_rpaths(dirs_with_libfiles, monkeypatch): + lib_to_dirs, all_dirs = dirs_with_libfiles + + def try_all_dirs(*args): + return all_dirs + + monkeypatch.setattr(MockCompiler, '_get_compiler_link_paths', try_all_dirs) + + expected_rpaths = set(lib_to_dirs['libstdc++'] + + lib_to_dirs['libgfortran']) + + compiler = MockCompiler() + retrieved_rpaths = compiler.implicit_rpaths() + assert set(retrieved_rpaths) == expected_rpaths + # Get the desired flag from the specified compiler spec. def flag_value(flag, spec): diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index c9f1c3737a..c193e66eee 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -6,6 +6,7 @@ import collections import copy import inspect +import itertools import os import os.path import shutil @@ -488,8 +489,48 @@ def mutable_database(database, _store_dir_and_cache): store_path.join('.spack-db').chmod(mode=0o555, rec=1) +@pytest.fixture() +def dirs_with_libfiles(tmpdir_factory): + lib_to_libfiles = { + 'libstdc++': ['libstdc++.so', 'libstdc++.tbd'], + 'libgfortran': ['libgfortran.a', 'libgfortran.dylib'], + 'libirc': ['libirc.a', 'libirc.so'] + } + + root = tmpdir_factory.mktemp('root') + lib_to_dirs = {} + i = 0 + for lib, libfiles in lib_to_libfiles.items(): + dirs = [] + for libfile in libfiles: + root.ensure(str(i), dir=True) + root.join(str(i)).ensure(libfile) + dirs.append(str(root.join(str(i)))) + i += 1 + lib_to_dirs[lib] = dirs + + all_dirs = list(itertools.chain.from_iterable(lib_to_dirs.values())) + + yield lib_to_dirs, all_dirs + + +@pytest.fixture(scope='function', autouse=True) +def disable_compiler_execution(monkeypatch): + def noop(*args): + return [] + + # Compiler.determine_implicit_rpaths actually runs the compiler. So this + # replaces that function with a noop that simulates finding no implicit + # RPATHs + monkeypatch.setattr( + spack.compiler.Compiler, + '_get_compiler_link_paths', + noop + ) + + @pytest.fixture(scope='function') -def install_mockery(tmpdir, config, mock_packages): +def install_mockery(tmpdir, config, mock_packages, monkeypatch): """Hooks a fake install directory, DB, and stage directory into Spack.""" real_store = spack.store.store spack.store.store = spack.store.Store(str(tmpdir.join('opt'))) diff --git a/lib/spack/spack/test/link_paths.py b/lib/spack/spack/test/link_paths.py index 33ac68cdc0..5f26a272c6 100644 --- a/lib/spack/spack/test/link_paths.py +++ b/lib/spack/spack/test/link_paths.py @@ -6,7 +6,7 @@ import os import spack.paths -from spack.compiler import _parse_implicit_rpaths +from spack.compiler import _parse_non_system_link_dirs #: directory with sample compiler data datadir = os.path.join(spack.paths.test_path, 'data', @@ -16,7 +16,7 @@ datadir = os.path.join(spack.paths.test_path, 'data', def check_link_paths(filename, paths): with open(os.path.join(datadir, filename)) as file: output = file.read() - detected_paths = _parse_implicit_rpaths(output) + detected_paths = _parse_non_system_link_dirs(output) actual = detected_paths expected = paths diff --git a/lib/spack/spack/test/llnl/util/filesystem.py b/lib/spack/spack/test/llnl/util/filesystem.py index f70844978e..0a8dc074fe 100644 --- a/lib/spack/spack/test/llnl/util/filesystem.py +++ b/lib/spack/spack/test/llnl/util/filesystem.py @@ -197,6 +197,16 @@ class TestInstallTree: assert not os.path.islink('dest/2') +def test_paths_containing_libs(dirs_with_libfiles): + lib_to_dirs, all_dirs = dirs_with_libfiles + + assert (set(fs.paths_containing_libs(all_dirs, ['libgfortran'])) == + set(lib_to_dirs['libgfortran'])) + + assert (set(fs.paths_containing_libs(all_dirs, ['libirc'])) == + set(lib_to_dirs['libirc'])) + + def test_move_transaction_commit(tmpdir): fake_library = tmpdir.mkdir('lib').join('libfoo.so') -- cgit v1.2.3-70-g09d2