]> git.lizzy.rs Git - rust.git/commitdiff
Rollup merge of #70947 - RalfJung:ctfe-no-read-mut-global, r=oli-obk
authorDylan DPC <dylan.dpc@gmail.com>
Tue, 14 Apr 2020 21:29:52 +0000 (23:29 +0200)
committerGitHub <noreply@github.com>
Tue, 14 Apr 2020 21:29:52 +0000 (23:29 +0200)
tighten CTFE safety net for accesses to globals

Previously, we only rejected reading from all statics. Now we also reject reading from any mutable global. Mutable globals are the true culprit here as their run-time value might be different from their compile-time values. Statics are just the approximation we use for that so far.

Also refactor the code a bit to make it clearer what is being checked and allowed.

r? @oli-obk

1  2 
src/librustc_mir/const_eval/machine.rs

index 3f9aa9ed02d2aeee76907cf7f94bed3032b7478b,3ddff6ea8ca4c3dddc85a287839025aa68461969..a1ab9a1c342c17a8ebc645a3250cedaaa7c08af9
@@@ -179,12 -179,9 +179,12 @@@ impl<'mir, 'tcx> interpret::Machine<'mi
  
      const GLOBAL_KIND: Option<!> = None; // no copying of globals from `tcx` to machine memory
  
 -    // We do not check for alignment to avoid having to carry an `Align`
 -    // in `ConstValue::ByRef`.
 -    const CHECK_ALIGN: bool = false;
 +    #[inline(always)]
 +    fn enforce_alignment(_memory_extra: &Self::MemoryExtra) -> bool {
 +        // We do not check for alignment to avoid having to carry an `Align`
 +        // in `ConstValue::ByRef`.
 +        false
 +    }
  
      #[inline(always)]
      fn enforce_validity(_ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
          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 {
+                 // Machine configuration allows us read from anything (e.g., `static` initializer).
+                 Ok(())
+             } 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(())
+             }
          }
      }
  }