Properly emit a slice of Invariant, add tests

Author: 5225225

compiler/rustc_codegen_ssa/src/mir/operand.rs

@@ -100,6 +100,22 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
                 let b_llval = bx.const_usize((end - start) as u64);
                 OperandValue::Pair(a_llval, b_llval)
             }
+            ConstValue::CustomSlice { data, length } => {
+                let Abi::ScalarPair(a_scalar, _) = layout.abi else {
+                    bug!("from_const: invalid ScalarPair layout: {:#?}", layout);
+                };
+                let a = Scalar::from_pointer(
+                    Pointer::new(bx.tcx().create_memory_alloc(data), Size::ZERO),
+                    &bx.tcx(),
+                );
+                let a_llval = bx.scalar_to_backend(
+                    a,
+                    a_scalar,
+                    bx.scalar_pair_element_backend_type(layout, 0, true),
+                );
+                let b_llval = bx.const_usize(length as u64);
+                OperandValue::Pair(a_llval, b_llval)
+            }
             ConstValue::ByRef { alloc, offset } => {
                 return bx.load_operand(bx.from_const_alloc(layout, alloc, offset));
             }

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -5,6 +5,7 @@
 use std::convert::TryFrom;
 
 use rustc_hir::def_id::DefId;
+use rustc_hir::lang_items::LangItem;
 use rustc_middle::mir::{
     self,
     interpret::{ConstValue, GlobalId, InterpResult, Scalar},
@@ -106,8 +107,8 @@ pub(crate) fn eval_nullary_intrinsic<'tcx>(
         },
         sym::validity_invariants_of => {
             ensure_monomorphic_enough(tcx, tp_ty)?;
-            let alloc = validity_invariants_of::alloc_validity_invariants_of(tcx, tp_ty);
-            ConstValue::Slice { data: alloc, start: 0, end: alloc.inner().len() }
+            let (data, length) = validity_invariants_of::alloc_validity_invariants_of(tcx, tp_ty);
+            ConstValue::CustomSlice { data, length }
         }
         other => bug!("`{}` is not a zero arg intrinsic", other),
     })
@@ -176,7 +177,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     sym::needs_drop => self.tcx.types.bool,
                     sym::type_id => self.tcx.types.u64,
                     sym::type_name => self.tcx.mk_static_str(),
-                    sym::validity_invariants_of => self.tcx.mk_static_bytes(),
+                    sym::validity_invariants_of => {
+                        let item = self.tcx.require_lang_item(LangItem::ValidityInvariant, None);
+                        let ty = self.tcx.type_of(item);
+                        self.tcx.mk_imm_ref(self.tcx.lifetimes.re_static, self.tcx.mk_slice(ty))
+                    }
                     _ => bug!(),
                 };
                 let val =

compiler/rustc_const_eval/src/interpret/intrinsics/validity_invariants_of.rs

@@ -1,11 +1,12 @@
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::lang_items::LangItem;
 use rustc_middle::mir::interpret::{AllocRange, Allocation, ConstAllocation, Scalar as MirScalar};
+use rustc_middle::mir::Mutability;
 use rustc_middle::ty::layout::LayoutCx;
 use rustc_middle::ty::{ParamEnv, ParamEnvAnd};
 use rustc_middle::ty::{Ty, TyCtxt};
 use rustc_target::abi::{
-    Abi, FieldsShape, HasDataLayout, Integer, Primitive, Scalar, Size, TyAndLayout, WrappingRange,
+    Abi, FieldsShape, HasDataLayout, Integer, Primitive, Scalar, Size, TyAndLayout, WrappingRange, Variants,
 };
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -25,7 +26,12 @@ struct InvariantKey {
 }
 
 // FIXME: Don't add duplicate invariants (maybe use a HashMap?)
-fn add_invariants<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, invs: &mut FxHashMap<InvariantKey, WrappingRange>, offset: Size) {
+fn add_invariants<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    ty: Ty<'tcx>,
+    invs: &mut FxHashMap<InvariantKey, WrappingRange>,
+    offset: Size,
+) {
     let x = tcx.layout_of(ParamEnvAnd { param_env: ParamEnv::reveal_all(), value: ty });
 
     if let Ok(layout) = x {
@@ -46,8 +52,14 @@ fn add_invariants<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, invs: &mut FxHashMap<In
             let _: Result<_, _> = invs.try_insert(InvariantKey { offset, size }, valid_range);
         }
 
+        //dbg!(&ty, &layout);
+        if !matches!(layout.layout.variants(), Variants::Single { .. }) {
+            // We *don't* want to look for fields inside enums.
+            return;
+        }
+
         let param_env = ParamEnv::reveal_all();
-        let unwrap = LayoutCx { tcx, param_env };
+        let layout_cx = LayoutCx { tcx, param_env };
 
         match layout.layout.fields() {
             FieldsShape::Primitive => {}
@@ -57,13 +69,13 @@ fn add_invariants<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, invs: &mut FxHashMap<In
                 // That would lead to false negatives, though.
                 for idx in 0..*count {
                     let off = offset + *stride * idx;
-                    let f = layout.field(&unwrap, idx as usize);
+                    let f = layout.field(&layout_cx, idx as usize);
                     add_invariants(tcx, f.ty, invs, off);
                 }
             }
             FieldsShape::Arbitrary { offsets, .. } => {
                 for (idx, &field_offset) in offsets.iter().enumerate() {
-                    let f = layout.field(&unwrap, idx);
+                    let f = layout.field(&layout_cx, idx);
                     if f.ty == ty {
                         // Some types contain themselves as fields, such as
                         // &mut [T]
@@ -90,7 +102,7 @@ fn get_layout_of_invariant<'tcx>(tcx: TyCtxt<'tcx>) -> TyAndLayout<'tcx, Ty<'tcx
 pub(crate) fn alloc_validity_invariants_of<'tcx>(
     tcx: TyCtxt<'tcx>,
     ty: Ty<'tcx>,
-) -> ConstAllocation<'tcx> {
+) -> (ConstAllocation<'tcx>, usize) {
     let mut invs = FxHashMap::default();
 
     let layout = tcx.data_layout();
@@ -126,22 +138,17 @@ pub(crate) fn alloc_validity_invariants_of<'tcx>(
 
         let offset_range = AllocRange { start: offset + start_off, size: Size::from_bytes(16) };
         alloc
-            .write_scalar(
-                &tcx,
-                offset_range,
-                MirScalar::from_u128(invariant.1.start).into(),
-            )
+            .write_scalar(&tcx, offset_range, MirScalar::from_u128(invariant.1.start).into())
             .unwrap();
 
         let offset_range = AllocRange { start: offset + end_off, size: Size::from_bytes(16) };
         alloc
-            .write_scalar(
-                &tcx,
-                offset_range,
-                MirScalar::from_u128(invariant.1.end).into(),
-            )
+            .write_scalar(&tcx, offset_range, MirScalar::from_u128(invariant.1.end).into())
             .unwrap();
     }
 
-    tcx.intern_const_alloc(alloc)
+    // The allocation is not mutable, we just needed write_scalar.
+    alloc.mutability = Mutability::Not;
+
+    (tcx.intern_const_alloc(alloc), invs.len())
 }

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -692,6 +692,19 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     self,
                 ))
             }
+            ConstValue::CustomSlice { data, length } => {
+                // We rely on mutability being set correctly in `data` to prevent writes
+                // where none should happen.
+                let ptr = Pointer::new(
+                    self.tcx.create_memory_alloc(data),
+                    Size::ZERO, // offset: 0
+                );
+                Operand::Immediate(Immediate::new_slice(
+                    Scalar::from_pointer(self.global_base_pointer(ptr)?, &*self.tcx),
+                    u64::try_from(length).unwrap(),
+                    self,
+                ))
+            }
         };
         Ok(OpTy { op, layout })
     }

compiler/rustc_middle/src/mir/interpret/value.rs

@@ -37,6 +37,9 @@ pub enum ConstValue<'tcx> {
     /// Used only for `&[u8]` and `&str`
     Slice { data: ConstAllocation<'tcx>, start: usize, end: usize },
 
+    /// Like `Slice`, but for types that aren't 1 byte long.
+    CustomSlice { data: ConstAllocation<'tcx>, length: usize },
+
     /// A value not represented/representable by `Scalar` or `Slice`
     ByRef {
         /// The backing memory of the value, may contain more memory than needed for just the value
@@ -61,6 +64,9 @@ impl<'a, 'tcx> Lift<'tcx> for ConstValue<'a> {
             ConstValue::ByRef { alloc, offset } => {
                 ConstValue::ByRef { alloc: tcx.lift(alloc)?, offset }
             }
+            ConstValue::CustomSlice { data, length } => {
+                ConstValue::CustomSlice { data: tcx.lift(data)?, length }
+            }
         })
     }
 }
@@ -69,7 +75,9 @@ impl<'tcx> ConstValue<'tcx> {
     #[inline]
     pub fn try_to_scalar(&self) -> Option<Scalar<AllocId>> {
         match *self {
-            ConstValue::ByRef { .. } | ConstValue::Slice { .. } => None,
+            ConstValue::ByRef { .. }
+            | ConstValue::Slice { .. }
+            | ConstValue::CustomSlice { .. } => None,
             ConstValue::Scalar(val) => Some(val),
         }
     }

compiler/rustc_middle/src/mir/pretty.rs

@@ -451,6 +451,7 @@ impl<'tcx> Visitor<'tcx> for ExtraComments<'tcx> {
             let fmt_val = |val: &ConstValue<'tcx>| match val {
                 ConstValue::Scalar(s) => format!("Scalar({:?})", s),
                 ConstValue::Slice { .. } => format!("Slice(..)"),
+                ConstValue::CustomSlice { .. } => format!("CustomSlice(..)"),
                 ConstValue::ByRef { .. } => format!("ByRef(..)"),
             };
 
@@ -679,7 +680,9 @@ pub fn write_allocations<'tcx>(
             ConstValue::Scalar(interpret::Scalar::Int { .. }) => {
                 Either::Left(Either::Right(std::iter::empty()))
             }
-            ConstValue::ByRef { alloc, .. } | ConstValue::Slice { data: alloc, .. } => {
+            ConstValue::ByRef { alloc, .. }
+            | ConstValue::Slice { data: alloc, .. }
+            | ConstValue::CustomSlice { data: alloc, .. } => {
                 Either::Right(alloc_ids_from_alloc(alloc))
             }
         }

compiler/rustc_middle/src/ty/context.rs

@@ -2339,11 +2339,6 @@ impl<'tcx> TyCtxt<'tcx> {
         self.mk_imm_ref(self.lifetimes.re_static, self.types.str_)
     }
 
-    #[inline]
-    pub fn mk_static_bytes(self) -> Ty<'tcx> {
-        self.mk_imm_ref(self.lifetimes.re_static, self.mk_slice(self.types.u8))
-    }
-
     #[inline]
     pub fn mk_adt(self, def: AdtDef<'tcx>, substs: SubstsRef<'tcx>) -> Ty<'tcx> {
         // Take a copy of substs so that we own the vectors inside.

compiler/rustc_typeck/src/check/intrinsic.rs

@@ -9,6 +9,7 @@ use crate::require_same_types;
 
 use rustc_errors::struct_span_err;
 use rustc_hir as hir;
+use rustc_hir::lang_items::LangItem;
 use rustc_middle::traits::{ObligationCause, ObligationCauseCode};
 use rustc_middle::ty::subst::Subst;
 use rustc_middle::ty::{self, TyCtxt};
@@ -192,7 +193,12 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) {
             sym::needs_drop => (1, Vec::new(), tcx.types.bool),
 
             sym::type_name => (1, Vec::new(), tcx.mk_static_str()),
-            sym::validity_invariants_of => (1, Vec::new(), tcx.mk_static_bytes()),
+            sym::validity_invariants_of => {
+                let item = tcx.require_lang_item(LangItem::ValidityInvariant, None);
+                let ty = tcx.type_of(item);
+                let slice = tcx.mk_imm_ref(tcx.lifetimes.re_static, tcx.mk_slice(ty));
+                (1, Vec::new(), slice)
+            }
             sym::type_id => (1, Vec::new(), tcx.types.u64),
             sym::offset | sym::arith_offset => (
                 1,

library/core/src/intrinsics.rs

@@ -1976,6 +1976,13 @@ extern "rust-intrinsic" {
     /// [`std::hint::black_box`]: crate::hint::black_box
     #[rustc_const_unstable(feature = "const_black_box", issue = "none")]
     pub fn black_box<T>(dummy: T) -> T;
+
+    /// Returns a list of invaraints that must be valid in order for T to be valid.
+    ///
+    /// This is used internally to allow for runtime assertions inside `MaybeUninit::assume_init`
+    #[cfg(not(bootstrap))]
+    #[rustc_const_unstable(feature = "const_intrinsic_validity_invariants_of", issue = "none")]
+    pub fn validity_invariants_of<T>() -> &'static [Invariant];
 }
 
 // Some functions are defined here because they accidentally got made
@@ -2138,10 +2145,10 @@ pub const unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: us
     }
 }
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq)]
 #[repr(u8)]
 #[allow(dead_code)] // https://github.com/rust-lang/rust/issues/85677
