Skip to content

Commit

Permalink
Rollup merge of rust-lang#63076 - RalfJung:miri-fn-ptr-alloc-size, r=…
Browse files Browse the repository at this point in the history
…oli-obk

Miri: fix determining size of an "extra function" allocation

Fixes [a bug](rust-lang/miri#862) introduced by rust-lang#62982. Best reviewed commit-by-commit.

r? @oli-obk
  • Loading branch information
Centril authored Jul 29, 2019
2 parents b52a95d + 0e602f1 commit 5922a19
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 39 deletions.
10 changes: 10 additions & 0 deletions src/librustc_mir/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,16 @@ 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> {
self.get_or(k, || Err(())).ok()
}

/// Mutable lookup.
fn get_mut(&mut self, k: K) -> Option<&mut V> {
self.get_mut_or(k, || Err(())).ok()
}
}

/// Methods of this trait signifies a point where CTFE evaluation would fail
Expand Down
82 changes: 43 additions & 39 deletions src/librustc_mir/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -535,48 +535,52 @@ 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
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));
}

// # 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);
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(_)) =>
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
// 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)
},
}
}

Expand Down

0 comments on commit 5922a19

Please sign in to comment.