-
Notifications
You must be signed in to change notification settings - Fork 82
Make unprefixed consistently override the system allocator #109
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
base: main
Are you sure you want to change the base?
Conversation
Welcome @madsmtm! It looks like this is your first PR to tikv/jemallocator 🎉 |
cf52544
to
1deee1e
Compare
use super::*; | ||
|
||
#[used] | ||
static USED_MALLOC: unsafe extern "C" fn(usize) -> *mut c_void = malloc; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any references on how these statics are processed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's the reference on #[used]
, and the equivalent Clang attribute and GCC attribute.
But these are somewhat vague, perhaps intentionally so as this is very much a linker concept? I don't really have a good reference on linkers, the best I can do is reference this piece of source code in rustc
that talks about a workaround for static libs, and the following section from the manual page for ld64
:
A static library (aka static archive) is a collection of .o files with a table of contents that lists the global symbols in the .o files. ld will only pull .o files out of a static library if needed to resolve some symbol reference. Unlike traditional linkers, ld will continually search a static library while linking.
(Note that Rust .rlib
s are internally archives / static libraries, and so the rules for static libaries apply to them as well).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or if you have more specific questions about how things work then I can try to answer them, to the best of my ability?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. How about adding a test to show it work as expected? You can add a dylib crate that allocs in the root directory and then add a test crate that links both the dylib and jemalloc-sys. If it works as expected the test shoud be able to use jemalloc's free to dealloc the pointer from dylib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay.
I found the closest thing to a reference link, and have rewritten the docs around it to hopefully be clearer.
I have also added two tests:
malloc_and_libc_are_interoperable_when_overridden
, which tests that the overriding actually works on macOS.test-dylib
, which tests that when linking a dylib, the symbol is correctly overridden. Note that I couldn't reproduce it with the current nightly, so something might have changed recently that makes this hack redundant nowadays? Unsure, though it doesn't hurt to have in any case.
Failed CI run of the first commit with just the tests: https://github.com/madsmtm/jemallocator/actions/runs/15490515399
Successful CI run after the second commit: https://github.com/madsmtm/jemallocator/actions/runs/15490429305
070e78b
to
c6670be
Compare
- A test that checks that jemalloc's malloc and libc's free are interoperable when unprefixed. - A test that checks that using unprefixed with a shared library still overrides the symbols in the shared library. Signed-off-by: Mads Marquart <[email protected]>
Signed-off-by: Mads Marquart <[email protected]>
The goal is to move the various workarounds for this feature in
rustc
to thejemalloc-sys
crate instead.I'm not entirely sure of the history here, but it is possible this was not done in the past because
#[used]
used to not work in libraries, see rust-lang/rust#95604 and rust-lang/rust#133491? In any case, we should be able to do it now.