From 02499c72c9cbc059dc58577923e8fe983876f173 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Mon, 30 Sep 2024 20:32:50 +0200 Subject: avoid rpath'ing default search paths (#44686) On sysroot systems like gentoo prefix, as well as nix/guix, our "is system path" logic is broken cause it's static. Talking about "the system paths" is not helpful, we have to talk about default search paths of the dynamic linker instead. If glibc is recent enough, we can query the dynamic loader's default search paths, which is a much more robust way to avoid registering rpaths to system dirs, which can shadow Spack dirs. This PR adds an **additional** filter on rpaths the compiler wrapper adds, dropping rpaths that are default search paths. The PR **does not** remove any of the original `is_system_path` code yet. This fixes issues where build systems run just-built executables linked against their *not-yet-installed libraries*, typically: ``` LD_LIBRARY_PATH=. ./exe ``` which happens in `perl`, `python`, and other non-cmake packages. If a default path is rpath'ed, it takes precedence over `LD_LIBRARY_PATH`, and a system library gets loaded instead of the just-built library in the stage dir, breaking the build. If default paths are not rpath'ed, then LD_LIBRARY_PATH takes precedence, as is desired. This PR additionally fixes an inconsistency in rpaths between cmake and non-cmake packages. The cmake build system computed rpaths by itself, but used a different order than computed for the compiler wrapper. In fact it's not necessary to compute rpaths at all, since we let cmake do that thanks to `CMAKE_INSTALL_RPATH_USE_LINK_PATH`. This covers rpaths for all dependencies. The only install rpaths we need to set are `/{lib,lib64}`, which cmake unfortunately omits, although it could also know these. Also, cmake does *not* delete rpaths added by the toolchain (i.e. Spack's compiler wrapper), so I don't think it should be controversial to simplify things. --- lib/spack/spack/build_environment.py | 56 +++++++++++++++++++++++++++++------- lib/spack/spack/compiler.py | 11 +++++-- lib/spack/spack/util/libc.py | 18 +++++++++++- 3 files changed, 71 insertions(+), 14 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index fd0adbf39d..cf771a10ac 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -37,13 +37,14 @@ import io import multiprocessing import os import re +import stat import sys import traceback import types from collections import defaultdict from enum import Flag, auto from itertools import chain -from typing import Dict, List, Set, Tuple +from typing import Dict, List, Optional, Set, Tuple import archspec.cpu @@ -74,6 +75,7 @@ import spack.stage import spack.store import spack.subprocess_context import spack.util.executable +import spack.util.libc from spack import traverse from spack.context import Context from spack.error import InstallError, NoHeadersError, NoLibrariesError @@ -435,6 +437,35 @@ def optimization_flags(compiler, target): return result +class FilterDefaultDynamicLinkerSearchPaths: + """Remove rpaths to directories that are default search paths of the dynamic linker.""" + + def __init__(self, dynamic_linker: Optional[str]) -> None: + # Identify directories by (inode, device) tuple, which handles symlinks too. + self.default_path_identifiers: Set[Tuple[int, int]] = set() + if not dynamic_linker: + return + for path in spack.util.libc.default_search_paths_from_dynamic_linker(dynamic_linker): + try: + s = os.stat(path) + if stat.S_ISDIR(s.st_mode): + self.default_path_identifiers.add((s.st_ino, s.st_dev)) + except OSError: + continue + + def is_dynamic_loader_default_path(self, p: str) -> bool: + try: + s = os.stat(p) + return (s.st_ino, s.st_dev) in self.default_path_identifiers + except OSError: + return False + + def __call__(self, dirs: List[str]) -> List[str]: + if not self.default_path_identifiers: + return dirs + return [p for p in dirs if not self.is_dynamic_loader_default_path(p)] + + def set_wrapper_variables(pkg, env): """Set environment variables used by the Spack compiler wrapper (which have the prefix `SPACK_`) and also add the compiler wrappers to PATH. @@ -508,17 +539,17 @@ def set_wrapper_variables(pkg, env): def update_compiler_args_for_dep(dep): if dep in link_deps and (not is_system_path(dep.prefix)): query = pkg.spec[dep.name] - dep_link_dirs = list() + dep_link_dirs = [] try: # In some circumstances (particularly for externals) finding # libraries packages can be time consuming, so indicate that # we are performing this operation (and also report when it # finishes). - tty.debug("Collecting libraries for {0}".format(dep.name)) + tty.debug(f"Collecting libraries for {dep.name}") dep_link_dirs.extend(query.libs.directories) - tty.debug("Libraries for {0} have been collected.".format(dep.name)) + tty.debug(f"Libraries for {dep.name} have been collected.") except NoLibrariesError: - tty.debug("No libraries found for {0}".format(dep.name)) + tty.debug(f"No libraries found for {dep.name}") for default_lib_dir in ["lib", "lib64"]: default_lib_prefix = os.path.join(dep.prefix, default_lib_dir) @@ -532,7 +563,7 @@ def set_wrapper_variables(pkg, env): try: _prepend_all(include_dirs, query.headers.directories) except NoHeadersError: - tty.debug("No headers found for {0}".format(dep.name)) + tty.debug(f"No headers found for {dep.name}") for dspec in pkg.spec.traverse(root=False, order="post"): if dspec.external: @@ -552,9 +583,18 @@ def set_wrapper_variables(pkg, env): lib_path = os.path.join(pkg.prefix, libdir) rpath_dirs.insert(0, lib_path) + filter_default_dynamic_linker_search_paths = FilterDefaultDynamicLinkerSearchPaths( + pkg.compiler.default_dynamic_linker + ) + link_dirs = list(dedupe(filter_system_paths(link_dirs))) include_dirs = list(dedupe(filter_system_paths(include_dirs))) rpath_dirs = list(dedupe(filter_system_paths(rpath_dirs))) + rpath_dirs = filter_default_dynamic_linker_search_paths(rpath_dirs) + + implicit_rpaths = filter_default_dynamic_linker_search_paths(pkg.compiler.implicit_rpaths()) + if implicit_rpaths: + env.set("SPACK_COMPILER_IMPLICIT_RPATHS", ":".join(implicit_rpaths)) # Spack managed directories include the stage, store and upstream stores. We extend this with # their real paths to make it more robust (e.g. /tmp vs /private/tmp on macOS). @@ -841,10 +881,6 @@ def setup_package(pkg, dirty, context: Context = Context.BUILD): load_external_modules(pkg) - implicit_rpaths = pkg.compiler.implicit_rpaths() - if implicit_rpaths: - env_mods.set("SPACK_COMPILER_IMPLICIT_RPATHS", ":".join(implicit_rpaths)) - # Make sure nothing's strange about the Spack environment. validate(env_mods, tty.warn) env_mods.apply_modifications() diff --git a/lib/spack/spack/compiler.py b/lib/spack/spack/compiler.py index 5e9b463dbb..8ed125ae24 100644 --- a/lib/spack/spack/compiler.py +++ b/lib/spack/spack/compiler.py @@ -415,14 +415,19 @@ class Compiler: return list(paths_containing_libs(link_dirs, all_required_libs)) @property - def default_libc(self) -> Optional["spack.spec.Spec"]: - """Determine libc targeted by the compiler from link line""" + def default_dynamic_linker(self) -> Optional[str]: + """Determine default dynamic linker from compiler link line""" output = self.compiler_verbose_output if not output: return None - dynamic_linker = spack.util.libc.parse_dynamic_linker(output) + return spack.util.libc.parse_dynamic_linker(output) + + @property + def default_libc(self) -> Optional["spack.spec.Spec"]: + """Determine libc targeted by the compiler from link line""" + dynamic_linker = self.default_dynamic_linker if not dynamic_linker: return None diff --git a/lib/spack/spack/util/libc.py b/lib/spack/spack/util/libc.py index 402bf6f244..55e8e3d26b 100644 --- a/lib/spack/spack/util/libc.py +++ b/lib/spack/spack/util/libc.py @@ -9,7 +9,7 @@ import re import shlex import sys from subprocess import PIPE, run -from typing import Optional +from typing import List, Optional import spack.spec import spack.util.elf @@ -34,6 +34,22 @@ def _libc_from_ldd(ldd: str) -> Optional["spack.spec.Spec"]: return None +def default_search_paths_from_dynamic_linker(dynamic_linker: str) -> List[str]: + """If the dynamic linker is glibc at a certain version, we can query the hard-coded library + search paths""" + try: + result = run([dynamic_linker, "--help"], stdout=PIPE, stderr=PIPE, check=False) + assert result.returncode == 0 + out = result.stdout.decode("utf-8") + except Exception: + return [] + + return [ + match.group(1).strip() + for match in re.finditer(r"^ (/.+) \(system search path\)$", out, re.MULTILINE) + ] + + def libc_from_dynamic_linker(dynamic_linker: str) -> Optional["spack.spec.Spec"]: if not os.path.exists(dynamic_linker): return None -- cgit v1.2.3-70-g09d2