Disable `DL_LOAD_PATH` prepending for `@`-paths on Darwin by staticfloat · Pull Request #42721 · JuliaLang/julia

staticfloat · 2021-10-20T20:17:41Z

Many thanks to Randy Rucker from Apple for pointing out that we were
technically relying on undefined behavior in dyld for loading things
via jl_dlopen("@loader_path/@rpath/libfoo.dylib"), due to the
composition of dlopen("@rpath/libfoo.dylib") in our Julia code, and
the DL_LOAD_PATH prepending we do in jl_load_dynamic_library().

This PR uses a slightly modified version of a patch emailed to me by
Randy, and should solve
JuliaLang/Downloads.jl#149 on macOS Monterey
where the undefined behavior in dyld has changed.

Many thanks to Randy Rucker from Apple for pointing out that we were technically relying on undefined behavior in `dyld` for loading things via `jl_dlopen("@loader_path/@rpath/libfoo.dylib")`, due to the composition of `dlopen("@rpath/libfoo.dylib")` in our Julia code, and the `DL_LOAD_PATH` prepending we do in `jl_load_dynamic_library()`. This PR uses a slightly modified version of a patch emailed to me by Randy, and should solve JuliaLang/Downloads.jl#149 on macOS Monterey where the undefined behavior in `dyld` has changed.

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

staticfloat · 2021-10-21T16:37:01Z

Confirmed to fix JuliaLang/Downloads.jl#149

vtjnash · 2021-10-21T16:41:50Z

Why is this code trying to handle full paths (with /) in them anyways?

Also, seems like we have special handling here for @executable_path that is now getting dropped on macOS?

staticfloat · 2021-10-21T16:56:41Z

Also, seems like we have special handling here for @executable_path that is now getting dropped on macOS?

Ironically, the @executable_path stuff is actually for everything other than macOS, since macOS supports that natively. I agree it's confusing though; what do you suggest? The @executable_path stuff is to emulate that functionality on Windows and other platforms that don't have a way to load something with a relative path from the executable itself.

Why is this code trying to handle full paths (with /) in them anyways?

I don't quite understand what you're asking.

vtjnash · 2021-10-21T19:48:38Z

I don't understand why this code is combining a/b with Dl_LOAD_PATH, and not just when given b. It is common for the shell, for example, to only search if given only a name and not a path.

staticfloat · 2021-10-21T20:06:28Z

Aha, I see what you mean. Yeah, that's not necessarily useful.

I think we can probably refactor this to be a three-step process:

Translate @executable_path/-style modnames to an absolute path.
Do abspath and is_atpath detection (the latter still macOS-only)
Enter DL_LOAD_PATH loop.

This way, we'll have consistent handling across platforms as much as possible, and we cut down on unnecessary jl_load_dynamic_library() calls.

vtjnash · 2021-10-21T20:13:46Z

I agree

staticfloat · 2021-10-21T20:33:24Z

Looking into this a bit deeper, I see that we are actually transforming the entries of DL_LOAD_PATH, not modname. E.g. we are allowing for DL_LOAD_PATH to contain @executable_path/../lib/, and then prepend it to all entries.

I had forgotten that that was how I designed this, and I'm not at all sure that this is precisely what we want. I don't think we're actively using this feature yet, so we can still change it if we want to. This was done to serve these changes from my JLL stdlibs PR, where I wanted to get Base able to dlopen("libpcre2-8") when that file lived in some very specific location. Looking back on it now, my gut says that the "right" way to do this would be for us to dlopen("@executable_path/../whatever"), but the reason we aren't doing that is because we just do ccall) without any explicit dlopen() on libpcre due to us needing libpcre so early on that we don't have access to niceties like joinpath(). We could, of course, hardcode a specific dlopen() instead. I'm undecided.

* Disable `DL_LOAD_PATH` prepending for `@`-paths on Darwin Many thanks to Randy Rucker from Apple for pointing out that we were technically relying on undefined behavior in `dyld` for loading things via `jl_dlopen("@loader_path/@rpath/libfoo.dylib")`, due to the composition of `dlopen("@rpath/libfoo.dylib")` in our Julia code, and the `DL_LOAD_PATH` prepending we do in `jl_load_dynamic_library()`. This PR uses a slightly modified version of a patch emailed to me by Randy, and should solve JuliaLang/Downloads.jl#149 on macOS Monterey where the undefined behavior in `dyld` has changed. * Apply suggestions from code review Co-authored-by: Jameson Nash <vtjnash@gmail.com> * Use `[]` instead of `*` Co-authored-by: Jameson Nash <vtjnash@gmail.com> (cherry picked from commit 76c2431)

