Auto merge of #86844 - bjorn3:global_alloc_improvements, r=pnkfelix

Support #[global_allocator] without the allocator shim

This makes it possible to use liballoc/libstd in combination with `--emit obj` if you use `#[global_allocator]`. This is what rust-for-linux uses right now and systemd may use in the future. Currently they have to depend on the exact implementation of the allocator shim to create one themself as `--emit obj` doesn't create an allocator shim.

Note that currently the allocator shim also defines the oom error handler, which is normally required too. Once `#![feature(default_alloc_error_handler)]` becomes the only option, this can be avoided. In addition when using only fallible allocator methods and either `--cfg no_global_oom_handling` for liballoc (like rust-for-linux) or `--gc-sections` no references to the oom error handler will exist.

To avoid this feature being insta-stable, you will have to define `__rust_no_alloc_shim_is_unstable` to avoid linker errors.

(Labeling this with both T-compiler and T-lang as it originally involved both an implementation detail and had an insta-stable user facing change. As noted above, the `__rust_no_alloc_shim_is_unstable` symbol requirement should prevent unintended dependence on this unstable feature.)

Author: bors

src/machine.rs

@@ -651,6 +651,10 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
 
     /// Sets up the "extern statics" for this machine.
     fn init_extern_statics(this: &mut MiriInterpCx<'mir, 'tcx>) -> InterpResult<'tcx> {