-enum InvariantSize {
+pub enum InvariantSize {
     U8 = 0,
     U16 = 1,
     U32 = 2,
@@ -2156,35 +2163,20 @@ enum InvariantSize {
 // Be sure to keep the fields in order (offset, size, start, end), validity_invariants_of.rs needs
 // that.
 #[cfg(not(bootstrap))]
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq)]
 #[lang = "ValidityInvariant"]
-struct Invariant {
+pub struct Invariant {
     /// The offset in bytes from the start of the struct
-    offset: usize,
+    pub offset: usize,
     /// The size/type of the invariant.
-    size: InvariantSize,
+    pub size: InvariantSize,
     /// The start point of the valid range of this field. Is allowed to be > valid_range_end, see
     /// <https://doc.rust-lang.org/nightly/nightly-rustc/rustc_target/abi/struct.WrappingRange.html>
     /// which this follows the semantics of.
-    valid_range_start: u128,
+    pub valid_range_start: u128,
 
     /// The end point of the range.
-    valid_range_end: u128,
-}
-
-#[cfg(not(bootstrap))]
-/// Returns a list of all validity invariants of the type.
-const fn validity_invariants_of<T>() -> &'static [Invariant] {
-    extern "rust-intrinsic" {
-        #[rustc_const_unstable(feature = "validity_invariants_of", issue = "none")]
-        pub fn validity_invariants_of<T>() -> &'static [u8];
-    }
-
-    let invariants: &'static [u8] = validity_invariants_of::<T>();
-    let sz = invariants.len() / core::mem::size_of::<Invariant>();
-
-    // SAFETY: we know this is valid because the intrinsic promises an aligned slice.
-    unsafe { core::slice::from_raw_parts(invariants.as_ptr().cast(), sz) }
+    pub valid_range_end: u128,
 }
 
 /// Copies `count * size_of::<T>()` bytes from `src` to `dst`. The source
@@ -2445,15 +2437,15 @@ where
 }
 
 #[cfg(bootstrap)]
-pub(crate) const unsafe fn assert_validity_of<T>(_: *const T) -> bool {
+pub const unsafe fn assert_validity_of<T>(_: *const T) -> bool {
     true
 }
 
 #[cfg(not(bootstrap))]
 /// Asserts that the value at `value` is a valid T.
 ///
-/// Best effort, and is UB if the value is invalid.
-pub(crate) unsafe fn assert_validity_of<T>(value: *const T) -> bool {
+/// Best effort, can miss some UB, and is UB if the value is invalid.
+pub unsafe fn assert_validity_of<T>(value: *const T) -> bool {
     // We have to do this, since we call assert_validity_of inside MaybeUninit::assume_init
     // and if we had used ptr::read_unaligned, that would be a recursive call.
     #[repr(packed)]
@@ -2493,11 +2485,11 @@ pub(crate) unsafe fn assert_validity_of<T>(value: *const T) -> bool {
 
             if start > end {
                 if !((start..=max).contains(&value) || (0..=end).contains(&value)) {
-                    return false;
+                    panic!("read value {value} which was not in range 0..={end} or {start}..={max} at offset {off} in type {}", core::any::type_name::<T>());
                 }
             } else {
                 if !(start..=end).contains(&value) {
-                    return false;
+                    panic!("read value {value} which was not in range {start}..={end} at offset {off} in type {}", core::any::type_name::<T>());
                 }
             }
         }

library/core/src/lib.rs

@@ -156,8 +156,6 @@
 #![feature(const_slice_from_ref)]
 #![feature(const_slice_index)]
 #![feature(const_is_char_boundary)]
-#![cfg_attr(not(bootstrap), feature(validity_invariants_of))]
-//
 // Language features:
 #![feature(abi_unadjusted)]
 #![feature(allow_internal_unsafe)]