fix cycle error when a static and a promoted are mutually recursive

This also now allows promoteds everywhere to point to 'extern static', because why not?
We still check that constants cannot transitively reach 'extern static' through references.
(We allow it through raw pointers.)
This commit is contained in:
Ralf Jung 2024-02-12 08:51:41 +01:00
parent 084ce5bdb5
commit 5fa69deb00
3 changed files with 29 additions and 24 deletions

View File

@ -356,22 +356,13 @@ pub fn const_validate_mplace<'mir, 'tcx>(
let mut inner = false; let mut inner = false;
while let Some((mplace, path)) = ref_tracking.todo.pop() { while let Some((mplace, path)) = ref_tracking.todo.pop() {
let mode = match ecx.tcx.static_mutability(cid.instance.def_id()) { let mode = match ecx.tcx.static_mutability(cid.instance.def_id()) {
Some(_) if cid.promoted.is_some() => { _ if cid.promoted.is_some() => CtfeValidationMode::Promoted,
// Promoteds in statics are consts that re allowed to point to statics.
CtfeValidationMode::Const {
allow_immutable_unsafe_cell: false,
allow_extern_static_ptrs: true,
}
}
Some(mutbl) => CtfeValidationMode::Static { mutbl }, // a `static` Some(mutbl) => CtfeValidationMode::Static { mutbl }, // a `static`
None => { None => {
// In normal `const` (not promoted), the outermost allocation is always only copied, // In normal `const` (not promoted), the outermost allocation is always only copied,
// so having `UnsafeCell` in there is okay despite them being in immutable memory. // so having `UnsafeCell` in there is okay despite them being in immutable memory.
let allow_immutable_unsafe_cell = cid.promoted.is_none() && !inner; let allow_immutable_unsafe_cell = cid.promoted.is_none() && !inner;
CtfeValidationMode::Const { CtfeValidationMode::Const { allow_immutable_unsafe_cell }
allow_immutable_unsafe_cell,
allow_extern_static_ptrs: false,
}
} }
}; };
ecx.const_validate_operand(&mplace.into(), path, &mut ref_tracking, mode)?; ecx.const_validate_operand(&mplace.into(), path, &mut ref_tracking, mode)?;

View File

@ -129,17 +129,20 @@ pub enum PathElem {
pub enum CtfeValidationMode { pub enum CtfeValidationMode {
/// Validation of a `static` /// Validation of a `static`
Static { mutbl: Mutability }, Static { mutbl: Mutability },
/// Validation of a `const` (including promoteds). /// Validation of a promoted.
Promoted,
/// Validation of a `const`.
/// `allow_immutable_unsafe_cell` says whether we allow `UnsafeCell` in immutable memory (which is the /// `allow_immutable_unsafe_cell` says whether we allow `UnsafeCell` in immutable memory (which is the
/// case for the top-level allocation of a `const`, where this is fine because the allocation will be /// case for the top-level allocation of a `const`, where this is fine because the allocation will be
/// copied at each use site). /// copied at each use site).
Const { allow_immutable_unsafe_cell: bool, allow_extern_static_ptrs: bool }, Const { allow_immutable_unsafe_cell: bool },
} }
impl CtfeValidationMode { impl CtfeValidationMode {
fn allow_immutable_unsafe_cell(self) -> bool { fn allow_immutable_unsafe_cell(self) -> bool {
match self { match self {
CtfeValidationMode::Static { .. } => false, CtfeValidationMode::Static { .. } => false,
CtfeValidationMode::Promoted { .. } => false,
CtfeValidationMode::Const { allow_immutable_unsafe_cell, .. } => { CtfeValidationMode::Const { allow_immutable_unsafe_cell, .. } => {
allow_immutable_unsafe_cell allow_immutable_unsafe_cell
} }
@ -149,6 +152,7 @@ impl CtfeValidationMode {
fn may_contain_mutable_ref(self) -> bool { fn may_contain_mutable_ref(self) -> bool {
match self { match self {
CtfeValidationMode::Static { mutbl } => mutbl == Mutability::Mut, CtfeValidationMode::Static { mutbl } => mutbl == Mutability::Mut,
CtfeValidationMode::Promoted { .. } => false,
CtfeValidationMode::Const { .. } => false, CtfeValidationMode::Const { .. } => false,
} }
} }
@ -476,34 +480,32 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
throw_validation_failure!(self.path, MutableRefToImmutable); throw_validation_failure!(self.path, MutableRefToImmutable);
} }
} }
// Mode-specific checks
match self.ctfe_mode { match self.ctfe_mode {
Some(CtfeValidationMode::Static { .. }) => { Some(
CtfeValidationMode::Static { .. }
| CtfeValidationMode::Promoted { .. },
) => {
// We skip recursively checking other statics. These statics must be sound by // 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 // themselves, and the only way to get broken statics here is by using
// unsafe code. // unsafe code.
// The reasons we don't check other statics is twofold. For one, in all // 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 // 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 // trigger cycle errors if we try to compute the value of the other static
// and that static refers back to us. // and that static refers back to us (potentially through a promoted).
// This could miss some UB, but that's fine. // This could miss some UB, but that's fine.
return Ok(()); return Ok(());
} }
Some(CtfeValidationMode::Const { Some(CtfeValidationMode::Const { .. }) => {
allow_extern_static_ptrs, ..
}) => {
// For consts on the other hand we have to recursively check; // For consts on the other hand we have to recursively check;
// pattern matching assumes a valid value. However we better make // pattern matching assumes a valid value. However we better make
// sure this is not mutable. // sure this is not mutable.
if is_mut { if is_mut {
throw_validation_failure!(self.path, ConstRefToMutable); throw_validation_failure!(self.path, ConstRefToMutable);
} }
// We can't recursively validate `extern static`, so we better reject them.
if self.ecx.tcx.is_foreign_item(did) { if self.ecx.tcx.is_foreign_item(did) {
if !allow_extern_static_ptrs { throw_validation_failure!(self.path, ConstRefToExtern);
throw_validation_failure!(self.path, ConstRefToExtern);
} else {
// We can't validate this...
return Ok(());
}
} }
} }
None => {} None => {}

View File

@ -0,0 +1,12 @@
// check-pass
struct Value {
values: &'static [&'static Value],
}
// This `static` recursively points to itself through a promoted (the slice).
static VALUE: Value = Value {
values: &[&VALUE],
};
fn main() {}