From 0389d09004b6e1ec9e2fce4e035591208b4533ac Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 24 Sep 2023 09:26:34 +0200 Subject: [PATCH] detect dyn-unsized arguments before they cause ICEs --- compiler/rustc_codegen_cranelift/src/base.rs | 9 ++- compiler/rustc_codegen_llvm/src/abi.rs | 23 +++++++- compiler/rustc_codegen_ssa/src/mir/mod.rs | 9 ++- .../src/interpret/eval_context.rs | 9 ++- compiler/rustc_middle/messages.ftl | 14 +++++ compiler/rustc_middle/src/error.rs | 19 ++++++ compiler/rustc_middle/src/mir/mod.rs | 59 ++++++++++++++++++- compiler/rustc_middle/src/mir/syntax.rs | 6 +- .../tests/fail/unsized-extern-type-arg.rs | 17 ++++++ .../tests/fail/unsized-extern-type-arg.stderr | 10 ++++ tests/ui/unsized-locals/extern-type.rs | 14 +++++ tests/ui/unsized-locals/extern-type.stderr | 18 ++++++ 12 files changed, 192 insertions(+), 15 deletions(-) create mode 100644 src/tools/miri/tests/fail/unsized-extern-type-arg.rs create mode 100644 src/tools/miri/tests/fail/unsized-extern-type-arg.stderr create mode 100644 tests/ui/unsized-locals/extern-type.rs create mode 100644 tests/ui/unsized-locals/extern-type.stderr diff --git a/compiler/rustc_codegen_cranelift/src/base.rs b/compiler/rustc_codegen_cranelift/src/base.rs index 9b5a6b89191e1..9ef590427eb19 100644 --- a/compiler/rustc_codegen_cranelift/src/base.rs +++ b/compiler/rustc_codegen_cranelift/src/base.rs @@ -250,9 +250,12 @@ pub(crate) fn verify_func( } fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) { - if let Err(err) = - fx.mir.post_mono_checks(fx.tcx, ty::ParamEnv::reveal_all(), |c| Ok(fx.monomorphize(c))) - { + if let Err(err) = fx.mir.post_mono_checks( + fx.tcx, + ty::ParamEnv::reveal_all(), + |c| Ok(fx.monomorphize(c)), + |ty| Ok(fx.monomorphize(ty)), + ) { err.emit_err(fx.tcx); fx.bcx.append_block_params_for_function_params(fx.block_map[START_BLOCK]); fx.bcx.switch_to_block(fx.block_map[START_BLOCK]); diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index 9e834b83df43c..3e982e58111db 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -393,8 +393,27 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> { // alignment, so this respects ABI compatibility. let ptr_ty = Ty::new_mut_ptr(cx.tcx, arg.layout.ty); let ptr_layout = cx.layout_of(ptr_ty); - llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 0, true)); - llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 1, true)); + if matches!(ptr_layout.abi, abi::Abi::ScalarPair(..)) { + llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 0, true)); + llargument_tys.push(ptr_layout.scalar_pair_element_llvm_type(cx, 1, true)); + } else { + // If this is not a ScalarPair, something went wrong. But this is possible + // e.g. with unsized locals of `extern` type, which can only be detected + // post-monomorphization. So we fill in some fake data (crucially, we add + // the right number of argument types). and rely on someone else showing a + // nice error. + assert!(ptr_layout.abi.is_scalar()); + let llvm_ty = ptr_layout.immediate_llvm_type(cx); + llargument_tys.push(llvm_ty); + llargument_tys.push(llvm_ty); + cx.tcx.sess.delay_span_bug( + rustc_span::DUMMY_SP, + format!( + "function argument with invalid unsized type {}", + arg.layout.ty + ), + ); + } continue; } PassMode::Indirect { attrs: _, meta_attrs: None, on_stack: _ } => { diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index 1e905a7c78e47..01f229ea9259a 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -212,9 +212,12 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( fx.per_local_var_debug_info = fx.compute_per_local_var_debug_info(&mut start_bx); // Rust post-monomorphization checks; we later rely on them. - if let Err(err) = - mir.post_mono_checks(cx.tcx(), ty::ParamEnv::reveal_all(), |c| Ok(fx.monomorphize(c))) - { + if let Err(err) = mir.post_mono_checks( + cx.tcx(), + ty::ParamEnv::reveal_all(), + |c| Ok(fx.monomorphize(c)), + |ty| Ok(fx.monomorphize(ty)), + ) { err.emit_err(cx.tcx()); // This IR shouldn't ever be emitted, but let's try to guard against any of this code // ever running. diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 83f2052d0f8c0..97064c22324a3 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -752,9 +752,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { if M::POST_MONO_CHECKS { // `ctfe_query` does some error message decoration that we want to be in effect here. self.ctfe_query(None, |tcx| { - body.post_mono_checks(*tcx, self.param_env, |c| { - self.subst_from_current_frame_and_normalize_erasing_regions(c) - }) + body.post_mono_checks( + *tcx, + self.param_env, + |c| self.subst_from_current_frame_and_normalize_erasing_regions(c), + |ty| self.subst_from_current_frame_and_normalize_erasing_regions(ty), + ) })?; } diff --git a/compiler/rustc_middle/messages.ftl b/compiler/rustc_middle/messages.ftl index 82162fd85711b..019a744b53856 100644 --- a/compiler/rustc_middle/messages.ftl +++ b/compiler/rustc_middle/messages.ftl @@ -52,6 +52,20 @@ middle_drop_check_overflow = overflow while adding drop-check rules for {$ty} .note = overflowed on {$overflow_ty} +middle_dyn_unsized_local = + {$is_arg -> + [true] function argument + *[other] local variable + } does not have a dynamically computable size + .note = `{$ty}` contains an `extern type`, which is not allowed in {$is_arg -> + [true] function arguments + *[other] local variables + } + +middle_dyn_unsized_param = + function parameter does not have a dynamically computable size + .note = `{$ty}` contains an `extern type`, which is not allowed in function parameters + middle_erroneous_constant = erroneous constant encountered middle_layout_references_error = diff --git a/compiler/rustc_middle/src/error.rs b/compiler/rustc_middle/src/error.rs index 3c5536570872a..771b226111287 100644 --- a/compiler/rustc_middle/src/error.rs +++ b/compiler/rustc_middle/src/error.rs @@ -53,6 +53,25 @@ pub struct LimitInvalid<'a> { pub error_str: &'a str, } +#[derive(Diagnostic)] +#[diag(middle_dyn_unsized_local)] +#[note] +pub struct DynUnsizedLocal<'tcx> { + #[primary_span] + pub span: Span, + pub ty: Ty<'tcx>, + pub is_arg: bool, +} + +#[derive(Diagnostic)] +#[diag(middle_dyn_unsized_param)] +#[note] +pub struct DynUnsizedParam<'tcx> { + #[primary_span] + pub span: Span, + pub ty: Ty<'tcx>, +} + #[derive(Diagnostic)] #[diag(middle_recursion_limit_reached)] #[help] diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 0bb1c66da0cbb..667e164bed400 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -2,6 +2,7 @@ //! //! [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/mir/index.html +use crate::error::{DynUnsizedLocal, DynUnsizedParam}; use crate::mir::interpret::{AllocRange, ConstAllocation, ErrorHandled, Scalar}; use crate::mir::visit::MirVisitable; use crate::ty::codec::{TyDecoder, TyEncoder}; @@ -586,14 +587,70 @@ impl<'tcx> Body<'tcx> { tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, normalize_const: impl Fn(Const<'tcx>) -> Result, ErrorHandled>, + normalize_ty: impl Fn(Ty<'tcx>) -> Result, ErrorHandled>, ) -> Result<(), ErrorHandled> { - // For now, the only thing we have to check is is to ensure that all the constants used in + // The main thing we have to check is is to ensure that all the constants used in // the body successfully evaluate. for &const_ in &self.required_consts { let c = normalize_const(const_.const_)?; c.eval(tcx, param_env, Some(const_.span))?; } + // The second thing we do is guard against unsized locals/arguments that do not have a dynamically computable size. + // Due to a lack of an appropriate trait, we currently have to hack this in + // as a post-mono check. + let mut guaranteed = None; // `Some` if any error was emitted + // First check all locals (including the arguments). + for (idx, local) in self.local_decls.iter_enumerated() { + let ty = local.ty; + // We get invoked in generic code, so we cannot force normalization. + let tail = + tcx.struct_tail_with_normalize(ty, |ty| normalize_ty(ty).unwrap_or(ty), || {}); + // If the unsized tail is an `extern type`, emit error. + if matches!(tail.kind(), ty::Foreign(_)) { + assert!(idx.as_u32() > 0); // cannot be the return local + let is_arg = (1..=self.arg_count).contains(&idx.as_usize()); + guaranteed = Some(tcx.sess.emit_err(DynUnsizedLocal { + span: local.source_info.span, + ty, + is_arg, + })); + } + } + // The above check covers the callee side. We also need to check the caller. + // For that we check all function calls. + for bb in self.basic_blocks.iter() { + let terminator = bb.terminator(); + if let TerminatorKind::Call { args, .. } = &terminator.kind { + for arg in args { + if let Operand::Move(place) = arg { + if place.is_indirect_first_projection() { + // Found an argument of the shape `Move(*arg)`. Make sure + // its size can be determined dynamically. + let ty = place.ty(&self.local_decls, tcx).ty; + let tail = tcx.struct_tail_with_normalize( + ty, + |ty| normalize_ty(ty).unwrap_or(ty), + || {}, + ); + // If the unsized tail is an `extern type`, emit error. + if matches!(tail.kind(), ty::Foreign(_)) { + guaranteed = Some(tcx.sess.emit_err(DynUnsizedParam { + span: terminator.source_info.span, + ty, + })); + } + } + } + } + } + } + // Above we made sure to report *all* errors by continuing even after reporting something. + // Here we make sure we return `Err` if there was any error. + if let Some(guaranteed) = guaranteed { + return Err(ErrorHandled::Reported(guaranteed.into(), DUMMY_SP)); + } + Ok(()) } } diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 8f651b2a2db4b..7c1fa6201e397 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -666,9 +666,9 @@ pub enum TerminatorKind<'tcx> { /// The function that’s being called. func: Operand<'tcx>, /// Arguments the function is called with. - /// These are owned by the callee, which is free to modify them. - /// This allows the memory occupied by "by-value" arguments to be - /// reused across function calls without duplicating the contents. + /// Any `Move` operands in this list are owned by the callee, which is free to modify them. + /// This allows the memory occupied by "by-value" arguments to be reused across function + /// calls without duplicating the contents. args: Vec>, /// Where the returned value will be written destination: Place<'tcx>, diff --git a/src/tools/miri/tests/fail/unsized-extern-type-arg.rs b/src/tools/miri/tests/fail/unsized-extern-type-arg.rs new file mode 100644 index 0000000000000..30f90184d4493 --- /dev/null +++ b/src/tools/miri/tests/fail/unsized-extern-type-arg.rs @@ -0,0 +1,17 @@ +#![feature(extern_types)] +#![feature(unsized_fn_params)] + +extern { + pub type E; +} + +fn test(_e: E) {} + +pub fn calltest(e: Box) { + test(*e) //~ERROR: does not have a dynamically computable size +} + +fn main() { + let b = Box::new(0u32); + calltest(unsafe { std::mem::transmute(b)} ); +} diff --git a/src/tools/miri/tests/fail/unsized-extern-type-arg.stderr b/src/tools/miri/tests/fail/unsized-extern-type-arg.stderr new file mode 100644 index 0000000000000..80c5709bf22c6 --- /dev/null +++ b/src/tools/miri/tests/fail/unsized-extern-type-arg.stderr @@ -0,0 +1,10 @@ +error: function parameter does not have a dynamically computable size + --> $DIR/unsized-extern-type-arg.rs:LL:CC + | +LL | test(*e) + | ^^^^^^^^ + | + = note: `E` contains an `extern type`, which is not allowed in function parameters + +error: aborting due to previous error + diff --git a/tests/ui/unsized-locals/extern-type.rs b/tests/ui/unsized-locals/extern-type.rs new file mode 100644 index 0000000000000..092e97ef57fee --- /dev/null +++ b/tests/ui/unsized-locals/extern-type.rs @@ -0,0 +1,14 @@ +// build-fail +#![feature(extern_types)] +#![feature(unsized_fn_params)] +#![crate_type = "lib"] + +extern { + pub type E; +} + +fn test(e: E) {} //~ERROR: does not have a dynamically computable size + +pub fn calltest(e: Box) { + test(*e) //~ERROR: does not have a dynamically computable size +} diff --git a/tests/ui/unsized-locals/extern-type.stderr b/tests/ui/unsized-locals/extern-type.stderr new file mode 100644 index 0000000000000..02236a7049fbb --- /dev/null +++ b/tests/ui/unsized-locals/extern-type.stderr @@ -0,0 +1,18 @@ +error: function argument does not have a dynamically computable size + --> $DIR/extern-type.rs:10:9 + | +LL | fn test(e: E) {} + | ^ + | + = note: `E` contains an `extern type`, which is not allowed in function arguments + +error: function parameter does not have a dynamically computable size + --> $DIR/extern-type.rs:13:5 + | +LL | test(*e) + | ^^^^^^^^ + | + = note: `E` contains an `extern type`, which is not allowed in function parameters + +error: aborting due to 2 previous errors +