Make unprefixed consistently override the system allocator #109

madsmtm · 2024-12-15T02:00:52Z

The goal is to move the various workarounds for this feature in rustc to the jemalloc-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.

ti-chi-bot · 2024-12-15T02:01:01Z

Welcome @madsmtm! It looks like this is your first PR to tikv/jemallocator 🎉

BusyJay · 2024-12-16T03:35:52Z

jemalloc-sys/src/lib.rs

+    use super::*;
+
+    #[used]
+    static USED_MALLOC: unsafe extern "C" fn(usize) -> *mut c_void = malloc;


Any references on how these statics are processed?

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 .rlibs are internally archives / static libraries, and so the rules for static libaries apply to them as well).

Or if you have more specific questions about how things work then I can try to answer them, to the best of my ability?

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.

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

I got the test-dylib test to not work without this PR, have pushed that as the first commit instead.

Failed CI run: https://github.com/madsmtm/jemallocator/actions/runs/17785024568
Succesful CI run: https://github.com/madsmtm/jemallocator/actions/runs/17784954361

madsmtm · 2025-09-16T09:14:41Z

Ping @BusyJay, could you take a look? This would be nice to get in, it'd simplify rustc's main function quite a bit.

BusyJay · 2025-09-16T11:56:32Z

jemalloc-sys/src/lib.rs

+    target_vendor = "apple"
+))]
+#[used]
+static USED_ZONE_REGISTER: unsafe extern "C" fn() = {


Better make it a new feature so that we can land it without a new minor version.

I'd argue this is more in the category "bugfix" rather than "feature"; the unprefixed_malloc_on_supported_platforms just plain didn't work on macOS before (unless you inserted these statics yourself like rustc), and now it does.

Or do you fear this will have a chance of breaking something for users?

unprefixed_malloc_on_supported_platforms just plain didn't work on macOS before

It actually works if unprefixed_malloc_on_supported_platforms is interpreted as its name instead of overriding system allocator. The symbol is still unprefixed on MacOS in the past and future.

Or do you fear this will have a chance of breaking something for users?

Yes, it's known not to override system allocator on MacOS.

Right. In the last commit, I've changed it to:

unprefixed_malloc_on_supported_platforms = [] override = ["unprefixed_malloc_on_supported_platforms"]

I've also made the statics only get emitted with that feature enabled.

- 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]>

madsmtm · 2025-09-17T02:57:38Z

I tested this a bit more thoroughly together with mimalloc in https://github.com/madsmtm/test-malloc-overriding btw, have updated the test included in this PR based on that.

jemalloc-sys/build.rs

jemalloc-sys/README.md

BusyJay · 2025-09-22T06:40:55Z

Rest LGTM

madsmtm · 2025-09-22T10:29:45Z

I think I've fixed the things you requested now

Signed-off-by: Mads Marquart <[email protected]>

madsmtm · 2025-09-22T11:55:15Z

Fixed CI

BusyJay · 2025-09-23T06:42:43Z

Thank you!

madsmtm · 2025-10-07T08:40:37Z

Would it be possible to get this published as v0.6.1? I can make a PR for it if you want?

…zelmann Simplify `jemalloc` setup In the past, `#[used]` had to appear in the top-level crate to have a consistent effect on the linker. This has been fixed a while ago for ELF with the introduction of the `symbols.o` file in rust-lang#95604, and more recently for Mach-O in rust-lang#133832, which means that libraries can now implement the required workarounds themselves. This allows moving these `#[used]` declarations out of our `main.rs`. Specifically, I have moved them into `tikv-jemalloc-sys` where they belong in tikv/jemallocator#109 and done the same for `mimalloc` in purpleprotocol/mimalloc_rust#146 (in case we want to experiment with switching to that one day). Test with: ```sh ./x build library src/tools/rustdoc src/tools/clippy --set rust.jemalloc=true # macOS lldb -- ./build/host/stage1/bin/rustc -vV (lldb) b _rjem_je_zone_register (lldb) run # Should breakpoint, this means that the allocator was properly linked # Linux lldb -- ./build/host/stage1/bin/rustc -vV (lldb) b malloc (lldb) run # Should breakpoint, inspect that the `malloc` symbol comes from the `rustc` binary and not from `libc` ``` try-job: `aarch64-gnu` try-job: `dist-aarch64-linux` try-job: `dist-x86_64-musl` try-job: `dist-x86_64-apple` try-job: `dist-aarch64-apple`

Rollup merge of #146627 - madsmtm:jemalloc-simplify, r=jdonszelmann Simplify `jemalloc` setup In the past, `#[used]` had to appear in the top-level crate to have a consistent effect on the linker. This has been fixed a while ago for ELF with the introduction of the `symbols.o` file in #95604, and more recently for Mach-O in #133832, which means that libraries can now implement the required workarounds themselves. This allows moving these `#[used]` declarations out of our `main.rs`. Specifically, I have moved them into `tikv-jemalloc-sys` where they belong in tikv/jemallocator#109 and done the same for `mimalloc` in purpleprotocol/mimalloc_rust#146 (in case we want to experiment with switching to that one day). Test with: ```sh ./x build library src/tools/rustdoc src/tools/clippy --set rust.jemalloc=true # macOS lldb -- ./build/host/stage1/bin/rustc -vV (lldb) b _rjem_je_zone_register (lldb) run # Should breakpoint, this means that the allocator was properly linked # Linux lldb -- ./build/host/stage1/bin/rustc -vV (lldb) b malloc (lldb) run # Should breakpoint, inspect that the `malloc` symbol comes from the `rustc` binary and not from `libc` ``` try-job: `aarch64-gnu` try-job: `dist-aarch64-linux` try-job: `dist-x86_64-musl` try-job: `dist-x86_64-apple` try-job: `dist-aarch64-apple`

