Linker failure when global allocator overridden in lib.rs

[mweber15]

I have a project that overrides the global allocator in lib.rs, and recently noticed that I can no longer link the test binary for my project with recent beta/nightly compilers. cargo bisect-rustc points to a2b1646 which certainly looks relevant. I reproduced the error using a slightly modified example from the global allocator documentation: https://doc.rust-lang.org/stable/std/alloc/trait.GlobalAlloc.html#example

If I move the allocator definition from lib.rs to main.rs, I no longer have a problem. Is that the right thing to do? If so, do I need to do anything else to make sure my tests use the desired allocator?

Code

I tried this code:

// main.rs
fn main() {
    testalloc::f();
}

// lib.rs
use std::alloc::{GlobalAlloc, Layout};
use std::cell::UnsafeCell;
use std::ptr::null_mut;
use std::sync::atomic::{
    AtomicUsize,
    Ordering::{Acquire, SeqCst},
};

const ARENA_SIZE: usize = 128 * 1024;
const MAX_SUPPORTED_ALIGN: usize = 4096;
#[repr(C, align(4096))] // 4096 == MAX_SUPPORTED_ALIGN
struct SimpleAllocator {
    arena: UnsafeCell<[u8; ARENA_SIZE]>,
    remaining: AtomicUsize, // we allocate from the top, counting down
}

pub const NO_ALLOC_SHIM_IS_UNSTABLE: &str = "__rust_no_alloc_shim_is_unstable";
#[global_allocator]
static ALLOCATOR: SimpleAllocator = SimpleAllocator {
    arena: UnsafeCell::new([0x55; ARENA_SIZE]),
    remaining: AtomicUsize::new(ARENA_SIZE),
};

unsafe impl Sync for SimpleAllocator {}

unsafe impl GlobalAlloc for SimpleAllocator {
    unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
        let size = layout.size();
        let align = layout.align();

        // `Layout` contract forbids making a `Layout` with align=0, or align not power of 2.
        // So we can safely use a mask to ensure alignment without worrying about UB.
        let align_mask_to_round_down = !(align - 1);

        if align > MAX_SUPPORTED_ALIGN {
            return null_mut();
        }

        let mut allocated = 0;
        if self
            .remaining
            .fetch_update(SeqCst, SeqCst, |mut remaining| {
                if size > remaining {
                    return None;
                }
                remaining -= size;
                remaining &= align_mask_to_round_down;
                allocated = remaining;
                Some(remaining)
            })
            .is_err()
        {
            return null_mut();
        };
        self.arena.get().cast::<u8>().add(allocated)
    }
    unsafe fn dealloc(&self, _ptr: *mut u8, _layout: Layout) {}
}

pub fn f() {
    let _s = format!("allocating a string!");
    let currently = ALLOCATOR.remaining.load(Acquire);
    println!("allocated so far: {}", ARENA_SIZE - currently);
}

#[cfg(test)]
mod test {}

I expected to see this happen:

$ cargo t
   Compiling testalloc v0.1.0 (/home/matt/src/testalloc)
    Finished test [unoptimized + debuginfo] target(s) in 0.36s

Instead, this happened:

$ cargo t
   Compiling testalloc v0.1.0 (/home/matt/src/testalloc)
error: linking with `cc` failed: exit status: 1

Version it worked on

searched nightlies: from nightly-2023-05-25 to nightly-2023-06-16
regressed nightly: nightly-2023-05-26
searched commit range: c373194...a2b1646
regressed commit: a2b1646

bisected with cargo-bisect-rustc v0.6.6

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc -- test

@rustbot modify labels: +regression-from-stable-to-beta -regression-untriaged

[mweber15]

Adding the full linker error output here in case it's of interest.
linker.txt

[mweber15]

I tried adding

pub const NO_ALLOC_SHIM_IS_UNSTABLE: &str = "__rust_no_alloc_shim_is_unstable";

based on a brief look at the change that cargo bisect-rustc pointed to. It doesn't seem to make any difference, but that's why it's in my repro above.

[bjorn3]

I can reproduce this. __rust_alloc and such are properly defined in the library rlib. It seems to be caused by the library order. Traditional linkers require symbols to have their uses in object files before their definition. So for circular dependencies --start-group and --end-group has to be used or libraries have to be mentioned multiple times. And indeed lld which doesn't have this restriction is entirely fine with this.

I think registering a circular dependency between the defining crate of #[global_allocator] and liballoc (and every crate that depends on it) at https://github.com/belovdv/rustc/blob/cbc064b341be231403d181402a786cce7f1c73f1/compiler/rustc_codegen_ssa/src/base.rs#L886-L919 would work.

Edit: Reading the code it references closer, I think adding __rust_{alloc,dealloc,realloc,alloc_zeroed} to linked_symbols would be enough to fix this.

I can't work on this tomorrow, but unless someone else picks it up, I will work on this.

[bjorn3]

I can't work on this tomorrow, but unless someone else picks it up, I will work on this.

Finished some work early. #112794 fixes the issue.

[mweber15]

@bjorn3, thank you for the quick fix!

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high