Rollup merge of rust-lang#95785 - RalfJung:interpret-size-mismatch, r…

…=oli-obk

interpret: err instead of ICE on size mismatches in to_bits_or_ptr_internal

We did this a while ago already for `to_i32()` and friends, but missed this one. That became quite annoying when I was debugging an ICE caused by `read_pointer` in a Miri shim where the code was passing an argument at the wrong type.

Having `scalar_to_ptr` be fallible is consistent with all the other `Scalar::to_*` methods being fallible. I added `unwrap` only in code outside the interpreter, which is no worse off than before now in terms of panics.

r? `@oli-obk`

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -167,17 +167,18 @@ pub(super) fn op_to_const<'tcx>(
  },
  Immediate::ScalarPair(a, b) => {
  // We know `offset` is relative to the allocation, so we can use `into_parts`.
- let (data, start) = match ecx.scalar_to_ptr(a.check_init().unwrap()).into_parts() {
- (Some(alloc_id), offset) => {
- (ecx.tcx.global_alloc(alloc_id).unwrap_memory(), offset.bytes())
- }
- (None, _offset) => (
- ecx.tcx.intern_const_alloc(Allocation::from_bytes_byte_aligned_immutable(
- b"" as &[u8],
- )),
- 0,
- ),
- };
+ let (data, start) =
+ match ecx.scalar_to_ptr(a.check_init().unwrap()).unwrap().into_parts() {
+ (Some(alloc_id), offset) => {
+ (ecx.tcx.global_alloc(alloc_id).unwrap_memory(), offset.bytes())
+ }
+ (None, _offset) => (
+ ecx.tcx.intern_const_alloc(
+ Allocation::from_bytes_byte_aligned_immutable(b"" as &[u8]),
+ ),
+ 0,
+ ),
+ };
  let len = b.to_machine_usize(ecx).unwrap();
  let start = start.try_into().unwrap();
  let len: usize = len.try_into().unwrap();

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -197,8 +197,8 @@ impl interpret::MayLeak for ! {
 }
 
 impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
- fn guaranteed_eq(&mut self, a: Scalar, b: Scalar) -> bool {
- match (a, b) {
+ fn guaranteed_eq(&mut self, a: Scalar, b: Scalar) -> InterpResult<'tcx, bool> {
+ Ok(match (a, b) {
  // Comparisons between integers are always known.
  (Scalar::Int { .. }, Scalar::Int { .. }) => a == b,
  // Equality with integers can never be known for sure.
@@ -207,25 +207,25 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
  // some things (like functions and vtables) do not have stable addresses
  // so we need to be careful around them (see e.g. #73722).
  (Scalar::Ptr(..), Scalar::Ptr(..)) => false,
- }
+ })
  }
 
- fn guaranteed_ne(&mut self, a: Scalar, b: Scalar) -> bool {
- match (a, b) {
+ fn guaranteed_ne(&mut self, a: Scalar, b: Scalar) -> InterpResult<'tcx, bool> {
+ Ok(match (a, b) {
  // Comparisons between integers are always known.
  (Scalar::Int(_), Scalar::Int(_)) => a != b,
  // Comparisons of abstract pointers with null pointers are known if the pointer
  // is in bounds, because if they are in bounds, the pointer can't be null.
  // Inequality with integers other than null can never be known for sure.
  (Scalar::Int(int), ptr @ Scalar::Ptr(..))
  | (ptr @ Scalar::Ptr(..), Scalar::Int(int)) => {
- int.is_null() && !self.scalar_may_be_null(ptr)
+ int.is_null() && !self.scalar_may_be_null(ptr)?
  }
  // FIXME: return `true` for at least some comparisons where we can reliably
  // determine the result of runtime inequality tests at compile-time.
  // Examples include comparison of addresses in different static items.
  (Scalar::Ptr(..), Scalar::Ptr(..)) => false,
- }
+ })
  }
 }
 
