Skip to content

Conversation

topolarity
Copy link
Member

ELF doesn't handle WEAK symbols dynamically the way it handles them statically.

Looking up overloaded WEAK symbols via a library-specific handle will often give you the empty stub (in libc.so.7 in this case) instead of the strong implementation elsewhere (ld-elf.so.1 here).

Even after the upstream fix, dlsym, dladdr and a ton of other symbols still have stubs with no trampoline in FreeBSD's libc:
https://cgit.freebsd.org/src/tree/lib/libc/gen/dlfcn.c?id=21a52f99440c9bec7679f3b0c5c9d888901c3694 Thankfully dl_iterate_phdr appears to be the only function that we directly ccall from Julia's Libdl so we can leave this fix incomplete for now.

Resolves #50846.

FreeBSD does not support looking up WEAK symbols in its libc via `dlsym`
if you're using a library-specific handle. It will give you the empty
stub in libc instead of, e.g., the strong implementation in ld-elf.so

This fix and the one that was merged upstream are pretty conspicuously
incomplete, since `dlsym`, `dladdr` and a ton of other symbols still
have stubs with no trampoline in libc:
https://cgit.freebsd.org/src/tree/lib/libc/gen/dlfcn.c?id=21a52f99440c9bec7679f3b0c5c9d888901c3694

