Skip to content

Commit

Permalink
Auto merge of rust-lang#116115 - RalfJung:dyn-unsized-args, r=<try>
Browse files Browse the repository at this point in the history
detect dyn-unsized arguments before they cause ICEs

"Fixes" rust-lang#115709 in a crude way, with a post-mono check. This is entirely caused by us not having a trait to express "type can have its size determined at runtime".
  • Loading branch information
bors committed Sep 24, 2023
2 parents 8a6bae2 + 0389d09 commit 6e5c4b6
Show file tree
Hide file tree
Showing 12 changed files with 192 additions and 15 deletions.
9 changes: 6 additions & 3 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
23 changes: 21 additions & 2 deletions compiler/rustc_codegen_llvm/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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: _ } => {
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_codegen_ssa/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
})?;
}

Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_middle/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
19 changes: 19 additions & 0 deletions compiler/rustc_middle/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
59 changes: 58 additions & 1 deletion compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -586,14 +587,70 @@ impl<'tcx> Body<'tcx> {
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
normalize_const: impl Fn(Const<'tcx>) -> Result<Const<'tcx>, ErrorHandled>,
normalize_ty: impl Fn(Ty<'tcx>) -> Result<Ty<'tcx>, 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(())
}
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Operand<'tcx>>,
/// Where the returned value will be written
destination: Place<'tcx>,
Expand Down
17 changes: 17 additions & 0 deletions src/tools/miri/tests/fail/unsized-extern-type-arg.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#![feature(extern_types)]
#![feature(unsized_fn_params)]

extern {
pub type E;
}

fn test(_e: E) {}

pub fn calltest(e: Box<E>) {
test(*e) //~ERROR: does not have a dynamically computable size
}

fn main() {
let b = Box::new(0u32);
calltest(unsafe { std::mem::transmute(b)} );
}
10 changes: 10 additions & 0 deletions src/tools/miri/tests/fail/unsized-extern-type-arg.stderr
Original file line number Diff line number Diff line change
@@ -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

14 changes: 14 additions & 0 deletions tests/ui/unsized-locals/extern-type.rs
Original file line number Diff line number Diff line change
@@ -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<E>) {
test(*e) //~ERROR: does not have a dynamically computable size
}
18 changes: 18 additions & 0 deletions tests/ui/unsized-locals/extern-type.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 6e5c4b6

Please sign in to comment.