ELF: verify needed libraries are resolvable by haampie · Pull Request #28109 · spack/spack

haampie · 2021-12-21T12:43:19Z

This commit adds a post-install hook that walks over the prefix and
verifies that each ELF executable/library that takes part in shared
linking can in fact resolve its direct depenendencies.

This is slightly simpler than running ldd in the sense that it is not
recursive (we only care about the package) and its verification logic is
very minimal (we require ET_DYN type elf files for libs, and they should
match the class / data / isa of the parent).

Also it is platform independent, so we don't have to worry about calling
the correct ldd.

This check does not fail the install, it just prints it as an error
message.

Examples

`llvm`

In [1]: import spack.hooks.check_dynamic_linking_elf

In [2]: spec = spack.store.db.query("llvm")[0]

In [3]: spack.hooks.check_dynamic_linking_elf.post_install(spec)

==> Error: Not all executables/libraries can resolve their dependencies:
bin/llvm-omp-device-info
        libomp.so => /home/harmen/spack/opt/spack/linux-ubuntu22.04-zen2/gcc-11.2.0/llvm-15.0.0-neuno6erto7nzlbttmzuepmgujk3c76t/bin/../lib/libomp.so
        libomptarget.so.15 => /home/harmen/spack/opt/spack/linux-ubuntu22.04-zen2/gcc-11.2.0/llvm-15.0.0-neuno6erto7nzlbttmzuepmgujk3c76t/bin/../lib/libomptarget.so.15
        libhwloc.so.15 => not found

lib/libomp.so
        libhwloc.so.15 => not found

lib/libompd.so
        libomp.so => not found
        libhwloc.so.15 => not found

lib/libomptarget.rtl.amdgpu.so.15
        libelf.so.1 => not found
        libz.so.1 => not found
        libtinfo.so.6 => not found

lib/libomptarget.rtl.cuda.so.15
        libelf.so.1 => not found
        libz.so.1 => not found
        libtinfo.so.6 => not found

lib/libomptarget.rtl.x86_64.so.15
        libffi.so.8 => not found
        libelf.so.1 => not found
        libz.so.1 => not found
        libtinfo.so.6 => not found

lib/libomptarget.so.15
        libz.so.1 => not found
        libtinfo.so.6 => not found

libexec/llvm/llvm-omp-device-info
        libomp.so => not found
        libomptarget.so.15 => not found
        libhwloc.so.15 => not found

`python`

lib/python3.9/lib-dynload/_crypt.cpython-39-x86_64-linux-gnu.so
        libcrypt.so.1 => not found

lib/python3.9/lib-dynload/nis.cpython-39-x86_64-linux-gnu.so
        libtirpc.so.3 => not found

`perl`

lib/5.36.0/x86_64-linux-thread-multi/CORE/libperl.so
        libcrypt.so.1 => not found

Performance

For me it takes 97.2 ms to process the LLVM prefix (6.8GB)

haampie · 2021-12-21T14:40:00Z

Notice that the above shows a legitimate problem with python picking up a system lib (libdb-5.3.so is not found in an rpath, and should likely have been libgdbm.so which is a lib installed by spack):

Which proves this is useful

scheibelp

I haven't thoroughly reviewed this yet but have some high-level questions about the implementation:

I'm wondering if we can avoid maintaining a whitelist of libraries which can be missing
I think this PR includes some optimization of RPATH rewriting which could be moved to another PR

The other comments are minor in comparison.

lib/spack/spack/hooks/check_dynamic_linking_elf.py

scheibelp · 2021-12-21T22:16:44Z

lib/spack/spack/hooks/check_dynamic_linking_elf.py

+    'libstdc++',
+    'libtsan',
+    'libubsan',
+])


Curating this list will likely be an ongoing (and initially-frequent) process. I'm wondering if we can circumvent this curation by searching for libraries that meet the following criteria:

The library is present in a dependency

We are getting it from somewhere else

This would involve perhaps creating a DB of all library files in all dependencies (which would be useful for other purposes e.g. better error messages when an installation fails because of missing symbols).

Makes sense, but it does not help with errors like #28114: if multiple libraries are supported and the build links to the first it finds on the system (e.g. berkely-db/intel-mkl on the system, whereas gdbm/openblas was the spack dependency).

I'm also thinking that something stateless is better than yet another cache

Also we have to deal with stuff like MKL which:

installs libs with the same filename, soname, arch in different directories depending on what interface the user needs (32 vs 64 bit integers on a 64 bit arch). There is actually no way of knowing the required library.

installs a copy of Qt for their updater/installer

So "the library is present in a dependency" is not a guarantee that it's meant to be exposed to the user, which is the reason I went for spec[x].libs.directories as candidate search paths.

Notice that the MKL issue also means we can't just randomly inject rpaths to MKL in the first place.

And... turns out .libs can't be trusted either. For mkl it returns glibc libs 😩. I don't see how to make any progress here.

Makes sense, but it does not help with errors like #28114: if multiple libraries are supported and the build links to the first it finds on the system (e.g. berkely-db/intel-mkl on the system, whereas gdbm/openblas was the spack dependency).

Sorry I don't think I understand: wouldn't running ldd for example give the locations of all found dependency libraries? In this case for example I think the error we would detect is:

berkeley-db installed by Spack has a library X

ldd reports finding X, but in a system path

Now we know there's an issue, because we should be getting X from the Spack package

One place where my suggestion wouldn't work is if Spack's Python package did not mention berkeley-db as a transitive dependency (in that case Spack may not install it, and hence may not know that X is provided). It looks like that might happen for example if one uses an external openssl.

So "the library is present in a dependency" is not a guarantee that it's meant to be exposed to the user,

.libs should not return such libraries, so we can also harvest the names from that (.libs is a LibraryList object which can give you .directories or .filenames - in the latter case the individual library files).

I'm also thinking that something stateless is better than yet another cache

I agree that caches have plenty of issues, although IMO in this case the benefits are worthwhile. If we can find a way to limit the complaints that end users get then this solution could be the right way to go. I think curating skip_list will take significant work:

