From d69c66811184707dfdb293673e58ce6121e3714f Mon Sep 17 00:00:00 2001 From: Ralf Jung <post@ralfj.de> Date: Thu, 9 Apr 2020 11:32:43 +0200 Subject: [PATCH 1/2] tighten CTFE safety net for accesses to globals --- src/librustc_mir/const_eval/machine.rs | 31 +++++++++++++++++++------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs index e926347147894..3d0c5ffae5210 100644 --- a/src/librustc_mir/const_eval/machine.rs +++ b/src/librustc_mir/const_eval/machine.rs @@ -350,15 +350,30 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter { static_def_id: Option<DefId>, is_write: bool, ) -> InterpResult<'tcx> { - if is_write && allocation.mutability == Mutability::Not { - Err(err_ub!(WriteToReadOnly(alloc_id)).into()) - } else if is_write { - Err(ConstEvalErrKind::ModifiedGlobal.into()) - } else if memory_extra.can_access_statics || static_def_id.is_none() { - // `static_def_id.is_none()` indicates this is not a static, but a const or so. - Ok(()) + if is_write { + // Write access. These are never allowed, but we give a targeted error message. + if allocation.mutability == Mutability::Not { + Err(err_ub!(WriteToReadOnly(alloc_id)).into()) + } else { + Err(ConstEvalErrKind::ModifiedGlobal.into()) + } } else { - Err(ConstEvalErrKind::ConstAccessesStatic.into()) + // Read access. These are usually allowed, with some exceptions. + if memory_extra.can_access_statics { + // This is allowed to read from anything. + Ok(()) + } else if allocation.mutability == Mutability::Mut || static_def_id.is_some() { + // This is a potentially dangerous read. + // We *must* error on any access to a mutable global here, as the content of + // this allocation may be different now and at run-time, so if we permit reading + // now we might return the wrong value. + // We conservatively also reject all statics here, but that could be relaxed + // in the future. + Err(ConstEvalErrKind::ConstAccessesStatic.into()) + } else { + // Immutable global, this read is fine. + Ok(()) + } } } } From a1f7e9a7250aea6f415fd62e88bbcb848baf73ff Mon Sep 17 00:00:00 2001 From: Ralf Jung <post@ralfj.de> Date: Fri, 10 Apr 2020 11:28:51 +0200 Subject: [PATCH 2/2] assert that only statics can possibly be mutable --- src/librustc_mir/const_eval/machine.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs index 3d0c5ffae5210..3ddff6ea8ca4c 100644 --- a/src/librustc_mir/const_eval/machine.rs +++ b/src/librustc_mir/const_eval/machine.rs @@ -360,18 +360,18 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter { } else { // Read access. These are usually allowed, with some exceptions. if memory_extra.can_access_statics { - // This is allowed to read from anything. + // Machine configuration allows us read from anything (e.g., `static` initializer). Ok(()) - } else if allocation.mutability == Mutability::Mut || static_def_id.is_some() { - // This is a potentially dangerous read. - // We *must* error on any access to a mutable global here, as the content of - // this allocation may be different now and at run-time, so if we permit reading - // now we might return the wrong value. - // We conservatively also reject all statics here, but that could be relaxed - // in the future. + } else if static_def_id.is_some() { + // Machine configuration does not allow us to read statics + // (e.g., `const` initializer). Err(ConstEvalErrKind::ConstAccessesStatic.into()) } else { // Immutable global, this read is fine. + // But make sure we never accept a read from something mutable, that would be + // unsound. The reason is that as the content of this allocation may be different + // now and at run-time, so if we permit reading now we might return the wrong value. + assert_eq!(allocation.mutability, Mutability::Not); Ok(()) } }