diff --git a/src/librustc_mir/interpret/error.rs b/src/librustc_mir/interpret/error.rs index 7cfff2d9810b..96911c10cca8 100644 --- a/src/librustc_mir/interpret/error.rs +++ b/src/librustc_mir/interpret/error.rs @@ -139,7 +139,7 @@ fn description(&self) -> &str { DoubleFree => "tried to deallocate dangling pointer", InvalidFunctionPointer => - "tried to use an integer pointer or a dangling pointer as a function pointer", + "tried to use a function pointer after offsetting it", InvalidBool => "invalid boolean value read", InvalidDiscriminant => diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 88fd254a2f25..c476046d3854 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -82,12 +82,55 @@ fn access_permitted(&self, frame: Option, access: AccessKind) -> bool { // Allocations and pointers //////////////////////////////////////////////////////////////////////////////// -#[derive(Copy, Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] -pub struct AllocId(pub u64); +#[derive(Copy, Clone, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub struct AllocId(u64); + +#[derive(Debug)] +enum AllocIdKind { + /// We can't ever have more than `usize::max_value` functions at the same time + /// since we never "deallocate" functions + Function(usize), + /// Locals and heap allocations (also statics for now, but those will get their + /// own variant soonish). + Runtime(u64), +} + +impl AllocIdKind { + fn into_alloc_id(self) -> AllocId { + match self { + AllocIdKind::Function(n) => AllocId(n as u64), + AllocIdKind::Runtime(n) => AllocId((1 << 63) | n), + } + } +} + +impl AllocId { + /// Currently yields the top bit to discriminate the `AllocIdKind`s + fn discriminant(self) -> u64 { + self.0 >> 63 + } + /// Yields everything but the discriminant bits + fn index(self) -> u64 { + self.0 & ((1 << 63) - 1) + } + fn destructure(self) -> AllocIdKind { + match self.discriminant() { + 0 => AllocIdKind::Function(self.index() as usize), + 1 => AllocIdKind::Runtime(self.index()), + n => bug!("got discriminant {} for AllocId", n), + } + } +} impl fmt::Display for AllocId { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", self.0) + write!(f, "{:?}", self.destructure()) + } +} + +impl fmt::Debug for AllocId { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{:?}", self.destructure()) } } @@ -186,10 +229,10 @@ pub struct Memory<'a, 'tcx, M: Machine<'tcx>> { pub data: M::MemoryData, /// Actual memory allocations (arbitrary bytes, may contain pointers into other allocations). - alloc_map: HashMap>, + alloc_map: HashMap>, - /// The AllocId to assign to the next new allocation. Always incremented, never gets smaller. - next_id: AllocId, + /// The AllocId to assign to the next new regular allocation. Always incremented, never gets smaller. + next_alloc_id: u64, /// Set of statics, constants, promoteds, vtables, ... to prevent `mark_static_initalized` from /// stepping out of its own allocations. This set only contains statics backed by an @@ -205,7 +248,7 @@ pub struct Memory<'a, 'tcx, M: Machine<'tcx>> { /// Function "allocations". They exist solely so pointers have something to point to, and /// we can figure out what they point to. - functions: HashMap>, + functions: Vec>, /// Inverse map of `functions` so we don't allocate a new pointer every time we need one function_alloc_cache: HashMap, AllocId>, @@ -231,9 +274,9 @@ pub fn new(layout: &'a TargetDataLayout, max_memory: u64, data: M::MemoryData) - Memory { data, alloc_map: HashMap::new(), - functions: HashMap::new(), + functions: Vec::new(), function_alloc_cache: HashMap::new(), - next_id: AllocId(0), + next_alloc_id: 0, layout, memory_size: max_memory, memory_usage: 0, @@ -245,20 +288,20 @@ pub fn new(layout: &'a TargetDataLayout, max_memory: u64, data: M::MemoryData) - } } - pub fn allocations(&self) -> ::std::collections::hash_map::Iter> { - self.alloc_map.iter() + pub fn allocations<'x>(&'x self) -> impl Iterator)> { + self.alloc_map.iter().map(|(&id, alloc)| (AllocIdKind::Runtime(id).into_alloc_id(), alloc)) } pub fn create_fn_alloc(&mut self, instance: ty::Instance<'tcx>) -> MemoryPointer { if let Some(&alloc_id) = self.function_alloc_cache.get(&instance) { return MemoryPointer::new(alloc_id, 0); } - let id = self.next_id; + let id = self.functions.len(); debug!("creating fn ptr: {}", id); - self.next_id.0 += 1; - self.functions.insert(id, instance); - self.function_alloc_cache.insert(instance, id); - MemoryPointer::new(id, 0) + self.functions.push(instance); + let alloc_id = AllocIdKind::Function(id).into_alloc_id(); + self.function_alloc_cache.insert(instance, alloc_id); + MemoryPointer::new(alloc_id, 0) } pub fn allocate_cached(&mut self, bytes: &[u8]) -> EvalResult<'tcx, MemoryPointer> { @@ -300,10 +343,10 @@ pub fn allocate( mutable: Mutability::Mutable, locks: RangeMap::new(), }; - let id = self.next_id; - self.next_id.0 += 1; + let id = self.next_alloc_id; + self.next_alloc_id += 1; self.alloc_map.insert(id, alloc); - Ok(MemoryPointer::new(id, 0)) + Ok(MemoryPointer::new(AllocIdKind::Runtime(id).into_alloc_id(), 0)) } pub fn reallocate( @@ -344,7 +387,13 @@ pub fn deallocate( return err!(DeallocateNonBasePtr); } - let alloc = match self.alloc_map.remove(&ptr.alloc_id) { + let alloc_id = match ptr.alloc_id.destructure() { + AllocIdKind::Function(_) => + return err!(DeallocatedWrongMemoryKind("function".to_string(), format!("{:?}", kind))), + AllocIdKind::Runtime(id) => id, + }; + + let alloc = match self.alloc_map.remove(&alloc_id) { Some(alloc) => alloc, None => return err!(DoubleFree), }; @@ -624,22 +673,22 @@ pub(crate) fn locks_lifetime_ended(&mut self, ending_region: Option) /// Allocation accessors impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> { pub fn get(&self, id: AllocId) -> EvalResult<'tcx, &Allocation> { - match self.alloc_map.get(&id) { - Some(alloc) => Ok(alloc), - None => match self.functions.get(&id) { - Some(_) => err!(DerefFunctionPointer), + match id.destructure() { + AllocIdKind::Function(_) => err!(DerefFunctionPointer), + AllocIdKind::Runtime(id) => match self.alloc_map.get(&id) { + Some(alloc) => Ok(alloc), None => err!(DanglingPointerDeref), - } + }, } } fn get_mut_unchecked(&mut self, id: AllocId) -> EvalResult<'tcx, &mut Allocation> { - match self.alloc_map.get_mut(&id) { - Some(alloc) => Ok(alloc), - None => match self.functions.get(&id) { - Some(_) => err!(DerefFunctionPointer), + match id.destructure() { + AllocIdKind::Function(_) => err!(DerefFunctionPointer), + AllocIdKind::Runtime(id) => match self.alloc_map.get_mut(&id) { + Some(alloc) => Ok(alloc), None => err!(DanglingPointerDeref), - } + }, } } @@ -657,12 +706,9 @@ pub fn get_fn(&self, ptr: MemoryPointer) -> EvalResult<'tcx, ty::Instance<'tcx>> return err!(InvalidFunctionPointer); } debug!("reading fn ptr: {}", ptr.alloc_id); - match self.functions.get(&ptr.alloc_id) { - Some(&fndef) => Ok(fndef), - None => match self.alloc_map.get(&ptr.alloc_id) { - Some(_) => err!(ExecuteMemory), - None => err!(InvalidFunctionPointer), - } + match ptr.alloc_id.destructure() { + AllocIdKind::Function(id) => Ok(self.functions[id]), + AllocIdKind::Runtime(_) => err!(ExecuteMemory), } } @@ -684,17 +730,18 @@ pub fn dump_allocs(&self, mut allocs: Vec) { let prefix_len = msg.len(); let mut relocations = vec![]; - let alloc = match (self.alloc_map.get(&id), self.functions.get(&id)) { - (Some(a), None) => a, - (None, Some(instance)) => { - trace!("{} {}", msg, instance); + let alloc = match id.destructure() { + AllocIdKind::Function(id) => { + trace!("{} {}", msg, self.functions[id]); continue; }, - (None, None) => { - trace!("{} (deallocated)", msg); - continue; + AllocIdKind::Runtime(id) => match self.alloc_map.get(&id) { + Some(a) => a, + None => { + trace!("{} (deallocated)", msg); + continue; + } }, - (Some(_), Some(_)) => bug!("miri invariant broken: an allocation id exists that points to both a function and a memory location"), }; for i in 0..(alloc.bytes.len() as u64) { @@ -745,7 +792,7 @@ pub fn leak_report(&self) -> usize { .iter() .filter_map(|(&key, val)| { if val.kind != Kind::Static { - Some(key) + Some(AllocIdKind::Runtime(key).into_alloc_id()) } else { None } @@ -834,6 +881,10 @@ pub fn mark_static_initalized(&mut self, alloc_id: AllocId, mutability: Mutabili trace!("mark_static_initalized {:?}, mutability: {:?}", alloc_id, mutability); // do not use `self.get_mut(alloc_id)` here, because we might have already marked a // sub-element or have circular pointers (e.g. `Rc`-cycles) + let alloc_id = match alloc_id.destructure() { + AllocIdKind::Function(_) => return Ok(()), + AllocIdKind::Runtime(id) => id, + }; let relocations = match self.alloc_map.get_mut(&alloc_id) { Some(&mut Allocation { ref mut relocations, ref mut kind, ref mut mutable, .. }) => { match *kind { @@ -854,8 +905,7 @@ pub fn mark_static_initalized(&mut self, alloc_id: AllocId, mutability: Mutabili // mark recursively mem::replace(relocations, Default::default()) }, - None if !self.functions.contains_key(&alloc_id) => return err!(DanglingPointerDeref), - _ => return Ok(()), + None => return err!(DanglingPointerDeref), }; // recurse into inner allocations for &alloc in relocations.values() { diff --git a/tests/compile-fail/fn_ptr_offset.rs b/tests/compile-fail/fn_ptr_offset.rs index 3e4c5d6ad391..45e32142a8c4 100644 --- a/tests/compile-fail/fn_ptr_offset.rs +++ b/tests/compile-fail/fn_ptr_offset.rs @@ -10,5 +10,5 @@ fn main() { let y : *mut u8 = unsafe { mem::transmute(x) }; let y = y.wrapping_offset(1); let x : fn() = unsafe { mem::transmute(y) }; - x(); //~ ERROR: tried to use an integer pointer or a dangling pointer as a function pointer + x(); //~ ERROR: tried to use a function pointer after offsetting it }