Auto merge of #116115 - RalfJung:dyn-unsized-args, r=<try>

detect dyn-unsized arguments before they cause ICEs

"Fixes" #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".

Author: bors

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]);

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: _ } => {

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.

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),
+                )
             })?;
         }
 

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 =

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]

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<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(())
     }
 }

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<Operand<'tcx>>,
         /// Where the returned value will be written
         destination: Place<'tcx>,

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<E>) {
+    test(*e) //~ERROR: does not have a dynamically computable size
+}
+
+fn main() {
+    let b = Box::new(0u32);
+    calltest(unsafe { std::mem::transmute(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
+

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<E>) {
+    test(*e) //~ERROR: does not have a dynamically computable size
+}

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
+