@@ -329,9 +329,9 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
  let a = ecx.read_immediate(&args[0])?.to_scalar()?;
  let b = ecx.read_immediate(&args[1])?.to_scalar()?;
  let cmp = if intrinsic_name == sym::ptr_guaranteed_eq {
- ecx.guaranteed_eq(a, b)
+ ecx.guaranteed_eq(a, b)?
  } else {
- ecx.guaranteed_ne(a, b)
+ ecx.guaranteed_ne(a, b)?
  };
  ecx.write_scalar(Scalar::from_bool(cmp), dest)?;
  }

compiler/rustc_const_eval/src/interpret/cast.rs

@@ -283,7 +283,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
  if let Some(entry_idx) = vptr_entry_idx {
  let entry_idx = u64::try_from(entry_idx).unwrap();
  let (old_data, old_vptr) = val.to_scalar_pair()?;
- let old_vptr = self.scalar_to_ptr(old_vptr);
+ let old_vptr = self.scalar_to_ptr(old_vptr)?;
  let new_vptr = self
  .read_new_vtable_after_trait_upcasting_from_vtable(old_vptr, entry_idx)?;
  self.write_immediate(Immediate::new_dyn_trait(old_data, new_vptr, self), dest)

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -640,7 +640,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
  Ok(Some((size, align)))
  }
  ty::Dynamic(..) => {
- let vtable = self.scalar_to_ptr(metadata.unwrap_meta());
+ let vtable = self.scalar_to_ptr(metadata.unwrap_meta())?;
  // Read size and align from vtable (already checks size).
  Ok(Some(self.read_size_and_align_from_vtable(vtable)?))
  }

compiler/rustc_const_eval/src/interpret/intern.rs

