From 828e7b685ade9d53a8f72f3509a98f2255787136 Mon Sep 17 00:00:00 2001 From: Ralf Jung 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: K, vacant: impl FnOnce() -> Result ) -> 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 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 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 { /// 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() } }