From a5e57c8055d25209cf1b06945dc628d7c0131394 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 3 Dec 2018 14:54:58 +0100 Subject: [PATCH 01/11] Monomorphize `AllocMap` struct --- src/librustc/mir/interpret/mod.rs | 21 ++++++++++----------- src/librustc/ty/context.rs | 2 +- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 9369b6e56f1d0..6ac80d72b1bc4 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -41,7 +41,6 @@ use ty::{self, TyCtxt, Instance}; use ty::layout::{self, Size}; use middle::region; use std::io; -use std::hash::Hash; use rustc_serialize::{Encoder, Decodable, Encodable}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::{Lock as Mutex, HashMapExt}; @@ -302,19 +301,19 @@ pub enum AllocType<'tcx, M> { Memory(M) } -pub struct AllocMap<'tcx, M> { +pub struct AllocMap<'tcx> { /// Lets you know what an AllocId refers to - id_to_type: FxHashMap>, + id_to_type: FxHashMap>, /// Used to ensure that functions and statics only get one associated AllocId - type_interner: FxHashMap, AllocId>, + type_interner: FxHashMap, AllocId>, /// The AllocId to assign to the next requested id. /// Always incremented, never gets smaller. next_id: AllocId, } -impl<'tcx, M: fmt::Debug + Eq + Hash + Clone> AllocMap<'tcx, M> { +impl<'tcx> AllocMap<'tcx> { pub fn new() -> Self { AllocMap { id_to_type: Default::default(), @@ -337,7 +336,7 @@ impl<'tcx, M: fmt::Debug + Eq + Hash + Clone> AllocMap<'tcx, M> { next } - fn intern(&mut self, alloc_type: AllocType<'tcx, M>) -> AllocId { + fn intern(&mut self, alloc_type: AllocType<'tcx, &'tcx Allocation>) -> AllocId { if let Some(&alloc_id) = self.type_interner.get(&alloc_type) { return alloc_id; } @@ -355,11 +354,11 @@ impl<'tcx, M: fmt::Debug + Eq + Hash + Clone> AllocMap<'tcx, M> { self.intern(AllocType::Function(instance)) } - pub fn get(&self, id: AllocId) -> Option> { + pub fn get(&self, id: AllocId) -> Option> { self.id_to_type.get(&id).cloned() } - pub fn unwrap_memory(&self, id: AllocId) -> M { + pub fn unwrap_memory(&self, id: AllocId) -> &'tcx Allocation { match self.get(id) { Some(AllocType::Memory(mem)) => mem, _ => bug!("expected allocation id {} to point to memory", id), @@ -370,19 +369,19 @@ impl<'tcx, M: fmt::Debug + Eq + Hash + Clone> AllocMap<'tcx, M> { self.intern(AllocType::Static(static_id)) } - pub fn allocate(&mut self, mem: M) -> AllocId { + pub fn allocate(&mut self, mem: &'tcx Allocation) -> AllocId { let id = self.reserve(); self.set_id_memory(id, mem); id } - pub fn set_id_memory(&mut self, id: AllocId, mem: M) { + pub fn set_id_memory(&mut self, id: AllocId, mem: &'tcx Allocation) { if let Some(old) = self.id_to_type.insert(id, AllocType::Memory(mem)) { bug!("tried to set allocation id {}, but it was already existing as {:#?}", id, old); } } - pub fn set_id_same_memory(&mut self, id: AllocId, mem: M) { + pub fn set_id_same_memory(&mut self, id: AllocId, mem: &'tcx Allocation) { self.id_to_type.insert_same(id, AllocType::Memory(mem)); } } diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 42a4de1682c39..d2d3a805f0a52 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -946,7 +946,7 @@ pub struct GlobalCtxt<'tcx> { /// Stores the value of constants (and deduplicates the actual memory) allocation_interner: Lock>, - pub alloc_map: Lock>, + pub alloc_map: Lock>, layout_interner: Lock>, From 3074fcb7e301c772cfb2301345cb3ee9d5fe3d4c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 3 Dec 2018 14:57:41 +0100 Subject: [PATCH 02/11] Monomorphize `AllocType` --- src/librustc/ich/impls_ty.rs | 2 +- src/librustc/mir/interpret/mod.rs | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index e6a7c20f79379..00ef3dd414bc9 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -338,7 +338,7 @@ impl_stable_hash_for!( ); impl_stable_hash_for!( - impl<'tcx, M> for enum mir::interpret::AllocType<'tcx, M> [ mir::interpret::AllocType ] { + impl<'tcx> for enum mir::interpret::AllocType<'tcx> [ mir::interpret::AllocType ] { Function(instance), Static(def_id), Memory(mem), diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 6ac80d72b1bc4..35719e49723ec 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -103,7 +103,7 @@ pub fn specialized_encode_alloc_id< tcx: TyCtxt<'a, 'tcx, 'tcx>, alloc_id: AllocId, ) -> Result<(), E::Error> { - let alloc_type: AllocType<'tcx, &'tcx Allocation> = + let alloc_type: AllocType<'tcx> = tcx.alloc_map.lock().get(alloc_id).expect("no value for AllocId"); match alloc_type { AllocType::Memory(alloc) => { @@ -291,22 +291,22 @@ impl fmt::Display for AllocId { } #[derive(Debug, Clone, Eq, PartialEq, Hash, RustcDecodable, RustcEncodable)] -pub enum AllocType<'tcx, M> { +pub enum AllocType<'tcx> { /// The alloc id is used as a function pointer Function(Instance<'tcx>), /// The alloc id points to a "lazy" static variable that did not get computed (yet). /// This is also used to break the cycle in recursive statics. Static(DefId), /// The alloc id points to memory - Memory(M) + Memory(&'tcx Allocation), } pub struct AllocMap<'tcx> { /// Lets you know what an AllocId refers to - id_to_type: FxHashMap>, + id_to_type: FxHashMap>, /// Used to ensure that functions and statics only get one associated AllocId - type_interner: FxHashMap, AllocId>, + type_interner: FxHashMap, AllocId>, /// The AllocId to assign to the next requested id. /// Always incremented, never gets smaller. @@ -336,7 +336,7 @@ impl<'tcx> AllocMap<'tcx> { next } - fn intern(&mut self, alloc_type: AllocType<'tcx, &'tcx Allocation>) -> AllocId { + fn intern(&mut self, alloc_type: AllocType<'tcx>) -> AllocId { if let Some(&alloc_id) = self.type_interner.get(&alloc_type) { return alloc_id; } @@ -354,7 +354,7 @@ impl<'tcx> AllocMap<'tcx> { self.intern(AllocType::Function(instance)) } - pub fn get(&self, id: AllocId) -> Option> { + pub fn get(&self, id: AllocId) -> Option> { self.id_to_type.get(&id).cloned() } From f6b3eb05415cbe57fbd4ddc22d054e7c94a00a84 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 3 Dec 2018 15:26:45 +0100 Subject: [PATCH 03/11] Function pointers are only equal to themselves, not to other function pointers to the same function --- src/librustc/mir/interpret/mod.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 35719e49723ec..49cbc90eb10c5 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -305,7 +305,7 @@ pub struct AllocMap<'tcx> { /// Lets you know what an AllocId refers to id_to_type: FxHashMap>, - /// Used to ensure that functions and statics only get one associated AllocId + /// Used to ensure that statics only get one associated AllocId type_interner: FxHashMap, AllocId>, /// The AllocId to assign to the next requested id. @@ -347,11 +347,14 @@ impl<'tcx> AllocMap<'tcx> { id } - // FIXME: Check if functions have identity. If not, we should not intern these, - // but instead create a new id per use. - // Alternatively we could just make comparing function pointers an error. + /// Functions cannot be identified by pointers, as asm-equal functions can get deduplicated + /// by the linker and functions can be duplicated across crates. + /// We thus generate a new `AllocId` for every mention of a function. This means that + /// `main as fn() == main as fn()` is false, while `let x = main as fn(); x == x` is true. pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> AllocId { - self.intern(AllocType::Function(instance)) + let id = self.reserve(); + self.id_to_type.insert(id, AllocType::Function(instance)); + id } pub fn get(&self, id: AllocId) -> Option> { From 3d278e91882b2a15a06242f1c0846986f259a3b7 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 3 Dec 2018 16:18:31 +0100 Subject: [PATCH 04/11] Document `AllocMap` --- src/librustc/mir/interpret/mod.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 49cbc90eb10c5..783bb6f521e69 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -322,8 +322,11 @@ impl<'tcx> AllocMap<'tcx> { } } - /// obtains a new allocation ID that can be referenced but does not + /// Obtains a new allocation ID that can be referenced but does not /// yet have an allocation backing it. + /// + /// Make sure to call `set_id_memory` or `set_id_same_memory` before returning such an + /// `AllocId` from a query. pub fn reserve( &mut self, ) -> AllocId { @@ -357,10 +360,14 @@ impl<'tcx> AllocMap<'tcx> { id } + /// Returns `None` in case the `AllocId` is dangling. + /// This function exists to allow const eval to detect the difference between evaluation- + /// local dangling pointers and allocations in constants/statics. pub fn get(&self, id: AllocId) -> Option> { self.id_to_type.get(&id).cloned() } + /// Panics if the `AllocId` does not refer to an `Allocation` pub fn unwrap_memory(&self, id: AllocId) -> &'tcx Allocation { match self.get(id) { Some(AllocType::Memory(mem)) => mem, @@ -368,24 +375,35 @@ impl<'tcx> AllocMap<'tcx> { } } + /// Generate an `AllocId` for a static or return a cached one in case this function has been + /// called on the same static before. pub fn intern_static(&mut self, static_id: DefId) -> AllocId { self.intern(AllocType::Static(static_id)) } + /// Intern the `Allocation` and return a new `AllocId`, even if there's already an identical + /// `Allocation` with a different `AllocId`. + // FIXME: is this really necessary? Can we ensure `FOO` and `BAR` being different after codegen + // in `static FOO: u32 = 42; static BAR: u32 = 42;` even if they reuse the same allocation + // inside rustc? pub fn allocate(&mut self, mem: &'tcx Allocation) -> AllocId { let id = self.reserve(); self.set_id_memory(id, mem); id } + /// Freeze an `AllocId` created with `reserve` by pointing it at an `Allocation`. Trying to + /// call this function twice, even with the same `Allocation` will ICE the compiler. pub fn set_id_memory(&mut self, id: AllocId, mem: &'tcx Allocation) { if let Some(old) = self.id_to_type.insert(id, AllocType::Memory(mem)) { bug!("tried to set allocation id {}, but it was already existing as {:#?}", id, old); } } + /// Freeze an `AllocId` created with `reserve` by pointing it at an `Allocation`. May be called + /// twice for the same `(AllocId, Allocation)` pair. pub fn set_id_same_memory(&mut self, id: AllocId, mem: &'tcx Allocation) { - self.id_to_type.insert_same(id, AllocType::Memory(mem)); + self.id_to_type.insert_same(id, AllocType::Memory(mem)); } } From a1e83a905f1ad82f2f35fd72b6b6797d7b60b8d0 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 3 Dec 2018 16:30:43 +0100 Subject: [PATCH 05/11] s/AllocKind/AllocDiscriminant/ --- src/librustc/mir/interpret/mod.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 783bb6f521e69..8d8128f7a19b9 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -89,7 +89,7 @@ impl ::rustc_serialize::UseSpecializedEncodable for AllocId {} impl ::rustc_serialize::UseSpecializedDecodable for AllocId {} #[derive(RustcDecodable, RustcEncodable)] -enum AllocKind { +enum AllocDiscriminant { Alloc, Fn, Static, @@ -108,18 +108,18 @@ pub fn specialized_encode_alloc_id< match alloc_type { AllocType::Memory(alloc) => { trace!("encoding {:?} with {:#?}", alloc_id, alloc); - AllocKind::Alloc.encode(encoder)?; + AllocDiscriminant::Alloc.encode(encoder)?; alloc.encode(encoder)?; } AllocType::Function(fn_instance) => { trace!("encoding {:?} with {:#?}", alloc_id, fn_instance); - AllocKind::Fn.encode(encoder)?; + AllocDiscriminant::Fn.encode(encoder)?; fn_instance.encode(encoder)?; } AllocType::Static(did) => { // referring to statics doesn't need to know about their allocations, // just about its DefId - AllocKind::Static.encode(encoder)?; + AllocDiscriminant::Static.encode(encoder)?; did.encode(encoder)?; } } @@ -188,10 +188,10 @@ impl<'s> AllocDecodingSession<'s> { let idx = decoder.read_u32()? as usize; let pos = self.state.data_offsets[idx] as usize; - // Decode the AllocKind now so that we know if we have to reserve an + // Decode the AllocDiscriminant now so that we know if we have to reserve an // AllocId. let (alloc_kind, pos) = decoder.with_position(pos, |decoder| { - let alloc_kind = AllocKind::decode(decoder)?; + let alloc_kind = AllocDiscriminant::decode(decoder)?; Ok((alloc_kind, decoder.position())) })?; @@ -207,7 +207,7 @@ impl<'s> AllocDecodingSession<'s> { ref mut entry @ State::Empty => { // We are allowed to decode match alloc_kind { - AllocKind::Alloc => { + AllocDiscriminant::Alloc => { // If this is an allocation, we need to reserve an // AllocId so we can decode cyclic graphs. let alloc_id = decoder.tcx().alloc_map.lock().reserve(); @@ -216,7 +216,7 @@ impl<'s> AllocDecodingSession<'s> { alloc_id); Some(alloc_id) }, - AllocKind::Fn | AllocKind::Static => { + AllocDiscriminant::Fn | AllocDiscriminant::Static => { // Fns and statics cannot be cyclic and their AllocId // is determined later by interning *entry = State::InProgressNonAlloc( @@ -250,7 +250,7 @@ impl<'s> AllocDecodingSession<'s> { // Now decode the actual data let alloc_id = decoder.with_position(pos, |decoder| { match alloc_kind { - AllocKind::Alloc => { + AllocDiscriminant::Alloc => { let allocation = <&'tcx Allocation as Decodable>::decode(decoder)?; // We already have a reserved AllocId. let alloc_id = alloc_id.unwrap(); @@ -258,7 +258,7 @@ impl<'s> AllocDecodingSession<'s> { decoder.tcx().alloc_map.lock().set_id_same_memory(alloc_id, allocation); Ok(alloc_id) }, - AllocKind::Fn => { + AllocDiscriminant::Fn => { assert!(alloc_id.is_none()); trace!("creating fn alloc id"); let instance = ty::Instance::decode(decoder)?; @@ -266,7 +266,7 @@ impl<'s> AllocDecodingSession<'s> { let alloc_id = decoder.tcx().alloc_map.lock().create_fn_alloc(instance); Ok(alloc_id) }, - AllocKind::Static => { + AllocDiscriminant::Static => { assert!(alloc_id.is_none()); trace!("creating extern static alloc id at"); let did = DefId::decode(decoder)?; From 2e00d648c059b56f0b2dc7a362a098aa627292fc Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 3 Dec 2018 16:31:20 +0100 Subject: [PATCH 06/11] s/AllocType/AllocKind/ --- src/librustc/ich/impls_ty.rs | 2 +- src/librustc/mir/interpret/mod.rs | 28 +++++++++++----------- src/librustc/mir/interpret/value.rs | 2 +- src/librustc/mir/mod.rs | 2 +- src/librustc_codegen_llvm/common.rs | 8 +++---- src/librustc_mir/interpret/memory.rs | 26 ++++++++++---------- src/librustc_mir/interpret/validity.rs | 4 ++-- src/librustc_mir/monomorphize/collector.rs | 8 +++---- 8 files changed, 40 insertions(+), 40 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 00ef3dd414bc9..56da6b719f4a7 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -338,7 +338,7 @@ impl_stable_hash_for!( ); impl_stable_hash_for!( - impl<'tcx> for enum mir::interpret::AllocType<'tcx> [ mir::interpret::AllocType ] { + impl<'tcx> for enum mir::interpret::AllocKind<'tcx> [ mir::interpret::AllocKind ] { Function(instance), Static(def_id), Memory(mem), diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 8d8128f7a19b9..fbfe0cdae6bed 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -103,20 +103,20 @@ pub fn specialized_encode_alloc_id< tcx: TyCtxt<'a, 'tcx, 'tcx>, alloc_id: AllocId, ) -> Result<(), E::Error> { - let alloc_type: AllocType<'tcx> = + let alloc_type: AllocKind<'tcx> = tcx.alloc_map.lock().get(alloc_id).expect("no value for AllocId"); match alloc_type { - AllocType::Memory(alloc) => { + AllocKind::Memory(alloc) => { trace!("encoding {:?} with {:#?}", alloc_id, alloc); AllocDiscriminant::Alloc.encode(encoder)?; alloc.encode(encoder)?; } - AllocType::Function(fn_instance) => { + AllocKind::Function(fn_instance) => { trace!("encoding {:?} with {:#?}", alloc_id, fn_instance); AllocDiscriminant::Fn.encode(encoder)?; fn_instance.encode(encoder)?; } - AllocType::Static(did) => { + AllocKind::Static(did) => { // referring to statics doesn't need to know about their allocations, // just about its DefId AllocDiscriminant::Static.encode(encoder)?; @@ -291,7 +291,7 @@ impl fmt::Display for AllocId { } #[derive(Debug, Clone, Eq, PartialEq, Hash, RustcDecodable, RustcEncodable)] -pub enum AllocType<'tcx> { +pub enum AllocKind<'tcx> { /// The alloc id is used as a function pointer Function(Instance<'tcx>), /// The alloc id points to a "lazy" static variable that did not get computed (yet). @@ -303,10 +303,10 @@ pub enum AllocType<'tcx> { pub struct AllocMap<'tcx> { /// Lets you know what an AllocId refers to - id_to_type: FxHashMap>, + id_to_type: FxHashMap>, /// Used to ensure that statics only get one associated AllocId - type_interner: FxHashMap, AllocId>, + type_interner: FxHashMap, AllocId>, /// The AllocId to assign to the next requested id. /// Always incremented, never gets smaller. @@ -339,7 +339,7 @@ impl<'tcx> AllocMap<'tcx> { next } - fn intern(&mut self, alloc_type: AllocType<'tcx>) -> AllocId { + fn intern(&mut self, alloc_type: AllocKind<'tcx>) -> AllocId { if let Some(&alloc_id) = self.type_interner.get(&alloc_type) { return alloc_id; } @@ -356,21 +356,21 @@ impl<'tcx> AllocMap<'tcx> { /// `main as fn() == main as fn()` is false, while `let x = main as fn(); x == x` is true. pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> AllocId { let id = self.reserve(); - self.id_to_type.insert(id, AllocType::Function(instance)); + self.id_to_type.insert(id, AllocKind::Function(instance)); id } /// Returns `None` in case the `AllocId` is dangling. /// This function exists to allow const eval to detect the difference between evaluation- /// local dangling pointers and allocations in constants/statics. - pub fn get(&self, id: AllocId) -> Option> { + pub fn get(&self, id: AllocId) -> Option> { self.id_to_type.get(&id).cloned() } /// Panics if the `AllocId` does not refer to an `Allocation` pub fn unwrap_memory(&self, id: AllocId) -> &'tcx Allocation { match self.get(id) { - Some(AllocType::Memory(mem)) => mem, + Some(AllocKind::Memory(mem)) => mem, _ => bug!("expected allocation id {} to point to memory", id), } } @@ -378,7 +378,7 @@ impl<'tcx> AllocMap<'tcx> { /// Generate an `AllocId` for a static or return a cached one in case this function has been /// called on the same static before. pub fn intern_static(&mut self, static_id: DefId) -> AllocId { - self.intern(AllocType::Static(static_id)) + self.intern(AllocKind::Static(static_id)) } /// Intern the `Allocation` and return a new `AllocId`, even if there's already an identical @@ -395,7 +395,7 @@ impl<'tcx> AllocMap<'tcx> { /// Freeze an `AllocId` created with `reserve` by pointing it at an `Allocation`. Trying to /// call this function twice, even with the same `Allocation` will ICE the compiler. pub fn set_id_memory(&mut self, id: AllocId, mem: &'tcx Allocation) { - if let Some(old) = self.id_to_type.insert(id, AllocType::Memory(mem)) { + if let Some(old) = self.id_to_type.insert(id, AllocKind::Memory(mem)) { bug!("tried to set allocation id {}, but it was already existing as {:#?}", id, old); } } @@ -403,7 +403,7 @@ impl<'tcx> AllocMap<'tcx> { /// Freeze an `AllocId` created with `reserve` by pointing it at an `Allocation`. May be called /// twice for the same `(AllocId, Allocation)` pair. pub fn set_id_same_memory(&mut self, id: AllocId, mem: &'tcx Allocation) { - self.id_to_type.insert_same(id, AllocType::Memory(mem)); + self.id_to_type.insert_same(id, AllocKind::Memory(mem)); } } diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 4bcba9d54674e..16374c92606ee 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -18,7 +18,7 @@ use super::{EvalResult, Pointer, PointerArithmetic, Allocation, AllocId, sign_ex /// Represents the result of a raw const operation, pre-validation. #[derive(Copy, Clone, Debug, Eq, PartialEq, RustcEncodable, RustcDecodable, Hash)] pub struct RawConst<'tcx> { - // the value lives here, at offset 0, and that allocation definitely is a `AllocType::Memory` + // the value lives here, at offset 0, and that allocation definitely is a `AllocKind::Memory` // (so you can use `AllocMap::unwrap_memory`). pub alloc_id: AllocId, pub ty: Ty<'tcx>, diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 368f83eb61127..47058f91525a8 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -2629,7 +2629,7 @@ pub fn fmt_const_val(f: &mut impl Write, const_val: &ty::Const<'_>) -> fmt::Resu if let Ref(_, &ty::TyS { sty: Str, .. }, _) = ty.sty { return ty::tls::with(|tcx| { let alloc = tcx.alloc_map.lock().get(ptr.alloc_id); - if let Some(interpret::AllocType::Memory(alloc)) = alloc { + if let Some(interpret::AllocKind::Memory(alloc)) = alloc { assert_eq!(len as usize as u128, len); let slice = &alloc.bytes[(ptr.offset.bytes() as usize)..][..(len as usize)]; diff --git a/src/librustc_codegen_llvm/common.rs b/src/librustc_codegen_llvm/common.rs index fd13421835c12..0d9c417f5229b 100644 --- a/src/librustc_codegen_llvm/common.rs +++ b/src/librustc_codegen_llvm/common.rs @@ -21,7 +21,7 @@ use value::Value; use rustc_codegen_ssa::traits::*; use rustc::ty::layout::{HasDataLayout, LayoutOf, self, TyLayout, Size}; -use rustc::mir::interpret::{Scalar, AllocType, Allocation}; +use rustc::mir::interpret::{Scalar, AllocKind, Allocation}; use consts::const_alloc_to_llvm; use rustc_codegen_ssa::mir::place::PlaceRef; @@ -318,7 +318,7 @@ impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> { Scalar::Ptr(ptr) => { let alloc_type = self.tcx.alloc_map.lock().get(ptr.alloc_id); let base_addr = match alloc_type { - Some(AllocType::Memory(alloc)) => { + Some(AllocKind::Memory(alloc)) => { let init = const_alloc_to_llvm(self, alloc); if alloc.mutability == Mutability::Mutable { self.static_addr_of_mut(init, alloc.align, None) @@ -326,10 +326,10 @@ impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> { self.static_addr_of(init, alloc.align, None) } } - Some(AllocType::Function(fn_instance)) => { + Some(AllocKind::Function(fn_instance)) => { self.get_fn(fn_instance) } - Some(AllocType::Static(def_id)) => { + Some(AllocKind::Static(def_id)) => { assert!(self.tcx.is_static(def_id).is_some()); self.get_static(def_id) } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 97d7e1586b811..cc96976e74a30 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -29,7 +29,7 @@ use syntax::ast::Mutability; use super::{ Pointer, AllocId, Allocation, GlobalId, AllocationExtra, - EvalResult, Scalar, EvalErrorKind, AllocType, PointerArithmetic, + EvalResult, Scalar, EvalErrorKind, AllocKind, PointerArithmetic, Machine, AllocMap, MayLeak, ErrorHandled, InboundsCheck, }; @@ -204,12 +204,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { None => { // Deallocating static memory -- always an error return match self.tcx.alloc_map.lock().get(ptr.alloc_id) { - Some(AllocType::Function(..)) => err!(DeallocatedWrongMemoryKind( + Some(AllocKind::Function(..)) => err!(DeallocatedWrongMemoryKind( "function".to_string(), format!("{:?}", kind), )), - Some(AllocType::Static(..)) | - Some(AllocType::Memory(..)) => err!(DeallocatedWrongMemoryKind( + Some(AllocKind::Static(..)) | + Some(AllocKind::Memory(..)) => err!(DeallocatedWrongMemoryKind( "static".to_string(), format!("{:?}", kind), )), @@ -326,15 +326,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { ) -> EvalResult<'tcx, Cow<'tcx, Allocation>> { let alloc = tcx.alloc_map.lock().get(id); let def_id = match alloc { - Some(AllocType::Memory(mem)) => { + Some(AllocKind::Memory(mem)) => { // We got tcx memory. Let the machine figure out whether and how to // turn that into memory with the right pointer tag. return Ok(M::adjust_static_allocation(mem, memory_extra)) } - Some(AllocType::Function(..)) => { + Some(AllocKind::Function(..)) => { return err!(DerefFunctionPointer) } - Some(AllocType::Static(did)) => { + Some(AllocKind::Static(did)) => { did } None => @@ -435,8 +435,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } // Could also be a fn ptr or extern static match self.tcx.alloc_map.lock().get(id) { - Some(AllocType::Function(..)) => (Size::ZERO, Align::from_bytes(1).unwrap()), - Some(AllocType::Static(did)) => { + Some(AllocKind::Function(..)) => (Size::ZERO, Align::from_bytes(1).unwrap()), + Some(AllocKind::Static(did)) => { // The only way `get` couldn't have worked here is if this is an extern static assert!(self.tcx.is_foreign_item(did)); // Use size and align of the type @@ -459,7 +459,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } trace!("reading fn ptr: {}", ptr.alloc_id); match self.tcx.alloc_map.lock().get(ptr.alloc_id) { - Some(AllocType::Function(instance)) => Ok(instance), + Some(AllocKind::Function(instance)) => Ok(instance), _ => Err(EvalErrorKind::ExecuteMemory.into()), } } @@ -557,16 +557,16 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { Err(()) => { // static alloc? match self.tcx.alloc_map.lock().get(id) { - Some(AllocType::Memory(alloc)) => { + Some(AllocKind::Memory(alloc)) => { self.dump_alloc_helper( &mut allocs_seen, &mut allocs_to_print, msg, alloc, " (immutable)".to_owned() ); } - Some(AllocType::Function(func)) => { + Some(AllocKind::Function(func)) => { trace!("{} {}", msg, func); } - Some(AllocType::Static(did)) => { + Some(AllocKind::Static(did)) => { trace!("{} {:?}", msg, did); } None => { diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index d98d05bc01b85..bf771789c8dee 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -17,7 +17,7 @@ use rustc::ty::layout::{self, Size, Align, TyLayout, LayoutOf, VariantIdx}; use rustc::ty; use rustc_data_structures::fx::FxHashSet; use rustc::mir::interpret::{ - Scalar, AllocType, EvalResult, EvalErrorKind, + Scalar, AllocKind, EvalResult, EvalErrorKind, }; use super::{ @@ -388,7 +388,7 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> "integer pointer in non-ZST reference", self.path); // Skip validation entirely for some external statics let alloc_kind = self.ecx.tcx.alloc_map.lock().get(ptr.alloc_id); - if let Some(AllocType::Static(did)) = alloc_kind { + if let Some(AllocKind::Static(did)) = alloc_kind { // `extern static` cannot be validated as they have no body. // FIXME: Statics from other crates are also skipped. // They might be checked at a different type, but for now we diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 7531f62fdab7b..ec5c57ee0ba68 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -197,7 +197,7 @@ use rustc::session::config; use rustc::mir::{self, Location, Promoted}; use rustc::mir::visit::Visitor as MirVisitor; use rustc::mir::mono::MonoItem; -use rustc::mir::interpret::{Scalar, GlobalId, AllocType, ErrorHandled}; +use rustc::mir::interpret::{Scalar, GlobalId, AllocKind, ErrorHandled}; use monomorphize::{self, Instance}; use rustc::util::nodemap::{FxHashSet, FxHashMap, DefIdMap}; @@ -1163,20 +1163,20 @@ fn collect_miri<'a, 'tcx>( ) { let alloc_type = tcx.alloc_map.lock().get(alloc_id); match alloc_type { - Some(AllocType::Static(did)) => { + Some(AllocKind::Static(did)) => { let instance = Instance::mono(tcx, did); if should_monomorphize_locally(tcx, &instance) { trace!("collecting static {:?}", did); output.push(MonoItem::Static(did)); } } - Some(AllocType::Memory(alloc)) => { + Some(AllocKind::Memory(alloc)) => { trace!("collecting {:?} with {:#?}", alloc_id, alloc); for &((), inner) in alloc.relocations.values() { collect_miri(tcx, inner, output); } }, - Some(AllocType::Function(fn_instance)) => { + Some(AllocKind::Function(fn_instance)) => { if should_monomorphize_locally(tcx, &fn_instance) { trace!("collecting {:?} with {:#?}", alloc_id, fn_instance); output.push(create_fn_mono_item(fn_instance)); From d6250191a6580317adb5c5fc3b1126c46f80d8d6 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 4 Dec 2018 09:39:09 +0100 Subject: [PATCH 07/11] s/alloc_type/alloc_kind/ --- src/librustc/mir/interpret/mod.rs | 14 +++++++------- src/librustc_codegen_llvm/common.rs | 4 ++-- src/librustc_mir/monomorphize/collector.rs | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index fbfe0cdae6bed..9708f7c6717f4 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -103,9 +103,9 @@ pub fn specialized_encode_alloc_id< tcx: TyCtxt<'a, 'tcx, 'tcx>, alloc_id: AllocId, ) -> Result<(), E::Error> { - let alloc_type: AllocKind<'tcx> = + let alloc_kind: AllocKind<'tcx> = tcx.alloc_map.lock().get(alloc_id).expect("no value for AllocId"); - match alloc_type { + match alloc_kind { AllocKind::Memory(alloc) => { trace!("encoding {:?} with {:#?}", alloc_id, alloc); AllocDiscriminant::Alloc.encode(encoder)?; @@ -339,14 +339,14 @@ impl<'tcx> AllocMap<'tcx> { next } - fn intern(&mut self, alloc_type: AllocKind<'tcx>) -> AllocId { - if let Some(&alloc_id) = self.type_interner.get(&alloc_type) { + fn intern(&mut self, alloc_kind: AllocKind<'tcx>) -> AllocId { + if let Some(&alloc_id) = self.type_interner.get(&alloc_kind) { return alloc_id; } let id = self.reserve(); - debug!("creating alloc_type {:?} with id {}", alloc_type, id); - self.id_to_type.insert(id, alloc_type.clone()); - self.type_interner.insert(alloc_type, id); + debug!("creating alloc_kind {:?} with id {}", alloc_kind, id); + self.id_to_type.insert(id, alloc_kind.clone()); + self.type_interner.insert(alloc_kind, id); id } diff --git a/src/librustc_codegen_llvm/common.rs b/src/librustc_codegen_llvm/common.rs index 0d9c417f5229b..194b6b4e2706d 100644 --- a/src/librustc_codegen_llvm/common.rs +++ b/src/librustc_codegen_llvm/common.rs @@ -316,8 +316,8 @@ impl ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> { } }, Scalar::Ptr(ptr) => { - let alloc_type = self.tcx.alloc_map.lock().get(ptr.alloc_id); - let base_addr = match alloc_type { + let alloc_kind = self.tcx.alloc_map.lock().get(ptr.alloc_id); + let base_addr = match alloc_kind { Some(AllocKind::Memory(alloc)) => { let init = const_alloc_to_llvm(self, alloc); if alloc.mutability == Mutability::Mutable { diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index ec5c57ee0ba68..289402d3b46a1 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -1161,8 +1161,8 @@ fn collect_miri<'a, 'tcx>( alloc_id: AllocId, output: &mut Vec>, ) { - let alloc_type = tcx.alloc_map.lock().get(alloc_id); - match alloc_type { + let alloc_kind = tcx.alloc_map.lock().get(alloc_id); + match alloc_kind { Some(AllocKind::Static(did)) => { let instance = Instance::mono(tcx, did); if should_monomorphize_locally(tcx, &instance) { From 66b2353a4cc0f21f6c9d0f7bd4a54d221ad09fa6 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 4 Dec 2018 09:58:36 +0100 Subject: [PATCH 08/11] Clear up comment --- src/librustc/mir/interpret/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 9708f7c6717f4..2082de51d7e5d 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -360,7 +360,9 @@ impl<'tcx> AllocMap<'tcx> { id } - /// Returns `None` in case the `AllocId` is dangling. + /// Returns `None` in case the `AllocId` is dangling. An `EvalContext` can still have a + /// local `Allocation` for that `AllocId`, but having such an `AllocId` in a constant is + /// illegal and will likely ICE. /// This function exists to allow const eval to detect the difference between evaluation- /// local dangling pointers and allocations in constants/statics. pub fn get(&self, id: AllocId) -> Option> { From 760df2c3d0e0ef2f12b164bfbf32348d555fbe15 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 4 Dec 2018 09:59:36 +0100 Subject: [PATCH 09/11] s/id_to_type/id_to_kind/ --- src/librustc/mir/interpret/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 2082de51d7e5d..7243e8ddc9b1f 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -303,7 +303,7 @@ pub enum AllocKind<'tcx> { pub struct AllocMap<'tcx> { /// Lets you know what an AllocId refers to - id_to_type: FxHashMap>, + id_to_kind: FxHashMap>, /// Used to ensure that statics only get one associated AllocId type_interner: FxHashMap, AllocId>, @@ -316,7 +316,7 @@ pub struct AllocMap<'tcx> { impl<'tcx> AllocMap<'tcx> { pub fn new() -> Self { AllocMap { - id_to_type: Default::default(), + id_to_kind: Default::default(), type_interner: Default::default(), next_id: AllocId(0), } @@ -345,7 +345,7 @@ impl<'tcx> AllocMap<'tcx> { } let id = self.reserve(); debug!("creating alloc_kind {:?} with id {}", alloc_kind, id); - self.id_to_type.insert(id, alloc_kind.clone()); + self.id_to_kind.insert(id, alloc_kind.clone()); self.type_interner.insert(alloc_kind, id); id } @@ -356,7 +356,7 @@ impl<'tcx> AllocMap<'tcx> { /// `main as fn() == main as fn()` is false, while `let x = main as fn(); x == x` is true. pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> AllocId { let id = self.reserve(); - self.id_to_type.insert(id, AllocKind::Function(instance)); + self.id_to_kind.insert(id, AllocKind::Function(instance)); id } @@ -366,7 +366,7 @@ impl<'tcx> AllocMap<'tcx> { /// This function exists to allow const eval to detect the difference between evaluation- /// local dangling pointers and allocations in constants/statics. pub fn get(&self, id: AllocId) -> Option> { - self.id_to_type.get(&id).cloned() + self.id_to_kind.get(&id).cloned() } /// Panics if the `AllocId` does not refer to an `Allocation` @@ -397,7 +397,7 @@ impl<'tcx> AllocMap<'tcx> { /// Freeze an `AllocId` created with `reserve` by pointing it at an `Allocation`. Trying to /// call this function twice, even with the same `Allocation` will ICE the compiler. pub fn set_id_memory(&mut self, id: AllocId, mem: &'tcx Allocation) { - if let Some(old) = self.id_to_type.insert(id, AllocKind::Memory(mem)) { + if let Some(old) = self.id_to_kind.insert(id, AllocKind::Memory(mem)) { bug!("tried to set allocation id {}, but it was already existing as {:#?}", id, old); } } @@ -405,7 +405,7 @@ impl<'tcx> AllocMap<'tcx> { /// Freeze an `AllocId` created with `reserve` by pointing it at an `Allocation`. May be called /// twice for the same `(AllocId, Allocation)` pair. pub fn set_id_same_memory(&mut self, id: AllocId, mem: &'tcx Allocation) { - self.id_to_type.insert_same(id, AllocKind::Memory(mem)); + self.id_to_kind.insert_same(id, AllocKind::Memory(mem)); } } From 982a9662c51b7746fd8aa1347034520667de2e8a Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 4 Dec 2018 10:01:58 +0100 Subject: [PATCH 10/11] Mention `alloc` in the name of the interning methods --- src/librustc/mir/interpret/mod.rs | 12 ++++++------ src/librustc_mir/interpret/memory.rs | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 7243e8ddc9b1f..4d0956f018f6a 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -255,7 +255,7 @@ impl<'s> AllocDecodingSession<'s> { // We already have a reserved AllocId. let alloc_id = alloc_id.unwrap(); trace!("decoded alloc {:?} {:#?}", alloc_id, allocation); - decoder.tcx().alloc_map.lock().set_id_same_memory(alloc_id, allocation); + decoder.tcx().alloc_map.lock().set_alloc_id_same_memory(alloc_id, allocation); Ok(alloc_id) }, AllocDiscriminant::Fn => { @@ -325,8 +325,8 @@ impl<'tcx> AllocMap<'tcx> { /// Obtains a new allocation ID that can be referenced but does not /// yet have an allocation backing it. /// - /// Make sure to call `set_id_memory` or `set_id_same_memory` before returning such an - /// `AllocId` from a query. + /// Make sure to call `set_alloc_id_memory` or `set_alloc_id_same_memory` before returning such + /// an `AllocId` from a query. pub fn reserve( &mut self, ) -> AllocId { @@ -390,13 +390,13 @@ impl<'tcx> AllocMap<'tcx> { // inside rustc? pub fn allocate(&mut self, mem: &'tcx Allocation) -> AllocId { let id = self.reserve(); - self.set_id_memory(id, mem); + self.set_alloc_id_memory(id, mem); id } /// Freeze an `AllocId` created with `reserve` by pointing it at an `Allocation`. Trying to /// call this function twice, even with the same `Allocation` will ICE the compiler. - pub fn set_id_memory(&mut self, id: AllocId, mem: &'tcx Allocation) { + pub fn set_alloc_id_memory(&mut self, id: AllocId, mem: &'tcx Allocation) { if let Some(old) = self.id_to_kind.insert(id, AllocKind::Memory(mem)) { bug!("tried to set allocation id {}, but it was already existing as {:#?}", id, old); } @@ -404,7 +404,7 @@ impl<'tcx> AllocMap<'tcx> { /// Freeze an `AllocId` created with `reserve` by pointing it at an `Allocation`. May be called /// twice for the same `(AllocId, Allocation)` pair. - pub fn set_id_same_memory(&mut self, id: AllocId, mem: &'tcx Allocation) { + pub fn set_alloc_id_same_memory(&mut self, id: AllocId, mem: &'tcx Allocation) { self.id_to_kind.insert_same(id, AllocKind::Memory(mem)); } } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index cc96976e74a30..a42821a8c0677 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -638,7 +638,7 @@ where // ensure llvm knows not to put this into immutable memory alloc.mutability = mutability; let alloc = self.tcx.intern_const_alloc(alloc); - self.tcx.alloc_map.lock().set_id_memory(alloc_id, alloc); + self.tcx.alloc_map.lock().set_alloc_id_memory(alloc_id, alloc); // recurse into inner allocations for &(_, alloc) in alloc.relocations.values() { // FIXME: Reusing the mutability here is likely incorrect. It is originally From 7181aa176393d601f43704d886e8e7afa4b14e45 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 4 Dec 2018 10:03:21 +0100 Subject: [PATCH 11/11] No need to export functions only used in the same module --- src/librustc/mir/interpret/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 4d0956f018f6a..d54a1c9bb82f7 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -404,7 +404,7 @@ impl<'tcx> AllocMap<'tcx> { /// Freeze an `AllocId` created with `reserve` by pointing it at an `Allocation`. May be called /// twice for the same `(AllocId, Allocation)` pair. - pub fn set_alloc_id_same_memory(&mut self, id: AllocId, mem: &'tcx Allocation) { + fn set_alloc_id_same_memory(&mut self, id: AllocId, mem: &'tcx Allocation) { self.id_to_kind.insert_same(id, AllocKind::Memory(mem)); } }