Auto merge of #3042 - rust-lang:rustup-2023-08-29, r=RalfJung

Automatic sync from rustc

3 files changed, 58 insertions, 18 deletions

diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs
index ca7c484ea31..42a2fb0330c 100644
--- a/compiler/rustc_const_eval/src/interpret/terminator.rs
+++ b/compiler/rustc_const_eval/src/interpret/terminator.rs
@@ -255,6 +255,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         caller_abi: &ArgAbi<'tcx, Ty<'tcx>>,
         callee_abi: &ArgAbi<'tcx, Ty<'tcx>>,
     ) -> bool {
+        let primitive_abi_compat = |a1: abi::Primitive, a2: abi::Primitive| -> bool {
+            match (a1, a2) {
+                // For integers, ignore the sign.
+                (abi::Primitive::Int(int_ty1, _sign1), abi::Primitive::Int(int_ty2, _sign2)) => {
+                    int_ty1 == int_ty2
+                }
+                // For everything else we require full equality.
+                _ => a1 == a2,
+            }
+        };
         // Heuristic for type comparison.
         let layout_compat = || {
             if caller_abi.layout.ty == callee_abi.layout.ty {
@@ -267,28 +277,24 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 // then who knows what happens.
                 return false;
             }
-            if caller_abi.layout.size != callee_abi.layout.size
-                || caller_abi.layout.align.abi != callee_abi.layout.align.abi
-            {
-                // This cannot go well...
-                return false;
-            }
-            // The rest *should* be okay, but we are extra conservative.
+            // This is tricky. Some ABIs split aggregates up into multiple registers etc, so we have
+            // to be super careful here. For the scalar ABIs we conveniently already have all the
+            // newtypes unwrapped etc, so in those cases we can just compare the scalar components.
+            // Everything else we just reject for now.
             match (caller_abi.layout.abi, callee_abi.layout.abi) {
-                // Different valid ranges are okay (once we enforce validity,
-                // that will take care to make it UB to leave the range, just
-                // like for transmute).
+                // Different valid ranges are okay (the validity check will complain if this leads
+                // to invalid transmutes).
                 (abi::Abi::Scalar(caller), abi::Abi::Scalar(callee)) => {
-                    caller.primitive() == callee.primitive()
+                    primitive_abi_compat(caller.primitive(), callee.primitive())
                 }
                 (
                     abi::Abi::ScalarPair(caller1, caller2),
                     abi::Abi::ScalarPair(callee1, callee2),
                 ) => {
-                    caller1.primitive() == callee1.primitive()
-                        && caller2.primitive() == callee2.primitive()
+                    primitive_abi_compat(caller1.primitive(), callee1.primitive())
+                        && primitive_abi_compat(caller2.primitive(), callee2.primitive())
                 }
-                // Be conservative
+                // Be conservative.
                 _ => false,
             }
         };
@@ -309,7 +315,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             return true;
         };
         let mode_compat = || match (&caller_abi.mode, &callee_abi.mode) {
-            (PassMode::Ignore, PassMode::Ignore) => true,
+            (PassMode::Ignore, PassMode::Ignore) => true, // can still be reached for the return type
             (PassMode::Direct(a1), PassMode::Direct(a2)) => arg_attr_compat(a1, a2),
             (PassMode::Pair(a1, b1), PassMode::Pair(a2, b2)) => {
                 arg_attr_compat(a1, a2) && arg_attr_compat(b1, b2)
@@ -326,7 +332,15 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             _ => false,
         };
 
+        // We have to check both. `layout_compat` is needed to reject e.g. `i32` vs `f32`,
+        // which is not reflected in `PassMode`. `mode_compat` is needed to reject `u8` vs `bool`,
+        // which have the same `abi::Primitive` but different `arg_ext`.
         if layout_compat() && mode_compat() {
+            // Something went very wrong if our checks don't even imply that the layout is the same.
+            assert!(
+                caller_abi.layout.size == callee_abi.layout.size
+                    && caller_abi.layout.align.abi == callee_abi.layout.align.abi
+            );
             return true;
         }
         trace!(
diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs
index b829f24ab7a..770c3f7f02c 100644
--- a/compiler/rustc_const_eval/src/transform/validate.rs
+++ b/compiler/rustc_const_eval/src/transform/validate.rs
@@ -20,6 +20,8 @@ use rustc_mir_dataflow::{Analysis, ResultsCursor};
 use rustc_target::abi::{Size, FIRST_VARIANT};
 use rustc_target::spec::abi::Abi;
 
+use crate::util::is_within_packed;
+
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 enum EdgeKind {
     Unwind,
@@ -93,6 +95,7 @@ impl<'tcx> MirPass<'tcx> for Validator {
         cfg_checker.visit_body(body);
         cfg_checker.check_cleanup_control_flow();
 
+        // Also run the TypeChecker.
         for (location, msg) in validate_types(tcx, self.mir_phase, param_env, body) {
             cfg_checker.fail(location, msg);
         }
@@ -427,14 +430,34 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
                 self.check_unwind_edge(location, *unwind);
 
                 // The call destination place and Operand::Move place used as an argument might be
-                // passed by a reference to the callee. Consequently they must be non-overlapping.
-                // Currently this simply checks for duplicate places.
+                // passed by a reference to the callee. Consequently they must be non-overlapping
+                // and cannot be packed. Currently this simply checks for duplicate places.
                 self.place_cache.clear();
                 self.place_cache.insert(destination.as_ref());
+                if is_within_packed(self.tcx, &self.body.local_decls, *destination).is_some() {
+                    // This is bad! The callee will expect the memory to be aligned.
+                    self.fail(
+                        location,
+                        format!(
+                            "encountered packed place in `Call` terminator destination: {:?}",
+                            terminator.kind,
+                        ),
+                    );
+                }
                 let mut has_duplicates = false;
                 for arg in args {
                     if let Operand::Move(place) = arg {
                         has_duplicates |= !self.place_cache.insert(place.as_ref());
+                        if is_within_packed(self.tcx, &self.body.local_decls, *place).is_some() {
+                            // This is bad! The callee will expect the memory to be aligned.
+                            self.fail(
+                                location,
+                                format!(
+                                    "encountered `Move` of a packed place in `Call` terminator: {:?}",
+                                    terminator.kind,
+                                ),
+                            );
+                        }
                     }
                 }
 
@@ -442,7 +465,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
                     self.fail(
                         location,
                         format!(
-                            "encountered overlapping memory in `Call` terminator: {:?}",
+                            "encountered overlapping memory in `Move` arguments to `Call` terminator: {:?}",
                             terminator.kind,
                         ),
                     );
@@ -541,6 +564,8 @@ impl<'a, 'tcx> Visitor<'tcx> for CfgChecker<'a, 'tcx> {
     }
 }
 
+/// A faster version of the validation pass that only checks those things which may break when apply
+/// generic substitutions.
 pub fn validate_types<'tcx>(
     tcx: TyCtxt<'tcx>,
     mir_phase: MirPhase,
diff --git a/compiler/rustc_const_eval/src/util/alignment.rs b/compiler/rustc_const_eval/src/util/alignment.rs
index c1f0ff260d2..2e0643afb39 100644
--- a/compiler/rustc_const_eval/src/util/alignment.rs
+++ b/compiler/rustc_const_eval/src/util/alignment.rs
@@ -34,6 +34,7 @@ where
             false
         }
         _ => {
+            // We cannot figure out the layout. Conservatively assume that this is disaligned.
             debug!("is_disaligned({:?}) - true", place);
             true
         }