Thankfully, this appears to be the only function that we directly
`ccall` from Julia's Libdl so we can leave this fix incomplete for now.
@ararslan ararslan added system:freebsd Affects only FreeBSD backport 1.10 Change should be backported to the 1.10 release labels Aug 30, 2023
// This is a workaround for FreeBSD <= 13.2 which do not have
// https://cgit.freebsd.org/src/commit/?id=21a52f99440c9bec7679f3b0c5c9d888901c3694
// (See https://github.com/JuliaLang/julia/issues/50846)
if (strcmp(f_name, "dl_iterate_phdr") == 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable to me as a stopgap solution but I wonder if there's a way we can more directly query whether dl_iterate_phdr resolved to libc does the right thing so that we don't have to perform this check on every call.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect this check is generally a lot cheaper than the dlsyms below, but thankfully the standard function/library pointer caching for ccall should mean we only do this once per call-site

@ararslan
Copy link
Member

Screenshot 2023-08-30 at 10 03 58 AM

💖

@topolarity topolarity added the merge me PR is reviewed. Merge when all tests are passing label Aug 30, 2023
@ararslan
Copy link
Member

The x86_64-linux-gnuassertrr failure is unrelated

@DilumAluthge DilumAluthge merged commit c659011 into JuliaLang:master Aug 31, 2023
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Aug 31, 2023
@topolarity topolarity deleted the fix-50846 branch August 31, 2023 13:59
KristofferC pushed a commit that referenced this pull request Sep 15, 2023
ELF doesn't handle WEAK symbols dynamically the way it handles them
statically.

Looking up overloaded WEAK symbols via a library-specific handle will
often give you the empty stub (in `libc.so.7` in this case) instead of
the strong implementation elsewhere (`ld-elf.so.1` here).

Even after the [upstream
fix](https://cgit.freebsd.org/src/commit/?id=21a52f99440c9bec7679f3b0c5c9d888901c3694),
`dlsym`, `dladdr` and a ton of other symbols still have stubs with no
trampoline in FreeBSD's libc:

https://cgit.freebsd.org/src/tree/lib/libc/gen/dlfcn.c?id=21a52f99440c9bec7679f3b0c5c9d888901c3694
Thankfully `dl_iterate_phdr` appears to be the only function that we
directly `ccall` from Julia's Libdl so we can leave this fix incomplete
for now.

Resolves #50846.

(cherry picked from commit c659011)
KristofferC added a commit that referenced this pull request Oct 2, 2023
Backported PRs:
- [x] #48625 <!-- add replace(io, str, patterns...) -->
- [x] #48387 <!-- Improve documentation of sort-related functions -->
- [x] #48363 <!-- Revise sort.md and docstrings in sort.jl -->
- [x] #48977 <!-- Update SparseArrays.jl stdlib for SuiteSparse 7 -->
- [x] #50719 <!-- fix `CyclePadding(::DataType)` -->
- [x] #50694 <!-- inference: permit recursive type traits -->
- [x] #50860 <!-- Add `Base.get_extension` to docs/API -->
- [x] #50594 <!-- Disallow non-index Integer types in isassigned -->
- [x] #50802 <!-- Makes IntrusiveLinkedListSynchronized mutable to avoid
allocation on poptask -->
- [x] #50858 <!-- Add a `threadpool` parameter to `Channel` constructor
-->
- [x] #50874 <!-- Restrict COFF to a single thread when symbol count is
high -->
- [x] #50822 <!-- Add default method for setmodifiers! -->
- [x] #50730 <!-- Fix integer overflow in `isapprox` -->
- [x] #50850 <!-- Remove weird Rational dispatch and add pi functions to
list -->
- [x] #50809 <!-- Limit type-printing in MethodError -->
- [x] #50915 <!-- Add note the `Task` about sticky bit -->
- [x] #50929 <!-- when widening tuple types in tmerge, only widen the
complex parts -->
- [x] #50928 <!-- Bump JuliaSyntax to 0.4.6 -->
- [x] #50959 <!-- Update libssh2 patches -->
- [x] #50823 <!-- Make ranges more robust with unsigned indexes. -->
- [x] #48542 <!-- Add docs on task-specific buffering using
multithreading -->
- [x] #50912 <!-- Separate foreign threads into a :foreign threadpool
-->
- [x] #51010 <!-- Add ORIGIN to SuiteSparse rpath on Linux/FreeBSD -->
- [x] #50753 <!-- cat: remove unused promote_eltype methods that confuse
inference -->
- [x] #51027 <!-- Implement realloc accounting correctly -->
- [x] #51019 <!-- fix a case of potentially use of undefined variable
when handling error in distributed message processing -->
- [x] #51039 <!-- Revert "Optimize findall(f, ::AbstractArray{Bool})
(#42202)" -->
- [x] #51036 <!-- add missing invoke edge for nospecialize targets -->
- [x] #51042 <!-- inference: fix return_type_tfunc modeling of concrete
functions -->
- [x] #51114 <!-- Workaround upstream FreeBSD issue #272992 -->
- [x] #50892 <!-- Add `JL_DLLIMPORT` to `small_typeof` declaration -->
- [x] #51154 <!-- broadcast: use recursion rather than ntuple to map
over a tuple -->
- [x] #51153 <!-- fix debug typo in "add missing invoke edge for
nospecialize targets (#51036)" -->
- [x] #51222 <!-- Check again if the tty is open inside the IO lock -->
- [x] #51236 <!-- Add lock around uv_unref during init -->
- [x] #51243 <!-- GMP: Gracefully handle more overflows. -->
- [x] #51254 <!-- Ryu: make sure adding zeros does not overwrite
trailing dot -->
- [x] #51175 <!-- shorten stale_age for cachefile lock -->
- [x] #51300 <!-- fix method definition error for bad vararg -->
- [x] #51307 <!-- fix force-throw ctrl-C on Windows -->
- [x] #51303 <!-- ensure revising structs is safe -->
- [x] #51393 
- [x] #51403 

Need manual backport:
- [x] #51009 <!-- fix #50562, regression in `in` of tuple of Symbols -->
- [x] #51053 <!-- Bump Statistics stdlib -->
- [x] #51013 <!-- fix #50709, type bound error due to free typevar in
sparam env -->
- [x] #51305 <!-- reduce test time for rounding and floatfuncs -->

Contains multiple commits, manual intervention needed:
- [ ] #50663 <!-- Fix Expr(:loopinfo) codegen -->
- [ ] #51035 <!-- refactor GC scanning code to reflect jl_binding_t are
now first class -->
- [ ] #51092 <!-- inference: fix bad effects for recursion -->
- [x] #51247 <!-- Check if malloc has succeeded before incrementing gc
stats -->
- [x] #51294 <!-- use LibGit2_jll for LibGit2 library -->

Non-merged PRs with backport label:
- [ ] #51132 <!-- Handle `AbstractQ` in concatenation -->
- [x] #51029 <!-- testdefs: make sure that if a test set changes the
active project, they change it back when they're done -->
- [ ] #50919 <!-- Code loading: do the "skipping mtime check for stdlib"
check regardless of the value of `ispath(f)` -->
- [ ] #50824 <!-- Add some aliasing warnings to docstrings for mutating
functions -->
- [x] #50385 <!-- Precompile pidlocks: add to NEWS and docs -->
- [ ] #49805 <!-- Limit TimeType subtraction to AbstractDateTime -->
@KristofferC KristofferC removed the backport 1.10 Change should be backported to the 1.10 release label Oct 2, 2023
nalimilan pushed a commit that referenced this pull request Nov 5, 2023
ELF doesn't handle WEAK symbols dynamically the way it handles them
statically.

Looking up overloaded WEAK symbols via a library-specific handle will
often give you the empty stub (in `libc.so.7` in this case) instead of
the strong implementation elsewhere (`ld-elf.so.1` here).

Even after the [upstream
fix](https://cgit.freebsd.org/src/commit/?id=21a52f99440c9bec7679f3b0c5c9d888901c3694),
`dlsym`, `dladdr` and a ton of other symbols still have stubs with no
trampoline in FreeBSD's libc:

https://cgit.freebsd.org/src/tree/lib/libc/gen/dlfcn.c?id=21a52f99440c9bec7679f3b0c5c9d888901c3694
Thankfully `dl_iterate_phdr` appears to be the only function that we
directly `ccall` from Julia's Libdl so we can leave this fix incomplete
for now.

Resolves #50846.

(cherry picked from commit c659011)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

system:freebsd Affects only FreeBSD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dllist() returns no items on FreeBSD

5 participants