Skip to content

Commit

Permalink
Auto merge of #109507 - Amanieu:panic-oom-payload, r=davidtwco
Browse files Browse the repository at this point in the history
Report allocation errors as panics

OOM is now reported as a panic but with a custom payload type (`AllocErrorPanicPayload`) which holds the layout that was passed to `handle_alloc_error`.

This should be review one commit at a time:
- The first commit adds `AllocErrorPanicPayload` and changes allocation errors to always be reported as panics.
- The second commit removes `#[alloc_error_handler]` and the `alloc_error_hook` API.

ACP: rust-lang/libs-team#192

Closes #51540
Closes #51245
  • Loading branch information
bors committed Apr 22, 2023
2 parents 21fab43 + 4da05e0 commit 39cf520
Show file tree
Hide file tree
Showing 58 changed files with 138 additions and 878 deletions.
2 changes: 0 additions & 2 deletions compiler/rustc_builtin_macros/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ builtin_macros_requires_cfg_pattern =
builtin_macros_expected_one_cfg_pattern = expected 1 cfg-pattern
builtin_macros_alloc_error_must_be_fn = alloc_error_handler must be a function
builtin_macros_assert_requires_boolean = macro requires a boolean expression as an argument
.label = boolean expression required
Expand Down
97 changes: 0 additions & 97 deletions compiler/rustc_builtin_macros/src/alloc_error_handler.rs

This file was deleted.

7 changes: 0 additions & 7 deletions compiler/rustc_builtin_macros/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,6 @@ pub(crate) struct OneCfgPattern {
pub(crate) span: Span,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_alloc_error_must_be_fn)]
pub(crate) struct AllocErrorMustBeFn {
#[primary_span]
pub(crate) span: Span,
}

#[derive(Diagnostic)]
#[diag(builtin_macros_assert_requires_boolean)]
pub(crate) struct AssertRequiresBoolean {
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_builtin_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ use rustc_expand::proc_macro::BangProcMacro;
use rustc_fluent_macro::fluent_messages;
use rustc_span::symbol::sym;

mod alloc_error_handler;
mod assert;
mod cfg;
mod cfg_accessible;
Expand Down Expand Up @@ -104,7 +103,6 @@ pub fn register_builtin_macros(resolver: &mut dyn ResolverExpand) {
}

register_attr! {
alloc_error_handler: alloc_error_handler::expand,
bench: test::expand_bench,
cfg_accessible: cfg_accessible::Expander,
cfg_eval: cfg_eval::expand,
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_codegen_cranelift/example/alloc_example.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![feature(start, core_intrinsics, alloc_error_handler)]
#![feature(start, core_intrinsics)]
#![no_std]

extern crate alloc;
Expand All @@ -22,11 +22,6 @@ fn panic_handler(_: &core::panic::PanicInfo) -> ! {
core::intrinsics::abort();
}

#[alloc_error_handler]
fn alloc_error_handler(_: alloc::alloc::Layout) -> ! {
core::intrinsics::abort();
}

#[start]
fn main(_argc: isize, _argv: *const *const u8) -> isize {
let world: Box<&str> = Box::new("Hello World!\0");
Expand Down
23 changes: 1 addition & 22 deletions compiler/rustc_codegen_cranelift/src/allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use crate::prelude::*;
use rustc_ast::expand::allocator::{AllocatorKind, AllocatorTy, ALLOCATOR_METHODS};
use rustc_codegen_ssa::base::allocator_kind_for_codegen;
use rustc_session::config::OomStrategy;
use rustc_span::symbol::sym;

/// Returns whether an allocator shim was created
pub(crate) fn codegen(
Expand All @@ -15,21 +14,14 @@ pub(crate) fn codegen(
unwind_context: &mut UnwindContext,
) -> bool {
let Some(kind) = allocator_kind_for_codegen(tcx) else { return false };
codegen_inner(
module,
unwind_context,
kind,
tcx.alloc_error_handler_kind(()).unwrap(),
tcx.sess.opts.unstable_opts.oom,
);
codegen_inner(module, unwind_context, kind, tcx.sess.opts.unstable_opts.oom);
true
}

fn codegen_inner(
module: &mut impl Module,
unwind_context: &mut UnwindContext,
kind: AllocatorKind,
alloc_error_handler_kind: AllocatorKind,
oom_strategy: OomStrategy,
) {
let usize_ty = module.target_config().pointer_type();
Expand Down Expand Up @@ -71,19 +63,6 @@ fn codegen_inner(
);
}

let sig = Signature {
call_conv: module.target_config().default_call_conv,
params: vec![AbiParam::new(usize_ty), AbiParam::new(usize_ty)],
returns: vec![],
};
crate::common::create_wrapper_function(
module,
unwind_context,
sig,
"__rust_alloc_error_handler",
&alloc_error_handler_kind.fn_name(sym::oom),
);

let data_id = module.declare_data(OomStrategy::SYMBOL, Linkage::Export, false, false).unwrap();
let mut data_ctx = DataContext::new();
data_ctx.set_align(1);
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_codegen_gcc/example/alloc_example.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![feature(start, core_intrinsics, alloc_error_handler, lang_items)]
#![feature(start, core_intrinsics, lang_items)]
#![no_std]

extern crate alloc;
Expand All @@ -21,11 +21,6 @@ fn panic_handler(_: &core::panic::PanicInfo) -> ! {
core::intrinsics::abort();
}

#[alloc_error_handler]
fn alloc_error_handler(_: alloc::alloc::Layout) -> ! {
core::intrinsics::abort();
}

#[lang = "eh_personality"]
fn eh_personality() -> ! {
loop {}
Expand Down
34 changes: 1 addition & 33 deletions compiler/rustc_codegen_gcc/src/allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ use rustc_ast::expand::allocator::{AllocatorKind, AllocatorTy, ALLOCATOR_METHODS
use rustc_middle::bug;
use rustc_middle::ty::TyCtxt;
use rustc_session::config::OomStrategy;
use rustc_span::symbol::sym;

use crate::GccContext;

pub(crate) unsafe fn codegen(tcx: TyCtxt<'_>, mods: &mut GccContext, _module_name: &str, kind: AllocatorKind, alloc_error_handler_kind: AllocatorKind) {
pub(crate) unsafe fn codegen(tcx: TyCtxt<'_>, mods: &mut GccContext, _module_name: &str, kind: AllocatorKind) {
let context = &mods.context;
let usize =
match tcx.sess.target.pointer_width {
Expand Down Expand Up @@ -87,37 +86,6 @@ pub(crate) unsafe fn codegen(tcx: TyCtxt<'_>, mods: &mut GccContext, _module_nam
// as described in https://github.com/rust-lang/rust/commit/77a96ed5646f7c3ee8897693decc4626fe380643
}

let types = [usize, usize];
let name = "__rust_alloc_error_handler".to_string();
let args: Vec<_> = types.iter().enumerate()
.map(|(index, typ)| context.new_parameter(None, *typ, &format!("param{}", index)))
.collect();
let func = context.new_function(None, FunctionType::Exported, void, &args, name, false);

if tcx.sess.target.default_hidden_visibility {
#[cfg(feature="master")]
func.add_attribute(FnAttribute::Visibility(gccjit::Visibility::Hidden));
}

let callee = alloc_error_handler_kind.fn_name(sym::oom);
let args: Vec<_> = types.iter().enumerate()
.map(|(index, typ)| context.new_parameter(None, *typ, &format!("param{}", index)))
.collect();
let callee = context.new_function(None, FunctionType::Extern, void, &args, callee, false);
#[cfg(feature="master")]
callee.add_attribute(FnAttribute::Visibility(gccjit::Visibility::Hidden));

let block = func.new_block("entry");

let args = args
.iter()
.enumerate()
.map(|(i, _)| func.get_param(i as i32).to_rvalue())
.collect::<Vec<_>>();
let _ret = context.new_call(None, callee, &args);
//llvm::LLVMSetTailCall(ret, True);
block.end_with_void_return(None);

let name = OomStrategy::SYMBOL.to_string();
let global = context.new_global(None, GlobalKind::Exported, i8, name);
let value = tcx.sess.opts.unstable_opts.oom.should_panic();
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_gcc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,11 @@ impl CodegenBackend for GccCodegenBackend {
}

impl ExtraBackendMethods for GccCodegenBackend {
fn codegen_allocator<'tcx>(&self, tcx: TyCtxt<'tcx>, module_name: &str, kind: AllocatorKind, alloc_error_handler_kind: AllocatorKind) -> Self::Module {
fn codegen_allocator<'tcx>(&self, tcx: TyCtxt<'tcx>, module_name: &str, kind: AllocatorKind) -> Self::Module {
let mut mods = GccContext {
context: Context::default(),
};
unsafe { allocator::codegen(tcx, &mut mods, module_name, kind, alloc_error_handler_kind); }
unsafe { allocator::codegen(tcx, &mut mods, module_name, kind); }
mods
}

Expand Down
48 changes: 0 additions & 48 deletions compiler/rustc_codegen_llvm/src/allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use rustc_ast::expand::allocator::{AllocatorKind, AllocatorTy, ALLOCATOR_METHODS
use rustc_middle::bug;
use rustc_middle::ty::TyCtxt;
use rustc_session::config::{DebugInfo, OomStrategy};
use rustc_span::symbol::sym;

use crate::debuginfo;
use crate::llvm::{self, False, True};
Expand All @@ -15,7 +14,6 @@ pub(crate) unsafe fn codegen(
module_llvm: &mut ModuleLlvm,
module_name: &str,
kind: AllocatorKind,
alloc_error_handler_kind: AllocatorKind,
) {
let llcx = &*module_llvm.llcx;
let llmod = module_llvm.llmod();
Expand Down Expand Up @@ -100,52 +98,6 @@ pub(crate) unsafe fn codegen(
llvm::LLVMDisposeBuilder(llbuilder);
}

// rust alloc error handler
let args = [usize, usize]; // size, align

let ty = llvm::LLVMFunctionType(void, args.as_ptr(), args.len() as c_uint, False);
let name = "__rust_alloc_error_handler";
let llfn = llvm::LLVMRustGetOrInsertFunction(llmod, name.as_ptr().cast(), name.len(), ty);
// -> ! DIFlagNoReturn
let no_return = llvm::AttributeKind::NoReturn.create_attr(llcx);
attributes::apply_to_llfn(llfn, llvm::AttributePlace::Function, &[no_return]);

if tcx.sess.target.default_hidden_visibility {
llvm::LLVMRustSetVisibility(llfn, llvm::Visibility::Hidden);
}
if tcx.sess.must_emit_unwind_tables() {
let uwtable = attributes::uwtable_attr(llcx);
attributes::apply_to_llfn(llfn, llvm::AttributePlace::Function, &[uwtable]);
}

let callee = alloc_error_handler_kind.fn_name(sym::oom);
let callee = llvm::LLVMRustGetOrInsertFunction(llmod, callee.as_ptr().cast(), callee.len(), ty);
// -> ! DIFlagNoReturn
attributes::apply_to_llfn(callee, llvm::AttributePlace::Function, &[no_return]);
llvm::LLVMRustSetVisibility(callee, llvm::Visibility::Hidden);

let llbb = llvm::LLVMAppendBasicBlockInContext(llcx, llfn, "entry\0".as_ptr().cast());

let llbuilder = llvm::LLVMCreateBuilderInContext(llcx);
llvm::LLVMPositionBuilderAtEnd(llbuilder, llbb);
let args = args
.iter()
.enumerate()
.map(|(i, _)| llvm::LLVMGetParam(llfn, i as c_uint))
.collect::<Vec<_>>();
let ret = llvm::LLVMRustBuildCall(
llbuilder,
ty,
callee,
args.as_ptr(),
args.len() as c_uint,
[].as_ptr(),
0 as c_uint,
);
llvm::LLVMSetTailCall(ret, True);
llvm::LLVMBuildRetVoid(llbuilder);
llvm::LLVMDisposeBuilder(llbuilder);

// __rust_alloc_error_handler_should_panic
let name = OomStrategy::SYMBOL;
let ll_g = llvm::LLVMRustGetOrInsertGlobal(llmod, name.as_ptr().cast(), name.len(), i8);
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_codegen_llvm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,10 @@ impl ExtraBackendMethods for LlvmCodegenBackend {
tcx: TyCtxt<'tcx>,
module_name: &str,
kind: AllocatorKind,
alloc_error_handler_kind: AllocatorKind,
) -> ModuleLlvm {
let mut module_llvm = ModuleLlvm::new_metadata(tcx, module_name);
unsafe {
allocator::codegen(tcx, &mut module_llvm, module_name, kind, alloc_error_handler_kind);
allocator::codegen(tcx, &mut module_llvm, module_name, kind);
}
module_llvm
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ fn exported_symbols_provider_local(
for symbol_name in ALLOCATOR_METHODS
.iter()
.map(|method| format!("__rust_{}", method.name))
.chain(["__rust_alloc_error_handler".to_string(), OomStrategy::SYMBOL.to_string()])
.chain([OomStrategy::SYMBOL.to_string()])
{
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, &symbol_name));

Expand Down
13 changes: 3 additions & 10 deletions compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -635,16 +635,9 @@ pub fn codegen_crate<B: ExtraBackendMethods>(
if let Some(kind) = allocator_kind_for_codegen(tcx) {
let llmod_id =
cgu_name_builder.build_cgu_name(LOCAL_CRATE, &["crate"], Some("allocator")).to_string();
let module_llvm = tcx.sess.time("write_allocator_module", || {
backend.codegen_allocator(
tcx,
&llmod_id,
kind,
// If allocator_kind is Some then alloc_error_handler_kind must
// also be Some.
tcx.alloc_error_handler_kind(()).unwrap(),
)
});
let module_llvm = tcx
.sess
.time("write_allocator_module", || backend.codegen_allocator(tcx, &llmod_id, kind));

ongoing_codegen.submit_pre_codegened_module_to_llvm(
tcx,
Expand Down
Loading

0 comments on commit 39cf520

Please sign in to comment.