global_allocator does not work with -C prefer-dynamic

[nicbn]

Could not find anything related to this online. Using RUSTFLAGS="-C prefer-dynamic" cargo run on this does not panic.

use std::alloc::{GlobalAlloc, Layout};

#[global_allocator]
static ALLOC: Alloc = Alloc;

pub struct Alloc;

unsafe impl GlobalAlloc for Alloc {
    unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
        panic!()
    }
    
    unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
        panic!()
    }
    
    unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
        panic!()
    }
    
    unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
        panic!()
    }
}

fn main() {
    let _ = Box::new(1);
}

Meta

rustc --version --verbose:

rustc 1.65.0-nightly (29e4a9ee0 2022-08-10)
binary: rustc
commit-hash: 29e4a9ee0253cd39e552a77f51f11f9a5f1c41e6
commit-date: 2022-08-10
host: x86_64-unknown-linux-gnu
release: 1.65.0-nightly
LLVM version: 14.0.6

[Noratrieb]

Does this also happen with -Cpanic=abort? Unwinding from GlobalAlloc is UB, so it would be useful to see a non-UB example.

[Nemo157]

Non-UB (I believe) example that fails for me (SIGSEGV normally, completes successfully with -C prefer-dynamic):

use std::alloc::{GlobalAlloc, Layout};

#[global_allocator]
static ALLOC: Alloc = Alloc;

pub struct Alloc;

#[allow(unconditional_recursion)]
fn abort() -> ! {
    abort()
}

unsafe impl GlobalAlloc for Alloc {
    unsafe fn alloc(&self, _layout: Layout) -> *mut u8 {
        abort()
    }

    unsafe fn alloc_zeroed(&self, _layout: Layout) -> *mut u8 {
        abort()
    }

    unsafe fn realloc(&self, _ptr: *mut u8, _layout: Layout, _new_size: usize) -> *mut u8 {
        abort()
    }

    unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) {
        abort()
    }
}

fn main() {
    let _ = Box::new(1);
}

[nicbn]

You can't use panic=abort with -C prefer-dynamic, because the stdlib you are linking against unwinds (or at least I never managed to get these flags together to compile). But you can do anything inside the function, loop {}, or just return ptr::null_mut(). It just won't do it.

[Noratrieb]

That's a very interesting case. Now, calls to GlobalAlloc are allowed to be optimized out, so this behavior isn't technically illegal, but it's certainly very odd and deserves some deeper investigation.

[nicbn]

Optimization is not the cause of what is going on. Also the allocator will be called before the Box::new by the runtime. You can try any usage for the allocator really, even calling alloc::alloc() directly.

[bjorn3]

It seems that #112794 accidentally fixed this. I was under the impression that rustc would error out when using #[global_allocator] when an upstream dylib already provided the allocator shim. I don't know if this will result in any inconsistencies where libstd will use std::alloc::System as global allocator and the main executable will use #[global_allocator] though on some systems. For ELF based systems it should be fine, but I'm worried that Mach-O based systems (apple) with two level namespaces cause problems and I don't know if PE (windows) supports this kind of interposition from the main executable to dll's.

[ChrisDenton]

I don't know if PE (windows) supports this kind of interposition from the main executable to dll's.

Not really, iirc. You could shim something by having the dll manually get an exported symbol from the exe when the dll is loaded. I'm not sure how I feel about that though.

[bjorn3]

Would making usage of #[global_allocator] when linking a dylib which contains an allocator shim a hard error be acceptable? It didn't work before anyway.

[ChrisDenton]

That sounds reasonable to me. And it doesn't rule out doing something differently in the future.

[workingjubilee]

At minimum we need to be issuing a diagnostic here, and we can FCP erroring on top.

[workingjubilee]

Would making usage of #[global_allocator] when linking a dylib which contains an allocator shim a hard error be acceptable? It didn't work before anyway.

@bjorn3 how would we do this?

@Zoxc Would any of this impact your experiments with improving rustc's allocator linkage?

It's not acceptable for Rust language features like #[global_allocator] to simply silently fail without a diagnostic when they are important for the program semantics of the source code.

[bjorn3]

how would we do this?

rust/compiler/rustc_codegen_ssa/src/base.rs

Lines 586 to 601 in 773fb88

	/// Decide allocator kind to codegen. If `Some(_)` this will be the same as
	/// `tcx.allocator_kind`, but it may be `None` in more cases (e.g. if using
	/// allocator definitions from a dylib dependency).
	pub fn allocator_kind_for_codegen(tcx: TyCtxt<'_>) -> Option<AllocatorKind> {
	// If the crate doesn't have an `allocator_kind` set then there's definitely
	// no shim to generate. Otherwise we also check our dependency graph for all
	// our output crate types. If anything there looks like its a `Dynamic`
	// linkage, then it's already got an allocator shim and we'll be using that
	// one instead. If nothing exists then it's our job to generate the
	// allocator!
	let any_dynamic_crate = tcx.dependency_formats(()).iter().any(|(_, list)| {
	use rustc_middle::middle::dependency_format::Linkage;
	list.iter().any(|&linkage| linkage == Linkage::Dynamic)
	});
	if any_dynamic_crate { None } else { tcx.allocator_kind(()) }
	}

is the logic for determining if an allocator shim is necessary or already included from upstream. We could check if the upstream already has an allocator shim and if so error on the global allocator. It seems like that logic is just a bit too eager about declaring any dynamic library to contain the allocator shim even if it doesn't link liballoc. (I guess creating a dynamic library which doesn't link libstd is really uncommon and if it doesn't link libstd depending on both the dylib and libstd is impossible anyway as libstd would be included as dylib too and thus libcore would get included twice.)

[Zoxc]

There should be logic to ensure that the crate graph only has 1 global allocator and give an error. That rule is not broken when using custom allocators in rustc.

[bjorn3]

That logic only checks for explicit global allocators. It doesn't consider the allocator shim that is generated for rust dylibs.

[comex]

For ELF based systems it should be fine, but I'm worried that Mach-O based systems (apple) with two level namespaces cause problems

On Mach-O based systems, weak symbols are exempted from two-level namespacing. Overriding with a strong definition should work as long as:

1. the default definition is marked weak, and
2. the image containing the strong override is linked against the image containing the default definition.
   On the other hand, someone might actually be relying on two-level namespacing, e.g. to have two separate Rust images loaded into a process which each link against a different libstd dylib. Relying on weak binds would break that.

There's also a more 'manual' way this could be done. The image containing the default implementation could also define a function pointer global variable, initialized to point to the default implementation. rustc would change all callers to use the function pointer instead of doing direct calls. And for an image that wanted to override the implementation, rustc would add a static initializer function that overwrites the function pointer. That should work on all platforms. But it's a bit iffy from a security perspective (you lose the benefit of RELRO and related mitigations).