Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

missing RPATH in secondary SO packed into the wheel #451

Open
mattip opened this issue Sep 14, 2023 · 9 comments
Open

missing RPATH in secondary SO packed into the wheel #451

mattip opened this issue Sep 14, 2023 · 9 comments

Comments

@mattip
Copy link

mattip commented Sep 14, 2023

Maybe related to #363 and others.

I may have something similar in a wheel build of OpenBLAS. It packages a OpenBLAS shared object, which depends on libgfortran, which depends on libquadmath. Both of the support so's are packed by auditwheel into the wheel, but the RPATH in libgfortran is not set to $ORIGIN, so libquadmath is not found. I uploaded the auditwheel-repaired wheel to testpypi so the issue can be reproduced:

$ python3.11 -m venv /tmp/test
$ source /tmp/test/bin/activate
$ pip install -i https://test.pypi.org/simple/ scipy-openblas32==0.3.23.293

# This works, `libopenblas_python` can find its dependencies:
$ python -m scipy_openblas32
OpenBLAS using 'b'OpenBLAS 0.3.23.dev DYNAMIC_ARCH NO_AFFINITY Zen MAX_THREADS=64''

# But ldd shows a problem when looking at libgfortran, note the "not found"
$ ldd /tmp/test/lib/python3.11/site-packages/scipy_openblas32/lib/libopenblas_python.so 
	linux-vdso.so.1 (0x00007ffd9fa9e000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007ff314c90000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007ff314c8b000)
	libgfortran-040039e1.so.5.0.0 => /tmp/test/lib/python3.11/site-packages/scipy_openblas32/lib/./libgfortran-040039e1.so.5.0.0 (0x00007ff314600000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007ff3143d8000)
	/lib64/ld-linux-x86-64.so.2 (0x00007ff316e3d000)
	libquadmath-96973f99.so.0.0.0 => /tmp/test/lib/python3.11/site-packages/scipy_openblas32/lib/./libquadmath-96973f99.so.0.0.0 (0x00007ff314000000)
	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007ff314c6d000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007ff314c4d000)
$ ldd /tmp/test/lib/python3.11/site-packages/scipy_openblas32/lib/libgfortran-040039e1.so.5.0.0 
	linux-vdso.so.1 (0x00007fff45797000)
	libquadmath-96973f99.so.0.0.0 => not found
	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007f32b68e7000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f32b6319000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f32b68c7000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f32b60f1000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f32b6936000)

This is only an issue when building a c-extension project (like NumPy or Scipy) that links to libopenblas_python: the linker cannot find libquadmath. Since the whole point of this package is to provide OpenBLAS for consumption by NumPy/SciPy, the failure to link is a problem.

As @rgommers points out, I can use patchelf to add an RPATH to the packaged libgfortran, but isn't this a bug in auditwheel?

@mattip
Copy link
Author

mattip commented Sep 14, 2023

@mayeut
Copy link
Member

mayeut commented Sep 16, 2023

This does not relate to #363

The RPATH modification is not required for runtime or when linking a shared library (as would be the usage of spicy_openblas).

The cc.links() test in numpy is trying to build an executable rather than a shared library and this is where things are going wrong.

While modifying the RPATH of libraries grafted by auditwheel would work in this case, I consider that adding some patchelf commands is always risky and would only be there to accommodate something that's not required at runtime or build time in the most generic usage of auditwheel.
If we want to do this, I'd say that it should not be the default (i.e. user needs to opt in this behavior).

@mattip, as a workaround, you could use -Wl,--allow-shlib-undefined in cc.links args (or -Wl,-fuse-ld=gold as the gold linker does not follow DT_NEEDED entries when resolving symbols in shared libraries)

@lkollar, any thoughts ?

@mayeut
Copy link
Member

mayeut commented Sep 16, 2023

Using the provided wheel (I used the ILP64 one) and the code from the cc.links() test in numpy inside a manylinux_2_28 image:

[root@64042783eba5 ~]# export OPENBLAS_PATH=/opt/_internal/cpython-3.10.13/lib/python3.10/site-packages/scipy_openblas64
[root@64042783eba5 ~]# gcc -otest -I${OPENBLAS_PATH}/include -lopenblas_python -L${OPENBLAS_PATH}/lib  -DBLAS_SYMBOL_SUFFIX=64_ --shared /auditwheel/test.c
[root@64042783eba5 ~]# gcc -otest -I${OPENBLAS_PATH}/include -lopenblas_python -L${OPENBLAS_PATH}/lib  -DBLAS_SYMBOL_SUFFIX=64_ -Wl,-fuse-ld=gold /auditwheel/test.c
[root@64042783eba5 ~]# gcc -otest -I${OPENBLAS_PATH}/include -lopenblas_python -L${OPENBLAS_PATH}/lib  -DBLAS_SYMBOL_SUFFIX=64_ -Wl,--allow-shlib-undefined /auditwheel/test.c
[root@64042783eba5 ~]# gcc -otest -I${OPENBLAS_PATH}/include -lopenblas_python -L${OPENBLAS_PATH}/lib  -DBLAS_SYMBOL_SUFFIX=64_ /auditwheel/test.c
/opt/rh/gcc-toolset-12/root/usr/libexec/gcc/x86_64-redhat-linux/12/ld: warning: libquadmath-96973f99.so.0.0.0, needed by /opt/_internal/cpython-3.10.13/lib/python3.10/site-packages/scipy_openblas64/lib/./libgfortran-040039e1.so.5.0.0, not found (try using -rpath or -rpath-link)
/opt/rh/gcc-toolset-12/root/usr/libexec/gcc/x86_64-redhat-linux/12/ld: /opt/_internal/cpython-3.10.13/lib/python3.10/site-packages/scipy_openblas64/lib/./libgfortran-040039e1.so.5.0.0: undefined reference to `sinhq@QUADMATH_1.0'
/opt/rh/gcc-toolset-12/root/usr/libexec/gcc/x86_64-redhat-linux/12/ld: /opt/_internal/cpython-3.10.13/lib/python3.10/site-packages/scipy_openblas64/lib/./libgfortran-040039e1.so.5.0.0: undefined reference to `erfcq@QUADMATH_1.0'
...

@lkollar
Copy link
Contributor

lkollar commented Sep 18, 2023

The RPATH only needs to be set in the Python extension as when the interpreter dlopens the extension, the dynamic loader will respect the extension's RPATH entries to load further dependencies, so those don't need the RPATH set. Minimising the amount of ELF patching makes it less likely that we mess something up, so if the above workaround doesn't work, I agree that exposing an extra flag in audithweel repair could be a solution.

@ofek
Copy link
Contributor

ofek commented Feb 3, 2024

This has been fixed by #478

@mattip
Copy link
Author

mattip commented Feb 5, 2024

I am still seeing the problem. I use auditwheel>=6 and download the artifact from the run, I am not seeing an RPATH in the libgfortran shared object packed into the wheel by auditwheel, and ldd still does not find the libquadmath.

@mattip
Copy link
Author

mattip commented Feb 26, 2024

@ofek this has not been fixed, as can be seen by downloading a manylinux artifact from https://github.com/MacPython/openblas-libs/actions/runs/7787953087?pr=136 and examining the libgfortran shared object.

@ofek
Copy link
Contributor

ofek commented Feb 26, 2024

Do you have any idea what the true fix might be?

@minrk
Copy link

minrk commented Aug 12, 2024

This has come up in pyzmq, and I think the reason #478 didn't fix it is it looks like it is correcting an incorrect RPATH, but the problem (at least in pyzmq) is that the bundled libraries lack RPATH entirely, when it perhaps should be added. Maybe that's the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants