Skip to content

Commit

Permalink
Auto merge of rust-lang#122580 - saethlin:compiler-builtins-can-panic…
Browse files Browse the repository at this point in the history
…, r=pnkfelix

"Handle" calls to upstream monomorphizations in compiler_builtins

This is pretty cooked, but I think it works.

compiler-builtins has a long-standing problem that at link time, its rlib cannot contain any calls to `core`. And yet, in codegen we _love_ inserting calls to symbols in `core`, generally from various panic entrypoints.

I intend this PR to attack that problem as completely as possible. When we generate a function call, we now check if we are generating a function call from `compiler_builtins` and whether the callee is a function which was not lowered in the current crate, meaning we will have to link to it.

If those conditions are met, actually generating the call is asking for a linker error. So we don't. If the callee diverges, we lower to an abort with the same behavior as `core::intrinsics::abort`. If the callee does not diverge, we produce an error. This means that compiler-builtins can contain panics, but they'll SIGILL instead of panicking. I made non-diverging calls a compile error because I'm guessing that they'd mostly get into compiler-builtins by someone making a mistake while working on the crate, and compile errors are better than linker errors. We could turn such calls into aborts as well if that's preferred.
  • Loading branch information
bors committed Mar 22, 2024
2 parents 1447f9d + 2f6fb23 commit b3df0d7
Show file tree
Hide file tree
Showing 17 changed files with 284 additions and 29 deletions.
40 changes: 32 additions & 8 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ version = "0.1.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9792d37ca5173d7e7f4fe453739a0671d0557915a030a383d6b866476bbc3e71"
dependencies = [
"object",
"object 0.32.2",
]

[[package]]
Expand Down Expand Up @@ -281,7 +281,7 @@ dependencies = [
"cfg-if",
"libc",
"miniz_oxide",
"object",
"object 0.32.2",
"rustc-demangle",
]

Expand Down Expand Up @@ -2636,10 +2636,21 @@ dependencies = [
"memchr",
"rustc-std-workspace-alloc",
"rustc-std-workspace-core",
"ruzstd",
"ruzstd 0.5.0",
"wasmparser",
]

[[package]]
name = "object"
version = "0.34.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d7090bae93f8585aad99e595b7073c5de9ba89fbd6b4e9f0cdd7a10177273ac8"
dependencies = [
"flate2",
"memchr",
"ruzstd 0.6.0",
]

[[package]]
name = "odht"
version = "0.3.1"
Expand Down Expand Up @@ -3323,6 +3334,7 @@ dependencies = [
name = "run_make_support"
version = "0.0.0"
dependencies = [
"object 0.34.0",
"wasmparser",
]

Expand Down Expand Up @@ -3634,7 +3646,7 @@ dependencies = [
"itertools 0.12.1",
"libc",
"measureme",
"object",
"object 0.32.2",
"rustc-demangle",
"rustc_ast",
"rustc_attr",
Expand Down Expand Up @@ -3670,7 +3682,7 @@ dependencies = [
"itertools 0.12.1",
"jobserver",
"libc",
"object",
"object 0.32.2",
"pathdiff",
"regex",
"rustc_arena",
Expand All @@ -3686,6 +3698,7 @@ dependencies = [
"rustc_macros",
"rustc_metadata",
"rustc_middle",
"rustc_monomorphize",
"rustc_query_system",
"rustc_serialize",
"rustc_session",
Expand Down Expand Up @@ -4630,7 +4643,7 @@ name = "rustc_target"
version = "0.0.0"
dependencies = [
"bitflags 2.4.2",
"object",
"object 0.32.2",
"rustc_abi",
"rustc_data_structures",
"rustc_feature",
Expand Down Expand Up @@ -4897,6 +4910,17 @@ dependencies = [
"twox-hash",
]

[[package]]
name = "ruzstd"
version = "0.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5174a470eeb535a721ae9fdd6e291c2411a906b96592182d05217591d5c5cf7b"
dependencies = [
"byteorder",
"derive_more",
"twox-hash",
]

[[package]]
name = "ryu"
version = "1.0.17"
Expand Down Expand Up @@ -5200,7 +5224,7 @@ dependencies = [
"hermit-abi",
"libc",
"miniz_oxide",
"object",
"object 0.32.2",
"panic_abort",
"panic_unwind",
"profiler_builtins",
Expand Down Expand Up @@ -5517,7 +5541,7 @@ checksum = "4db52ee8fec06e119b692ef3dd2c4cf621a99204c1b8c47407870ed050305b9b"
dependencies = [
"gimli",
"hashbrown",
"object",
"object 0.32.2",
"tracing",
]

Expand Down
9 changes: 0 additions & 9 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,6 @@ exclude = [
]

[profile.release.package.compiler_builtins]
# The compiler-builtins crate cannot reference libcore, and its own CI will
# verify that this is the case. This requires, however, that the crate is built
# without overflow checks and debug assertions. Forcefully disable debug
# assertions and overflow checks here which should ensure that even if these
# assertions are enabled for libstd we won't enable them for compiler_builtins
# which should ensure we still link everything correctly.
debug-assertions = false
overflow-checks = false

# For compiler-builtins we always use a high number of codegen units.
# The goal here is to place every single intrinsic into its own object
# file to avoid symbol clashes with the system libgcc if possible. Note
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_codegen_cranelift/src/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ use std::borrow::Cow;

use cranelift_codegen::ir::SigRef;
use cranelift_module::ModuleError;
use rustc_codegen_ssa::errors::CompilerBuiltinsCannotCall;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::ty::layout::FnAbiOf;
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_monomorphize::is_call_from_compiler_builtins_to_upstream_monomorphization;
use rustc_session::Session;
use rustc_span::source_map::Spanned;
use rustc_target::abi::call::{Conv, FnAbi};
Expand Down Expand Up @@ -372,6 +375,17 @@ pub(crate) fn codegen_terminator_call<'tcx>(
ty::Instance::expect_resolve(fx.tcx, ty::ParamEnv::reveal_all(), def_id, fn_args)
.polymorphize(fx.tcx);

if is_call_from_compiler_builtins_to_upstream_monomorphization(fx.tcx, instance) {
if target.is_some() {
let caller = with_no_trimmed_paths!(fx.tcx.def_path_str(fx.instance.def_id()));
let callee = with_no_trimmed_paths!(fx.tcx.def_path_str(def_id));
fx.tcx.dcx().emit_err(CompilerBuiltinsCannotCall { caller, callee });
} else {
fx.bcx.ins().trap(TrapCode::User(0));
return;
}
}

if fx.tcx.symbol_name(instance).name.starts_with("llvm.") {
crate::intrinsics::codegen_llvm_intrinsic_call(
fx,
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use rustc_index::IndexVec;
use rustc_middle::ty::adjustment::PointerCoercion;
use rustc_middle::ty::layout::FnAbiOf;
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_monomorphize::is_call_from_compiler_builtins_to_upstream_monomorphization;

use crate::constant::ConstantCx;
use crate::debuginfo::FunctionDebugContext;
Expand Down Expand Up @@ -999,6 +1000,12 @@ fn codegen_panic_inner<'tcx>(
let def_id = fx.tcx.require_lang_item(lang_item, span);

let instance = Instance::mono(fx.tcx, def_id).polymorphize(fx.tcx);

if is_call_from_compiler_builtins_to_upstream_monomorphization(fx.tcx, instance) {
fx.bcx.ins().trap(TrapCode::User(0));
return;
}

let symbol_name = fx.tcx.symbol_name(instance).name;

fx.lib_call(
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_cranelift/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ extern crate rustc_hir;
extern crate rustc_incremental;
extern crate rustc_index;
extern crate rustc_metadata;
extern crate rustc_monomorphize;
extern crate rustc_session;
extern crate rustc_span;
extern crate rustc_target;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ rustc_index = { path = "../rustc_index" }
rustc_macros = { path = "../rustc_macros" }
rustc_metadata = { path = "../rustc_metadata" }
rustc_middle = { path = "../rustc_middle" }
rustc_monomorphize = { path = "../rustc_monomorphize" }
rustc_query_system = { path = "../rustc_query_system" }
rustc_serialize = { path = "../rustc_serialize" }
rustc_session = { path = "../rustc_session" }
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_codegen_ssa/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ codegen_ssa_cgu_not_recorded =
codegen_ssa_check_installed_visual_studio = please ensure that Visual Studio 2017 or later, or Build Tools for Visual Studio were installed with the Visual C++ option.
codegen_ssa_compiler_builtins_cannot_call =
`compiler_builtins` cannot call functions through upstream monomorphizations; encountered invalid call from `{$caller}` to `{$callee}`
codegen_ssa_copy_path = could not copy {$from} to {$to}: {$error}
codegen_ssa_copy_path_buf = unable to copy {$source_file} to {$output_path}: {$error}
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_codegen_ssa/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use rustc_hir::LangItem;
use rustc_middle::mir;
use rustc_middle::ty::Instance;
use rustc_middle::ty::{self, layout::TyAndLayout, Ty, TyCtxt};
use rustc_span::Span;

Expand Down Expand Up @@ -120,11 +121,11 @@ pub fn build_langcall<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
bx: &Bx,
span: Option<Span>,
li: LangItem,
) -> (Bx::FnAbiOfResult, Bx::Value) {
) -> (Bx::FnAbiOfResult, Bx::Value, Instance<'tcx>) {
let tcx = bx.tcx();
let def_id = tcx.require_lang_item(li, span);
let instance = ty::Instance::mono(tcx, def_id);
(bx.fn_abi_of_instance(instance, ty::List::empty()), bx.get_fn_addr(instance))
(bx.fn_abi_of_instance(instance, ty::List::empty()), bx.get_fn_addr(instance), instance)
}

// To avoid UB from LLVM, these two functions mask RHS with an
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_codegen_ssa/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1030,3 +1030,10 @@ pub struct FailedToGetLayout<'tcx> {
pub struct ErrorCreatingRemarkDir {
pub error: std::io::Error,
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_compiler_builtins_cannot_call)]
pub struct CompilerBuiltinsCannotCall {
pub caller: String,
pub callee: String,
}
49 changes: 41 additions & 8 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use super::{CachedLlbb, FunctionCx, LocalRef};

use crate::base;
use crate::common::{self, IntPredicate};
use crate::errors::CompilerBuiltinsCannotCall;
use crate::meth;
use crate::traits::*;
use crate::MemFlags;
Expand All @@ -16,6 +17,7 @@ use rustc_middle::mir::{self, AssertKind, BasicBlock, SwitchTargets, UnwindTermi
use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf, ValidityRequirement};
use rustc_middle::ty::print::{with_no_trimmed_paths, with_no_visible_paths};
use rustc_middle::ty::{self, Instance, Ty};
use rustc_monomorphize::is_call_from_compiler_builtins_to_upstream_monomorphization;
use rustc_session::config::OptLevel;
use rustc_span::{source_map::Spanned, sym, Span};
use rustc_target::abi::call::{ArgAbi, FnAbi, PassMode, Reg};
Expand Down Expand Up @@ -157,8 +159,28 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
destination: Option<(ReturnDest<'tcx, Bx::Value>, mir::BasicBlock)>,
mut unwind: mir::UnwindAction,
copied_constant_arguments: &[PlaceRef<'tcx, <Bx as BackendTypes>::Value>],
instance: Option<Instance<'tcx>>,
mergeable_succ: bool,
) -> MergingSucc {
let tcx = bx.tcx();
if let Some(instance) = instance {
if is_call_from_compiler_builtins_to_upstream_monomorphization(tcx, instance) {
if destination.is_some() {
let caller = with_no_trimmed_paths!(tcx.def_path_str(fx.instance.def_id()));
let callee = with_no_trimmed_paths!(tcx.def_path_str(instance.def_id()));
tcx.dcx().emit_err(CompilerBuiltinsCannotCall { caller, callee });
} else {
info!(
"compiler_builtins call to diverging function {:?} replaced with abort",
instance.def_id()
);
bx.abort();
bx.unreachable();
return MergingSucc::False;
}
}
}

// If there is a cleanup block and the function we're calling can unwind, then
// do an invoke, otherwise do a call.
let fn_ty = bx.fn_decl_backend_type(fn_abi);
Expand Down Expand Up @@ -480,6 +502,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let ty = location.ty(self.mir, bx.tcx()).ty;
let ty = self.monomorphize(ty);
let drop_fn = Instance::resolve_drop_in_place(bx.tcx(), ty);
let instance = drop_fn.clone();

if let ty::InstanceDef::DropGlue(_, None) = drop_fn.def {
// we don't actually need to drop anything.
Expand Down Expand Up @@ -582,6 +605,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
Some((ReturnDest::Nothing, target)),
unwind,
&[],
Some(instance),
mergeable_succ,
)
}
Expand Down Expand Up @@ -658,10 +682,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}
};

let (fn_abi, llfn) = common::build_langcall(bx, Some(span), lang_item);
let (fn_abi, llfn, instance) = common::build_langcall(bx, Some(span), lang_item);

// Codegen the actual panic invoke/call.
let merging_succ = helper.do_call(self, bx, fn_abi, llfn, &args, None, unwind, &[], false);
let merging_succ =
helper.do_call(self, bx, fn_abi, llfn, &args, None, unwind, &[], Some(instance), false);
assert_eq!(merging_succ, MergingSucc::False);
MergingSucc::False
}
Expand All @@ -677,7 +702,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
self.set_debug_loc(bx, terminator.source_info);

// Obtain the panic entry point.
let (fn_abi, llfn) = common::build_langcall(bx, Some(span), reason.lang_item());
let (fn_abi, llfn, instance) = common::build_langcall(bx, Some(span), reason.lang_item());

// Codegen the actual panic invoke/call.
let merging_succ = helper.do_call(
Expand All @@ -689,6 +714,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
None,
mir::UnwindAction::Unreachable,
&[],
Some(instance),
false,
);
assert_eq!(merging_succ, MergingSucc::False);
Expand Down Expand Up @@ -738,7 +764,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let msg = bx.const_str(&msg_str);

// Obtain the panic entry point.
let (fn_abi, llfn) =
let (fn_abi, llfn, instance) =
common::build_langcall(bx, Some(source_info.span), LangItem::PanicNounwind);

// Codegen the actual panic invoke/call.
Expand All @@ -751,6 +777,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
target.as_ref().map(|bb| (ReturnDest::Nothing, *bb)),
unwind,
&[],
Some(instance),
mergeable_succ,
)
} else {
Expand Down Expand Up @@ -798,6 +825,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
ty::FnPtr(_) => (None, Some(callee.immediate())),
_ => bug!("{} is not callable", callee.layout.ty),
};

let def = instance.map(|i| i.def);

if let Some(ty::InstanceDef::DropGlue(_, None)) = def {
Expand Down Expand Up @@ -1106,6 +1134,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
destination,
unwind,
&copied_constant_arguments,
instance,
mergeable_succ,
)
}
Expand Down Expand Up @@ -1664,11 +1693,15 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

self.set_debug_loc(&mut bx, mir::SourceInfo::outermost(self.mir.span));

let (fn_abi, fn_ptr) = common::build_langcall(&bx, None, reason.lang_item());
let fn_ty = bx.fn_decl_backend_type(fn_abi);
let (fn_abi, fn_ptr, instance) = common::build_langcall(&bx, None, reason.lang_item());
if is_call_from_compiler_builtins_to_upstream_monomorphization(bx.tcx(), instance) {
bx.abort();
} else {
let fn_ty = bx.fn_decl_backend_type(fn_abi);

let llret = bx.call(fn_ty, None, Some(fn_abi), fn_ptr, &[], funclet.as_ref());
bx.apply_attrs_to_cleanup_callsite(llret);
let llret = bx.call(fn_ty, None, Some(fn_abi), fn_ptr, &[], funclet.as_ref());
bx.apply_attrs_to_cleanup_callsite(llret);
}

bx.unreachable();

Expand Down
Loading

0 comments on commit b3df0d7

Please sign in to comment.