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

Consistently use RUNPATH in our libraries #46464

Merged
merged 2 commits into from
Aug 24, 2022
Merged

Conversation

staticfloat
Copy link
Member

When loading dependencies on Linux, we can either use RPATH or
RUNPATH as a list of relative paths to search for libraries. The
difference, for our purposes, mainly lies within how this interacts with
LD_LIBRARY_PATH: RPATH is searched first, then LD_LIBRARY_PATH,
then RUNPATH. So by using RUNPATH here, we are explicitly allowing
ourselves to be overridden by LD_LIBRARY_PATH. This is fine, as long
as we are consistent across our entire library line, however in the
v1.8.0 release, there was an inconsistency, reported in [0].

The inconsistency occured because of the following confluence of factors:

  • Ancient ld builds (such as the one used in our build environment)
    do not default to using RUNPATH, but instead use RPATH.
  • patchelf, when it rewrites the RPATH, will default to using
    RUNPATH instead.
  • We were only using patchelf on libjulia-internal, not on
    libjulia-codegen, which was newly added in v1.8.

These three factors together caused us to ship a binary with RUNPATH
in libjulia-internal, but RPATH in libjulia-codegen, which caused
loading to fail in [0] due to first libjulia-internal being loaded,
(which brought in the external libstdc++), then libjulia-codegen
failed to load (because it found an incompatible libstdc++), causing
the mysterious compiler error.

This PR fixes this twofold; first, when building the libraries in the
first place, we pass --enable-new-dtags to the linker to encourage it
to use runpath when possible. This removes the possibility for a
missing patchelf invocation to break things in this way. Second, we
apply patchelf properly to libjulia-codegen as well.

[0] #46409

When loading dependencies on Linux, we can either use `RPATH` or
`RUNPATH` as a list of relative paths to search for libraries.  The
difference, for our purposes, mainly lies within how this interacts with
`LD_LIBRARY_PATH`: `RPATH` is searched first, then `LD_LIBRARY_PATH`,
then `RUNPATH`.  So by using `RUNPATH` here, we are explicitly allowing
ourselves to be overridden by `LD_LIBRARY_PATH`.  This is fine, as long
as we are consistent across our entire library line, however in the
`v1.8.0` release, there was an inconsistency, reported in [0].

The inconsistency occured because of the following confluence of factors:

 - Ancient `ld` builds (such as the one used in our build environment)
   do not default to using `RUNPATH`, but instead use `RPATH`.
 - `patchelf`, when it rewrites the RPATH, will default to using
   `RUNPATH` instead.
 - We were only using `patchelf` on `libjulia-internal`, not on
   `libjulia-codegen`, which was newly added in `v1.8`.

These three factors together caused us to ship a binary with `RUNPATH`
in `libjulia-internal`, but `RPATH` in `libjulia-codegen`, which caused
loading to fail in [0] due to first `libjulia-internal` being loaded,
(which brought in the external `libstdc++`), then `libjulia-codegen`
failed to load (because it found an incompatible `libstdc++`), causing
the mysterious compiler error.

This PR fixes this twofold; first, when building the libraries in the
first place, we pass `--enable-new-dtags` to the linker to encourage it
to use `runpath` when possible.  This removes the possibility for a
missing `patchelf` invocation to break things in this way.  Second, we
apply `patchelf` properly to `libjulia-codegen` as well.

[0] #46409
@giordano giordano added building Build system, or building Julia or its dependencies backport 1.8 Change should be backported to release-1.8 labels Aug 23, 2022
Make.inc Outdated Show resolved Hide resolved
Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>
@staticfloat staticfloat merged commit 3b1c54d into master Aug 24, 2022
@staticfloat staticfloat deleted the sf/runpath_not_rpath branch August 24, 2022 15:38
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Sep 7, 2022
vtjnash added a commit to JuliaPackaging/Yggdrasil that referenced this pull request Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
building Build system, or building Julia or its dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants