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.

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<dir>[/\\].*)')
 _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 @@ 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')