Split the alloc id address space into functions and normal allocs

instead of interleaving them as before.
The next step is to also separate out static memory into its own
address space.

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

src/librustc_mir/interpret/memory.rs

@@ -82,12 +82,55 @@ fn access_permitted(&self, frame: Option<usize>, 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<AllocId, Allocation<M::MemoryKinds>>,
+    alloc_map: HashMap<u64, Allocation<M::MemoryKinds>>,
 
-    /// 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<AllocId, ty::Instance<'tcx>>,
+    functions: Vec<ty::Instance<'tcx>>,
 
     /// Inverse map of `functions` so we don't allocate a new pointer every time we need one
     function_alloc_cache: HashMap<ty::Instance<'tcx>, 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<AllocId, Allocation<M::MemoryKinds>> {
-        self.alloc_map.iter()
+    pub fn allocations<'x>(&'x self) -> impl Iterator<Item = (AllocId, &'x Allocation<M::MemoryKinds>)> {
+        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<CodeExtent>)
 /// Allocation accessors
 impl<'a, 'tcx, M: Machine<'tcx>> Memory<'a, 'tcx, M> {
     pub fn get(&self, id: AllocId) -> EvalResult<'tcx, &Allocation<M::MemoryKinds>> {
-        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<M::MemoryKinds>> {
-        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<AllocId>) {
             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() {

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
 }