From 4bf6734f6ffc38745adadd5a8f407630e234685b Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 15 Mar 2020 14:54:53 +0100 Subject: [PATCH 01/15] clean up E0436 explanation --- src/librustc_error_codes/error_codes/E0436.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/librustc_error_codes/error_codes/E0436.md b/src/librustc_error_codes/error_codes/E0436.md index b1afd9e960337..48ecc49e92f54 100644 --- a/src/librustc_error_codes/error_codes/E0436.md +++ b/src/librustc_error_codes/error_codes/E0436.md @@ -1,5 +1,4 @@ -The functional record update syntax is only allowed for structs. (Struct-like -enum variants don't qualify, for example.) +The functional record update syntax was used on something other than a struct. Erroneous code example: @@ -24,7 +23,9 @@ fn one_up_competitor(competitor_frequency: PublicationFrequency) } ``` -Rewrite the expression without functional record update syntax: +The functional record update syntax is only allowed for structs (struct-like +enum variants don't qualify, for example). To fix the previous code, rewrite the +expression without functional record update syntax: ``` enum PublicationFrequency { From a7ab7b136ee347e7b80523994a060e6784b3b219 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sun, 22 Mar 2020 18:21:09 -0700 Subject: [PATCH 02/15] #[track_caller] on core::ops::{Index, IndexMut}. --- src/libcore/ops/index.rs | 2 ++ src/libcore/slice/mod.rs | 6 +++++ src/libcore/str/mod.rs | 2 ++ .../std-panic-locations.rs | 22 +++++++++++++++++++ 4 files changed, 32 insertions(+) diff --git a/src/libcore/ops/index.rs b/src/libcore/ops/index.rs index aae0691122415..64dd633f75d2b 100644 --- a/src/libcore/ops/index.rs +++ b/src/libcore/ops/index.rs @@ -65,6 +65,7 @@ pub trait Index { /// Performs the indexing (`container[index]`) operation. #[stable(feature = "rust1", since = "1.0.0")] + #[cfg_attr(not(bootstrap), track_caller)] fn index(&self, index: Idx) -> &Self::Output; } @@ -166,5 +167,6 @@ see chapter in The Book : Index { /// Performs the mutable indexing (`container[index]`) operation. #[stable(feature = "rust1", since = "1.0.0")] + #[cfg_attr(not(bootstrap), track_caller)] fn index_mut(&mut self, index: Idx) -> &mut Self::Output; } diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index 0e12e6360da95..2140a7be9efe8 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -2306,6 +2306,7 @@ impl [T] { /// assert_eq!(&bytes, b"Hello, Wello!"); /// ``` #[stable(feature = "copy_within", since = "1.37.0")] + #[track_caller] pub fn copy_within>(&mut self, src: R, dest: usize) where T: Copy, @@ -2721,18 +2722,21 @@ where #[inline(never)] #[cold] +#[track_caller] fn slice_index_len_fail(index: usize, len: usize) -> ! { panic!("index {} out of range for slice of length {}", index, len); } #[inline(never)] #[cold] +#[track_caller] fn slice_index_order_fail(index: usize, end: usize) -> ! { panic!("slice index starts at {} but ends at {}", index, end); } #[inline(never)] #[cold] +#[track_caller] fn slice_index_overflow_fail() -> ! { panic!("attempted to index slice up to maximum usize"); } @@ -2804,11 +2808,13 @@ pub trait SliceIndex: private_slice_index::Sealed { /// Returns a shared reference to the output at this location, panicking /// if out of bounds. #[unstable(feature = "slice_index_methods", issue = "none")] + #[cfg_attr(not(bootstrap), track_caller)] fn index(self, slice: &T) -> &Self::Output; /// Returns a mutable reference to the output at this location, panicking /// if out of bounds. #[unstable(feature = "slice_index_methods", issue = "none")] + #[cfg_attr(not(bootstrap), track_caller)] fn index_mut(self, slice: &mut T) -> &mut Self::Output; } diff --git a/src/libcore/str/mod.rs b/src/libcore/str/mod.rs index 6ad0e68a88f3b..013ca182c13cd 100644 --- a/src/libcore/str/mod.rs +++ b/src/libcore/str/mod.rs @@ -1794,6 +1794,7 @@ mod traits { #[inline(never)] #[cold] + #[track_caller] fn str_index_overflow_fail() -> ! { panic!("attempted to index str up to maximum usize"); } @@ -2185,6 +2186,7 @@ fn truncate_to_char_boundary(s: &str, mut max: usize) -> (bool, &str) { #[inline(never)] #[cold] +#[track_caller] fn slice_error_fail(s: &str, begin: usize, end: usize) -> ! { const MAX_DISPLAY_LENGTH: usize = 256; let (truncated, s_trunc) = truncate_to_char_boundary(s, MAX_DISPLAY_LENGTH); diff --git a/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs b/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs index be13076b8af52..35a2956ee26b8 100644 --- a/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs +++ b/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs @@ -2,10 +2,14 @@ // ignore-wasm32-bare compiled with panic=abort by default #![feature(option_expect_none, option_unwrap_none)] +#![allow(unconditional_panic)] //! Test that panic locations for `#[track_caller]` functions in std have the correct //! location reported. +use std::collections::{BTreeMap, HashMap, VecDeque}; +use std::ops::{Index, IndexMut}; + fn main() { // inspect the `PanicInfo` we receive to ensure the right file is the source std::panic::set_hook(Box::new(|info| { @@ -35,4 +39,22 @@ fn main() { let fine: Result<(), ()> = Ok(()); assert_panicked(|| fine.unwrap_err()); assert_panicked(|| fine.expect_err("")); + + let mut small = [0]; // the implementation backing str, vec, etc + assert_panicked(move || { small.index(1); }); + assert_panicked(move || { small[1]; }); + assert_panicked(move || { small.index_mut(1); }); + assert_panicked(move || { small[1] += 1; }); + + let sorted: BTreeMap = Default::default(); + assert_panicked(|| { sorted.index(&false); }); + assert_panicked(|| { sorted[&false]; }); + + let unsorted: HashMap = Default::default(); + assert_panicked(|| { unsorted.index(&false); }); + assert_panicked(|| { unsorted[&false]; }); + + let weirdo: VecDeque<()> = Default::default(); + assert_panicked(|| { weirdo.index(1); }); + assert_panicked(|| { weirdo[1]; }); } From 4dda632faf777e415977d28b9697df6cff09b01b Mon Sep 17 00:00:00 2001 From: Without Boats Date: Tue, 24 Mar 2020 00:33:25 +0100 Subject: [PATCH 03/15] IoSlice/IoSliceMut should be Send and Sync --- src/libstd/io/mod.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/libstd/io/mod.rs b/src/libstd/io/mod.rs index 83c492fecf974..4323dfff26f62 100644 --- a/src/libstd/io/mod.rs +++ b/src/libstd/io/mod.rs @@ -951,6 +951,12 @@ pub trait Read { #[repr(transparent)] pub struct IoSliceMut<'a>(sys::io::IoSliceMut<'a>); +#[stable(feature = "iovec-send-sync", since = "1.44.0")] +unsafe impl<'a> Send for IoSliceMut<'a> { } + +#[stable(feature = "iovec-send-sync", since = "1.44.0")] +unsafe impl<'a> Sync for IoSliceMut<'a> { } + #[stable(feature = "iovec", since = "1.36.0")] impl<'a> fmt::Debug for IoSliceMut<'a> { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { @@ -1054,6 +1060,12 @@ impl<'a> DerefMut for IoSliceMut<'a> { #[repr(transparent)] pub struct IoSlice<'a>(sys::io::IoSlice<'a>); +#[stable(feature = "iovec-send-sync", since = "1.43.0")] +unsafe impl<'a> Send for IoSlice<'a> { } + +#[stable(feature = "iovec-send-sync", since = "1.43.0")] +unsafe impl<'a> Sync for IoSlice<'a> { } + #[stable(feature = "iovec", since = "1.36.0")] impl<'a> fmt::Debug for IoSlice<'a> { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { From 3cc4ef93260adc89eca3799d93757ff1a4c7b43f Mon Sep 17 00:00:00 2001 From: Without Boats Date: Tue, 24 Mar 2020 00:34:48 +0100 Subject: [PATCH 04/15] correct rustc version --- src/libstd/io/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstd/io/mod.rs b/src/libstd/io/mod.rs index 4323dfff26f62..1022123347ee5 100644 --- a/src/libstd/io/mod.rs +++ b/src/libstd/io/mod.rs @@ -1060,10 +1060,10 @@ impl<'a> DerefMut for IoSliceMut<'a> { #[repr(transparent)] pub struct IoSlice<'a>(sys::io::IoSlice<'a>); -#[stable(feature = "iovec-send-sync", since = "1.43.0")] +#[stable(feature = "iovec-send-sync", since = "1.44.0")] unsafe impl<'a> Send for IoSlice<'a> { } -#[stable(feature = "iovec-send-sync", since = "1.43.0")] +#[stable(feature = "iovec-send-sync", since = "1.44.0")] unsafe impl<'a> Sync for IoSlice<'a> { } #[stable(feature = "iovec", since = "1.36.0")] From eaa6488ca7550428ce70d6f43e906a8e36def943 Mon Sep 17 00:00:00 2001 From: Jubilee <46493976+workingjubilee@users.noreply.github.com> Date: Mon, 23 Mar 2020 22:54:06 -0700 Subject: [PATCH 05/15] Request "-Z unstable-options" for unstable options Explicitly requests the "-Z unstable-options" flag if someone attempts to use a cargo option gated by it. This enhances discoverability, particularly in the instance where the user is on the nightly compiler but isn't using the flag. --- src/libtest/cli.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libtest/cli.rs b/src/libtest/cli.rs index 5317063b80d11..aac454c023c80 100644 --- a/src/libtest/cli.rs +++ b/src/libtest/cli.rs @@ -213,7 +213,7 @@ macro_rules! unstable_optflag { let opt = $matches.opt_present($option_name); if !$allow_unstable && opt { return Err(format!( - "The \"{}\" flag is only accepted on the nightly compiler", + "The \"{}\" flag is only accepted on the nightly compiler with -Z unstable-options", $option_name )); } From 9d9649adea5d1bb745b86814791e5fb72589a005 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 21 Mar 2020 19:19:10 +0100 Subject: [PATCH 06/15] move ModifiedStatic error to ConstEval errors, and generally adjust terminology from "static" to "global" where appropriate --- src/librustc/mir/interpret/error.rs | 9 -- src/librustc/ty/context.rs | 1 + src/librustc_mir/const_eval/error.rs | 6 ++ src/librustc_mir/const_eval/machine.rs | 16 +-- src/librustc_mir/interpret/eval_context.rs | 4 +- src/librustc_mir/interpret/intern.rs | 4 +- src/librustc_mir/interpret/machine.rs | 35 ++++--- src/librustc_mir/interpret/memory.rs | 110 +++++++++++---------- src/librustc_mir/interpret/operand.rs | 6 +- src/librustc_mir/interpret/place.rs | 10 +- src/librustc_mir/transform/const_prop.rs | 21 ++-- 11 files changed, 117 insertions(+), 105 deletions(-) diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index 8f06b9a69bd15..f787195245212 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -453,9 +453,6 @@ pub enum UnsupportedOpInfo { ReadForeignStatic(DefId), /// Could not find MIR for a function. NoMirFor(DefId), - /// Modified a static during const-eval. - /// FIXME: move this to `ConstEvalErrKind` through a machine hook. - ModifiedStatic, /// Encountered a pointer where we needed raw bytes. ReadPointerAsBytes, /// Encountered raw bytes where we needed a pointer. @@ -471,12 +468,6 @@ impl fmt::Debug for UnsupportedOpInfo { write!(f, "tried to read from foreign (extern) static {:?}", did) } NoMirFor(did) => write!(f, "could not load MIR for {:?}", did), - ModifiedStatic => write!( - f, - "tried to modify a static's initial value from another static's \ - initializer" - ), - ReadPointerAsBytes => write!(f, "unable to turn pointer into raw bytes",), ReadBytesAsPointer => write!(f, "unable to turn bytes into a pointer"), } diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 75842fd554941..1dc2f2a6f248b 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -984,6 +984,7 @@ pub struct GlobalCtxt<'tcx> { /// Stores the value of constants (and deduplicates the actual memory) allocation_interner: ShardedHashMap<&'tcx Allocation, ()>, + /// Stores memory for globals (statics/consts). pub alloc_map: Lock>, layout_interner: ShardedHashMap<&'tcx LayoutDetails, ()>, diff --git a/src/librustc_mir/const_eval/error.rs b/src/librustc_mir/const_eval/error.rs index dc23eba643e3e..fd46340f03ad6 100644 --- a/src/librustc_mir/const_eval/error.rs +++ b/src/librustc_mir/const_eval/error.rs @@ -12,6 +12,7 @@ use crate::interpret::{ConstEvalErr, InterpErrorInfo, Machine}; pub enum ConstEvalErrKind { NeedsRfc(String), ConstAccessesStatic, + ModifiedGlobal, AssertFailure(AssertKind), Panic { msg: Symbol, line: u32, col: u32, file: Symbol }, } @@ -33,6 +34,11 @@ impl fmt::Display for ConstEvalErrKind { write!(f, "\"{}\" needs an rfc before being allowed inside constants", msg) } ConstAccessesStatic => write!(f, "constant accesses static"), + ModifiedGlobal => write!( + f, + "modifying a static's initial value from another static's \ + initializer" + ), AssertFailure(ref msg) => write!(f, "{:?}", msg), Panic { msg, line, col, file } => { write!(f, "the evaluated program panicked at '{}', {}:{}:{}", msg, file, line, col) diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs index ab88a92ea7bc2..65dca16e739f5 100644 --- a/src/librustc_mir/const_eval/machine.rs +++ b/src/librustc_mir/const_eval/machine.rs @@ -8,8 +8,8 @@ use std::hash::Hash; use rustc_data_structures::fx::FxHashMap; use rustc::mir::AssertMessage; -use rustc_span::source_map::Span; use rustc_span::symbol::Symbol; +use rustc_span::{def_id::DefId, Span}; use crate::interpret::{ self, AllocId, Allocation, GlobalId, ImmTy, InterpCx, InterpResult, Memory, MemoryKind, OpTy, @@ -167,7 +167,7 @@ impl interpret::MayLeak for ! { } impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter { - type MemoryKinds = !; + type MemoryKind = !; type PointerTag = (); type ExtraFnVal = !; @@ -177,7 +177,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter { type MemoryMap = FxHashMap, Allocation)>; - const STATIC_KIND: Option = None; // no copying of statics allowed + const GLOBAL_KIND: Option = None; // no copying of globals allowed // We do not check for alignment to avoid having to carry an `Align` // in `ConstValue::ByRef`. @@ -317,7 +317,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter { } #[inline(always)] - fn tag_static_base_pointer(_memory_extra: &MemoryExtra, _id: AllocId) -> Self::PointerTag {} + fn tag_global_base_pointer(_memory_extra: &MemoryExtra, _id: AllocId) -> Self::PointerTag {} fn box_alloc( _ecx: &mut InterpCx<'mir, 'tcx, Self>, @@ -345,11 +345,15 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter { Ok(()) } - fn before_access_static( + fn before_access_global( memory_extra: &MemoryExtra, _allocation: &Allocation, + def_id: Option, + is_write: bool, ) -> InterpResult<'tcx> { - if memory_extra.can_access_statics { + if is_write { + Err(ConstEvalErrKind::ModifiedGlobal.into()) + } else if memory_extra.can_access_statics || def_id.is_none() { Ok(()) } else { Err(ConstEvalErrKind::ConstAccessesStatic.into()) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index ac593d0845a7d..c50146f295adb 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -253,8 +253,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// This represents a *direct* access to that memory, as opposed to access /// through a pointer that was created by the program. #[inline(always)] - pub fn tag_static_base_pointer(&self, ptr: Pointer) -> Pointer { - self.memory.tag_static_base_pointer(ptr) + pub fn tag_global_base_pointer(&self, ptr: Pointer) -> Pointer { + self.memory.tag_global_base_pointer(ptr) } #[inline(always)] diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index 90b8a4932991e..b9ed69842f1a9 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -16,7 +16,7 @@ use super::{AllocId, Allocation, InterpCx, MPlaceTy, Machine, MemoryKind, Scalar pub trait CompileTimeMachine<'mir, 'tcx> = Machine< 'mir, 'tcx, - MemoryKinds = !, + MemoryKind = !, PointerTag = (), ExtraFnVal = !, FrameExtra = (), @@ -104,7 +104,7 @@ fn intern_shallow<'rt, 'mir, 'tcx, M: CompileTimeMachine<'mir, 'tcx>>( MemoryKind::Stack | MemoryKind::Vtable | MemoryKind::CallerLocation => {} } // Set allocation mutability as appropriate. This is used by LLVM to put things into - // read-only memory, and also by Miri when evluating other constants/statics that + // read-only memory, and also by Miri when evaluating other globals that // access this one. if mode == InternMode::Static { // When `ty` is `None`, we assume no interior mutability. diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 88cb74ebf8c98..6fe7054855480 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -7,7 +7,7 @@ use std::hash::Hash; use rustc::mir; use rustc::ty::{self, Ty}; -use rustc_span::Span; +use rustc_span::{def_id::DefId, Span}; use super::{ AllocId, Allocation, AllocationExtra, Frame, ImmTy, InterpCx, InterpResult, Memory, MemoryKind, @@ -79,7 +79,7 @@ pub trait AllocMap { /// and some use case dependent behaviour can instead be applied. pub trait Machine<'mir, 'tcx>: Sized { /// Additional memory kinds a machine wishes to distinguish from the builtin ones - type MemoryKinds: ::std::fmt::Debug + MayLeak + Eq + 'static; + type MemoryKind: ::std::fmt::Debug + MayLeak + Eq + 'static; /// Tag tracked alongside every pointer. This is used to implement "Stacked Borrows" /// . @@ -105,16 +105,17 @@ pub trait Machine<'mir, 'tcx>: Sized { /// Memory's allocation map type MemoryMap: AllocMap< AllocId, - (MemoryKind, Allocation), + (MemoryKind, Allocation), > + Default + Clone; - /// The memory kind to use for copied statics -- or None if statics should not be mutated - /// and thus any such attempt will cause a `ModifiedStatic` error to be raised. + /// The memory kind to use for copied global memory (held in `tcx`) -- + /// or None if such memory should not be mutated and thus any such attempt will cause + /// a `ModifiedStatic` error to be raised. /// Statics are copied under two circumstances: When they are mutated, and when - /// `tag_allocation` or `find_foreign_static` (see below) returns an owned allocation + /// `tag_allocation` (see below) returns an owned allocation /// that is added to the memory so that the work is not done twice. - const STATIC_KIND: Option; + const GLOBAL_KIND: Option; /// Whether memory accesses should be alignment-checked. const CHECK_ALIGN: bool; @@ -207,11 +208,13 @@ pub trait Machine<'mir, 'tcx>: Sized { Ok(()) } - /// Called before a `Static` value is accessed. + /// Called before a global allocation is accessed. #[inline] - fn before_access_static( + fn before_access_global( _memory_extra: &Self::MemoryExtra, _allocation: &Allocation, + _def_id: Option, + _is_write: bool, ) -> InterpResult<'tcx> { Ok(()) } @@ -231,10 +234,10 @@ pub trait Machine<'mir, 'tcx>: Sized { /// it contains (in relocations) tagged. The way we construct allocations is /// to always first construct it without extra and then add the extra. /// This keeps uniform code paths for handling both allocations created by CTFE - /// for statics, and allocations created by Miri during evaluation. + /// for globals, and allocations created by Miri during evaluation. /// /// `kind` is the kind of the allocation being tagged; it can be `None` when - /// it's a static and `STATIC_KIND` is `None`. + /// it's a global and `GLOBAL_KIND` is `None`. /// /// This should avoid copying if no work has to be done! If this returns an owned /// allocation (because a copy had to be done to add tags or metadata), machine memory will @@ -243,20 +246,20 @@ pub trait Machine<'mir, 'tcx>: Sized { /// /// Also return the "base" tag to use for this allocation: the one that is used for direct /// accesses to this allocation. If `kind == STATIC_KIND`, this tag must be consistent - /// with `tag_static_base_pointer`. + /// with `tag_global_base_pointer`. fn init_allocation_extra<'b>( memory_extra: &Self::MemoryExtra, id: AllocId, alloc: Cow<'b, Allocation>, - kind: Option>, + kind: Option>, ) -> (Cow<'b, Allocation>, Self::PointerTag); - /// Return the "base" tag for the given *static* allocation: the one that is used for direct - /// accesses to this static/const/fn allocation. If `id` is not a static allocation, + /// Return the "base" tag for the given *global* allocation: the one that is used for direct + /// accesses to this static/const/fn allocation. If `id` is not a global allocation, /// this will return an unusable tag (i.e., accesses will be UB)! /// /// Expects `id` to be already canonical, if needed. - fn tag_static_base_pointer(memory_extra: &Self::MemoryExtra, id: AllocId) -> Self::PointerTag; + fn tag_global_base_pointer(memory_extra: &Self::MemoryExtra, id: AllocId) -> Self::PointerTag; /// Executes a retagging operation #[inline] diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 277a77af3fd56..df56b3f957782 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -80,12 +80,12 @@ pub struct Memory<'mir, 'tcx, M: Machine<'mir, 'tcx>> { /// Allocations local to this instance of the miri engine. The kind /// helps ensure that the same mechanism is used for allocation and /// deallocation. When an allocation is not found here, it is a - /// static and looked up in the `tcx` for read access. Some machines may - /// have to mutate this map even on a read-only access to a static (because + /// global and looked up in the `tcx` for read access. Some machines may + /// have to mutate this map even on a read-only access to a global (because /// they do pointer provenance tracking and the allocations in `tcx` have /// the wrong type), so we let the machine override this type. - /// Either way, if the machine allows writing to a static, doing so will - /// create a copy of the static allocation here. + /// Either way, if the machine allows writing to a global, doing so will + /// create a copy of the global allocation here. // FIXME: this should not be public, but interning currently needs access to it pub(super) alloc_map: M::MemoryMap, @@ -130,9 +130,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { /// This represents a *direct* access to that memory, as opposed to access /// through a pointer that was created by the program. #[inline] - pub fn tag_static_base_pointer(&self, ptr: Pointer) -> Pointer { + pub fn tag_global_base_pointer(&self, ptr: Pointer) -> Pointer { let id = M::canonical_alloc_id(self, ptr.alloc_id); - ptr.with_tag(M::tag_static_base_pointer(&self.extra, id)) + ptr.with_tag(M::tag_global_base_pointer(&self.extra, id)) } pub fn create_fn_alloc( @@ -149,23 +149,23 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { id } }; - self.tag_static_base_pointer(Pointer::from(id)) + self.tag_global_base_pointer(Pointer::from(id)) } pub fn allocate( &mut self, size: Size, align: Align, - kind: MemoryKind, + kind: MemoryKind, ) -> Pointer { let alloc = Allocation::undef(size, align); self.allocate_with(alloc, kind) } - pub fn allocate_static_bytes( + pub fn allocate_bytes( &mut self, bytes: &[u8], - kind: MemoryKind, + kind: MemoryKind, ) -> Pointer { let alloc = Allocation::from_byte_aligned_bytes(bytes); self.allocate_with(alloc, kind) @@ -174,13 +174,13 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { pub fn allocate_with( &mut self, alloc: Allocation, - kind: MemoryKind, + kind: MemoryKind, ) -> Pointer { let id = self.tcx.alloc_map.lock().reserve(); debug_assert_ne!( Some(kind), - M::STATIC_KIND.map(MemoryKind::Machine), - "dynamically allocating static memory" + M::GLOBAL_KIND.map(MemoryKind::Machine), + "dynamically allocating global memory" ); let (alloc, tag) = M::init_allocation_extra(&self.extra, id, Cow::Owned(alloc), Some(kind)); self.alloc_map.insert(id, (kind, alloc.into_owned())); @@ -193,7 +193,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { old_size_and_align: Option<(Size, Align)>, new_size: Size, new_align: Align, - kind: MemoryKind, + kind: MemoryKind, ) -> InterpResult<'tcx, Pointer> { if ptr.offset.bytes() != 0 { throw_ub_format!( @@ -215,9 +215,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { Ok(new_ptr) } - /// Deallocate a local, or do nothing if that local has been made into a static + /// Deallocate a local, or do nothing if that local has been made into a global. pub fn deallocate_local(&mut self, ptr: Pointer) -> InterpResult<'tcx> { - // The allocation might be already removed by static interning. + // The allocation might be already removed by global interning. // This can only really happen in the CTFE instance, not in miri. if self.alloc_map.contains_key(&ptr.alloc_id) { self.deallocate(ptr, None, MemoryKind::Stack) @@ -230,7 +230,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { &mut self, ptr: Pointer, old_size_and_align: Option<(Size, Align)>, - kind: MemoryKind, + kind: MemoryKind, ) -> InterpResult<'tcx> { trace!("deallocating: {}", ptr.alloc_id); @@ -244,7 +244,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { let (alloc_kind, mut alloc) = match self.alloc_map.remove(&ptr.alloc_id) { Some(alloc) => alloc, None => { - // Deallocating static memory -- always an error + // Deallocating global memory -- always an error return Err(match self.tcx.alloc_map.lock().get(ptr.alloc_id) { Some(GlobalAlloc::Function(..)) => err_ub_format!("deallocating a function"), Some(GlobalAlloc::Static(..)) | Some(GlobalAlloc::Memory(..)) => { @@ -403,43 +403,42 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { /// Allocation accessors impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { - /// Helper function to obtain the global (tcx) allocation for a static. + /// Helper function to obtain a global (tcx) allocation. /// This attempts to return a reference to an existing allocation if /// one can be found in `tcx`. That, however, is only possible if `tcx` and /// this machine use the same pointer tag, so it is indirected through /// `M::tag_allocation`. - /// - /// Notice that every static has two `AllocId` that will resolve to the same - /// thing here: one maps to `GlobalAlloc::Static`, this is the "lazy" ID, - /// and the other one is maps to `GlobalAlloc::Memory`, this is returned by - /// `const_eval_raw` and it is the "resolved" ID. - /// The resolved ID is never used by the interpreted progrma, it is hidden. - /// The `GlobalAlloc::Memory` branch here is still reachable though; when a static - /// contains a reference to memory that was created during its evaluation (i.e., not to - /// another static), those inner references only exist in "resolved" form. - /// - /// Assumes `id` is already canonical. - fn get_static_alloc( + fn get_global_alloc( memory_extra: &M::MemoryExtra, tcx: TyCtxtAt<'tcx>, id: AllocId, + is_write: bool, ) -> InterpResult<'tcx, Cow<'tcx, Allocation>> { let alloc = tcx.alloc_map.lock().get(id); - let alloc = match alloc { - Some(GlobalAlloc::Memory(mem)) => Cow::Borrowed(mem), + let (alloc, def_id) = match alloc { + Some(GlobalAlloc::Memory(mem)) => (mem, None), Some(GlobalAlloc::Function(..)) => throw_ub!(DerefFunctionPointer(id)), None => throw_ub!(PointerUseAfterFree(id)), Some(GlobalAlloc::Static(def_id)) => { - // We got a "lazy" static that has not been computed yet. + // Notice that every static has two `AllocId` that will resolve to the same + // thing here: one maps to `GlobalAlloc::Static`, this is the "lazy" ID, + // and the other one is maps to `GlobalAlloc::Memory`, this is returned by + // `const_eval_raw` and it is the "resolved" ID. + // The resolved ID is never used by the interpreted progrma, it is hidden. + // The `GlobalAlloc::Memory` branch here is still reachable though; when a static + // contains a reference to memory that was created during its evaluation (i.e., not + // to another static), those inner references only exist in "resolved" form. + // + // Assumes `id` is already canonical. if tcx.is_foreign_item(def_id) { - trace!("get_static_alloc: foreign item {:?}", def_id); + trace!("get_global_alloc: foreign item {:?}", def_id); throw_unsup!(ReadForeignStatic(def_id)) } - trace!("get_static_alloc: Need to compute {:?}", def_id); + trace!("get_global_alloc: Need to compute {:?}", def_id); let instance = Instance::mono(tcx.tcx, def_id); let gid = GlobalId { instance, promoted: None }; - // use the raw query here to break validation cycles. Later uses of the static - // will call the full query anyway + // Use the raw query here to break validation cycles. Later uses of the static + // will call the full query anyway. let raw_const = tcx.const_eval_raw(ty::ParamEnv::reveal_all().and(gid)).map_err(|err| { // no need to report anything, the const_eval call takes care of that @@ -454,18 +453,19 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { let id = raw_const.alloc_id; let allocation = tcx.alloc_map.lock().unwrap_memory(id); - M::before_access_static(memory_extra, allocation)?; - Cow::Borrowed(allocation) + (allocation, Some(def_id)) } }; + M::before_access_global(memory_extra, alloc, def_id, is_write)?; + let alloc = Cow::Borrowed(alloc); // We got tcx memory. Let the machine initialize its "extra" stuff. let (alloc, tag) = M::init_allocation_extra( memory_extra, id, // always use the ID we got as input, not the "hidden" one. alloc, - M::STATIC_KIND.map(MemoryKind::Machine), + M::GLOBAL_KIND.map(MemoryKind::Machine), ); - debug_assert_eq!(tag, M::tag_static_base_pointer(memory_extra, id)); + debug_assert_eq!(tag, M::tag_global_base_pointer(memory_extra, id)); Ok(alloc) } @@ -478,10 +478,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { let id = M::canonical_alloc_id(self, id); // The error type of the inner closure here is somewhat funny. We have two // ways of "erroring": An actual error, or because we got a reference from - // `get_static_alloc` that we can actually use directly without inserting anything anywhere. + // `get_global_alloc` that we can actually use directly without inserting anything anywhere. // So the error type is `InterpResult<'tcx, &Allocation>`. let a = self.alloc_map.get_or(id, || { - let alloc = Self::get_static_alloc(&self.extra, self.tcx, id).map_err(Err)?; + let alloc = Self::get_global_alloc(&self.extra, self.tcx, id, /*is_write*/ false) + .map_err(Err)?; match alloc { Cow::Borrowed(alloc) => { // We got a ref, cheaply return that as an "error" so that the @@ -490,8 +491,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } Cow::Owned(alloc) => { // Need to put it into the map and return a ref to that - let kind = M::STATIC_KIND.expect( - "I got an owned allocation that I have to copy but the machine does \ + let kind = M::GLOBAL_KIND.expect( + "I got a global allocation that I have to copy but the machine does \ not expect that to happen", ); Ok((MemoryKind::Machine(kind), alloc)) @@ -515,16 +516,17 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { let tcx = self.tcx; let memory_extra = &self.extra; let a = self.alloc_map.get_mut_or(id, || { - // Need to make a copy, even if `get_static_alloc` is able + // Need to make a copy, even if `get_global_alloc` is able // to give us a cheap reference. - let alloc = Self::get_static_alloc(memory_extra, tcx, id)?; + let alloc = Self::get_global_alloc(memory_extra, tcx, id, /*is_write*/ true)?; if alloc.mutability == Mutability::Not { throw_ub!(WriteToReadOnly(id)) } - match M::STATIC_KIND { - Some(kind) => Ok((MemoryKind::Machine(kind), alloc.into_owned())), - None => throw_unsup!(ModifiedStatic), - } + let kind = M::GLOBAL_KIND.expect( + "I got a global allocation that I have to copy but the machine does \ + not expect that to happen", + ); + Ok((MemoryKind::Machine(kind), alloc.into_owned())) }); // Unpack the error type manually because type inference doesn't // work otherwise (and we cannot help it because `impl Trait`) @@ -553,7 +555,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // # Regular allocations // Don't use `self.get_raw` here as that will // a) cause cycles in case `id` refers to a static - // b) duplicate a static's allocation in miri + // b) duplicate a global's allocation in miri if let Some((_, alloc)) = self.alloc_map.get(id) { return Ok((alloc.size, alloc.align)); } @@ -728,7 +730,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { ); } Err(()) => { - // static alloc? + // global alloc? match self.tcx.alloc_map.lock().get(id) { Some(GlobalAlloc::Memory(alloc)) => { self.dump_alloc_helper( diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 4c82172ae4517..90fb7eb2bb3ac 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -517,7 +517,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { layout: Option>, ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> { let tag_scalar = |scalar| match scalar { - Scalar::Ptr(ptr) => Scalar::Ptr(self.tag_static_base_pointer(ptr)), + Scalar::Ptr(ptr) => Scalar::Ptr(self.tag_global_base_pointer(ptr)), Scalar::Raw { data, size } => Scalar::Raw { data, size }, }; // Early-return cases. @@ -547,7 +547,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let id = self.tcx.alloc_map.lock().create_memory_alloc(alloc); // We rely on mutability being set correctly in that allocation to prevent writes // where none should happen. - let ptr = self.tag_static_base_pointer(Pointer::new(id, offset)); + let ptr = self.tag_global_base_pointer(Pointer::new(id, offset)); Operand::Indirect(MemPlace::from_ptr(ptr, layout.align.abi)) } ConstValue::Scalar(x) => Operand::Immediate(tag_scalar(x).into()), @@ -559,7 +559,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Size::from_bytes(start as u64), // offset: `start` ); Operand::Immediate(Immediate::new_slice( - self.tag_static_base_pointer(ptr).into(), + self.tag_global_base_pointer(ptr).into(), (end - start) as u64, // len: `end - start` self, )) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 027e33abc7bb1..6cf11c071e4f7 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -290,7 +290,7 @@ where Tag: ::std::fmt::Debug + Copy + Eq + Hash + 'static, M: Machine<'mir, 'tcx, PointerTag = Tag>, // FIXME: Working around https://github.com/rust-lang/rust/issues/24159 - M::MemoryMap: AllocMap, Allocation)>, + M::MemoryMap: AllocMap, Allocation)>, M::AllocExtra: AllocationExtra, { /// Take a value, which represents a (thin or wide) reference, and make it a place. @@ -1015,7 +1015,7 @@ where pub fn allocate( &mut self, layout: TyLayout<'tcx>, - kind: MemoryKind, + kind: MemoryKind, ) -> MPlaceTy<'tcx, M::PointerTag> { let ptr = self.memory.allocate(layout.size, layout.align.abi, kind); MPlaceTy::from_aligned_ptr(ptr, layout) @@ -1025,9 +1025,9 @@ where pub fn allocate_str( &mut self, str: &str, - kind: MemoryKind, + kind: MemoryKind, ) -> MPlaceTy<'tcx, M::PointerTag> { - let ptr = self.memory.allocate_static_bytes(str.as_bytes(), kind); + let ptr = self.memory.allocate_bytes(str.as_bytes(), kind); let meta = Scalar::from_uint(str.len() as u128, self.pointer_size()); let mplace = MemPlace { ptr: ptr.into(), @@ -1118,7 +1118,7 @@ where ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { // This must be an allocation in `tcx` assert!(self.tcx.alloc_map.lock().get(raw.alloc_id).is_some()); - let ptr = self.tag_static_base_pointer(Pointer::from(raw.alloc_id)); + let ptr = self.tag_global_base_pointer(Pointer::from(raw.alloc_id)); let layout = self.layout_of(raw.ty)?; Ok(MPlaceTy::from_aligned_ptr(ptr, layout)) } diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 8899f12b15361..850fc15f12cf7 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -25,7 +25,7 @@ use rustc_hir::def::DefKind; use rustc_hir::HirId; use rustc_index::vec::IndexVec; use rustc_session::lint; -use rustc_span::Span; +use rustc_span::{def_id::DefId, Span}; use rustc_trait_selection::traits; use crate::const_eval::error_to_const_error; @@ -162,7 +162,7 @@ impl<'tcx> MirPass<'tcx> for ConstProp { struct ConstPropMachine; impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { - type MemoryKinds = !; + type MemoryKind = !; type PointerTag = (); type ExtraFnVal = !; @@ -172,7 +172,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { type MemoryMap = FxHashMap, Allocation)>; - const STATIC_KIND: Option = None; + const GLOBAL_KIND: Option = None; const CHECK_ALIGN: bool = false; @@ -247,7 +247,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { } #[inline(always)] - fn tag_static_base_pointer(_memory_extra: &(), _id: AllocId) -> Self::PointerTag {} + fn tag_global_base_pointer(_memory_extra: &(), _id: AllocId) -> Self::PointerTag {} fn box_alloc( _ecx: &mut InterpCx<'mir, 'tcx, Self>, @@ -270,14 +270,19 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { l.access() } - fn before_access_static( + fn before_access_global( _memory_extra: &(), allocation: &Allocation, + _def_id: Option, + is_write: bool, ) -> InterpResult<'tcx> { - // if the static allocation is mutable or if it has relocations (it may be legal to mutate - // the memory behind that in the future), then we can't const prop it + if is_write { + throw_machine_stop_str!("can't write to global"); + } + // If the static allocation is mutable or if it has relocations (it may be legal to mutate + // the memory behind that in the future), then we can't const prop it. if allocation.mutability == Mutability::Mut || allocation.relocations().len() > 0 { - throw_machine_stop_str!("can't eval mutable statics in ConstProp") + throw_machine_stop_str!("can't eval mutable statics in ConstProp"); } Ok(()) From f70af91e51709cbffe195aa698e287be56eeaa47 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 21 Mar 2020 19:47:52 +0100 Subject: [PATCH 07/15] bless; add test for mutating a static --- ...ssign-to-static-within-other-static.stderr | 2 +- .../miri_unleashed/mutable_const.stderr | 2 +- .../consts/miri_unleashed/mutating_global.rs | 15 ++++++++++ .../miri_unleashed/mutating_global.stderr | 29 +++++++++++++++++++ ...ic_mut_containing_mut_ref2.mut_refs.stderr | 2 +- .../static_mut_containing_mut_ref3.stderr | 2 +- 6 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/consts/miri_unleashed/mutating_global.rs create mode 100644 src/test/ui/consts/miri_unleashed/mutating_global.stderr diff --git a/src/test/ui/consts/const-eval/assign-to-static-within-other-static.stderr b/src/test/ui/consts/const-eval/assign-to-static-within-other-static.stderr index cb4d35b9a1809..bf5e476d80045 100644 --- a/src/test/ui/consts/const-eval/assign-to-static-within-other-static.stderr +++ b/src/test/ui/consts/const-eval/assign-to-static-within-other-static.stderr @@ -2,7 +2,7 @@ error[E0080]: could not evaluate static initializer --> $DIR/assign-to-static-within-other-static.rs:10:5 | LL | FOO = 5; - | ^^^^^^^ tried to modify a static's initial value from another static's initializer + | ^^^^^^^ modifying a static's initial value from another static's initializer error: aborting due to previous error diff --git a/src/test/ui/consts/miri_unleashed/mutable_const.stderr b/src/test/ui/consts/miri_unleashed/mutable_const.stderr index 8456e8ec6870d..29a13b18e5ba3 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_const.stderr +++ b/src/test/ui/consts/miri_unleashed/mutable_const.stderr @@ -11,7 +11,7 @@ LL | / const MUTATING_BEHIND_RAW: () = { LL | | // Test that `MUTABLE_BEHIND_RAW` is actually immutable, by doing this at const time. LL | | unsafe { LL | | *MUTABLE_BEHIND_RAW = 99 - | | ^^^^^^^^^^^^^^^^^^^^^^^^ writing to alloc1 which is read-only + | | ^^^^^^^^^^^^^^^^^^^^^^^^ modifying a static's initial value from another static's initializer LL | | } LL | | }; | |__- diff --git a/src/test/ui/consts/miri_unleashed/mutating_global.rs b/src/test/ui/consts/miri_unleashed/mutating_global.rs new file mode 100644 index 0000000000000..acc6fb026cd69 --- /dev/null +++ b/src/test/ui/consts/miri_unleashed/mutating_global.rs @@ -0,0 +1,15 @@ +// compile-flags: -Zunleash-the-miri-inside-of-you + +// Make sure we cannot mutate globals. + +static mut GLOBAL: i32 = 0; + +const MUTATING_GLOBAL: () = { + unsafe { + GLOBAL = 99 //~ ERROR any use of this value will cause an error + //~^ WARN skipping const checks + //~| WARN skipping const checks + } +}; + +fn main() {} diff --git a/src/test/ui/consts/miri_unleashed/mutating_global.stderr b/src/test/ui/consts/miri_unleashed/mutating_global.stderr new file mode 100644 index 0000000000000..4e67d2c0fb85e --- /dev/null +++ b/src/test/ui/consts/miri_unleashed/mutating_global.stderr @@ -0,0 +1,29 @@ +warning: skipping const checks + --> $DIR/mutating_global.rs:9:9 + | +LL | GLOBAL = 99 + | ^^^^^^ + +warning: skipping const checks + --> $DIR/mutating_global.rs:9:9 + | +LL | GLOBAL = 99 + | ^^^^^^ + +error: any use of this value will cause an error + --> $DIR/mutating_global.rs:9:9 + | +LL | / const MUTATING_GLOBAL: () = { +LL | | unsafe { +LL | | GLOBAL = 99 + | | ^^^^^^^^^^^ modifying a static's initial value from another static's initializer +LL | | +LL | | +LL | | } +LL | | }; + | |__- + | + = note: `#[deny(const_err)]` on by default + +error: aborting due to previous error + diff --git a/src/test/ui/consts/static_mut_containing_mut_ref2.mut_refs.stderr b/src/test/ui/consts/static_mut_containing_mut_ref2.mut_refs.stderr index b43fbc86f99f2..8db75dd63cf2a 100644 --- a/src/test/ui/consts/static_mut_containing_mut_ref2.mut_refs.stderr +++ b/src/test/ui/consts/static_mut_containing_mut_ref2.mut_refs.stderr @@ -2,7 +2,7 @@ error[E0080]: could not evaluate static initializer --> $DIR/static_mut_containing_mut_ref2.rs:7:45 | LL | pub static mut STDERR_BUFFER: () = unsafe { *(&mut STDERR_BUFFER_SPACE) = 42; }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ tried to modify a static's initial value from another static's initializer + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ modifying a static's initial value from another static's initializer error: aborting due to previous error diff --git a/src/test/ui/consts/static_mut_containing_mut_ref3.stderr b/src/test/ui/consts/static_mut_containing_mut_ref3.stderr index e88e49b097af2..91f9dbd8d0b9e 100644 --- a/src/test/ui/consts/static_mut_containing_mut_ref3.stderr +++ b/src/test/ui/consts/static_mut_containing_mut_ref3.stderr @@ -2,7 +2,7 @@ error[E0080]: could not evaluate static initializer --> $DIR/static_mut_containing_mut_ref3.rs:3:31 | LL | static mut BAR: () = unsafe { FOO.0 = 99; }; - | ^^^^^^^^^^ tried to modify a static's initial value from another static's initializer + | ^^^^^^^^^^ modifying a static's initial value from another static's initializer error: aborting due to previous error From 69cf211d06fc23148c5e3e5d5e732c1b0a0384f0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 21 Mar 2020 20:44:39 +0100 Subject: [PATCH 08/15] get back the more precise error message --- src/librustc_mir/const_eval/machine.rs | 8 ++++++-- src/librustc_mir/interpret/machine.rs | 1 + src/librustc_mir/interpret/memory.rs | 2 +- src/librustc_mir/transform/const_prop.rs | 1 + src/test/ui/consts/miri_unleashed/mutable_const.stderr | 2 +- 5 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs index 65dca16e739f5..ff91ddec946cb 100644 --- a/src/librustc_mir/const_eval/machine.rs +++ b/src/librustc_mir/const_eval/machine.rs @@ -8,6 +8,7 @@ use std::hash::Hash; use rustc_data_structures::fx::FxHashMap; use rustc::mir::AssertMessage; +use rustc_ast::ast::Mutability; use rustc_span::symbol::Symbol; use rustc_span::{def_id::DefId, Span}; @@ -347,11 +348,14 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter { fn before_access_global( memory_extra: &MemoryExtra, - _allocation: &Allocation, + alloc_id: AllocId, + allocation: &Allocation, def_id: Option, is_write: bool, ) -> InterpResult<'tcx> { - if is_write { + 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 || def_id.is_none() { Ok(()) diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 6fe7054855480..b820b11e9460d 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -212,6 +212,7 @@ pub trait Machine<'mir, 'tcx>: Sized { #[inline] fn before_access_global( _memory_extra: &Self::MemoryExtra, + _alloc_id: AllocId, _allocation: &Allocation, _def_id: Option, _is_write: bool, diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index df56b3f957782..87db44a96e7b3 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -456,7 +456,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { (allocation, Some(def_id)) } }; - M::before_access_global(memory_extra, alloc, def_id, is_write)?; + M::before_access_global(memory_extra, id, alloc, def_id, is_write)?; let alloc = Cow::Borrowed(alloc); // We got tcx memory. Let the machine initialize its "extra" stuff. let (alloc, tag) = M::init_allocation_extra( diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 850fc15f12cf7..17b8f3de75138 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -272,6 +272,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { fn before_access_global( _memory_extra: &(), + _alloc_id: AllocId, allocation: &Allocation, _def_id: Option, is_write: bool, diff --git a/src/test/ui/consts/miri_unleashed/mutable_const.stderr b/src/test/ui/consts/miri_unleashed/mutable_const.stderr index 29a13b18e5ba3..8456e8ec6870d 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_const.stderr +++ b/src/test/ui/consts/miri_unleashed/mutable_const.stderr @@ -11,7 +11,7 @@ LL | / const MUTATING_BEHIND_RAW: () = { LL | | // Test that `MUTABLE_BEHIND_RAW` is actually immutable, by doing this at const time. LL | | unsafe { LL | | *MUTABLE_BEHIND_RAW = 99 - | | ^^^^^^^^^^^^^^^^^^^^^^^^ modifying a static's initial value from another static's initializer + | | ^^^^^^^^^^^^^^^^^^^^^^^^ writing to alloc1 which is read-only LL | | } LL | | }; | |__- From 58a56cc8c088cbe51d1c6d324ab6f9bb03e365fd Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 21 Mar 2020 23:20:43 +0100 Subject: [PATCH 09/15] bless you --- src/test/ui/write-to-static-mut-in-static.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/write-to-static-mut-in-static.stderr b/src/test/ui/write-to-static-mut-in-static.stderr index 4349f6e89c119..6c2bd13d433ad 100644 --- a/src/test/ui/write-to-static-mut-in-static.stderr +++ b/src/test/ui/write-to-static-mut-in-static.stderr @@ -2,7 +2,7 @@ error[E0080]: could not evaluate static initializer --> $DIR/write-to-static-mut-in-static.rs:2:33 | LL | pub static mut B: () = unsafe { A = 1; }; - | ^^^^^ tried to modify a static's initial value from another static's initializer + | ^^^^^ modifying a static's initial value from another static's initializer error[E0391]: cycle detected when const-evaluating `C` --> $DIR/write-to-static-mut-in-static.rs:5:34 From 7a73b879cb560858e5f9c29f724cd305d6a4116b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 22 Mar 2020 09:23:19 +0100 Subject: [PATCH 10/15] fix const_prop ICE --- src/librustc_mir/const_eval/error.rs | 8 +++----- src/librustc_mir/const_eval/machine.rs | 1 + src/librustc_mir/interpret/machine.rs | 1 + src/librustc_mir/interpret/memory.rs | 5 ++++- src/librustc_mir/transform/const_prop.rs | 8 ++++++-- 5 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/const_eval/error.rs b/src/librustc_mir/const_eval/error.rs index fd46340f03ad6..aa30f43df9350 100644 --- a/src/librustc_mir/const_eval/error.rs +++ b/src/librustc_mir/const_eval/error.rs @@ -34,11 +34,9 @@ impl fmt::Display for ConstEvalErrKind { write!(f, "\"{}\" needs an rfc before being allowed inside constants", msg) } ConstAccessesStatic => write!(f, "constant accesses static"), - ModifiedGlobal => write!( - f, - "modifying a static's initial value from another static's \ - initializer" - ), + ModifiedGlobal => { + write!(f, "modifying a static's initial value from another static's initializer") + } AssertFailure(ref msg) => write!(f, "{:?}", msg), Panic { msg, line, col, file } => { write!(f, "the evaluated program panicked at '{}', {}:{}:{}", msg, file, line, col) diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs index ff91ddec946cb..8f4501cc3fb69 100644 --- a/src/librustc_mir/const_eval/machine.rs +++ b/src/librustc_mir/const_eval/machine.rs @@ -358,6 +358,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter { } else if is_write { Err(ConstEvalErrKind::ModifiedGlobal.into()) } else if memory_extra.can_access_statics || def_id.is_none() { + // `def_id.is_none()` indicates this is not a static, but a const or so. Ok(()) } else { Err(ConstEvalErrKind::ConstAccessesStatic.into()) diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index b820b11e9460d..cc87c2916862b 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -209,6 +209,7 @@ pub trait Machine<'mir, 'tcx>: Sized { } /// Called before a global allocation is accessed. + /// `def_id` is `Some` if this is the "lazy" allocation of a static. #[inline] fn before_access_global( _memory_extra: &Self::MemoryExtra, diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 87db44a96e7b3..110f2ffd9d78c 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -416,7 +416,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { ) -> InterpResult<'tcx, Cow<'tcx, Allocation>> { let alloc = tcx.alloc_map.lock().get(id); let (alloc, def_id) = match alloc { - Some(GlobalAlloc::Memory(mem)) => (mem, None), + Some(GlobalAlloc::Memory(mem)) => { + // Memory of a constant or promoted or anonymous memory referenced by a static. + (mem, None) + } Some(GlobalAlloc::Function(..)) => throw_ub!(DerefFunctionPointer(id)), None => throw_ub!(PointerUseAfterFree(id)), Some(GlobalAlloc::Static(def_id)) => { diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 17b8f3de75138..ef2d5404541b9 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -274,7 +274,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { _memory_extra: &(), _alloc_id: AllocId, allocation: &Allocation, - _def_id: Option, + def_id: Option, is_write: bool, ) -> InterpResult<'tcx> { if is_write { @@ -282,7 +282,11 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { } // If the static allocation is mutable or if it has relocations (it may be legal to mutate // the memory behind that in the future), then we can't const prop it. - if allocation.mutability == Mutability::Mut || allocation.relocations().len() > 0 { + // FIXME: we only check statics here (that have a `DefId`), not other mutable allocations. + // Why that? + if def_id.is_some() + && (allocation.mutability == Mutability::Mut || allocation.relocations().len() > 0) + { throw_machine_stop_str!("can't eval mutable statics in ConstProp"); } From 5a62054b06d276006907f64bf071d042cf9b9edd Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 24 Mar 2020 13:58:00 +0100 Subject: [PATCH 11/15] Clean up E0454 --- src/librustc_error_codes/error_codes/E0454.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc_error_codes/error_codes/E0454.md b/src/librustc_error_codes/error_codes/E0454.md index 80eb91e43d16b..23ca6c7824df1 100644 --- a/src/librustc_error_codes/error_codes/E0454.md +++ b/src/librustc_error_codes/error_codes/E0454.md @@ -1,4 +1,6 @@ -A link name was given with an empty name. Erroneous code example: +A link name was given with an empty name. + +Erroneous code example: ```compile_fail,E0454 #[link(name = "")] extern {} From 1939b4c940be8569eadbb556e3987534a3d26594 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 24 Mar 2020 14:31:55 +0100 Subject: [PATCH 12/15] actually we can reject all reads from mutable allocs in const-prop --- src/librustc_mir/transform/const_prop.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index ef2d5404541b9..6f3cdfdcd7e24 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -282,12 +282,11 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { } // If the static allocation is mutable or if it has relocations (it may be legal to mutate // the memory behind that in the future), then we can't const prop it. - // FIXME: we only check statics here (that have a `DefId`), not other mutable allocations. - // Why that? - if def_id.is_some() - && (allocation.mutability == Mutability::Mut || allocation.relocations().len() > 0) - { - throw_machine_stop_str!("can't eval mutable statics in ConstProp"); + if allocation.mutability == Mutability::Mut { + throw_machine_stop_str!("can't eval mutable globals in ConstProp"); + } + if def_id.is_some() && allocation.relocations().len() > 0 { + throw_machine_stop_str!("can't eval statics with pointers in ConstProp"); } Ok(()) From 03c64bf532ceec915f74460daf5344bb8ccf23d3 Mon Sep 17 00:00:00 2001 From: Without Boats Date: Tue, 24 Mar 2020 15:39:29 +0100 Subject: [PATCH 13/15] spaces between braces really ruin readability --- src/libstd/io/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libstd/io/mod.rs b/src/libstd/io/mod.rs index 1022123347ee5..88d556229e4cb 100644 --- a/src/libstd/io/mod.rs +++ b/src/libstd/io/mod.rs @@ -952,10 +952,10 @@ pub trait Read { pub struct IoSliceMut<'a>(sys::io::IoSliceMut<'a>); #[stable(feature = "iovec-send-sync", since = "1.44.0")] -unsafe impl<'a> Send for IoSliceMut<'a> { } +unsafe impl<'a> Send for IoSliceMut<'a> {} #[stable(feature = "iovec-send-sync", since = "1.44.0")] -unsafe impl<'a> Sync for IoSliceMut<'a> { } +unsafe impl<'a> Sync for IoSliceMut<'a> {} #[stable(feature = "iovec", since = "1.36.0")] impl<'a> fmt::Debug for IoSliceMut<'a> { @@ -1061,10 +1061,10 @@ impl<'a> DerefMut for IoSliceMut<'a> { pub struct IoSlice<'a>(sys::io::IoSlice<'a>); #[stable(feature = "iovec-send-sync", since = "1.44.0")] -unsafe impl<'a> Send for IoSlice<'a> { } +unsafe impl<'a> Send for IoSlice<'a> {} #[stable(feature = "iovec-send-sync", since = "1.44.0")] -unsafe impl<'a> Sync for IoSlice<'a> { } +unsafe impl<'a> Sync for IoSlice<'a> {} #[stable(feature = "iovec", since = "1.36.0")] impl<'a> fmt::Debug for IoSlice<'a> { From 42b10e51c18ad37f671dc289aa0f183d4dbceab9 Mon Sep 17 00:00:00 2001 From: Kornel Date: Tue, 24 Mar 2020 13:33:35 +0000 Subject: [PATCH 14/15] must_use on split_off #70194 --- src/liballoc/collections/vec_deque.rs | 1 + src/liballoc/string.rs | 1 + src/liballoc/tests/string.rs | 4 ++-- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/liballoc/collections/vec_deque.rs b/src/liballoc/collections/vec_deque.rs index 9d56f17700a85..69284fbf1b37d 100644 --- a/src/liballoc/collections/vec_deque.rs +++ b/src/liballoc/collections/vec_deque.rs @@ -1876,6 +1876,7 @@ impl VecDeque { /// assert_eq!(buf2, [2, 3]); /// ``` #[inline] + #[must_use = "use `.truncate()` if you don't need the other half"] #[stable(feature = "split_off", since = "1.4.0")] pub fn split_off(&mut self, at: usize) -> Self { let len = self.len(); diff --git a/src/liballoc/string.rs b/src/liballoc/string.rs index 0e48f1548e6d1..7c89d38caa4e6 100644 --- a/src/liballoc/string.rs +++ b/src/liballoc/string.rs @@ -1461,6 +1461,7 @@ impl String { /// ``` #[inline] #[stable(feature = "string_split_off", since = "1.16.0")] + #[must_use = "use `.truncate()` if you don't need the other half"] pub fn split_off(&mut self, at: usize) -> String { assert!(self.is_char_boundary(at)); let other = self.vec.split_off(at); diff --git a/src/liballoc/tests/string.rs b/src/liballoc/tests/string.rs index 08859b2b24bde..d2f09eb4a7568 100644 --- a/src/liballoc/tests/string.rs +++ b/src/liballoc/tests/string.rs @@ -266,14 +266,14 @@ fn test_split_off_empty() { fn test_split_off_past_end() { let orig = "Hello, world!"; let mut split = String::from(orig); - split.split_off(orig.len() + 1); + let _ = split.split_off(orig.len() + 1); } #[test] #[should_panic] fn test_split_off_mid_char() { let mut orig = String::from("山"); - orig.split_off(1); + let _ = orig.split_off(1); } #[test] From 33cd9a2515cb58657634928bf3b3915795822bb9 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Tue, 24 Mar 2020 20:21:50 +0100 Subject: [PATCH 15/15] Mark hotplug_codegen_backend as ignore-stage1 --- src/test/run-make-fulldeps/hotplug_codegen_backend/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/run-make-fulldeps/hotplug_codegen_backend/Makefile b/src/test/run-make-fulldeps/hotplug_codegen_backend/Makefile index e203ec2737fc7..d8ceace7fff25 100644 --- a/src/test/run-make-fulldeps/hotplug_codegen_backend/Makefile +++ b/src/test/run-make-fulldeps/hotplug_codegen_backend/Makefile @@ -1,5 +1,7 @@ include ../tools.mk +# ignore-stage1 + all: /bin/echo || exit 0 # This test requires /bin/echo to exist $(RUSTC) the_backend.rs --crate-name the_backend --crate-type dylib \