Rollup merge of #147725 - bjorn3:remove_oom_panic, r=Amanieu

Remove -Zoom=panic

There are major questions remaining about the reentrancy that this allows. It doesn't have any users on github outside of a single project that uses it in a panic=abort project to show backtraces. It can still be emulated through `#[alloc_error_handler]` or `set_alloc_error_hook` depending on if you use the standard library or not. And finally it makes it harder to do various improvements to the allocator shim.

With this PR the sole remaining symbol in the allocator shim that is not effectively emulating weak symbols is the symbol that prevents skipping the allocator shim on stable even when it would otherwise be empty because libstd + `#[global_allocator]` is used.

Closes https://github.com/rust-lang/rust/issues/43596
Fixes https://github.com/rust-lang/rust/issues/126683

compiler/rustc_codegen_cranelift/src/allocator.rs

@@ -6,7 +6,6 @@ use rustc_ast::expand::allocator::{
     AllocatorMethod, AllocatorTy, NO_ALLOC_SHIM_IS_UNSTABLE, default_fn_name, global_fn_name,
 };
 use rustc_codegen_ssa::base::{allocator_kind_for_codegen, allocator_shim_contents};
-use rustc_session::config::OomStrategy;
 use rustc_symbol_mangling::mangle_internal_symbol;
 
 use crate::prelude::*;
@@ -15,16 +14,11 @@ use crate::prelude::*;
 pub(crate) fn codegen(tcx: TyCtxt<'_>, module: &mut dyn Module) -> bool {
     let Some(kind) = allocator_kind_for_codegen(tcx) else { return false };
     let methods = allocator_shim_contents(tcx, kind);
-    codegen_inner(tcx, module, &methods, tcx.sess.opts.unstable_opts.oom);
+    codegen_inner(tcx, module, &methods);
     true
 }
 
-fn codegen_inner(
-    tcx: TyCtxt<'_>,
-    module: &mut dyn Module,
-    methods: &[AllocatorMethod],
-    oom_strategy: OomStrategy,
-) {
+fn codegen_inner(tcx: TyCtxt<'_>, module: &mut dyn Module, methods: &[AllocatorMethod]) {
     let usize_ty = module.target_config().pointer_type();
 
     for method in methods {
@@ -65,35 +59,6 @@ fn codegen_inner(
         );
     }
 
-    {
-        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 {
             call_conv: module.target_config().default_call_conv,

compiler/rustc_codegen_gcc/src/allocator.rs

@@ -1,12 +1,11 @@
 #[cfg(feature = "master")]
 use gccjit::FnAttribute;
-use gccjit::{Context, FunctionType, RValue, ToRValue, Type};
+use gccjit::{Context, FunctionType, ToRValue, Type};
 use rustc_ast::expand::allocator::{
     AllocatorMethod, AllocatorTy, NO_ALLOC_SHIM_IS_UNSTABLE, default_fn_name, global_fn_name,
 };
 use rustc_middle::bug;
 use rustc_middle::ty::TyCtxt;
-use rustc_session::config::OomStrategy;
 use rustc_symbol_mangling::mangle_internal_symbol;
 
 use crate::GccContext;
@@ -59,14 +58,6 @@ pub(crate) unsafe fn codegen(
         create_wrapper_function(tcx, context, &from_name, Some(&to_name), &types, output);
     }
 
-    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,
         context,
@@ -77,34 +68,6 @@ 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(),
-        )));
-
-        // FIXME(antoyo): cg_llvm sets AlwaysInline, but AlwaysInline is different in GCC and using
-        // it here will causes linking errors when using LTO.
-        func.add_attribute(FnAttribute::Inline);
-    }
-
-    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

@@ -7,13 +7,13 @@ use rustc_codegen_ssa::traits::BaseTypeCodegenMethods as _;
 use rustc_middle::bug;
 use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
 use rustc_middle::ty::TyCtxt;
-use rustc_session::config::{DebugInfo, OomStrategy};
+use rustc_session::config::DebugInfo;
 use rustc_symbol_mangling::mangle_internal_symbol;
 
 use crate::attributes::llfn_attrs_from_instance;
 use crate::builder::SBuilder;
 use crate::declare::declare_simple_fn;
