Skip to content

Commit 2d936b4

Browse files
oli-obkoli-obk
committed
Deduplicate logic for checking the mutability of allocations
1 parent 61f66e8 commit 2d936b4

File tree

1 file changed

+75
-79
lines changed

1 file changed

+75
-79
lines changed

‎compiler/rustc_const_eval/src/interpret/validity.rs‎

Lines changed: 75 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use std::num::NonZero;
1010
use either::{Left, Right};
1111

1212
use hir::def::DefKind;
13+
use hir::def_id::DefId;
1314
use rustc_ast::Mutability;
1415
use rustc_data_structures::fx::FxHashSet;
1516
use rustc_hir as hir;
@@ -449,73 +450,40 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
449450
// `!` is a ZST and we want to validate it.
450451
if let Ok((alloc_id, _offset, _prov)) = self.ecx.ptr_try_get_alloc_id(place.ptr()) {
451452
let mut skip_recursive_check = false;
452-
// Let's see what kind of memory this points to.
453-
// `unwrap` since dangling pointers have already been handled.
454-
let alloc_kind = self.ecx.tcx.try_get_global_alloc(alloc_id).unwrap();
455-
let alloc_actual_mutbl = match alloc_kind {
456-
GlobalAlloc::Static(did) => {
457-
// Special handling for pointers to statics (irrespective of their type).
458-
assert!(!self.ecx.tcx.is_thread_local_static(did));
459-
assert!(self.ecx.tcx.is_static(did));
460-
let DefKind::Static { mutability, nested } = self.ecx.tcx.def_kind(did)
461-
else {
462-
bug!()
463-
};
464-
// Mode-specific checks
465-
match self.ctfe_mode {
466-
Some(
467-
CtfeValidationMode::Static { .. }
468-
| CtfeValidationMode::Promoted { .. },
469-
) => {
470-
// We skip recursively checking other statics. These statics must be sound by
471-
// themselves, and the only way to get broken statics here is by using
472-
// unsafe code.
473-
// The reasons we don't check other statics is twofold. For one, in all
474-
// sound cases, the static was already validated on its own, and second, we
475-
// trigger cycle errors if we try to compute the value of the other static
476-
// and that static refers back to us (potentially through a promoted).
477-
// This could miss some UB, but that's fine.
478-
// We still walk nested allocations, as they are fundamentally part of this validation run.
479-
// This means we will also recurse into nested statics of *other*
480-
// statics, even though we do not recurse into other statics directly.
481-
// That's somewhat inconsistent but harmless.
482-
skip_recursive_check = !nested;
483-
}
484-
Some(CtfeValidationMode::Const { .. }) => {
485-
// We can't recursively validate `extern static`, so we better reject them.
486-
if self.ecx.tcx.is_foreign_item(did) {
487-
throw_validation_failure!(self.path, ConstRefToExtern);
488-
}
489-
}
490-
None => {}
453+
let (alloc_actual_mutbl, is_static) = mutability(self.ecx, alloc_id);
454+
if let Some((did, nested)) = is_static {
455+
// Special handling for pointers to statics (irrespective of their type).
456+
assert!(!self.ecx.tcx.is_thread_local_static(did));
457+
assert!(self.ecx.tcx.is_static(did));
458+
// Mode-specific checks
459+
match self.ctfe_mode {
460+
Some(
461+
CtfeValidationMode::Static { .. } | CtfeValidationMode::Promoted { .. },
462+
) => {
463+
// We skip recursively checking other statics. These statics must be sound by
464+
// themselves, and the only way to get broken statics here is by using
465+
// unsafe code.
466+
// The reasons we don't check other statics is twofold. For one, in all
467+
// sound cases, the static was already validated on its own, and second, we
468+
// trigger cycle errors if we try to compute the value of the other static
469+
// and that static refers back to us (potentially through a promoted).
470+
// This could miss some UB, but that's fine.
471+
// We still walk nested allocations, as they are fundamentally part of this validation run.
472+
// This means we will also recurse into nested statics of *other*
473+
// statics, even though we do not recurse into other statics directly.
474+
// That's somewhat inconsistent but harmless.
475+
skip_recursive_check = !nested;
491476
}
492-
// Return alloc mutability. For "root" statics we look at the type to account for interior
493-
// mutability; for nested statics we have no type and directly use the annotated mutability.
494-
if nested {
495-
mutability
496-
} else {
497-
match mutability {
498-
Mutability::Not
499-
if !self
500-
.ecx
501-
.tcx
502-
.type_of(did)
503-
.no_bound_vars()
504-
.expect("statics should not have generic parameters")
505-
.is_freeze(*self.ecx.tcx, ty::ParamEnv::reveal_all()) =>
506-
{
507-
Mutability::Mut
508-
}
509-
_ => mutability,
477+
Some(CtfeValidationMode::Const { .. }) => {
478+
// We can't recursively validate `extern static`, so we better reject them.
479+
if self.ecx.tcx.is_foreign_item(did) {
480+
throw_validation_failure!(self.path, ConstRefToExtern);
510481
}
511482
}
483+
None => {}
512484
}
513-
GlobalAlloc::Memory(alloc) => alloc.inner().mutability,
514-
GlobalAlloc::Function(..) | GlobalAlloc::VTable(..) => {
515-
// These are immutable, we better don't allow mutable pointers here.
516-
Mutability::Not
517-
}
518-
};
485+
}
486+
519487
// Mutability check.
520488
// If this allocation has size zero, there is no actual mutability here.
521489
let (size, _align, _alloc_kind) = self.ecx.get_alloc_info(alloc_id);
@@ -713,28 +681,56 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
713681
fn in_mutable_memory(&self, op: &OpTy<'tcx, M::Provenance>) -> bool {
714682
if let Some(mplace) = op.as_mplace_or_imm().left() {
715683
if let Some(alloc_id) = mplace.ptr().provenance.and_then(|p| p.get_alloc_id()) {
716-
let mutability = match self.ecx.tcx.global_alloc(alloc_id) {
717-
GlobalAlloc::Static(did) => {
718-
let DefKind::Static { mutability, nested } = self.ecx.tcx.def_kind(did)
719-
else {
720-
bug!()
721-
};
722-
if nested {
723-
mutability
724-
} else {
725-
self.ecx.memory.alloc_map.get(alloc_id).unwrap().1.mutability
726-
}
727-
}
728-
GlobalAlloc::Memory(alloc) => alloc.inner().mutability,
729-
_ => span_bug!(self.ecx.tcx.span, "not a memory allocation"),
730-
};
731-
return mutability == Mutability::Mut;
684+
return mutability(self.ecx, alloc_id).0.is_mut();
732685
}
733686
}
734687
false
735688
}
736689
}
737690

691+
/// Returns whether the allocation is mutable, and whether it's actually a static.
692+
/// For "root" statics we look at the type to account for interior
693+
/// mutability; for nested statics we have no type and directly use the annotated mutability.
694+
fn mutability<'mir, 'tcx: 'mir>(
695+
ecx: &InterpCx<'mir, 'tcx, impl Machine<'mir, 'tcx>>,
696+
alloc_id: AllocId,
697+
) -> (Mutability, Option<(DefId, bool)>) {
698+
// Let's see what kind of memory this points to.
699+
// We're not using `try_global_alloc` since dangling pointers have already been handled.
700+
match ecx.tcx.global_alloc(alloc_id) {
701+
GlobalAlloc::Static(did) => {
702+
let DefKind::Static { mutability, nested } = ecx.tcx.def_kind(did) else { bug!() };
703+
let mutability = if nested {
704+
mutability
705+
} else {
706+
let mutability = match mutability {
707+
Mutability::Not
708+
if !ecx
709+
.tcx
710+
.type_of(did)
711+
.no_bound_vars()
712+
.expect("statics should not have generic parameters")
713+
.is_freeze(*ecx.tcx, ty::ParamEnv::reveal_all()) =>
714+
{
715+
Mutability::Mut
716+
}
717+
_ => mutability,
718+
};
719+
if let Some((_, alloc)) = ecx.memory.alloc_map.get(alloc_id) {
720+
assert_eq!(alloc.mutability, mutability);
721+
}
722+
mutability
723+
};
724+
(mutability, Some((did, nested)))
725+
}
726+
GlobalAlloc::Memory(alloc) => (alloc.inner().mutability, None),
727+
GlobalAlloc::Function(..) | GlobalAlloc::VTable(..) => {
728+
// These are immutable, we better don't allow mutable pointers here.
729+
(Mutability::Not, None)
730+
}
731+
}
732+
}
733+
738734
impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
739735
for ValidityVisitor<'rt, 'mir, 'tcx, M>
740736
{

0 commit comments

Comments
 (0)