Skip to content

const validation: properly ignore zero-sized UnsafeCell #143046

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
/// Returns the actual dynamic size and alignment of the place at the given type.
/// Only the "meta" (metadata) part of the place matters.
/// This can fail to provide an answer for extern types.
pub(super) fn size_and_align_of(
pub(super) fn size_and_align_from_meta(
&self,
metadata: &MemPlaceMeta<M::Provenance>,
layout: &TyAndLayout<'tcx>,
Expand All @@ -388,7 +388,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
// adjust alignment and size for them?
let field = layout.field(self, layout.fields.count() - 1);
let Some((unsized_size, mut unsized_align)) =
self.size_and_align_of(metadata, &field)?
self.size_and_align_from_meta(metadata, &field)?
else {
// A field with an extern type. We don't know the actual dynamic size
// or the alignment.
Expand Down Expand Up @@ -450,11 +450,11 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
}
}
#[inline]
pub fn size_and_align_of_mplace(
pub fn size_and_align_of_val(
&self,
mplace: &MPlaceTy<'tcx, M::Provenance>,
val: &impl Projectable<'tcx, M::Provenance>,
) -> InterpResult<'tcx, Option<(Size, Align)>> {
self.size_and_align_of(&mplace.meta(), &mplace.layout)
self.size_and_align_from_meta(&val.meta(), &val.layout())
}

/// Jump to the given block.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
// dereferenceable!
let place = self.ref_to_mplace(&self.read_immediate(&args[0])?)?;
let (size, align) = self
.size_and_align_of_mplace(&place)?
.size_and_align_of_val(&place)?
.ok_or_else(|| err_unsup_format!("`extern type` does not have known layout"))?;

let result = match intrinsic_name {
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ where
) -> InterpResult<'tcx, Option<AllocRef<'_, 'tcx, M::Provenance, M::AllocExtra, M::Bytes>>>
{
let (size, _align) = self
.size_and_align_of_mplace(mplace)?
.size_and_align_of_val(mplace)?
.unwrap_or((mplace.layout.size, mplace.layout.align.abi));
// We check alignment separately, and *after* checking everything else.
// If an access is both OOB and misaligned, we want to see the bounds error.
Expand All @@ -486,7 +486,7 @@ where
) -> InterpResult<'tcx, Option<AllocRefMut<'_, 'tcx, M::Provenance, M::AllocExtra, M::Bytes>>>
{
let (size, _align) = self
.size_and_align_of_mplace(mplace)?
.size_and_align_of_val(mplace)?
.unwrap_or((mplace.layout.size, mplace.layout.align.abi));
// We check alignment separately, and raise that error *after* checking everything else.
// If an access is both OOB and misaligned, we want to see the bounds error.
Expand Down Expand Up @@ -888,11 +888,11 @@ where
trace!("copy_op: {:?} <- {:?}: {}", *dest, src, dest.layout().ty);

let dest = dest.force_mplace(self)?;
let Some((dest_size, _)) = self.size_and_align_of_mplace(&dest)? else {
let Some((dest_size, _)) = self.size_and_align_of_val(&dest)? else {
span_bug!(self.cur_span(), "copy_op needs (dynamically) sized values")
};
if cfg!(debug_assertions) {
let src_size = self.size_and_align_of_mplace(&src)?.unwrap().0;
let src_size = self.size_and_align_of_val(&src)?.unwrap().0;
assert_eq!(src_size, dest_size, "Cannot copy differently-sized data");
} else {
// As a cheap approximation, we compare the fixed parts of the size.
Expand Down Expand Up @@ -980,7 +980,7 @@ where
kind: MemoryKind<M::MemoryKind>,
meta: MemPlaceMeta<M::Provenance>,
) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
let Some((size, align)) = self.size_and_align_of(&meta, &layout)? else {
let Some((size, align)) = self.size_and_align_from_meta(&meta, &layout)? else {
span_bug!(self.cur_span(), "cannot allocate space for `extern` type, size is not known")
};
let ptr = self.allocate_ptr(size, align, kind, AllocInit::Uninit)?;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/projection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ where
// Re-use parent metadata to determine dynamic field layout.
// With custom DSTS, this *will* execute user-defined code, but the same
// happens at run-time so that's okay.
match self.size_and_align_of(&base_meta, &field_layout)? {
match self.size_and_align_from_meta(&base_meta, &field_layout)? {
Some((_, align)) => {
// For packed types, we need to cap alignment.
let align = if let ty::Adt(def, _) = base.layout().ty.kind()
Expand Down
15 changes: 10 additions & 5 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
}
// Make sure this is dereferenceable and all.
let size_and_align = try_validation!(
self.ecx.size_and_align_of_mplace(&place),
self.ecx.size_and_align_of_val(&place),
self.path,
Ub(InvalidMeta(msg)) => match msg {
InvalidMetaKind::SliceTooBig => InvalidMetaSliceTooLarge { ptr_kind },
Expand Down Expand Up @@ -906,7 +906,7 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
let (_prov, start_offset) = mplace.ptr().into_parts();
let (size, _align) = self
.ecx
.size_and_align_of_mplace(&mplace)?
.size_and_align_of_val(&mplace)?
.unwrap_or((mplace.layout.size, mplace.layout.align.abi));
// If there is no padding at all, we can skip the rest: check for
// a single data range covering the entire value.
Expand Down Expand Up @@ -1086,8 +1086,10 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
) -> InterpResult<'tcx> {
// Special check for CTFE validation, preventing `UnsafeCell` inside unions in immutable memory.
if self.ctfe_mode.is_some_and(|c| !c.allow_immutable_unsafe_cell()) {
if !val.layout.is_zst() && !val.layout.ty.is_freeze(*self.ecx.tcx, self.ecx.typing_env)
{
// Unsized unions are currently not a thing, but let's keep this code consistent with
// the check in `visit_value`.
let zst = self.ecx.size_and_align_of_val(val)?.is_some_and(|(s, _a)| s.bytes() == 0);
if !zst && !val.layout.ty.is_freeze(*self.ecx.tcx, self.ecx.typing_env) {
if !self.in_mutable_memory(val) {
throw_validation_failure!(self.path, UnsafeCellInImmutable);
}
Expand Down Expand Up @@ -1131,7 +1133,10 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,

// Special check preventing `UnsafeCell` in the inner part of constants
if self.ctfe_mode.is_some_and(|c| !c.allow_immutable_unsafe_cell()) {
if !val.layout.is_zst()
// Exclude ZST values. We need to compute the dynamic size/align to properly
// handle slices and trait objects.
let zst = self.ecx.size_and_align_of_val(val)?.is_some_and(|(s, _a)| s.bytes() == 0);
if !zst
&& let Some(def) = val.layout.ty.ty_adt_def()
&& def.is_unsafe_cell()
{
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/gvn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1625,7 +1625,7 @@ fn op_to_prop_const<'tcx>(
// If this constant is already represented as an `Allocation`,
// try putting it into global memory to return it.
if let Either::Left(mplace) = op.as_mplace_or_imm() {
let (size, _align) = ecx.size_and_align_of_mplace(&mplace).discard_err()??;
let (size, _align) = ecx.size_and_align_of_val(&mplace).discard_err()??;

// Do not try interning a value that contains provenance.
// Due to https://github.com/rust-lang/rust/issues/79738, doing so could lead to bugs.
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@ trait EvalContextPrivExt<'tcx, 'ecx>: crate::MiriInterpCxExt<'tcx> {
info: RetagInfo, // diagnostics info about this retag
) -> InterpResult<'tcx, MPlaceTy<'tcx>> {
let this = self.eval_context_mut();
let size = this.size_and_align_of_mplace(place)?.map(|(size, _)| size);
let size = this.size_and_align_of_val(place)?.map(|(size, _)| size);
// FIXME: If we cannot determine the size (because the unsized tail is an `extern type`),
// bail out -- we cannot reasonably figure out which memory range to reborrow.
// See https://github.com/rust-lang/unsafe-code-guidelines/issues/276.
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
// - if the pointer is not reborrowed (raw pointer) then we override the size
// to do a zero-length reborrow.
let reborrow_size = this
.size_and_align_of_mplace(place)?
.size_and_align_of_val(place)?
.map(|(size, _)| size)
.unwrap_or(place.layout.size);
trace!("Creating new permission: {:?} with size {:?}", new_perm, reborrow_size);
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
trace!("visit_frozen(place={:?}, size={:?})", *place, size);
debug_assert_eq!(
size,
this.size_and_align_of_mplace(place)?
this.size_and_align_of_val(place)?
.map(|(size, _)| size)
.unwrap_or_else(|| place.layout.size)
);
Expand Down Expand Up @@ -530,7 +530,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
trace!("unsafe_cell_action on {:?}", place.ptr());
// We need a size to go on.
let unsafe_cell_size = this
.size_and_align_of_mplace(place)?
.size_and_align_of_val(place)?
.map(|(size, _)| size)
// for extern types, just cover what we can
.unwrap_or_else(|| place.layout.size);
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/consts/unsafe_cell_in_const.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//! Ensure we do not complain about zero-sized `UnsafeCell` in a const in any form.
//! See <https://github.com/rust-lang/rust/issues/142948>.

//@ check-pass
use std::cell::UnsafeCell;

const X1: &mut UnsafeCell<[i32; 0]> = UnsafeCell::from_mut(&mut []);

const X2: &mut UnsafeCell<[i32]> = UnsafeCell::from_mut(&mut []);

trait Trait {}
impl Trait for [i32; 0] {}
const X3: &mut UnsafeCell<dyn Trait> = UnsafeCell::from_mut(&mut []);

fn main() {}
Loading