adjust const-eval and Miri to more relaxed ZST access rules

Author: RalfJung

compiler/rustc_const_eval/messages.ftl

@@ -69,9 +69,6 @@ const_eval_deref_function_pointer =
     accessing {$allocation} which contains a function
 const_eval_deref_vtable_pointer =
     accessing {$allocation} which contains a vtable
-const_eval_different_allocations =
-    `{$name}` called on pointers into different allocations
-
 const_eval_division_by_zero =
     dividing by zero
 const_eval_division_overflow =
@@ -241,12 +238,18 @@ const_eval_not_enough_caller_args =
 const_eval_nullary_intrinsic_fail =
     could not evaluate nullary intrinsic
 
+const_eval_offset_from_different_allocations =
+    `{$name}` called on pointers into different allocations
+const_eval_offset_from_different_integers =
+    `{$name}` called on different pointers without provenance (i.e., without an associated allocation)
 const_eval_offset_from_overflow =
     `{$name}` called when first pointer is too far ahead of second
-
-const_eval_offset_from_test = out-of-bounds `offset_from`
+const_eval_offset_from_test =
+    out-of-bounds `offset_from`
 const_eval_offset_from_underflow =
     `{$name}` called when first pointer is too far before second
+const_eval_offset_from_unsigned_overflow =
+    `ptr_offset_from_unsigned` called when first pointer has smaller offset than second: {$a_offset} < {$b_offset}
 
 const_eval_operator_non_const =
     cannot call non-const operator in {const_eval_const_context}s
@@ -394,8 +397,6 @@ const_eval_unreachable = entering unreachable code
 const_eval_unreachable_unwind =
     unwinding past a stack frame that does not allow unwinding
 
-const_eval_unsigned_offset_from_overflow =
-    `ptr_offset_from_unsigned` called when first pointer has smaller offset than second: {$a_offset} < {$b_offset}
 const_eval_unsized_local = unsized locals are not supported
 const_eval_unstable_const_fn = `{$def_path}` is not yet stable as a const fn
 

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -8,21 +8,15 @@ use rustc_middle::ty::layout::{LayoutOf as _, ValidityRequirement};
 use rustc_middle::ty::GenericArgsRef;
 use rustc_middle::ty::{Ty, TyCtxt};
 use rustc_middle::{
-    mir::{
-        self,
-        interpret::{
-            Allocation, ConstAllocation, GlobalId, InterpResult, PointerArithmetic, Scalar,
-        },
-        BinOp, ConstValue, NonDivergingIntrinsic,
-    },
+    mir::{self, BinOp, ConstValue, NonDivergingIntrinsic},
     ty::layout::TyAndLayout,
 };
 use rustc_span::symbol::{sym, Symbol};
 use rustc_target::abi::Size;
 
 use super::{
-    util::ensure_monomorphic_enough, CheckInAllocMsg, ImmTy, InterpCx, Machine, OpTy, PlaceTy,
-    Pointer,
+    util::ensure_monomorphic_enough, Allocation, CheckInAllocMsg, ConstAllocation, GlobalId, ImmTy,
+    InterpCx, InterpResult, Machine, OpTy, PlaceTy, Pointer, PointerArithmetic, Provenance, Scalar,
 };
 
 use crate::fluent_generated as fluent;