@@ -202,7 +202,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: CompileTimeMachine<'mir, 'tcx, const_eval::Memory
  if let ty::Dynamic(..) =
  tcx.struct_tail_erasing_lifetimes(referenced_ty, self.ecx.param_env).kind()
  {
- let ptr = self.ecx.scalar_to_ptr(mplace.meta.unwrap_meta());
+ let ptr = self.ecx.scalar_to_ptr(mplace.meta.unwrap_meta())?;
  if let Some(alloc_id) = ptr.provenance {
  // Explicitly choose const mode here, since vtables are immutable, even
  // if the reference of the fat pointer is mutable.

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -1102,30 +1102,38 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
 
 /// Machine pointer introspection.
 impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
- pub fn scalar_to_ptr(&self, scalar: Scalar<M::PointerTag>) -> Pointer<Option<M::PointerTag>> {
+ pub fn scalar_to_ptr(
+ &self,
+ scalar: Scalar<M::PointerTag>,
+ ) -> InterpResult<'tcx, Pointer<Option<M::PointerTag>>> {
  // We use `to_bits_or_ptr_internal` since we are just implementing the method people need to
  // call to force getting out a pointer.
- match scalar.to_bits_or_ptr_internal(self.pointer_size()) {
- Err(ptr) => ptr.into(),
- Ok(bits) => {
- let addr = u64::try_from(bits).unwrap();
- let ptr = M::ptr_from_addr(&self, addr);
- if addr == 0 {
- assert!(ptr.provenance.is_none(), "null pointer can never have an AllocId");
+ Ok(
+ match scalar
+ .to_bits_or_ptr_internal(self.pointer_size())
+ .map_err(|s| err_ub!(ScalarSizeMismatch(s)))?
+ {
+ Err(ptr) => ptr.into(),
+ Ok(bits) => {
+ let addr = u64::try_from(bits).unwrap();
+ let ptr = M::ptr_from_addr(&self, addr);
+ if addr == 0 {
+ assert!(ptr.provenance.is_none(), "null pointer can never have an AllocId");
+ }
+ ptr
  }
- ptr
- }
- }
+ },
+ )
  }
 
  /// Test if this value might be null.
  /// If the machine does not support ptr-to-int casts, this is conservative.
- pub fn scalar_may_be_null(&self, scalar: Scalar<M::PointerTag>) -> bool {
- match scalar.try_to_int() {
+ pub fn scalar_may_be_null(&self, scalar: Scalar<M::PointerTag>) -> InterpResult<'tcx, bool> {
+ Ok(match scalar.try_to_int() {
  Ok(int) => int.is_null(),
  Err(_) => {
  // Can only happen during CTFE.
- let ptr = self.scalar_to_ptr(scalar);
+ let ptr = self.scalar_to_ptr(scalar)?;
  match self.ptr_try_get_alloc_id(ptr) {
  Ok((alloc_id, offset, _)) => {
  let (size, _align) = self
@@ -1138,7 +1146,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
  Err(_offset) => bug!("a non-int scalar is always a pointer"),
  }
  }
- }
+ })
  }
 
  /// Turning a "maybe pointer" into a proper pointer (and some information

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -342,7 +342,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
  &self,
  op: &OpTy<'tcx, M::PointerTag>,
  ) -> InterpResult<'tcx, Pointer<Option<M::PointerTag>>> {
- Ok(self.scalar_to_ptr(self.read_scalar(op)?.check_init()?))
+ self.scalar_to_ptr(self.read_scalar(op)?.check_init()?)
  }
 
  // Turn the wide MPlace into a string (must already be dereferenced!)
@@ -738,7 +738,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
  // okay. Everything else, we conservatively reject.
  let ptr_valid = niche_start == 0
  && variants_start == variants_end
- && !self.scalar_may_be_null(tag_val);
+ && !self.scalar_may_be_null(tag_val)?;
  if !ptr_valid {
  throw_ub!(InvalidTag(dbg_val))
  }

compiler/rustc_const_eval/src/interpret/place.rs

@@ -281,7 +281,7 @@ where
  };
 
  let mplace = MemPlace {
- ptr: self.scalar_to_ptr(ptr.check_init()?),
+ ptr: self.scalar_to_ptr(ptr.check_init()?)?,
  // We could use the run-time alignment here. For now, we do not, because
  // the point of tracking the alignment here is to make sure that the *static*
  // alignment information emitted with the loads is correct. The run-time
@@ -1104,7 +1104,7 @@ where
  &self,
  mplace: &MPlaceTy<'tcx, M::PointerTag>,
  ) -> InterpResult<'tcx, (ty::Instance<'tcx>, MPlaceTy<'tcx, M::PointerTag>)> {
- let vtable = self.scalar_to_ptr(mplace.vtable()); // also sanity checks the type
+ let vtable = self.scalar_to_ptr(mplace.vtable())?; // also sanity checks the type
  let (instance, ty) = self.read_drop_type_from_vtable(vtable)?;
  let layout = self.layout_of(ty)?;
 

compiler/rustc_const_eval/src/interpret/terminator.rs

@@ -519,7 +519,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
  .kind(),
  ty::Dynamic(..)
  ));
- let vtable = self.scalar_to_ptr(receiver_place.meta.unwrap_meta());
+ let vtable = self.scalar_to_ptr(receiver_place.meta.unwrap_meta())?;
  let fn_val = self.get_vtable_slot(vtable, u64::try_from(idx).unwrap())?;
 
  // `*mut receiver_place.layout.ty` is almost the layout that we

compiler/rustc_const_eval/src/interpret/traits.rs

@@ -50,7 +50,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
  let vtable_slot = self
  .get_ptr_alloc(vtable_slot, ptr_size, self.tcx.data_layout.pointer_align.abi)?
  .expect("cannot be a ZST");
- let fn_ptr = self.scalar_to_ptr(vtable_slot.read_ptr_sized(Size::ZERO)?.check_init()?);
+ let fn_ptr = self.scalar_to_ptr(vtable_slot.read_ptr_sized(Size::ZERO)?.check_init()?)?;
  self.get_ptr_fn(fn_ptr)
  }
 
