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.
2019-09-17 15:45:21 -07:00 · 2019-09-17 15:45:21 -07:00 · 141a1648e6
commit 141a1648e6
parent b11a98abf0
14 changed files with 193 additions and 54 deletions
--- a/lib/spack/llnl/util/filesystem.py
+++ b/lib/spack/llnl/util/filesystem.py
@ -9,6 +9,7 @@
 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)
--- 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:
--- a/lib/spack/spack/compiler.py
+++ b/lib/spack/spack/compiler.py
@ -9,7 +9,9 @@
 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 @@ def tokenize_flags(flags_str):
 _LIBPATH_ARG = re.compile(r'^[-/](LIBPATH|libpath):(?P<dir>.*)')


-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 @@ def fc_rpath_arg(self):

    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 @@ def __init__(self, cspec, operating_system, target,
        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 @@ def check(exe):
    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.
+    def implicit_rpaths(self):
+        if self.enable_implicit_rpaths is False:
+            return []

-        If it is not overridden, it is assumed to not be supported.
+        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)
+
+        all_required_libs = (
+            list(self.required_libs) + Compiler._all_compiler_rpath_libraries)
+        return list(paths_containing_libs(link_dirs, all_required_libs))
+
+    @property
+    def required_libs(self):
+        """For executables created with this compiler, the compiler libraries
+        that would be generally required to run it.
        """
+        # By default every compiler returns the empty list
+        return []

    @classmethod
-    def parse_implicit_rpaths(cls, 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
-
-        Subclasses can override this to customize.
-        """
-        return _parse_implicit_rpaths(string)
-
-    @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 @@ def determine_implicit_rpaths(cls, paths):
            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 @@ def determine_implicit_rpaths(cls, paths):
        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
--- a/lib/spack/spack/compilers/init.py
+++ b/lib/spack/spack/compilers/init.py
@ -30,7 +30,7 @@
 _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 _default(cmp_id, paths):
        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]

--- a/lib/spack/spack/compilers/arm.py
+++ b/lib/spack/spack/compilers/arm.py
@ -69,6 +69,8 @@ def c11_flag(self):
    def pic_flag(self):
        return "-fPIC"

+    required_libs = ['libclang', 'libflang']
+
    @classmethod
    def fc_version(cls, fc):
        return cls.default_version(fc)
--- a/lib/spack/spack/compilers/clang.py
+++ b/lib/spack/spack/compilers/clang.py
@ -177,6 +177,8 @@ def c11_flag(self):
    def pic_flag(self):
        return "-fPIC"

+    required_libs = ['libclang']
+
    @classmethod
    @llnl.util.lang.memoized
    def default_version(cls, comp):
--- a/lib/spack/spack/compilers/gcc.py
+++ b/lib/spack/spack/compilers/gcc.py
@ -113,6 +113,8 @@ def c11_flag(self):
    def pic_flag(self):
        return "-fPIC"

+    required_libs = ['libgcc', 'libgfortran']
+
    @classmethod
    def default_version(cls, cc):
        """Older versions of gcc use the ``-dumpversion`` option.
--- 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'):
--- a/lib/spack/spack/compilers/pgi.py
+++ b/lib/spack/spack/compilers/pgi.py
@ -48,6 +48,8 @@ def cxx11_flag(self):
    def pic_flag(self):
        return "-fpic"

+    required_libs = ['libpgc', 'libpgf90']
+
    @property
    def c99_flag(self):
        if self.version >= ver('12.10'):
--- a/lib/spack/spack/schema/compilers.py
+++ b/lib/spack/spack/schema/compilers.py
@ -62,8 +62,11 @@
                                              {'type': 'null'},
                                              {'type': 'array'}]},
                        'implicit_rpaths': {
-                            'type': 'array',
-                            'items': {'type': 'string'}
+                            'anyOf': [
+                                {'type': 'array',
+                                 'items': {'type': 'string'}},
+                                {'type': 'boolean'}
+                            ]
                        },
                        'environment': {
                            'type': 'object',
--- a/lib/spack/spack/test/compilers.py
+++ b/lib/spack/spack/test/compilers.py
@ -170,6 +170,24 @@ def name(self):
    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):
--- 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')))
--- 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 @@
 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
--- a/lib/spack/spack/test/llnl/util/filesystem.py
+++ b/lib/spack/spack/test/llnl/util/filesystem.py
@ -197,6 +197,16 @@ def test_symlinks_false(self, stage):
            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')