mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #56461 - oli-obk:alloc_ids, r=RalfJung

Browse Source 
Some cleanups around `AllocId` management

r? @eddyb
cc @RalfJung
This commit is contained in:
bors 2018-12-13 12:36:13 +00:00
commit 7489ee9c6f
9 changed files with 101 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/librustc/ich/impls_ty.rs
View File

@ -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::AllocKind<'tcx> [ mir::interpret::AllocKind ] {
Function(instance),
Static(def_id),
Memory(mem),

116
src/librustc/mir/interpret/mod.rs
View File

@ -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};
@ -90,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,
@ -104,23 +103,23 @@ 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_kind: AllocKind<'tcx> =
tcx.alloc_map.lock().get(alloc_id).expect("no value for AllocId");
match alloc_type {
AllocType::Memory(alloc) => {
match alloc_kind {
AllocKind::Memory(alloc) => {
trace!("encoding {:?} with {:#?}", alloc_id, alloc);
AllocKind::Alloc.encode(encoder)?;
AllocDiscriminant::Alloc.encode(encoder)?;
alloc.encode(encoder)?;
}
AllocType::Function(fn_instance) => {
AllocKind::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) => {
AllocKind::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)?;
}
}
@ -189,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()))
})?;
@ -208,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();
@ -217,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(
@ -251,15 +250,15 @@ 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();
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)
},
AllocKind::Fn => {
AllocDiscriminant::Fn => {
assert!(alloc_id.is_none());
trace!("creating fn alloc id");
let instance = ty::Instance::decode(decoder)?;
@ -267,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)?;
@ -292,39 +291,42 @@ impl fmt::Display for AllocId {
}
#[derive(Debug, Clone, Eq, PartialEq, Hash, RustcDecodable, RustcEncodable)]
pub enum AllocType<'tcx, M> {
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).
/// 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, M> {
pub struct AllocMap<'tcx> {
/// Lets you know what an AllocId refers to
id_to_type: FxHashMap<AllocId, AllocType<'tcx, M>>,
id_to_kind: FxHashMap<AllocId, AllocKind<'tcx>>,
/// Used to ensure that functions and statics only get one associated AllocId
type_interner: FxHashMap<AllocType<'tcx, M>, AllocId>,
/// Used to ensure that statics only get one associated AllocId
type_interner: FxHashMap<AllocKind<'tcx>, 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(),
id_to_kind: Default::default(),
type_interner: Default::default(),
next_id: AllocId(0),
}
}
/// 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_alloc_id_memory` or `set_alloc_id_same_memory` before returning such
/// an `AllocId` from a query.
pub fn reserve(
&mut self,
) -> AllocId {
@ -337,53 +339,73 @@ impl<'tcx, M: fmt::Debug + Eq + Hash + Clone> AllocMap<'tcx, M> {
next
}
fn intern(&mut self, alloc_type: AllocType<'tcx, M>) -> 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_kind.insert(id, alloc_kind.clone());
self.type_interner.insert(alloc_kind, id);
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_kind.insert(id, AllocKind::Function(instance));
id
}
pub fn get(&self, id: AllocId) -> Option<AllocType<'tcx, M>> {
self.id_to_type.get(&id).cloned()
/// 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<AllocKind<'tcx>> {
self.id_to_kind.get(&id).cloned()
}
pub fn unwrap_memory(&self, id: AllocId) -> M {
/// 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),
}
}
/// 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))
}
pub fn allocate(&mut self, mem: M) -> AllocId {
/// 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);
self.set_alloc_id_memory(id, mem);
id
}
pub fn set_id_memory(&mut self, id: AllocId, mem: M) {
if let Some(old) = self.id_to_type.insert(id, AllocType::Memory(mem)) {
/// 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_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);
}
}
pub fn set_id_same_memory(&mut self, id: AllocId, mem: M) {
self.id_to_type.insert_same(id, AllocType::Memory(mem));
/// Freeze an `AllocId` created with `reserve` by pointing it at an `Allocation`. May be called
/// twice for the same `(AllocId, Allocation)` pair.
fn set_alloc_id_same_memory(&mut self, id: AllocId, mem: &'tcx Allocation) {
self.id_to_kind.insert_same(id, AllocKind::Memory(mem));
}
}

2
src/librustc/mir/interpret/value.rs
View File

@ -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>,

2
src/librustc/mir/mod.rs
View File

@ -2637,7 +2637,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)];

2
src/librustc/ty/context.rs
View File

@ -946,7 +946,7 @@ pub struct GlobalCtxt<'tcx> {
/// Stores the value of constants (and deduplicates the actual memory)
allocation_interner: Lock<FxHashMap<&'tcx Allocation, ()>>,
pub alloc_map: Lock<interpret::AllocMap<'tcx, &'tcx Allocation>>,
pub alloc_map: Lock<interpret::AllocMap<'tcx>>,
layout_interner: Lock<FxHashMap<&'tcx LayoutDetails, ()>>,

12
src/librustc_codegen_llvm/common.rs
View File

@ -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;
@ -316,9 +316,9 @@ 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)) => {
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 {
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)
}

28
src/librustc_mir/interpret/memory.rs
View File

@ -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<M::PointerTag, M::AllocExtra>>> {
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 => {
@ -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

4
src/librustc_mir/interpret/validity.rs
View File

@ -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

12
src/librustc_mir/monomorphize/collector.rs
View File

@ -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};
@ -1161,22 +1161,22 @@ fn collect_miri<'a, 'tcx>(
alloc_id: AllocId,
output: &mut Vec<MonoItem<'tcx>>,
) {
let alloc_type = tcx.alloc_map.lock().get(alloc_id);
match alloc_type {
Some(AllocType::Static(did)) => {
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) {
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));