@@ -236,106 +230,125 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             }
             sym::ptr_offset_from | sym::ptr_offset_from_unsigned => {
                 let a = self.read_pointer(&args[0])?;
+                let a_parts = self.ptr_try_get_alloc_id(a);
                 let b = self.read_pointer(&args[1])?;
+                let b_parts = self.ptr_try_get_alloc_id(b);
+
+                // If the pointers' addresses are the same, then different rules apply.
+                let same_addr = match (a_parts, b_parts) {
+                    (Err(a_addr), Err(b_addr)) => a_addr == b_addr,
+                    _ if M::Provenance::OFFSET_IS_ADDR => a.addr() == b.addr(),
+                    _ => {
+                        // We can't determine whether they are the same, so we have to assume they
+                        // are different.
+                        false
+                    }
+                };
 
-                let usize_layout = self.layout_of(self.tcx.types.usize)?;
-                let isize_layout = self.layout_of(self.tcx.types.isize)?;
-
-                // Get offsets for both that are at least relative to the same base.
-                let (a_offset, b_offset) =
-                    match (self.ptr_try_get_alloc_id(a), self.ptr_try_get_alloc_id(b)) {
-                        (Err(a), Err(b)) => {
-                            // Neither pointer points to an allocation.
-                            // If these are inequal or null, this *will* fail the deref check below.
-                            (a, b)
-                        }
-                        (Err(_), _) | (_, Err(_)) => {
-                            // We managed to find a valid allocation for one pointer, but not the other.
-                            // That means they are definitely not pointing to the same allocation.
-                            throw_ub_custom!(
-                                fluent::const_eval_different_allocations,
-                                name = intrinsic_name,
-                            );
-                        }
-                        (Ok((a_alloc_id, a_offset, _)), Ok((b_alloc_id, b_offset, _))) => {
-                            // Found allocation for both. They must be into the same allocation.
-                            if a_alloc_id != b_alloc_id {
+                if same_addr {
+                    // Distance: 0. No further requirements.
+                    self.write_scalar(Scalar::from_target_usize(0, self), dest)?;
+                } else {
+                    let usize_layout = self.layout_of(self.tcx.types.usize)?;
+                    let isize_layout = self.layout_of(self.tcx.types.isize)?;
+
+                    // The addresses are not the same, so we have to do some extra checks and some maths.
+                    let (a_offset, b_offset) =
+                        match (self.ptr_try_get_alloc_id(a), self.ptr_try_get_alloc_id(b)) {
+                            (Ok((a_alloc_id, a_offset, _)), Ok((b_alloc_id, b_offset, _))) => {
+                                // Found allocation for both. They must be into the same allocation.
+                                if a_alloc_id != b_alloc_id {
+                                    throw_ub_custom!(
+                                        fluent::const_eval_offset_from_different_allocations,
+                                        name = intrinsic_name,
+                                    );
+                                }
+                                // Use these offsets for distance calculation.
+                                (a_offset.bytes(), b_offset.bytes())
+                            }
+                            (Err(_), Err(_)) => {
+                                throw_ub_custom!(
+                                    fluent::const_eval_offset_from_different_integers,
+                                    name = intrinsic_name,
+                                );
+                            }
+                            _ => {
+                                // They don't both have an allocation, and they are also not the same.
+                                // This is never allowed.
+                                throw_ub_custom!(
+                                    fluent::const_eval_offset_from_different_allocations,
+                                    name = intrinsic_name,
+                                );
+                            }
+                        };
+
+                    // Compute distance.
+                    let dist = {
+                        // Addresses are unsigned, so this is a `usize` computation. We have to do the
+                        // overflow check separately anyway.
+                        let (val, overflowed) = {
+                            let a_offset = ImmTy::from_uint(a_offset, usize_layout);
+                            let b_offset = ImmTy::from_uint(b_offset, usize_layout);
+                            self.overflowing_binary_op(BinOp::Sub, &a_offset, &b_offset)?
+                        };
+                        if overflowed {
+                            // a < b
+                            if intrinsic_name == sym::ptr_offset_from_unsigned {
+                                throw_ub_custom!(
+                                    fluent::const_eval_offset_from_unsigned_overflow,
+                                    a_offset = a_offset,
+                                    b_offset = b_offset,
+                                );
+                            }
+                            // The signed form of the intrinsic allows this. If we interpret the
+                            // difference as isize, we'll get the proper signed difference. If that
+                            // seems *positive*, they were more than isize::MAX apart.
+                            let dist = val.to_scalar().to_target_isize(self)?;
+                            if dist >= 0 {
+                                throw_ub_custom!(
+                                    fluent::const_eval_offset_from_underflow,
+                                    name = intrinsic_name,
+                                );
+                            }
+                            dist
+                        } else {
+                            // b >= a
+                            let dist = val.to_scalar().to_target_isize(self)?;
+                            // If converting to isize produced a *negative* result, we had an overflow
+                            // because they were more than isize::MAX apart.
+                            if dist < 0 {
                                 throw_ub_custom!(
-                                    fluent::const_eval_different_allocations,
+                                    fluent::const_eval_offset_from_overflow,
                                     name = intrinsic_name,
                                 );
                             }
-                            // Use these offsets for distance calculation.
-                            (a_offset.bytes(), b_offset.bytes())
+                            dist
                         }
                     };
 
-                // Compute distance.
-                let dist = {
-                    // Addresses are unsigned, so this is a `usize` computation. We have to do the
-                    // overflow check separately anyway.
-                    let (val, overflowed) = {
-                        let a_offset = ImmTy::from_uint(a_offset, usize_layout);
-                        let b_offset = ImmTy::from_uint(b_offset, usize_layout);
-                        self.overflowing_binary_op(BinOp::Sub, &a_offset, &b_offset)?
-                    };
-                    if overflowed {
-                        // a < b
-                        if intrinsic_name == sym::ptr_offset_from_unsigned {
-                            throw_ub_custom!(
-                                fluent::const_eval_unsigned_offset_from_overflow,
-                                a_offset = a_offset,
-                                b_offset = b_offset,
-                            );
-                        }
-                        // The signed form of the intrinsic allows this. If we interpret the
-                        // difference as isize, we'll get the proper signed difference. If that
-                        // seems *positive*, they were more than isize::MAX apart.
-                        let dist = val.to_scalar().to_target_isize(self)?;
-                        if dist >= 0 {
-                            throw_ub_custom!(
-                                fluent::const_eval_offset_from_underflow,
-                                name = intrinsic_name,
-                            );
-                        }
-                        dist
+                    // Check that the range between them is dereferenceable ("in-bounds or one past the
+                    // end of the same allocation"). This is like the check in ptr_offset_inbounds.
+                    let min_ptr = if dist >= 0 { b } else { a };
+                    self.check_ptr_access(
+                        min_ptr,
+                        Size::from_bytes(dist.unsigned_abs()),
+                        CheckInAllocMsg::OffsetFromTest,
+                    )?;
+
+                    // Perform division by size to compute return value.
+                    let ret_layout = if intrinsic_name == sym::ptr_offset_from_unsigned {
+                        assert!(0 <= dist && dist <= self.target_isize_max());
+                        usize_layout
                     } else {
-                        // b >= a
-                        let dist = val.to_scalar().to_target_isize(self)?;
-                        // If converting to isize produced a *negative* result, we had an overflow
-                        // because they were more than isize::MAX apart.
-                        if dist < 0 {
-                            throw_ub_custom!(
-                                fluent::const_eval_offset_from_overflow,
-                                name = intrinsic_name,
-                            );
-                        }
-                        dist
-                    }
-                };
-
-                // Check that the range between them is dereferenceable ("in-bounds or one past the
-                // end of the same allocation"). This is like the check in ptr_offset_inbounds.
-                let min_ptr = if dist >= 0 { b } else { a };
-                self.check_ptr_access(
-                    min_ptr,
-                    Size::from_bytes(dist.unsigned_abs()),
-                    CheckInAllocMsg::OffsetFromTest,
-                )?;
-
-                // Perform division by size to compute return value.
-                let ret_layout = if intrinsic_name == sym::ptr_offset_from_unsigned {
-                    assert!(0 <= dist && dist <= self.target_isize_max());
-                    usize_layout
-                } else {
-                    assert!(self.target_isize_min() <= dist && dist <= self.target_isize_max());
-                    isize_layout
-                };
-                let pointee_layout = self.layout_of(instance_args.type_at(0))?;
-                // If ret_layout is unsigned, we checked that so is the distance, so we are good.
-                let val = ImmTy::from_int(dist, ret_layout);
-                let size = ImmTy::from_int(pointee_layout.size.bytes(), ret_layout);
-                self.exact_div(&val, &size, dest)?;
+                        assert!(self.target_isize_min() <= dist && dist <= self.target_isize_max());
+                        isize_layout
+                    };
+                    let pointee_layout = self.layout_of(instance_args.type_at(0))?;
+                    // If ret_layout is unsigned, we checked that so is the distance, so we are good.
+                    let val = ImmTy::from_int(dist, ret_layout);
+                    let size = ImmTy::from_int(pointee_layout.size.bytes(), ret_layout);
+                    self.exact_div(&val, &size, dest)?;
+                }
             }
 
             sym::assert_inhabited

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -396,6 +396,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     /// to the allocation it points to. Supports both shared and mutable references, as the actual
     /// checking is offloaded to a helper closure.
     ///
+    /// `alloc_size` will always be called if `ptr` has an `AllocId`, even if the size is 0.
+    ///
     /// Returns `None` if and only if the size is 0.
     fn check_and_deref_ptr<T>(
         &self,
@@ -410,16 +412,21 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     ) -> InterpResult<'tcx, Option<T>> {
         Ok(match self.ptr_try_get_alloc_id(ptr) {
             Err(addr) => {
-                // We couldn't get a proper allocation. This is only okay if the access size is 0,
-                // and the address is not null.
-                if size.bytes() > 0 || addr == 0 {
-                    throw_ub!(DanglingIntPointer(addr, msg));
+                // Everything is okay with size 0.
+                if size.bytes() == 0 {
+                    return Ok(None);
                 }
-                None
+                // We couldn't get a proper allocation.
+                throw_ub!(DanglingIntPointer(addr, msg));
             }
             Ok((alloc_id, offset, prov)) => {
+                // We must always call `alloc_size` when the `ptr` has an `AllocId`.
                 let (alloc_size, _alloc_align, ret_val) = alloc_size(alloc_id, offset, prov)?;
-                // Test bounds. This also ensures non-null.
+                // Everything is okay with size 0, even out-of-bounds accesses.
+                if size.bytes() == 0 {
+                    return Ok(None);
+                }
+                // Test bounds.
                 // It is sufficient to check this for the end pointer. Also check for overflow!
                 if offset.checked_add(size, &self.tcx).map_or(true, |end| end > alloc_size) {
                     throw_ub!(PointerOutOfBounds {
@@ -430,14 +437,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                         msg,
                     })
                 }
-                // Ensure we never consider the null pointer dereferenceable.
-                if M::Provenance::OFFSET_IS_ADDR {
-                    assert_ne!(ptr.addr(), Size::ZERO);
-                }
 
-                // We can still be zero-sized in this branch, in which case we have to
-                // return `None`.
-                if size.bytes() == 0 { None } else { Some(ret_val) }
+                Some(ret_val)
             }
         })
     }

compiler/rustc_const_eval/src/interpret/validity.rs

@@ -435,6 +435,10 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
                 found_bytes: has.bytes()
             },
         );
+        // Make sure this is non-null. (ZST references can be dereferenceable and null.)
+        if self.ecx.scalar_may_be_null(Scalar::from_maybe_pointer(place.ptr(), self.ecx))? {
+            throw_validation_failure!(self.path, NullPtr { ptr_kind })
+        }
         // Do not allow pointers to uninhabited types.
         if place.layout.abi.is_uninhabited() {
             let ty = place.layout.ty;
@@ -802,6 +806,8 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
         trace!("visit_value: {:?}, {:?}", *op, op.layout);
 
         // Check primitive types -- the leaves of our recursive descent.
+        // We assume that the Scalar validity range does not restrict these values
+        // any further than `try_visit_primitive` does!
         if self.try_visit_primitive(op)? {
             return Ok(());
         }