Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

detect dyn-unsized arguments before they cause ICEs #116115

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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