From cb4cd7366415deb659641edcff695aa2e4ffb105 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 22 Aug 2022 21:25:03 -0400 Subject: [PATCH] extra sanity check against consts pointing to mutable memory --- .../src/interpret/validity.rs | 72 ++++++++++++------- .../const-points-to-static.32bit.stderr | 2 +- .../const-points-to-static.64bit.stderr | 2 +- .../const_refers_to_static2.32bit.stderr | 4 +- .../const_refers_to_static2.64bit.stderr | 4 +- ..._refers_to_static_cross_crate.32bit.stderr | 4 +- ..._refers_to_static_cross_crate.64bit.stderr | 4 +- 7 files changed, 55 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs index f1b1855c3ec74..0e60f0c7ef131 100644 --- a/compiler/rustc_const_eval/src/interpret/validity.rs +++ b/compiler/rustc_const_eval/src/interpret/validity.rs @@ -8,6 +8,7 @@ use std::convert::TryFrom; use std::fmt::Write; use std::num::NonZeroUsize; +use rustc_ast::Mutability; use rustc_data_structures::fx::FxHashSet; use rustc_hir as hir; use rustc_middle::mir::interpret::InterpError; @@ -411,34 +412,51 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' // Proceed recursively even for ZST, no reason to skip them! // `!` is a ZST and we want to validate it. if let Ok((alloc_id, _offset, _prov)) = self.ecx.ptr_try_get_alloc_id(place.ptr) { - // Special handling for pointers to statics (irrespective of their type). + // Let's see what kind of memory this points to. let alloc_kind = self.ecx.tcx.try_get_global_alloc(alloc_id); - if let Some(GlobalAlloc::Static(did)) = alloc_kind { - assert!(!self.ecx.tcx.is_thread_local_static(did)); - assert!(self.ecx.tcx.is_static(did)); - if matches!( - self.ctfe_mode, - Some(CtfeValidationMode::Const { allow_static_ptrs: false, .. }) - ) { - // See const_eval::machine::MemoryExtra::can_access_statics for why - // this check is so important. - // This check is reachable when the const just referenced the static, - // but never read it (so we never entered `before_access_global`). - throw_validation_failure!(self.path, - { "a {} pointing to a static variable", kind } - ); + match alloc_kind { + Some(GlobalAlloc::Static(did)) => { + // Special handling for pointers to statics (irrespective of their type). + assert!(!self.ecx.tcx.is_thread_local_static(did)); + assert!(self.ecx.tcx.is_static(did)); + if matches!( + self.ctfe_mode, + Some(CtfeValidationMode::Const { allow_static_ptrs: false, .. }) + ) { + // See const_eval::machine::MemoryExtra::can_access_statics for why + // this check is so important. + // This check is reachable when the const just referenced the static, + // but never read it (so we never entered `before_access_global`). + throw_validation_failure!(self.path, + { "a {} pointing to a static variable in a constant", kind } + ); + } + // 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 + // unsafe code. + // 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 + // trigger cycle errors if we try to compute the value of the other static + // and that static refers back to us. + // We might miss const-invalid data, + // but things are still sound otherwise (in particular re: consts + // referring to statics). + return Ok(()); } - // We skip checking other statics. These statics must be sound by - // themselves, and the only way to get broken statics here is by using - // unsafe code. - // 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 - // trigger cycle errors if we try to compute the value of the other static - // and that static refers back to us. - // We might miss const-invalid data, - // but things are still sound otherwise (in particular re: consts - // referring to statics). - return Ok(()); + Some(GlobalAlloc::Memory(alloc)) => { + if alloc.inner().mutability == Mutability::Mut + && matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. })) + { + // This should be unreachable, but if someone manages to copy a pointer + // out of a `static`, then that pointer might point to mutable memory, + // and we would catch that here. + throw_validation_failure!(self.path, + { "a {} pointing to mutable memory in a constant", kind } + ); + } + } + // Nothing to check for these. + None | Some(GlobalAlloc::Function(..) | GlobalAlloc::VTable(..)) => {} } } let path = &self.path; @@ -544,7 +562,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' } ty::Ref(_, ty, mutbl) => { if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. })) - && *mutbl == hir::Mutability::Mut + && *mutbl == Mutability::Mut { // A mutable reference inside a const? That does not seem right (except if it is // a ZST). diff --git a/src/test/ui/consts/const-points-to-static.32bit.stderr b/src/test/ui/consts/const-points-to-static.32bit.stderr index 97825dd0eb517..c7a435a1ee3fc 100644 --- a/src/test/ui/consts/const-points-to-static.32bit.stderr +++ b/src/test/ui/consts/const-points-to-static.32bit.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/const-points-to-static.rs:6:1 | LL | const TEST: &u8 = &MY_STATIC; - | ^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable + | ^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable in a constant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 4, align: 4) { diff --git a/src/test/ui/consts/const-points-to-static.64bit.stderr b/src/test/ui/consts/const-points-to-static.64bit.stderr index 0d4a5a8ce4f33..4d5b8eac541df 100644 --- a/src/test/ui/consts/const-points-to-static.64bit.stderr +++ b/src/test/ui/consts/const-points-to-static.64bit.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/const-points-to-static.rs:6:1 | LL | const TEST: &u8 = &MY_STATIC; - | ^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable + | ^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable in a constant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 8, align: 8) { diff --git a/src/test/ui/consts/miri_unleashed/const_refers_to_static2.32bit.stderr b/src/test/ui/consts/miri_unleashed/const_refers_to_static2.32bit.stderr index b3ad81e49bc16..14173ac9f69b4 100644 --- a/src/test/ui/consts/miri_unleashed/const_refers_to_static2.32bit.stderr +++ b/src/test/ui/consts/miri_unleashed/const_refers_to_static2.32bit.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/const_refers_to_static2.rs:11:1 | LL | const REF_INTERIOR_MUT: &usize = { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable in a constant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 4, align: 4) { @@ -13,7 +13,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/const_refers_to_static2.rs:18:1 | LL | const READ_IMMUT: &usize = { - | ^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable + | ^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable in a constant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 4, align: 4) { diff --git a/src/test/ui/consts/miri_unleashed/const_refers_to_static2.64bit.stderr b/src/test/ui/consts/miri_unleashed/const_refers_to_static2.64bit.stderr index 24bd070928265..e7e51a41856e2 100644 --- a/src/test/ui/consts/miri_unleashed/const_refers_to_static2.64bit.stderr +++ b/src/test/ui/consts/miri_unleashed/const_refers_to_static2.64bit.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/const_refers_to_static2.rs:11:1 | LL | const REF_INTERIOR_MUT: &usize = { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable in a constant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 8, align: 8) { @@ -13,7 +13,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/const_refers_to_static2.rs:18:1 | LL | const READ_IMMUT: &usize = { - | ^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable + | ^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable in a constant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 8, align: 8) { diff --git a/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.32bit.stderr b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.32bit.stderr index 20a96b57f29b9..3a22b06891629 100644 --- a/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.32bit.stderr +++ b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.32bit.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/const_refers_to_static_cross_crate.rs:12:1 | LL | const SLICE_MUT: &[u8; 1] = { - | ^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable + | ^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable in a constant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 4, align: 4) { @@ -19,7 +19,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/const_refers_to_static_cross_crate.rs:17:1 | LL | const U8_MUT: &u8 = { - | ^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable + | ^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable in a constant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 4, align: 4) { diff --git a/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.64bit.stderr b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.64bit.stderr index dfa0f76baa186..39c874d6498b2 100644 --- a/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.64bit.stderr +++ b/src/test/ui/consts/miri_unleashed/const_refers_to_static_cross_crate.64bit.stderr @@ -2,7 +2,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/const_refers_to_static_cross_crate.rs:12:1 | LL | const SLICE_MUT: &[u8; 1] = { - | ^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable + | ^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable in a constant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 8, align: 8) { @@ -19,7 +19,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/const_refers_to_static_cross_crate.rs:17:1 | LL | const U8_MUT: &u8 = { - | ^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable + | ^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a reference pointing to a static variable in a constant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior. = note: the raw bytes of the constant (size: 8, align: 8) {