From 886946395d33356e2073f4f2112514c366871958 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Wed, 19 Jul 2023 11:48:31 +0200 Subject: drop redundant rpaths post install (#38976) Spack heuristically adds `/lib` and `/lib64` as rpath entries, as it doesn't know what the install dir is going to be ahead of the build. This PR cleans up non-existing, absolute paths[^1], which 1. avoids redundant stat calls at runtime 2. drops redundant rpaths in `patchelf`, making it relocatable -- you don't need patchelf recursively then. [^1]: It also removes relative paths not starting with `$` (so, `$ORIGIN/../lib` is retained -- we _could_ interpolate `$ORIGIN`, but that's hard to get right when symlinks have to be taken into account). Relative paths _are_ supported in glibc, but are relative to _the current working directory_, which is madness, and it would be better to drop those paths. --- lib/spack/spack/hooks/drop_redundant_rpaths.py | 124 +++++++++++++++++++++++++ lib/spack/spack/test/util/elf.py | 28 ++++++ 2 files changed, 152 insertions(+) create mode 100644 lib/spack/spack/hooks/drop_redundant_rpaths.py (limited to 'lib') diff --git a/lib/spack/spack/hooks/drop_redundant_rpaths.py b/lib/spack/spack/hooks/drop_redundant_rpaths.py new file mode 100644 index 0000000000..4cbbf5359d --- /dev/null +++ b/lib/spack/spack/hooks/drop_redundant_rpaths.py @@ -0,0 +1,124 @@ +# Copyright 2013-2023 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +import os +from typing import IO, Optional, Tuple + +import llnl.util.tty as tty +from llnl.util.filesystem import BaseDirectoryVisitor, visit_directory_tree + +from spack.util.elf import ElfParsingError, parse_elf + + +def should_keep(path: bytes) -> bool: + """Return True iff path starts with $ (typically for $ORIGIN/${ORIGIN}) or is + absolute and exists.""" + return path.startswith(b"$") or (os.path.isabs(path) and os.path.lexists(path)) + + +def _drop_redundant_rpaths(f: IO) -> Optional[Tuple[bytes, bytes]]: + """Drop redundant entries from rpath. + + Args: + f: File object to patch opened in r+b mode. + + Returns: + A tuple of the old and new rpath if the rpath was patched, None otherwise. + """ + try: + elf = parse_elf(f, interpreter=False, dynamic_section=True) + except ElfParsingError: + return None + + # Nothing to do. + if not elf.has_rpath: + return None + + old_rpath_str = elf.dt_rpath_str + new_rpath_str = b":".join(p for p in old_rpath_str.split(b":") if should_keep(p)) + + # Nothing to write. + if old_rpath_str == new_rpath_str: + return None + + # Pad with 0 bytes. + pad = len(old_rpath_str) - len(new_rpath_str) + + # This can never happen since we're filtering, but lets be defensive. + if pad < 0: + return None + + # The rpath is at a given offset in the string table used by the + # dynamic section. + rpath_offset = elf.pt_dynamic_strtab_offset + elf.rpath_strtab_offset + + f.seek(rpath_offset) + f.write(new_rpath_str + b"\x00" * pad) + return old_rpath_str, new_rpath_str + + +def drop_redundant_rpaths(path: str) -> Optional[Tuple[bytes, bytes]]: + """Drop redundant entries from rpath. + + Args: + path: Path to a potential ELF file to patch. + + Returns: + A tuple of the old and new rpath if the rpath was patched, None otherwise. + """ + try: + with open(path, "r+b") as f: + return _drop_redundant_rpaths(f) + except OSError: + return None + + +class ElfFilesWithRPathVisitor(BaseDirectoryVisitor): + """Visitor that collects all elf files that have an rpath""" + + def __init__(self): + # Map from (ino, dev) -> path. We need 1 path per file, if there are hardlinks, + # we don't need to store the path multiple times. + self.visited = set() + + def visit_file(self, root, rel_path, depth): + filepath = os.path.join(root, rel_path) + s = os.lstat(filepath) + identifier = (s.st_ino, s.st_dev) + + # We're hitting a hardlink or symlink of an excluded lib, no need to parse. + if identifier in self.visited: + return + + self.visited.add(identifier) + + result = drop_redundant_rpaths(filepath) + + if result is not None: + old, new = result + tty.debug(f"Patched rpath in {rel_path} from {old!r} to {new!r}") + + def visit_symlinked_file(self, root, rel_path, depth): + pass + + def before_visit_dir(self, root, rel_path, depth): + # Always enter dirs + return True + + def before_visit_symlinked_dir(self, root, rel_path, depth): + # Never enter symlinked dirs + return False + + +def post_install(spec, explicit=None): + # Skip externals + if spec.external: + return + + # Only enable on platforms using ELF. + if not spec.satisfies("platform=linux") and not spec.satisfies("platform=cray"): + return + + visit_directory_tree(spec.prefix, ElfFilesWithRPathVisitor()) diff --git a/lib/spack/spack/test/util/elf.py b/lib/spack/spack/test/util/elf.py index b80072d015..6380bb7910 100644 --- a/lib/spack/spack/test/util/elf.py +++ b/lib/spack/spack/test/util/elf.py @@ -14,6 +14,7 @@ import llnl.util.filesystem as fs import spack.platforms import spack.util.elf as elf import spack.util.executable +from spack.hooks.drop_redundant_rpaths import drop_redundant_rpaths # note that our elf parser is platform independent... but I guess creating an elf file @@ -159,3 +160,30 @@ def test_elf_get_and_replace_rpaths(binary_with_rpaths): [(b"/short-a", b"/very/long/prefix-a"), (b"/short-b", b"/very/long/prefix-b")] ), ) + + +@pytest.mark.requires_executables("gcc") +@skip_unless_linux +def test_drop_redundant_rpath(tmpdir, binary_with_rpaths): + """Test the post install hook that drops redundant rpath entries""" + + # Use existing and non-existing dirs in tmpdir + non_existing_dirs = [str(tmpdir.join("a")), str(tmpdir.join("b"))] + existing_dirs = [str(tmpdir.join("c")), str(tmpdir.join("d"))] + all_dirs = non_existing_dirs + existing_dirs + + tmpdir.ensure("c", dir=True) + tmpdir.ensure("d", dir=True) + + # Create a binary with rpaths to both existing and non-existing dirs + binary = binary_with_rpaths(rpaths=all_dirs) + + # Verify that the binary has all the rpaths + # sometimes compilers add extra rpaths, so we test for a subset + assert set(all_dirs).issubset(elf.get_rpaths(binary)) + + # Test whether the right rpaths are dropped + drop_redundant_rpaths(binary) + new_rpaths = elf.get_rpaths(binary) + assert set(existing_dirs).issubset(new_rpaths) + assert set(non_existing_dirs).isdisjoint(new_rpaths) -- cgit v1.2.3-60-g2f50