Regression in global_allocator when using prefer-dynamic on 1.71.0 and above

[alexkornitzer]

Since 1.71.0 a regression has been added into the compiler causing segmentation faults in applications compiled using prefer-dynamic, this issue is not present in 1.70.0. It appears that the system allocator is still being used in some cases causing segmentation faults.

Code

https://github.com/alexkornitzer/dylib-errors/tree/error/jemalloc

use jemallocator::Jemalloc;

#[global_allocator]
static GLOBAL: Jemalloc = Jemalloc;

fn main() {
    let a = std::thread::spawn(move || {});
    a.join().unwrap();
    println!("didn''t crash");
}

Version it worked on

It most recently worked on: 1.70.0

Version with regression

1.71.0

Have run git bisect and the regression was introduced with the following commit: a2b1646

[lqd]

cc @bjorn3 for that bisection to #86844

[bjorn3]

As far as I understand this shouldn't work on Windows ever before my PR as on Windows symbol preemption across dll's doesn't happen, so inside std.dll it would resolve __rust_alloc to the system allocator unconditionally. cc #100781 (comment)

[alexkornitzer]

This is happening for me on macOS and Linux. What is odd if that cc above is accurate is that global_allocator and -C prefer-dynamic has worked for years for me, its only with 1.71.0 and above that its now broken.

[bjorn3]

I mean it should always have been broken on Windows as far as I understand and now is broken on other platforms too.

[alexkornitzer]

Ah right gotcha, just did not get why Windows was being brought in scope but then again I was not clear that linux was my deployment target and macos my development target. Is there anything I can do to help you fix the regression?

[bjorn3]

When I wrote #86844 I expected rustc to error on #[global_allocator] when a dylib with the allocator shim (like libstd.so) is linked. Instead it turns out that rustc accepts it just fine, but ignores #[global_allocator] entirely. For example the following doesn't panic when compiled with -Cprefer-dynamic with rustc 1.70:

use std::alloc::*;

struct MyAlloc;

unsafe impl GlobalAlloc for MyAlloc {
    unsafe fn alloc(&self, _: Layout) -> *mut u8 { todo!() }
    unsafe fn dealloc(&self, _: *mut u8, _: Layout) { todo!() }
}

#[global_allocator]
static ALLOC: MyAlloc = MyAlloc;

fn main() {
    Box::new(0);
}

and __rust_alloc comes from libstd.so. On ELF based systems it would be possible for rustc to generate the allocator shim and then thus have #[global_allocator] apply, but for Windows and likely macOS it would result in the exact kind of crash you are observing here as preempting symbols from dynamic libraries is not possible and thus the main executable and dynamic libraries will observe a different allocator. The least confusing fix is likely to give an error instead.

[alexkornitzer]

So basically on 1.70.0 and below, when -C prefer-dynamic was set with a #[global_allocator] it was actually just ignoring it and using the default system allocator? If this is the case then yep raising an error makes total sense cause having silent segmentation faults is definitely not good and difficult to debug as the root cause.

[bjorn3]

So basically on 1.70.0 and below, when -C prefer-dynamic was set with a #[global_allocator] it was actually just ignoring it and using the default system allocator?

Yes

[apiraino]

Removing the prioritization label as the comments seem to point out that now this is the intended behaviour. Though probably can be surprising to some users, so I wonder about the actionable here.

@rustbot label -I-prioritize

[alexkornitzer]

@apiraino surely its important to get the compiler to throw an error here or people are going to get unexpected segfaults. This is probably not that apparent due to the rare use of both features, but rustc is now creating invalid binaries.

[apiraino]

ok @alexkornitzer so what would be the desired behaviour in 1.71.0 instead of crashing? Some kind of safeguard preventing it (and maybe giving useful info to the user)? Thanks for helping me figuring the context :-)

[alexkornitzer]

Yeah that would be ideal, which I think is what @bjorn3 suggested but never actioned? (#100781 (comment))

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium