From 828e7b685ade9d53a8f72f3509a98f2255787136 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sun, 28 Jul 2019 12:58:39 +0200
Subject: [PATCH 1/3] miri: add get and get_mut to AllocMap; use that in
 get_size_and_align and avoid rightwards drift

---
 src/librustc_mir/interpret/machine.rs | 16 ++++++
 src/librustc_mir/interpret/memory.rs  | 74 +++++++++++++--------------
 2 files changed, 52 insertions(+), 38 deletions(-)

diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs
index f37c474fa4fff..5808b0b7748f4 100644
--- a/src/librustc_mir/interpret/machine.rs
+++ b/src/librustc_mir/interpret/machine.rs
@@ -54,6 +54,22 @@ pub trait AllocMap<K: Hash + Eq, V> {
         k: K,
         vacant: impl FnOnce() -> Result<V, E>
     ) -> Result<&mut V, E>;
+
+    /// Read-only lookup.
+    fn get(&self, k: K) -> Option<&V> {
+        match self.get_or(k, || Err(())) {
+            Ok(v) => Some(v),
+            Err(()) => None,
+        }
+    }
+
+    /// Mutable lookup.
+    fn get_mut(&mut self, k: K) -> Option<&mut V> {
+        match self.get_mut_or(k, || Err(())) {
+            Ok(v) => Some(v),
+            Err(()) => None,
+        }
+    }
 }
 
 /// Methods of this trait signifies a point where CTFE evaluation would fail
diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
index 4575784ac3703..12be9baec132f 100644
--- a/src/librustc_mir/interpret/memory.rs
+++ b/src/librustc_mir/interpret/memory.rs
@@ -538,45 +538,43 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
         // Don't use `self.get` here as that will
         // a) cause cycles in case `id` refers to a static
         // b) duplicate a static's allocation in miri
-        match self.alloc_map.get_or(id, || Err(())) {
-            Ok((_, alloc)) => Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)),
-            Err(()) => {
-                // Not a local allocation, check the global `tcx.alloc_map`.
-
-                // Can't do this in the match argument, we may get cycle errors since the lock would
-                // be held throughout the match.
-                let alloc = self.tcx.alloc_map.lock().get(id);
-                match alloc {
-                    Some(GlobalAlloc::Static(did)) => {
-                        // Use size and align of the type.
-                        let ty = self.tcx.type_of(did);
-                        let layout = self.tcx.layout_of(ParamEnv::empty().and(ty)).unwrap();
-                        Ok((layout.size, layout.align.abi))
-                    },
-                    Some(GlobalAlloc::Memory(alloc)) =>
-                        // Need to duplicate the logic here, because the global allocations have
-                        // different associated types than the interpreter-local ones.
-                        Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)),
-                    Some(GlobalAlloc::Function(_)) => {
-                        if let AllocCheck::Dereferencable = liveness {
-                            // The caller requested no function pointers.
-                            err!(DerefFunctionPointer)
-                        } else {
-                            Ok((Size::ZERO, Align::from_bytes(1).unwrap()))
-                        }
-                    },
-                    // The rest must be dead.
-                    None => if let AllocCheck::MaybeDead = liveness {
-                        // Deallocated pointers are allowed, we should be able to find
-                        // them in the map.
-                        Ok(*self.dead_alloc_map.get(&id)
-                            .expect("deallocated pointers should all be recorded in \
-                                    `dead_alloc_map`"))
-                    } else {
-                        err!(DanglingPointerDeref)
-                    },
+        if let Some((_, alloc)) = self.alloc_map.get(id) {
+            return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align));
+        }
+        // Not a local allocation, check the global `tcx.alloc_map`.
+
+        // Can't do this in the match argument, we may get cycle errors since the lock would
+        // be held throughout the match.
+        let alloc = self.tcx.alloc_map.lock().get(id);
+        match alloc {
+            Some(GlobalAlloc::Static(did)) => {
+                // Use size and align of the type.
+                let ty = self.tcx.type_of(did);
+                let layout = self.tcx.layout_of(ParamEnv::empty().and(ty)).unwrap();
+                Ok((layout.size, layout.align.abi))
+            },
+            Some(GlobalAlloc::Memory(alloc)) =>
+                // Need to duplicate the logic here, because the global allocations have
+                // different associated types than the interpreter-local ones.
+                Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)),
+            Some(GlobalAlloc::Function(_)) => {
+                if let AllocCheck::Dereferencable = liveness {
+                    // The caller requested no function pointers.
+                    err!(DerefFunctionPointer)
+                } else {
+                    Ok((Size::ZERO, Align::from_bytes(1).unwrap()))
                 }
-            }
+            },
+            // The rest must be dead.
+            None => if let AllocCheck::MaybeDead = liveness {
+                // Deallocated pointers are allowed, we should be able to find
+                // them in the map.
+                Ok(*self.dead_alloc_map.get(&id)
+                    .expect("deallocated pointers should all be recorded in \
+                            `dead_alloc_map`"))
+            } else {
+                err!(DanglingPointerDeref)
+            },
         }
     }
 