+        // "__rust_no_alloc_shim_is_unstable"
+        let val = ImmTy::from_int(0, this.machine.layouts.u8);
+        Self::alloc_extern_static(this, "__rust_no_alloc_shim_is_unstable", val)?;
+
         match this.tcx.sess.target.os.as_ref() {
             "linux" => {
                 // "environ"

src/shims/foreign_items.rs

@@ -347,7 +347,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     /// Emulates calling the internal __rust_* allocator functions
     fn emulate_allocator(
         &mut self,
-        symbol: Symbol,
         default: impl FnOnce(&mut MiriInterpCx<'mir, 'tcx>) -> InterpResult<'tcx>,
     ) -> InterpResult<'tcx, EmulateByNameResult<'mir, 'tcx>> {
         let this = self.eval_context_mut();
@@ -359,11 +358,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
         match allocator_kind {
             AllocatorKind::Global => {
-                let (body, instance) = this
-                    .lookup_exported_symbol(symbol)?
-                    .expect("symbol should be present if there is a global allocator");
-
-                Ok(EmulateByNameResult::MirBody(body, instance))
+                // When `#[global_allocator]` is used, `__rust_*` is defined by the macro expansion
+                // of this attribute. As such we have to call an exported Rust function,
+                // and not execute any Miri shim. Somewhat unintuitively doing so is done
+                // by returning `NotSupported`, which triggers the `lookup_exported_symbol`
+                // fallback case in `emulate_foreign_item`.
+                return Ok(EmulateByNameResult::NotSupported);
             }
             AllocatorKind::Default => {
                 default(this)?;
@@ -558,11 +558,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
             // Rust allocation
             "__rust_alloc" | "miri_alloc" => {
-                let [size, align] = this.check_shim(abi, Abi::Rust, link_name, args)?;
-                let size = this.read_target_usize(size)?;
-                let align = this.read_target_usize(align)?;
-
                 let default = |this: &mut MiriInterpCx<'mir, 'tcx>| {
+                    // Only call `check_shim` when `#[global_allocator]` isn't used. When that
+                    // macro is used, we act like no shim exists, so that the exported function can run.
+                    let [size, align] = this.check_shim(abi, Abi::Rust, link_name, args)?;
+                    let size = this.read_target_usize(size)?;
+                    let align = this.read_target_usize(align)?;
+
                     Self::check_alloc_request(size, align)?;
 
                     let memory_kind = match link_name.as_str() {
@@ -581,8 +583,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 };
 
                 match link_name.as_str() {
-                    "__rust_alloc" =>
-                        return this.emulate_allocator(Symbol::intern("__rg_alloc"), default),
+                    "__rust_alloc" => return this.emulate_allocator(default),
                     "miri_alloc" => {
                         default(this)?;
                         return Ok(EmulateByNameResult::NeedsJumping);
@@ -591,11 +592,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 }
             }
             "__rust_alloc_zeroed" => {
-                let [size, align] = this.check_shim(abi, Abi::Rust, link_name, args)?;
-                let size = this.read_target_usize(size)?;
-                let align = this.read_target_usize(align)?;
+                return this.emulate_allocator(|this| {
+                    // See the comment for `__rust_alloc` why `check_shim` is only called in the
+                    // default case.
+                    let [size, align] = this.check_shim(abi, Abi::Rust, link_name, args)?;
+                    let size = this.read_target_usize(size)?;
+                    let align = this.read_target_usize(align)?;
 
-                return this.emulate_allocator(Symbol::intern("__rg_alloc_zeroed"), |this| {
                     Self::check_alloc_request(size, align)?;
 
                     let ptr = this.allocate_ptr(
@@ -614,12 +617,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 });
             }
             "__rust_dealloc" | "miri_dealloc" => {
-                let [ptr, old_size, align] = this.check_shim(abi, Abi::Rust, link_name, args)?;
-                let ptr = this.read_pointer(ptr)?;
-                let old_size = this.read_target_usize(old_size)?;
-                let align = this.read_target_usize(align)?;
-
                 let default = |this: &mut MiriInterpCx<'mir, 'tcx>| {
+                    // See the comment for `__rust_alloc` why `check_shim` is only called in the
+                    // default case.
+                    let [ptr, old_size, align] =
+                        this.check_shim(abi, Abi::Rust, link_name, args)?;
+                    let ptr = this.read_pointer(ptr)?;
+                    let old_size = this.read_target_usize(old_size)?;
+                    let align = this.read_target_usize(align)?;
+
                     let memory_kind = match link_name.as_str() {
                         "__rust_dealloc" => MiriMemoryKind::Rust,
                         "miri_dealloc" => MiriMemoryKind::Miri,
@@ -635,8 +641,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 };
 
                 match link_name.as_str() {
-                    "__rust_dealloc" =>
-                        return this.emulate_allocator(Symbol::intern("__rg_dealloc"), default),
+                    "__rust_dealloc" => {
+                        return this.emulate_allocator(default);
+                    }
                     "miri_dealloc" => {
                         default(this)?;
                         return Ok(EmulateByNameResult::NeedsJumping);
@@ -645,15 +652,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 }
             }
             "__rust_realloc" => {
-                let [ptr, old_size, align, new_size] =
-                    this.check_shim(abi, Abi::Rust, link_name, args)?;
-                let ptr = this.read_pointer(ptr)?;
-                let old_size = this.read_target_usize(old_size)?;
-                let align = this.read_target_usize(align)?;
-                let new_size = this.read_target_usize(new_size)?;
-                // No need to check old_size; we anyway check that they match the allocation.
+                return this.emulate_allocator(|this| {
+                    // See the comment for `__rust_alloc` why `check_shim` is only called in the
+                    // default case.
+                    let [ptr, old_size, align, new_size] =
+                        this.check_shim(abi, Abi::Rust, link_name, args)?;
+                    let ptr = this.read_pointer(ptr)?;
+                    let old_size = this.read_target_usize(old_size)?;
+                    let align = this.read_target_usize(align)?;
+                    let new_size = this.read_target_usize(new_size)?;
+                    // No need to check old_size; we anyway check that they match the allocation.
 
-                return this.emulate_allocator(Symbol::intern("__rg_realloc"), |this| {
                     Self::check_alloc_request(new_size, align)?;
 
                     let align = Align::from_bytes(align).unwrap();

tests/fail/memleak.stderr

@@ -1,8 +1,8 @@
 error: memory leaked: ALLOC (Rust heap, size: 4, align: 4), allocated here:
   --> RUSTLIB/alloc/src/alloc.rs:LL:CC
    |
-LL |     unsafe { __rust_alloc(layout.size(), layout.align()) }
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |         __rust_alloc(layout.size(), layout.align())
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: inside `std::alloc::alloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
    = note: inside `std::alloc::Global::alloc_impl` at RUSTLIB/alloc/src/alloc.rs:LL:CC

tests/fail/memleak_rc.32bit.stderr

@@ -1,8 +1,8 @@
 error: memory leaked: ALLOC (Rust heap, size: 16, align: 4), allocated here:
   --> RUSTLIB/alloc/src/alloc.rs:LL:CC
    |
-LL |     unsafe { __rust_alloc(layout.size(), layout.align()) }
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |         __rust_alloc(layout.size(), layout.align())
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: inside `std::alloc::alloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
    = note: inside `std::alloc::Global::alloc_impl` at RUSTLIB/alloc/src/alloc.rs:LL:CC

tests/fail/memleak_rc.64bit.stderr

@@ -1,8 +1,8 @@
 error: memory leaked: ALLOC (Rust heap, size: 32, align: 8), allocated here:
   --> RUSTLIB/alloc/src/alloc.rs:LL:CC
    |
-LL |     unsafe { __rust_alloc(layout.size(), layout.align()) }
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |         __rust_alloc(layout.size(), layout.align())
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: inside `std::alloc::alloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
    = note: inside `std::alloc::Global::alloc_impl` at RUSTLIB/alloc/src/alloc.rs:LL:CC

tests/pass/shims/fs.rs

@@ -31,7 +31,7 @@ fn main() {
 }
 
 fn host_to_target_path(path: String) -> PathBuf {
-    use std::ffi::{CStr, CString};
+    use std::ffi::{c_char, CStr, CString};
 
     let path = CString::new(path).unwrap();
     let mut out = Vec::with_capacity(1024);