@@ -75,7 +75,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
  .check_init()?;
  // We *need* an instance here, no other kind of function value, to be able
  // to determine the type.
- let drop_instance = self.get_ptr_fn(self.scalar_to_ptr(drop_fn))?.as_instance()?;
+ let drop_instance = self.get_ptr_fn(self.scalar_to_ptr(drop_fn)?)?.as_instance()?;
  trace!("Found drop fn: {:?}", drop_instance);
  let fn_sig = drop_instance.ty(*self.tcx, self.param_env).fn_sig(*self.tcx);
  let fn_sig = self.tcx.normalize_erasing_late_bound_regions(self.param_env, fn_sig);
@@ -132,7 +132,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
  .get_ptr_alloc(vtable_slot, pointer_size, self.tcx.data_layout.pointer_align.abi)?
  .expect("cannot be a ZST");
 
- let new_vtable = self.scalar_to_ptr(new_vtable.read_ptr_sized(Size::ZERO)?.check_init()?);
+ let new_vtable =
+ self.scalar_to_ptr(new_vtable.read_ptr_sized(Size::ZERO)?.check_init()?)?;
 
  Ok(new_vtable)
  }

compiler/rustc_const_eval/src/interpret/validity.rs

@@ -312,7 +312,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
  let tail = self.ecx.tcx.struct_tail_erasing_lifetimes(pointee.ty, self.ecx.param_env);
  match tail.kind() {
  ty::Dynamic(..) => {
- let vtable = self.ecx.scalar_to_ptr(meta.unwrap_meta());
+ let vtable = self.ecx.scalar_to_ptr(meta.unwrap_meta())?;
  // Direct call to `check_ptr_access_align` checks alignment even on CTFE machines.
  try_validation!(
  self.ecx.check_ptr_access_align(
@@ -577,7 +577,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
 
  // If we check references recursively, also check that this points to a function.
  if let Some(_) = self.ref_tracking {
- let ptr = self.ecx.scalar_to_ptr(value);
+ let ptr = self.ecx.scalar_to_ptr(value)?;
  let _fn = try_validation!(
  self.ecx.get_ptr_fn(ptr),
  self.path,
@@ -590,7 +590,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
  // FIXME: Check if the signature matches
  } else {
  // Otherwise (for standalone Miri), we have to still check it to be non-null.
- if self.ecx.scalar_may_be_null(value) {
+ if self.ecx.scalar_may_be_null(value)? {
  throw_validation_failure!(self.path, { "a null function pointer" });
  }
  }
@@ -667,7 +667,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
  // We support 2 kinds of ranges here: full range, and excluding zero.
  if start == 1 && end == max_value {
  // Only null is the niche. So make sure the ptr is NOT null.
- if self.ecx.scalar_may_be_null(value) {
+ if self.ecx.scalar_may_be_null(value)? {
  throw_validation_failure!(self.path,
  { "a potentially null pointer" }
  expected {

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

@@ -15,8 +15,8 @@ use rustc_target::abi::{Align, HasDataLayout, Size};
 
 use super::{
  read_target_uint, write_target_uint, AllocId, InterpError, InterpResult, Pointer, Provenance,
- ResourceExhaustionInfo, Scalar, ScalarMaybeUninit, UndefinedBehaviorInfo, UninitBytesAccess,
- UnsupportedOpInfo,
+ ResourceExhaustionInfo, Scalar, ScalarMaybeUninit, ScalarSizeMismatch, UndefinedBehaviorInfo,
+ UninitBytesAccess, UnsupportedOpInfo,
 };
 use crate::ty;
 
@@ -81,6 +81,8 @@ impl<'tcx, Tag, Extra> ConstAllocation<'tcx, Tag, Extra> {
 /// is added when converting to `InterpError`.
 #[derive(Debug)]
 pub enum AllocError {
+ /// A scalar had the wrong size.
+ ScalarSizeMismatch(ScalarSizeMismatch),
  /// Encountered a pointer where we needed raw bytes.
  ReadPointerAsBytes,
  /// Partially overwriting a pointer.
@@ -90,10 +92,19 @@ pub enum AllocError {
 }
 pub type AllocResult<T = ()> = Result<T, AllocError>;
 
+impl From<ScalarSizeMismatch> for AllocError {
+ fn from(s: ScalarSizeMismatch) -> Self {
+ AllocError::ScalarSizeMismatch(s)
+ }
+}
+
 impl AllocError {
  pub fn to_interp_error<'tcx>(self, alloc_id: AllocId) -> InterpError<'tcx> {
  use AllocError::*;
  match self {
+ ScalarSizeMismatch(s) => {
+ InterpError::UndefinedBehavior(UndefinedBehaviorInfo::ScalarSizeMismatch(s))
+ }
  ReadPointerAsBytes => InterpError::Unsupported(UnsupportedOpInfo::ReadPointerAsBytes),
  PartialPointerOverwrite(offset) => InterpError::Unsupported(
  UnsupportedOpInfo::PartialPointerOverwrite(Pointer::new(alloc_id, offset)),
@@ -425,7 +436,7 @@ impl<Tag: Provenance, Extra> Allocation<Tag, Extra> {
 
  // `to_bits_or_ptr_internal` is the right method because we just want to store this data
  // as-is into memory.
- let (bytes, provenance) = match val.to_bits_or_ptr_internal(range.size) {
+ let (bytes, provenance) = match val.to_bits_or_ptr_internal(range.size)? {
  Err(val) => {
  let (provenance, offset) = val.into_parts();
  (u128::from(offset.bytes()), Some(provenance))

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

@@ -221,6 +221,13 @@ pub struct UninitBytesAccess {
  pub uninit_size: Size,
 }
 
+/// Information about a size mismatch.
+#[derive(Debug)]
+pub struct ScalarSizeMismatch {
+ pub target_size: u64,
+ pub data_size: u64,
+}
+
 /// Error information for when the program caused Undefined Behavior.
 pub enum UndefinedBehaviorInfo<'tcx> {
  /// Free-form case. Only for errors that are never caught!
@@ -298,10 +305,7 @@ pub enum UndefinedBehaviorInfo<'tcx> {
  /// Working with a local that is not currently live.
  DeadLocal,
  /// Data size is not equal to target size.
- ScalarSizeMismatch {
- target_size: u64,
- data_size: u64,
- },
+ ScalarSizeMismatch(ScalarSizeMismatch),
  /// A discriminant of an uninhabited enum variant is written.
  UninhabitedEnumVariantWritten,
 }
@@ -408,7 +412,7 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> {
  "using uninitialized data, but this operation requires initialized memory"
  ),
  DeadLocal => write!(f, "accessing a dead local variable"),
- ScalarSizeMismatch { target_size, data_size } => write!(
+ ScalarSizeMismatch(self::ScalarSizeMismatch { target_size, data_size }) => write!(
  f,
  "scalar size mismatch: expected {} bytes but got {} bytes instead",
  target_size, data_size

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

@@ -120,7 +120,8 @@ use crate::ty::{self, Instance, Ty, TyCtxt};
 pub use self::error::{
  struct_error, CheckInAllocMsg, ErrorHandled, EvalToAllocationRawResult, EvalToConstValueResult,
  InterpError, InterpErrorInfo, InterpResult, InvalidProgramInfo, MachineStopType,
- ResourceExhaustionInfo, UndefinedBehaviorInfo, UninitBytesAccess, UnsupportedOpInfo,
+ ResourceExhaustionInfo, ScalarSizeMismatch, UndefinedBehaviorInfo, UninitBytesAccess,
+ UnsupportedOpInfo,
 };
 
 pub use self::value::{get_slice_bytes, ConstAlloc, ConstValue, Scalar, ScalarMaybeUninit};