Rollup merge of #143387 - dpaoliello:shouldpanicfn, r=bjorn3

Make __rust_alloc_error_handler_should_panic a function

Fixes #143253

`__rust_alloc_error_handler_should_panic` is a static but was being exported as a function.

For most targets this doesn't matter, but Arm64EC Windows uses different decorations for exported variables vs functions, hence it fails to link when `-Z oom=abort` is enabled.

We've had issues in the past with statics like this (see #141061) but the tldr; is that Arm64EC needs symbols correctly exported as either a function or data, and data MUST and MUST ONLY be marked `dllimport` when the symbol is being imported from another binary, which is non-trivial to calculate for these compiler-generated statics.

So, instead, the easiest thing to do is to make `__rust_alloc_error_handler_should_panic` a function instead.

Since `__rust_alloc_error_handler_should_panic` isn't involved in any linking shenanigans, I've marked it as `AlwaysInline` with the hopes that the various backends will see that it is just returning a constant and perform the same optimizations as the previous implementation.

r? `@bjorn3`

Author: matthiaskrgr

compiler/rustc_codegen_cranelift/src/allocator.rs

@@ -84,19 +84,34 @@ fn codegen_inner(
         &mangle_internal_symbol(tcx, alloc_error_handler_name(alloc_error_handler_kind)),
     );
 