Simplify `jemalloc` setup In the past, `#[used]` had to appear in the top-level crate to have a consistent effect on the linker. This has been fixed a while ago for ELF with the introduction of the `symbols.o` file in rust-lang/rust#95604, and more recently for Mach-O in rust-lang/rust#133832, which means that libraries can now implement the required workarounds themselves. This allows moving these `#[used]` declarations out of our `main.rs`. Specifically, I have moved them into `tikv-jemalloc-sys` where they belong in tikv/jemallocator#109 and done the same for `mimalloc` in purpleprotocol/mimalloc_rust#146 (in case we want to experiment with switching to that one day). Test with: ```sh ./x build library src/tools/rustdoc src/tools/clippy --set rust.jemalloc=true # macOS lldb -- ./build/host/stage1/bin/rustc -vV (lldb) b _rjem_je_zone_register (lldb) run # Should breakpoint, this means that the allocator was properly linked # Linux lldb -- ./build/host/stage1/bin/rustc -vV (lldb) b malloc (lldb) run # Should breakpoint, inspect that the `malloc` symbol comes from the `rustc` binary and not from `libc` ``` try-job: `aarch64-gnu` try-job: `dist-aarch64-linux` try-job: `dist-x86_64-musl` try-job: `dist-x86_64-apple` try-job: `dist-aarch64-apple`

ti-chi-bot bot added the dco-signoff: no label Dec 15, 2024

madsmtm force-pushed the used-unprefixed branch from cf52544 to 1deee1e Compare December 15, 2024 02:01

ti-chi-bot bot added dco-signoff: yes and removed dco-signoff: no labels Dec 15, 2024

madsmtm mentioned this pull request Dec 15, 2024

macOS: Fix linking statically jemalloc/jemalloc#2649

Open

BusyJay reviewed Dec 16, 2024

View reviewed changes

madsmtm force-pushed the used-unprefixed branch 3 times, most recently from 070e78b to c6670be Compare June 6, 2025 12:31

ti-chi-bot bot added dco-signoff: no and removed dco-signoff: yes labels Jun 6, 2025

madsmtm force-pushed the used-unprefixed branch from c6670be to 9bc0fd9 Compare June 6, 2025 12:31

ti-chi-bot bot added dco-signoff: yes and removed dco-signoff: no labels Jun 6, 2025

madsmtm requested a review from BusyJay June 6, 2025 12:48

This was referenced Sep 16, 2025

Enable dyld interposing on Apple platforms when overriding purpleprotocol/mimalloc_rust#145

Merged

Add #[used] static to always override purpleprotocol/mimalloc_rust#146

Merged

Simplify jemalloc setup rust-lang/rust#146627

Merged

BusyJay reviewed Sep 16, 2025

View reviewed changes

madsmtm added 2 commits September 17, 2025 04:36

Add tests for unprefixed

7d0b060

- 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]>

Make unprefixed consistently override the system allocator

bd94cbe

Signed-off-by: Mads Marquart <[email protected]>

madsmtm force-pushed the used-unprefixed branch from ea4c927 to bd94cbe Compare September 17, 2025 02:36

ti-chi-bot bot added dco-signoff: no and removed dco-signoff: yes labels Sep 18, 2025

madsmtm force-pushed the used-unprefixed branch from a7ea610 to b2813fc Compare September 18, 2025 10:43

ti-chi-bot bot added dco-signoff: yes and removed dco-signoff: no labels Sep 18, 2025

BusyJay reviewed Sep 22, 2025

View reviewed changes

jemalloc-sys/build.rs Outdated Show resolved Hide resolved

jemalloc-sys/README.md Outdated Show resolved Hide resolved

madsmtm force-pushed the used-unprefixed branch 2 times, most recently from 25dd274 to 82b4a4e Compare September 22, 2025 10:28

madsmtm requested a review from BusyJay September 22, 2025 10:29

BusyJay approved these changes Sep 22, 2025

View reviewed changes

Add "override_allocator_on_supported_platforms" feature flag

fc4d596

Signed-off-by: Mads Marquart <[email protected]>

madsmtm force-pushed the used-unprefixed branch from 82b4a4e to fc4d596 Compare September 22, 2025 11:54

madsmtm requested a review from BusyJay September 22, 2025 11:55

BusyJay merged commit 925d696 into tikv:main Sep 23, 2025
10 checks passed

madsmtm deleted the used-unprefixed branch September 24, 2025 19:49

madsmtm mentioned this pull request Oct 15, 2025

Release v0.6.1 #147

Merged

Make unprefixed consistently override the system allocator #109

Make unprefixed consistently override the system allocator #109

Uh oh!

Conversation

madsmtm commented Dec 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ti-chi-bot bot commented Dec 15, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madsmtm Dec 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madsmtm commented Sep 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madsmtm commented Sep 17, 2025

Uh oh!

Uh oh!

Uh oh!

BusyJay commented Sep 22, 2025

Uh oh!

madsmtm commented Sep 22, 2025

Uh oh!

madsmtm commented Sep 22, 2025

Uh oh!

Uh oh!

BusyJay commented Sep 23, 2025

Uh oh!

madsmtm commented Oct 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

madsmtm commented Dec 15, 2024 •

edited

Loading

madsmtm Dec 16, 2024 •

edited

Loading