drop redundant rpaths post install (#38976)

Spack heuristically adds `<install prefix>/lib` and `<install prefix>/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

@@ -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())

lib/spack/spack/test/util/elf.py

@@ -14,6 +14,7 @@
 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)