-    let data_id = module
-        .declare_data(
-            &mangle_internal_symbol(tcx, OomStrategy::SYMBOL),
-            Linkage::Export,
-            false,
-            false,
-        )
-        .unwrap();
-    let mut data = DataDescription::new();
-    data.set_align(1);
-    let val = oom_strategy.should_panic();
-    data.define(Box::new([val]));
-    module.define_data(data_id, &data).unwrap();
+    {
+        let sig = Signature {
+            call_conv: module.target_config().default_call_conv,
+            params: vec![],
+            returns: vec![AbiParam::new(types::I8)],
+        };
+        let func_id = module
+            .declare_function(
+                &mangle_internal_symbol(tcx, OomStrategy::SYMBOL),
+                Linkage::Export,
+                &sig,
+            )
+            .unwrap();
+        let mut ctx = Context::new();
+        ctx.func.signature = sig;
+        {
+            let mut func_ctx = FunctionBuilderContext::new();
+            let mut bcx = FunctionBuilder::new(&mut ctx.func, &mut func_ctx);
+
+            let block = bcx.create_block();
+            bcx.switch_to_block(block);
+            let value = bcx.ins().iconst(types::I8, oom_strategy.should_panic() as i64);
+            bcx.ins().return_(&[value]);
+            bcx.seal_all_blocks();
+            bcx.finalize();
+        }
+        module.define_function(func_id, &mut ctx).unwrap();
+    }
 
     {
         let sig = Signature {

compiler/rustc_codegen_gcc/src/allocator.rs

@@ -1,6 +1,6 @@
-use gccjit::{Context, FunctionType, GlobalKind, ToRValue, Type};
 #[cfg(feature = "master")]
-use gccjit::{FnAttribute, VarAttribute};
+use gccjit::FnAttribute;
+use gccjit::{Context, FunctionType, RValue, ToRValue, Type};
 use rustc_ast::expand::allocator::{
     ALLOCATOR_METHODS, AllocatorKind, AllocatorTy, NO_ALLOC_SHIM_IS_UNSTABLE,
     alloc_error_handler_name, default_fn_name, global_fn_name,
@@ -71,15 +71,13 @@ pub(crate) unsafe fn codegen(
         None,
     );
 
-    let name = mangle_internal_symbol(tcx, OomStrategy::SYMBOL);
-    let global = context.new_global(None, GlobalKind::Exported, i8, name);
-    #[cfg(feature = "master")]
-    global.add_attribute(VarAttribute::Visibility(symbol_visibility_to_gcc(
-        tcx.sess.default_visibility(),
-    )));
-    let value = tcx.sess.opts.unstable_opts.oom.should_panic();
-    let value = context.new_rvalue_from_int(i8, value as i32);
-    global.global_set_initializer_rvalue(value);
+    create_const_value_function(
+        tcx,
+        context,
+        &mangle_internal_symbol(tcx, OomStrategy::SYMBOL),
+        i8,
+        context.new_rvalue_from_int(i8, tcx.sess.opts.unstable_opts.oom.should_panic() as i32),
+    );
 
     create_wrapper_function(
         tcx,
@@ -91,6 +89,30 @@ pub(crate) unsafe fn codegen(
     );
 }
 
+fn create_const_value_function(
+    tcx: TyCtxt<'_>,
+    context: &Context<'_>,
+    name: &str,
+    output: Type<'_>,
+    value: RValue<'_>,
+) {
+    let func = context.new_function(None, FunctionType::Exported, output, &[], name, false);
+
+    #[cfg(feature = "master")]
+    func.add_attribute(FnAttribute::Visibility(symbol_visibility_to_gcc(
+        tcx.sess.default_visibility(),
+    )));
+
+    func.add_attribute(FnAttribute::AlwaysInline);
+
+    if tcx.sess.must_emit_unwind_tables() {
+        // TODO(antoyo): emit unwind tables.
+    }
+
+    let block = func.new_block("entry");
+    block.end_with_return(None, value);
+}
+
 fn create_wrapper_function(
     tcx: TyCtxt<'_>,
     context: &Context<'_>,

compiler/rustc_codegen_llvm/src/allocator.rs

@@ -11,7 +11,7 @@ use rustc_symbol_mangling::mangle_internal_symbol;
 
 use crate::builder::SBuilder;
 use crate::declare::declare_simple_fn;
-use crate::llvm::{self, False, True, Type};
+use crate::llvm::{self, False, True, Type, Value};
 use crate::{SimpleCx, attributes, debuginfo};
 
 pub(crate) unsafe fn codegen(
@@ -73,13 +73,14 @@ pub(crate) unsafe fn codegen(
     );
 
     unsafe {
-        // __rust_alloc_error_handler_should_panic
-        let name = mangle_internal_symbol(tcx, OomStrategy::SYMBOL);
-        let ll_g = cx.declare_global(&name, i8);
-        llvm::set_visibility(ll_g, llvm::Visibility::from_generic(tcx.sess.default_visibility()));
-        let val = tcx.sess.opts.unstable_opts.oom.should_panic();
-        let llval = llvm::LLVMConstInt(i8, val as u64, False);
-        llvm::set_initializer(ll_g, llval);
+        // __rust_alloc_error_handler_should_panic_v2
+        create_const_value_function(
+            tcx,
+            &cx,
+            &mangle_internal_symbol(tcx, OomStrategy::SYMBOL),
+            &i8,
+            &llvm::LLVMConstInt(i8, tcx.sess.opts.unstable_opts.oom.should_panic() as u64, False),
+        );
 
         // __rust_no_alloc_shim_is_unstable_v2
         create_wrapper_function(
@@ -100,6 +101,34 @@ pub(crate) unsafe fn codegen(
     }
 }
 
+fn create_const_value_function(
+    tcx: TyCtxt<'_>,
+    cx: &SimpleCx<'_>,
+    name: &str,
+    output: &Type,
+    value: &Value,
+) {
+    let ty = cx.type_func(&[], output);
+    let llfn = declare_simple_fn(
+        &cx,
+        name,
+        llvm::CallConv::CCallConv,
+        llvm::UnnamedAddr::Global,
+        llvm::Visibility::from_generic(tcx.sess.default_visibility()),
+        ty,
+    );
+
+    attributes::apply_to_llfn(
+        llfn,
+        llvm::AttributePlace::Function,
+        &[llvm::AttributeKind::AlwaysInline.create_attr(cx.llcx)],
+    );
+
+    let llbb = unsafe { llvm::LLVMAppendBasicBlockInContext(cx.llcx, llfn, c"entry".as_ptr()) };
+    let mut bx = SBuilder::build(&cx, llbb);
+    bx.ret(value);
+}
+
 fn create_wrapper_function(
     tcx: TyCtxt<'_>,
     cx: &SimpleCx<'_>,

compiler/rustc_session/src/config.rs

@@ -3340,7 +3340,7 @@ pub enum OomStrategy {
 }
 
 impl OomStrategy {
-    pub const SYMBOL: &'static str = "__rust_alloc_error_handler_should_panic";
+    pub const SYMBOL: &'static str = "__rust_alloc_error_handler_should_panic_v2";
 
     pub fn should_panic(self) -> u8 {
         match self {

library/alloc/src/alloc.rs

@@ -429,10 +429,10 @@ pub mod __alloc_error_handler {
             // This symbol is emitted by rustc next to __rust_alloc_error_handler.
             // Its value depends on the -Zoom={panic,abort} compiler option.
             #[rustc_std_internal_symbol]
-            static __rust_alloc_error_handler_should_panic: u8;
+            fn __rust_alloc_error_handler_should_panic_v2() -> u8;
         }
 
-        if unsafe { __rust_alloc_error_handler_should_panic != 0 } {
+        if unsafe { __rust_alloc_error_handler_should_panic_v2() != 0 } {
             panic!("memory allocation of {size} bytes failed")
         } else {
             core::panicking::panic_nounwind_fmt(

library/std/src/alloc.rs

@@ -349,10 +349,10 @@ fn default_alloc_error_hook(layout: Layout) {
         // This symbol is emitted by rustc next to __rust_alloc_error_handler.
         // Its value depends on the -Zoom={panic,abort} compiler option.
         #[rustc_std_internal_symbol]
-        static __rust_alloc_error_handler_should_panic: u8;
+        fn __rust_alloc_error_handler_should_panic_v2() -> u8;
     }
 
-    if unsafe { __rust_alloc_error_handler_should_panic != 0 } {
+    if unsafe { __rust_alloc_error_handler_should_panic_v2() != 0 } {
         panic!("memory allocation of {} bytes failed", layout.size());
     } else {
         // This is the default path taken on OOM, and the only path taken on stable with std.

src/tools/miri/src/shims/extern_static.rs

@@ -1,7 +1,5 @@
 //! Provides the `extern static` that this platform expects.
 
-use rustc_symbol_mangling::mangle_internal_symbol;
-
 use crate::*;
 
 impl<'tcx> MiriMachine<'tcx> {
@@ -45,15 +43,6 @@ impl<'tcx> MiriMachine<'tcx> {
 
     /// Sets up the "extern statics" for this machine.
     pub fn init_extern_statics(ecx: &mut MiriInterpCx<'tcx>) -> InterpResult<'tcx> {
-        // "__rust_alloc_error_handler_should_panic"
-        let val = ecx.tcx.sess.opts.unstable_opts.oom.should_panic();
-        let val = ImmTy::from_int(val, ecx.machine.layouts.u8);
-        Self::alloc_extern_static(
-            ecx,
-            &mangle_internal_symbol(*ecx.tcx, "__rust_alloc_error_handler_should_panic"),
-            val,
-        )?;
-
         if ecx.target_os_is_unix() {
             // "environ" is mandated by POSIX.
             let environ = ecx.machine.env_vars.unix().environ();

src/tools/miri/src/shims/foreign_items.rs

@@ -615,6 +615,12 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 // This is a no-op shim that only exists to prevent making the allocator shims instantly stable.
                 let [] = this.check_shim(abi, CanonAbi::Rust, link_name, args)?;
             }
+            name if name == this.mangle_internal_symbol("__rust_alloc_error_handler_should_panic_v2") => {
+                // Gets the value of the `oom` option.
+                let [] = this.check_shim(abi, CanonAbi::Rust, link_name, args)?;
+                let val = this.tcx.sess.opts.unstable_opts.oom.should_panic();
+                this.write_int(val, dest)?;
+            }
 
             // C memory handling functions
             "memcmp" => {

src/tools/miri/tests/pass/alloc-access-tracking.rs

@@ -1,7 +1,7 @@
 #![no_std]
 #![no_main]
-//@compile-flags: -Zmiri-track-alloc-id=19 -Zmiri-track-alloc-accesses -Cpanic=abort
-//@normalize-stderr-test: "id 19" -> "id $$ALLOC"
+//@compile-flags: -Zmiri-track-alloc-id=18 -Zmiri-track-alloc-accesses -Cpanic=abort
+//@normalize-stderr-test: "id 18" -> "id $$ALLOC"
 //@only-target: linux # alloc IDs differ between OSes (due to extern static allocations)
 
 extern "Rust" {