From c119291b55f2a14a2b0e13fbd3d54a9692f1066f Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sun, 28 Jul 2019 13:02:01 +0200
Subject: [PATCH 2/3] get_size_and_align: fix handling of function pointers

---
 src/librustc_mir/interpret/memory.rs | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
index 12be9baec132f..87dd7738410ee 100644
--- a/src/librustc_mir/interpret/memory.rs
+++ b/src/librustc_mir/interpret/memory.rs
@@ -535,14 +535,26 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
         id: AllocId,
         liveness: AllocCheck,
     ) -> InterpResult<'static, (Size, Align)> {
+        // # Regular allocations
         // Don't use `self.get` here as that will
         // a) cause cycles in case `id` refers to a static
         // b) duplicate a static's allocation in miri
         if let Some((_, alloc)) = self.alloc_map.get(id) {
             return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align));
         }
-        // Not a local allocation, check the global `tcx.alloc_map`.
 
+        // # Function pointers
+        // (both global from `alloc_map` and local from `extra_fn_ptr_map`)
+        if let Ok(_) = self.get_fn_alloc(id) {
+            return if let AllocCheck::Dereferencable = liveness {
+                // The caller requested no function pointers.
+                err!(DerefFunctionPointer)
+            } else {
+                Ok((Size::ZERO, Align::from_bytes(1).unwrap()))
+            };
+        }
+
+        // # Statics
         // Can't do this in the match argument, we may get cycle errors since the lock would
         // be held throughout the match.
         let alloc = self.tcx.alloc_map.lock().get(id);
@@ -557,14 +569,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
                 // Need to duplicate the logic here, because the global allocations have
                 // different associated types than the interpreter-local ones.
                 Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)),
-            Some(GlobalAlloc::Function(_)) => {
-                if let AllocCheck::Dereferencable = liveness {
-                    // The caller requested no function pointers.
-                    err!(DerefFunctionPointer)
-                } else {
-                    Ok((Size::ZERO, Align::from_bytes(1).unwrap()))
-                }
-            },
+            Some(GlobalAlloc::Function(_)) =>
+                bug!("We already checked function pointers above"),
             // The rest must be dead.
             None => if let AllocCheck::MaybeDead = liveness {
                 // Deallocated pointers are allowed, we should be able to find

From 0e602f10b55da3b751859d862a8cddba719ecd6f Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sun, 28 Jul 2019 22:30:19 +0200
Subject: [PATCH 3/3] replace match by ok()

---
 src/librustc_mir/interpret/machine.rs | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs
index 5808b0b7748f4..e3f16a3c9ea45 100644
--- a/src/librustc_mir/interpret/machine.rs
+++ b/src/librustc_mir/interpret/machine.rs
@@ -57,18 +57,12 @@ pub trait AllocMap<K: Hash + Eq, V> {
 
     /// Read-only lookup.
     fn get(&self, k: K) -> Option<&V> {
-        match self.get_or(k, || Err(())) {
-            Ok(v) => Some(v),
-            Err(()) => None,
-        }
+        self.get_or(k, || Err(())).ok()
     }
 
     /// Mutable lookup.
     fn get_mut(&mut self, k: K) -> Option<&mut V> {
-        match self.get_mut_or(k, || Err(())) {
-            Ok(v) => Some(v),
-            Err(()) => None,
-        }
+        self.get_mut_or(k, || Err(())).ok()
     }
 }