Skip to content

Commit e8ffa21

Browse files
committed
better document const-pattern dynamic soundness checks, and fix a soundness hole
1 parent db98d32 commit e8ffa21

File tree

3 files changed

+24
-9
lines changed

3 files changed

+24
-9
lines changed

src/librustc_mir/const_eval/eval_queries.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ fn validate_and_turn_into_const<'tcx>(
193193
mplace.into(),
194194
path,
195195
&mut ref_tracking,
196-
/*may_ref_to_static*/ is_static,
196+
/*may_ref_to_static*/ ecx.memory.extra.can_access_statics,
197197
)?;
198198
}
199199
}

src/librustc_mir/const_eval/machine.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,12 @@ pub struct CompileTimeInterpreter<'mir, 'tcx> {
9999

100100
#[derive(Copy, Clone, Debug)]
101101
pub struct MemoryExtra {
102-
/// Whether this machine may read from statics
102+
/// We need to make sure consts never point to anything mutable, even recursively. That is
103+
/// relied on for pattern matching on consts with references.
104+
/// To achieve this, two pieces have to work together:
105+
/// * Interning makes everything outside of statics immutable.
106+
/// * Pointers to allocations inside of statics can never leak outside, to a non-static global.
107+
/// This boolean here controls the second part.
103108
pub(super) can_access_statics: bool,
104109
}
105110

@@ -337,6 +342,8 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
337342
} else if static_def_id.is_some() {
338343
// Machine configuration does not allow us to read statics
339344
// (e.g., `const` initializer).
345+
// See const_eval::machine::MemoryExtra::can_access_statics for why
346+
// this check is so important.
340347
Err(ConstEvalErrKind::ConstAccessesStatic.into())
341348
} else {
342349
// Immutable global, this read is fine.

src/librustc_mir/interpret/validity.rs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -404,19 +404,27 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
404404
// Skip validation entirely for some external statics
405405
let alloc_kind = self.ecx.tcx.alloc_map.lock().get(ptr.alloc_id);
406406
if let Some(GlobalAlloc::Static(did)) = alloc_kind {
407-
// `extern static` cannot be validated as they have no body.
408-
// FIXME: Statics from other crates are also skipped.
409-
// They might be checked at a different type, but for now we
410-
// want to avoid recursing too deeply. This is not sound!
411-
if !did.is_local() || self.ecx.tcx.is_foreign_item(did) {
412-
return Ok(());
413-
}
407+
// See const_eval::machine::MemoryExtra::can_access_statics for why
408+
// this check is so important.
409+
// This check is reachable when the const just referenced the static,
410+
// but never read it (so we never entered `before_access_global`).
411+
// We also need to do it here instead of going on to avoid running
412+
// into the `before_access_global` check during validation.
414413
if !self.may_ref_to_static && self.ecx.tcx.is_static(did) {
415414
throw_validation_failure!(
416415
format_args!("a {} pointing to a static variable", kind),
417416
self.path
418417
);
419418
}
419+
// `extern static` cannot be validated as they have no body.
420+
// FIXME: Statics from other crates are also skipped.
421+
// They might be checked at a different type, but for now we
422+
// want to avoid recursing too deeply. We might miss const-invalid data,
423+
// but things are still sound otherwise (in particular re: consts
424+
// referring to statics).
425+
if !did.is_local() || self.ecx.tcx.is_foreign_item(did) {
426+
return Ok(());
427+
}
420428
}
421429
}
422430
// Proceed recursively even for ZST, no reason to skip them!

0 commit comments

Comments
 (0)