I assume each different compiler will have libraries (right now you have ones associated with GCC, but I assume there will need to be at least one entry for Intel, nvhpc, go, etc.

Also, whether or not it's "OK" to find some of these on the system depends on whether we've installed a compiler with Spack

I assume there may be minor differences for different operating systems (MacOS, and perhaps other Linux distributions)

That doesn't actually seem so bad to manage, now that I list it out. I wonder though if I'm missing something.

Perhaps this can be mitigated by only optionally running this post-install hook. I think that for example it would always make sense to run this for all CI jobs on all systems.

lib/spack/spack/object_file/__init__.py

lib/spack/spack/object_file/elf.py

scheibelp · 2021-12-21T22:44:02Z

lib/spack/spack/hooks/check_dynamic_linking_elf.py

+        if s.external:
+            continue
+        try:
+            dirs = spec[s.name].libs.directories


I think this is causing a CI failure:

spack.build_environment.ChildError: AttributeError: Query of package 'mpileaks' for 'libs' failed

(at https://github.com/spack/spack/runs/4598014455?check_suite_focus=true):

this is now calling .libs for every installed package

the default implementation fails if no libs are found

the mock packages don't have libraries

So one solution for example would be to define .libs for the mock mpileaks package.

I think this hook should not run in tests, only maybe in an integration test.

Generally the build tests and CI jobs are sufficient to exercise this so I mostly agree. I think at least one unit test should run the hook though (so e.g. if something about the API of .libs changes, it is easy to see that the hook also needs to be updated).

lib/spack/spack/relocate.py

haampie · 2022-11-08T13:47:00Z

I've now resurrected and rebased this PR to basically just check if dependencies can be resolved.

haampie · 2022-11-08T17:27:27Z

Another "improvement" could be checking that the resolved paths are actually inside the prefixes of the non-external direct link deps, and if not, warn about that. So verify that

any(realpath(resolved_lib).startswith(realpath(s.prefix)) for s in [spec] + spec.dependencies(deptype="link"))

Or maybe include transitive link deps, since maybe the direct dep is linked statically and pulls in transitive deps that are doing shared linking.

Detected using spack#28109

adamjstewart · 2022-11-12T17:22:45Z

Does this close #1304?

haampie · 2022-11-12T17:53:02Z

No, doesn't resolve symbols

haampie · 2022-11-14T12:38:44Z

@spackbot rebuild everything

spackbot-app · 2022-11-14T12:38:49Z

I've started that pipeline for you!

Detected using spack#28109 ``` bin/ccache-swig libz.so.1 => not found ```

haampie · 2022-11-14T14:00:25Z

I've enabled this now for e4s as a strict post-install check.

There's tons of failures, including cuda/nvhpc/intel-mkl that ship binaries

This commit adds a post-install hook that walks over the prefix and verifies that each ELF executable/library that takes part in shared linking can in fact resolve its direct depenendencies. This is slightly simpler than running `ldd` in the sense that it is not recursive (we only care about the package) and its verification logic is very minimal (we require ET_DYN type elf files for libs, and they should match the class / data / isa as the parent). Also it is platform independent, so we don't have to worry about calling the correct `ldd`. This check does not *fail* the install, it just prints it as an error message.

haampie · 2022-11-14T17:06:23Z

If we add a "strict" mode for CI, what do we do with stuff like cuda / intel-mkl / nvhpc that assume zlib to be a system dependency, or worse, have dependencies on internal deps like Qt5 and fail to set $ORIGIN type of rpaths?

Do we make per-package exceptions in packages.yaml? Do we use patchelf to fix up the binaries? Do we add a per-package ignore list for directories/files?

Detected using #28109

Detected using spack#28109

haampie · 2024-11-01T11:33:05Z

Closing this in favor of #47365

spackbot-app bot added the tests General test capability(ies) label Dec 21, 2021

haampie force-pushed the feature/check-needed-libs-elf-files branch from 0579a95 to b824425 Compare December 21, 2021 13:50

spackbot-app bot added libraries update-package labels Dec 21, 2021

haampie force-pushed the feature/check-needed-libs-elf-files branch 2 times, most recently from 1535df1 to 7397928 Compare December 21, 2021 14:15

haampie mentioned this pull request Dec 21, 2021

Installation issue: Python links to system berkely-db and crypt #28114

Closed

4 tasks

scheibelp self-assigned this Dec 21, 2021

scheibelp reviewed Dec 21, 2021

View reviewed changes

haampie force-pushed the feature/check-needed-libs-elf-files branch from ff74354 to 8125f23 Compare November 8, 2022 13:43

spackbot-app bot added the core PR affects Spack core functionality label Nov 8, 2022

haampie force-pushed the feature/check-needed-libs-elf-files branch from 8125f23 to 1eeacbb Compare November 8, 2022 13:44

haampie changed the title ~~Warn when elf files with dt needed section can't locate libraries in direct link type deps~~ ELF: verify needed libraries are resolvable Nov 8, 2022

haampie requested a review from trws November 8, 2022 13:48

haampie mentioned this pull request Nov 11, 2022

fix perl libxcrypt.so dep #33846

Merged

haampie added a commit to haampie/spack that referenced this pull request Nov 11, 2022

fix perl libxcrypt.so dep

ee39666

Detected using spack#28109

haampie mentioned this pull request Nov 11, 2022

python: missing libxcrypt dep #33847

Merged

spackbot-app bot added defaults gitlab Issues related to gitlab integration labels Nov 14, 2022

haampie force-pushed the feature/check-needed-libs-elf-files branch from 09677b5 to d96d107 Compare November 14, 2022 10:49

haampie mentioned this pull request Nov 14, 2022

swig: needs zlib #33890

Merged

haampie added a commit to haampie/spack that referenced this pull request Nov 14, 2022

swig: needs zlib

fdb48cb

Detected using spack#28109 ``` bin/ccache-swig libz.so.1 => not found ```

haampie added 5 commits November 14, 2022 16:35

strict mode

3ff888b

style

233e8bf

set config:shared_linking:strict:true in e4s

4ce7fac

add libudev to skip list

f6fbd42

haampie force-pushed the feature/check-needed-libs-elf-files branch from 3fe8cb2 to f6fbd42 Compare November 14, 2022 15:35

This was referenced Nov 14, 2022

openssh: add libxcrypt #33892

Merged

util-linux: fix deps #33893

Merged

glib: add missing libelf dep #33894

Merged

tgamblin self-requested a review November 14, 2022 15:53

alalazo pushed a commit that referenced this pull request Nov 14, 2022

fix perl libxcrypt.so dep (#33846)

bf1b2a8

Detected using #28109

charmoniumQ pushed a commit to charmoniumQ/spack that referenced this pull request Nov 19, 2022

fix perl libxcrypt.so dep (spack#33846)

941b185

Detected using spack#28109

haampie mentioned this pull request Nov 26, 2022

OpenMP is broken for [email protected] #33071

Closed

3 tasks

amd-toolchain-support pushed a commit to amd-toolchain-support/spack that referenced this pull request Feb 16, 2023

fix perl libxcrypt.so dep (spack#33846)

392a5db

Detected using spack#28109

haampie mentioned this pull request Nov 1, 2024

config:shared_linking:missing_library_policy to error/warn about accidental use of system libraries on linux/freebsd #47365

Merged

haampie closed this Nov 1, 2024

Conversation

haampie commented Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Examples

llvm

python

perl

Performance

Uh oh!

haampie commented Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scheibelp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haampie Dec 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haampie Dec 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

haampie commented Nov 8, 2022

Uh oh!

haampie commented Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamjstewart commented Nov 12, 2022

Uh oh!

haampie commented Nov 12, 2022

Uh oh!

haampie commented Nov 14, 2022

Uh oh!

spackbot-app bot commented Nov 14, 2022

Uh oh!

haampie commented Nov 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haampie commented Nov 14, 2022

Uh oh!

haampie commented Nov 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

haampie commented Dec 21, 2021 •

edited

Loading

`llvm`

`python`

`perl`

haampie commented Dec 21, 2021 •

edited

Loading

haampie Dec 22, 2021 •

edited

Loading

haampie Dec 22, 2021 •

edited

Loading

haampie commented Nov 8, 2022 •

edited

Loading

haampie commented Nov 14, 2022 •

edited

Loading