-use crate::llvm::{self, FALSE, FromGeneric, TRUE, Type, Value};
+use crate::llvm::{self, FromGeneric, TRUE, Type};
 use crate::{SimpleCx, attributes, debuginfo};
 
 pub(crate) unsafe fn codegen(
@@ -28,7 +28,6 @@ pub(crate) unsafe fn codegen(
         64 => cx.type_i64(),
         tws => bug!("Unsupported target word size for int: {}", tws),
     };
-    let i8 = cx.type_i8();
     let i8p = cx.type_ptr();
 
     for method in methods {
@@ -87,17 +86,6 @@ pub(crate) unsafe fn codegen(
         );
     }
 
-    // __rust_alloc_error_handler_should_panic_v2
-    create_const_value_function(
-        tcx,
-        &cx,
-        &mangle_internal_symbol(tcx, OomStrategy::SYMBOL),
-        &i8,
-        unsafe {
-            llvm::LLVMConstInt(i8, tcx.sess.opts.unstable_opts.oom.should_panic() as u64, FALSE)
-        },
-    );
-
     // __rust_no_alloc_shim_is_unstable_v2
     create_wrapper_function(
         tcx,
@@ -117,34 +105,6 @@ 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_codegen_ssa/src/back/symbol_export.rs

@@ -15,7 +15,7 @@ use rustc_middle::middle::exported_symbols::{
 use rustc_middle::query::LocalCrate;
 use rustc_middle::ty::{self, GenericArgKind, GenericArgsRef, Instance, SymbolName, Ty, TyCtxt};
 use rustc_middle::util::Providers;
-use rustc_session::config::{CrateType, OomStrategy};
+use rustc_session::config::CrateType;
 use rustc_symbol_mangling::mangle_internal_symbol;
 use rustc_target::spec::{Arch, Os, TlsModel};
 use tracing::debug;
@@ -493,7 +493,6 @@ pub(crate) fn allocator_shim_symbols(
         .map(move |method| mangle_internal_symbol(tcx, global_fn_name(method.name).as_str()))
         .chain([
             mangle_internal_symbol(tcx, global_fn_name(ALLOC_ERROR_HANDLER).as_str()),
-            mangle_internal_symbol(tcx, OomStrategy::SYMBOL),
             mangle_internal_symbol(tcx, NO_ALLOC_SHIM_IS_UNSTABLE),
         ])
         .map(move |symbol_name| {

compiler/rustc_interface/src/tests.rs

@@ -14,10 +14,10 @@ use rustc_session::config::{
     CoverageOptions, DebugInfo, DumpMonoStatsFormat, ErrorOutputType, ExternEntry, ExternLocation,
     Externs, FmtDebug, FunctionReturn, InliningThreshold, Input, InstrumentCoverage,
     InstrumentXRay, LinkSelfContained, LinkerPluginLto, LocationDetail, LtoCli, MirIncludeSpans,
-    NextSolverConfig, Offload, OomStrategy, Options, OutFileName, OutputType, OutputTypes,
-    PAuthKey, PacRet, Passes, PatchableFunctionEntry, Polonius, ProcMacroExecutionStrategy, Strip,
-    SwitchWithOptPath, SymbolManglingVersion, WasiExecModel, build_configuration,
-    build_session_options, rustc_optgroups,
+    NextSolverConfig, Offload, Options, OutFileName, OutputType, OutputTypes, PAuthKey, PacRet,
+    Passes, PatchableFunctionEntry, Polonius, ProcMacroExecutionStrategy, Strip, SwitchWithOptPath,
+    SymbolManglingVersion, WasiExecModel, build_configuration, build_session_options,
+    rustc_optgroups,
 };
 use rustc_session::lint::Level;
 use rustc_session::search_paths::SearchPath;
@@ -839,7 +839,6 @@ fn test_unstable_options_tracking_hash() {
     tracked!(no_unique_section_names, true);
     tracked!(offload, vec![Offload::Enable]);
     tracked!(on_broken_pipe, OnBrokenPipe::Kill);
-    tracked!(oom, OomStrategy::Panic);
     tracked!(osx_rpath_install_name, true);
     tracked!(packed_bundled_libs, true);
     tracked!(panic_abort_tests, true);

compiler/rustc_session/src/config.rs

@@ -3127,8 +3127,8 @@ pub(crate) mod dep_tracking {
         AnnotateMoves, AutoDiff, BranchProtection, CFGuard, CFProtection, CollapseMacroDebuginfo,
         CoverageOptions, CrateType, DebugInfo, DebugInfoCompression, ErrorOutputType, FmtDebug,
         FunctionReturn, InliningThreshold, InstrumentCoverage, InstrumentXRay, LinkerPluginLto,
-        LocationDetail, LtoCli, MirStripDebugInfo, NextSolverConfig, Offload, OomStrategy,
-        OptLevel, OutFileName, OutputType, OutputTypes, PatchableFunctionEntry, Polonius,
+        LocationDetail, LtoCli, MirStripDebugInfo, NextSolverConfig, Offload, OptLevel,
+        OutFileName, OutputType, OutputTypes, PatchableFunctionEntry, Polonius,
         RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm, SplitDwarfKind,
         SwitchWithOptPath, SymbolManglingVersion, WasiExecModel,
     };
@@ -3227,7 +3227,6 @@ pub(crate) mod dep_tracking {
         LocationDetail,
         FmtDebug,
         BranchProtection,
-        OomStrategy,
         LanguageIdentifier,
         NextSolverConfig,
         PatchableFunctionEntry,
@@ -3340,27 +3339,6 @@ pub(crate) mod dep_tracking {
     }
 }
 
-/// Default behavior to use in out-of-memory situations.
-#[derive(Clone, Copy, PartialEq, Hash, Debug, Encodable, Decodable, HashStable_Generic)]
-pub enum OomStrategy {
-    /// Generate a panic that can be caught by `catch_unwind`.
-    Panic,
-
-    /// Abort the process immediately.
-    Abort,
-}
-
-impl OomStrategy {
-    pub const SYMBOL: &'static str = "__rust_alloc_error_handler_should_panic_v2";
-
-    pub fn should_panic(self) -> u8 {
-        match self {
-            OomStrategy::Panic => 1,
-            OomStrategy::Abort => 0,
-        }
-    }
-}
-
 /// How to run proc-macro code when building this crate
 #[derive(Clone, Copy, PartialEq, Hash, Debug)]
 pub enum ProcMacroExecutionStrategy {

compiler/rustc_session/src/options.rs

@@ -806,7 +806,6 @@ mod desc {
     pub(crate) const parse_on_broken_pipe: &str = "either `kill`, `error`, or `inherit`";
     pub(crate) const parse_patchable_function_entry: &str = "either two comma separated integers (total_nops,prefix_nops), with prefix_nops <= total_nops, or one integer (total_nops)";
     pub(crate) const parse_opt_panic_strategy: &str = parse_panic_strategy;
-    pub(crate) const parse_oom_strategy: &str = "either `panic` or `abort`";
     pub(crate) const parse_relro_level: &str = "one of: `full`, `partial`, or `off`";
     pub(crate) const parse_sanitizers: &str = "comma separated list of sanitizers: `address`, `cfi`, `dataflow`, `hwaddress`, `kcfi`, `kernel-address`, `leak`, `memory`, `memtag`, `safestack`, `shadow-call-stack`, `thread`, or 'realtime'";
     pub(crate) const parse_sanitizer_memory_track_origins: &str = "0, 1, or 2";
@@ -1242,15 +1241,6 @@ pub mod parse {
         false
     }
 
-    pub(crate) fn parse_oom_strategy(slot: &mut OomStrategy, v: Option<&str>) -> bool {
-        match v {
-            Some("panic") => *slot = OomStrategy::Panic,
-            Some("abort") => *slot = OomStrategy::Abort,
-            _ => return false,
-        }
-        true
-    }
-
     pub(crate) fn parse_relro_level(slot: &mut Option<RelroLevel>, v: Option<&str>) -> bool {
         match v {
             Some(s) => match s.parse::<RelroLevel>() {
@@ -2529,8 +2519,6 @@ options! {
         Currently the only option available"),
     on_broken_pipe: OnBrokenPipe = (OnBrokenPipe::Default, parse_on_broken_pipe, [TRACKED],
         "behavior of std::io::ErrorKind::BrokenPipe (SIGPIPE)"),
-    oom: OomStrategy = (OomStrategy::Abort, parse_oom_strategy, [TRACKED],
-        "panic strategy for out-of-memory handling"),
     osx_rpath_install_name: bool = (false, parse_bool, [TRACKED],
         "pass `-install_name @rpath/...` to the macOS linker (default: no)"),
     packed_bundled_libs: bool = (false, parse_bool, [TRACKED],

library/alloc/src/alloc.rs

@@ -426,20 +426,9 @@ pub mod __alloc_error_handler {
     // `#[alloc_error_handler]`.
     #[rustc_std_internal_symbol]
     pub unsafe fn __rdl_alloc_error_handler(size: usize, _align: usize) -> ! {
-        unsafe extern "Rust" {
-            // 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]
-            fn __rust_alloc_error_handler_should_panic_v2() -> u8;
-        }
-
-        if unsafe { __rust_alloc_error_handler_should_panic_v2() != 0 } {
-            panic!("memory allocation of {size} bytes failed")
-        } else {
-            core::panicking::panic_nounwind_fmt(
-                format_args!("memory allocation of {size} bytes failed"),
-                /* force_no_backtrace */ false,
-            )
-        }
+        core::panicking::panic_nounwind_fmt(
+            format_args!("memory allocation of {size} bytes failed"),
+            /* force_no_backtrace */ false,
+        )
     }
 }

library/std/src/alloc.rs

@@ -357,17 +357,6 @@ fn default_alloc_error_hook(layout: Layout) {
         return;
     }
 
-    unsafe extern "Rust" {
-        // 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]
-        fn __rust_alloc_error_handler_should_panic_v2() -> u8;
-    }
-
-    if unsafe { __rust_alloc_error_handler_should_panic_v2() != 0 } {
-        panic!("memory allocation of {} bytes failed", layout.size());
-    }
-
     // This is the default path taken on OOM, and the only path taken on stable with std.
     // Crucially, it does *not* call any user-defined code, and therefore users do not have to
     // worry about allocation failure causing reentrancy issues. That makes it different from

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

@@ -12,7 +12,6 @@ use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
 use rustc_middle::mir::interpret::AllocInit;
 use rustc_middle::ty::{Instance, Ty};
 use rustc_middle::{mir, ty};
-use rustc_session::config::OomStrategy;
 use rustc_span::Symbol;
 use rustc_target::callconv::FnAbi;
 use rustc_target::spec::{Arch, Os};
@@ -305,18 +304,12 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
         // Here we dispatch all the shims for foreign functions. If you have a platform specific
         // shim, add it to the corresponding submodule.
         match link_name.as_str() {
-            // Magic functions Rust emits (and not as part of the allocator shim).
+            // Magic function Rust emits (and not as part of the allocator shim).
             name if name == this.mangle_internal_symbol(NO_ALLOC_SHIM_IS_UNSTABLE) => {
                 // This is a no-op shim that only exists to prevent making the allocator shims
                 // instantly stable.
                 let [] = this.check_shim_sig_lenient(abi, CanonAbi::Rust, link_name, args)?;
             }
-            name if name == this.mangle_internal_symbol(OomStrategy::SYMBOL) => {
-                // Gets the value of the `oom` option.
-                let [] = this.check_shim_sig_lenient(abi, CanonAbi::Rust, link_name, args)?;
-                let val = this.tcx.sess.opts.unstable_opts.oom.should_panic();
-                this.write_int(val, dest)?;
-            }
 
             // Miri-specific extern functions
             "miri_alloc" => {

src/tools/miri/tests/panic/alloc_error_handler_panic.rs

@@ -1,18 +0,0 @@
-//@compile-flags: -Zoom=panic
-#![feature(allocator_api)]
-
-use std::alloc::*;
-
-struct Bomb;
-impl Drop for Bomb {
-    fn drop(&mut self) {
-        eprintln!("yes we are unwinding!");
-    }
-}
-
-#[allow(unreachable_code, unused_variables)]
-fn main() {
-    let bomb = Bomb;
-    handle_alloc_error(Layout::for_value(&0));
-    std::mem::forget(bomb); // defuse unwinding bomb
-}

src/tools/miri/tests/panic/alloc_error_handler_panic.stderr

@@ -1,6 +0,0 @@
-
-thread 'main' ($TID) panicked at RUSTLIB/std/src/alloc.rs:LL:CC:
-memory allocation of 4 bytes failed
-note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
-note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect
-yes we are unwinding!

tests/ui/panics/alloc_error_hook-unwind.rs

@@ -1,17 +1,19 @@
-//! Test that out-of-memory conditions trigger catchable panics with `-Z oom=panic`.
+//! Test that out-of-memory conditions trigger catchable panics with `set_alloc_error_hook`.
 
-//@ compile-flags: -Z oom=panic
 //@ run-pass
-//@ no-prefer-dynamic
 //@ needs-unwind
 //@ only-linux
 //@ ignore-backends: gcc
 
+#![feature(alloc_error_hook)]
+
 use std::hint::black_box;
 use std::mem::forget;
 use std::panic::catch_unwind;
 
 fn main() {
+    std::alloc::set_alloc_error_hook(|_| panic!());
+
     let panic = catch_unwind(|| {
         // This is guaranteed to exceed even the size of the address space
         for _ in 0..16 {