Skip to content

Commit

Permalink
Deduplicate logic for checking the mutability of allocations
Browse files Browse the repository at this point in the history
  • Loading branch information
@oli-obk
oli-obk committed Apr 4, 2024
1 parent 61f66e8 commit 2d936b4
Showing 1 changed file with 75 additions and 79 deletions.
154 changes: 75 additions & 79 deletions compiler/rustc_const_eval/src/interpret/validity.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use std::num::NonZero;
use either::{Left, Right};

use hir::def::DefKind;
use hir::def_id::DefId;
use rustc_ast::Mutability;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
Expand Down Expand Up @@ -449,73 +450,40 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
// `!` is a ZST and we want to validate it.
if let Ok((alloc_id, _offset, _prov)) = self.ecx.ptr_try_get_alloc_id(place.ptr()) {
let mut skip_recursive_check = false;
// Let's see what kind of memory this points to.
// `unwrap` since dangling pointers have already been handled.
let alloc_kind = self.ecx.tcx.try_get_global_alloc(alloc_id).unwrap();
let alloc_actual_mutbl = match alloc_kind {
GlobalAlloc::Static(did) => {
// Special handling for pointers to statics (irrespective of their type).
assert!(!self.ecx.tcx.is_thread_local_static(did));
assert!(self.ecx.tcx.is_static(did));
let DefKind::Static { mutability, nested } = self.ecx.tcx.def_kind(did)
else {
bug!()
};
// Mode-specific checks
match self.ctfe_mode {
Some(
CtfeValidationMode::Static { .. }
| CtfeValidationMode::Promoted { .. },
) => {
// We skip recursively checking other statics. These statics must be sound by
// themselves, and the only way to get broken statics here is by using
// unsafe code.
// The reasons we don't check other statics is twofold. For one, in all
// sound cases, the static was already validated on its own, and second, we
// trigger cycle errors if we try to compute the value of the other static
// and that static refers back to us (potentially through a promoted).
// This could miss some UB, but that's fine.
// We still walk nested allocations, as they are fundamentally part of this validation run.
// This means we will also recurse into nested statics of *other*
// statics, even though we do not recurse into other statics directly.
// That's somewhat inconsistent but harmless.
skip_recursive_check = !nested;
}
Some(CtfeValidationMode::Const { .. }) => {
// We can't recursively validate `extern static`, so we better reject them.
if self.ecx.tcx.is_foreign_item(did) {
throw_validation_failure!(self.path, ConstRefToExtern);
}
}
None => {}
let (alloc_actual_mutbl, is_static) = mutability(self.ecx, alloc_id);
if let Some((did, nested)) = is_static {
// Special handling for pointers to statics (irrespective of their type).
assert!(!self.ecx.tcx.is_thread_local_static(did));
assert!(self.ecx.tcx.is_static(did));
// Mode-specific checks
match self.ctfe_mode {
Some(
CtfeValidationMode::Static { .. } | CtfeValidationMode::Promoted { .. },
) => {
// We skip recursively checking other statics. These statics must be sound by
// themselves, and the only way to get broken statics here is by using
// unsafe code.
// The reasons we don't check other statics is twofold. For one, in all
// sound cases, the static was already validated on its own, and second, we
// trigger cycle errors if we try to compute the value of the other static
// and that static refers back to us (potentially through a promoted).
// This could miss some UB, but that's fine.
// We still walk nested allocations, as they are fundamentally part of this validation run.
// This means we will also recurse into nested statics of *other*
// statics, even though we do not recurse into other statics directly.
// That's somewhat inconsistent but harmless.
skip_recursive_check = !nested;
}
// Return alloc mutability. For "root" statics we look at the type to account for interior
// mutability; for nested statics we have no type and directly use the annotated mutability.
if nested {
mutability
} else {
match mutability {
Mutability::Not
if !self
.ecx
.tcx
.type_of(did)
.no_bound_vars()
.expect("statics should not have generic parameters")
.is_freeze(*self.ecx.tcx, ty::ParamEnv::reveal_all()) =>
{
Mutability::Mut
}
_ => mutability,
Some(CtfeValidationMode::Const { .. }) => {
// We can't recursively validate `extern static`, so we better reject them.
if self.ecx.tcx.is_foreign_item(did) {
throw_validation_failure!(self.path, ConstRefToExtern);
}
}
None => {}
}
GlobalAlloc::Memory(alloc) => alloc.inner().mutability,
GlobalAlloc::Function(..) | GlobalAlloc::VTable(..) => {
// These are immutable, we better don't allow mutable pointers here.
Mutability::Not
}
};
}

// Mutability check.
// If this allocation has size zero, there is no actual mutability here.
let (size, _align, _alloc_kind) = self.ecx.get_alloc_info(alloc_id);
Expand Down Expand Up @@ -713,28 +681,56 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
fn in_mutable_memory(&self, op: &OpTy<'tcx, M::Provenance>) -> bool {
if let Some(mplace) = op.as_mplace_or_imm().left() {
if let Some(alloc_id) = mplace.ptr().provenance.and_then(|p| p.get_alloc_id()) {
let mutability = match self.ecx.tcx.global_alloc(alloc_id) {
GlobalAlloc::Static(did) => {
let DefKind::Static { mutability, nested } = self.ecx.tcx.def_kind(did)
else {
bug!()
};
if nested {
mutability
} else {
self.ecx.memory.alloc_map.get(alloc_id).unwrap().1.mutability
}
}
GlobalAlloc::Memory(alloc) => alloc.inner().mutability,
_ => span_bug!(self.ecx.tcx.span, "not a memory allocation"),
};
return mutability == Mutability::Mut;
return mutability(self.ecx, alloc_id).0.is_mut();
}
}
false
}
}

/// Returns whether the allocation is mutable, and whether it's actually a static.
/// For "root" statics we look at the type to account for interior
/// mutability; for nested statics we have no type and directly use the annotated mutability.
fn mutability<'mir, 'tcx: 'mir>(
ecx: &InterpCx<'mir, 'tcx, impl Machine<'mir, 'tcx>>,
alloc_id: AllocId,
) -> (Mutability, Option<(DefId, bool)>) {
// Let's see what kind of memory this points to.
// We're not using `try_global_alloc` since dangling pointers have already been handled.
match ecx.tcx.global_alloc(alloc_id) {
GlobalAlloc::Static(did) => {
let DefKind::Static { mutability, nested } = ecx.tcx.def_kind(did) else { bug!() };
let mutability = if nested {
mutability
} else {
let mutability = match mutability {
Mutability::Not
if !ecx
.tcx
.type_of(did)
.no_bound_vars()
.expect("statics should not have generic parameters")
.is_freeze(*ecx.tcx, ty::ParamEnv::reveal_all()) =>
{
Mutability::Mut
}
_ => mutability,
};
if let Some((_, alloc)) = ecx.memory.alloc_map.get(alloc_id) {
assert_eq!(alloc.mutability, mutability);
}
mutability
};
(mutability, Some((did, nested)))
}
GlobalAlloc::Memory(alloc) => (alloc.inner().mutability, None),
GlobalAlloc::Function(..) | GlobalAlloc::VTable(..) => {
// These are immutable, we better don't allow mutable pointers here.
(Mutability::Not, None)
}
}
}

impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
for ValidityVisitor<'rt, 'mir, 'tcx, M>
{
Expand Down

0 comments on commit 2d936b4

Please sign in to comment.