…42721) * Disable `DL_LOAD_PATH` prepending for `@`-paths on Darwin Many thanks to Randy Rucker from Apple for pointing out that we were technically relying on undefined behavior in `dyld` for loading things via `jl_dlopen("@loader_path/@rpath/libfoo.dylib")`, due to the composition of `dlopen("@rpath/libfoo.dylib")` in our Julia code, and the `DL_LOAD_PATH` prepending we do in `jl_load_dynamic_library()`. This PR uses a slightly modified version of a patch emailed to me by Randy, and should solve JuliaLang/Downloads.jl#149 on macOS Monterey where the undefined behavior in `dyld` has changed. * Apply suggestions from code review Co-authored-by: Jameson Nash <vtjnash@gmail.com> * Use `[]` instead of `*` Co-authored-by: Jameson Nash <vtjnash@gmail.com>

* Disable `DL_LOAD_PATH` prepending for `@`-paths on Darwin Many thanks to Randy Rucker from Apple for pointing out that we were technically relying on undefined behavior in `dyld` for loading things via `jl_dlopen("@loader_path/@rpath/libfoo.dylib")`, due to the composition of `dlopen("@rpath/libfoo.dylib")` in our Julia code, and the `DL_LOAD_PATH` prepending we do in `jl_load_dynamic_library()`. This PR uses a slightly modified version of a patch emailed to me by Randy, and should solve JuliaLang/Downloads.jl#149 on macOS Monterey where the undefined behavior in `dyld` has changed. * Apply suggestions from code review Co-authored-by: Jameson Nash <vtjnash@gmail.com> * Use `[]` instead of `*` Co-authored-by: Jameson Nash <vtjnash@gmail.com> (cherry picked from commit 76c2431)

staticfloat added backport 1.6 Change should be backported to release-1.6 backport 1.7 labels Oct 20, 2021

vtjnash reviewed Oct 20, 2021

View reviewed changes

Comment thread src/dlload.c Outdated

vtjnash reviewed Oct 20, 2021

View reviewed changes

Comment thread src/dlload.c Outdated

staticfloat and others added 2 commits October 20, 2021 14:25

Apply suggestions from code review

c477ade

Co-authored-by: Jameson Nash <vtjnash@gmail.com>

Use [] instead of *

27cb0c4

staticfloat merged commit 76c2431 into master Oct 21, 2021

staticfloat deleted the sf/apple_dlpath_atpaths branch October 21, 2021 16:37

staticfloat mentioned this pull request Oct 21, 2021

Error: curl_easy_setopt: 48 JuliaLang/Downloads.jl#149

Closed

KristofferC mentioned this pull request Oct 22, 2021

release-1.7: Backports for 1.7.0/1.7.0-rc3 #42765

Merged

66 tasks

Roger-luo mentioned this pull request Oct 27, 2021

Downloads.Curl issue with macOS 12: Error: curl_easy_setopt: 48 #42814

Closed

KristofferC mentioned this pull request Oct 29, 2021

release-1.6: Backports for julia-1.6.4 #42147

Merged

95 tasks

mgkuhn mentioned this pull request Oct 31, 2021

Julia 1.7 does not start on macOS Monterey #42875

Closed

fredrikekre mentioned this pull request Nov 7, 2021

Unable to add unregistered packages on macOS Monterey JuliaLang/Pkg.jl#2788

Closed

KristofferC removed backport 1.6 Change should be backported to release-1.6 backport 1.7 labels Nov 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Disable `DL_LOAD_PATH` prepending for `@`-paths on Darwin#42721

Disable `DL_LOAD_PATH` prepending for `@`-paths on Darwin#42721
staticfloat merged 3 commits intomasterfrom
sf/apple_dlpath_atpaths

staticfloat commented Oct 20, 2021

Uh oh!

Uh oh!

Uh oh!

staticfloat commented Oct 21, 2021

Uh oh!

vtjnash commented Oct 21, 2021

Uh oh!

staticfloat commented Oct 21, 2021

Uh oh!

vtjnash commented Oct 21, 2021

Uh oh!

staticfloat commented Oct 21, 2021

Uh oh!

vtjnash commented Oct 21, 2021

Uh oh!

staticfloat commented Oct 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

staticfloat commented Oct 20, 2021

Uh oh!

Uh oh!

Uh oh!

staticfloat commented Oct 21, 2021

Uh oh!

vtjnash commented Oct 21, 2021

Uh oh!

staticfloat commented Oct 21, 2021

Uh oh!

vtjnash commented Oct 21, 2021

Uh oh!

staticfloat commented Oct 21, 2021

Uh oh!

vtjnash commented Oct 21, 2021

Uh oh!

staticfloat commented Oct 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants