Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't access a static just for its size and alignment #62982

Merged
merged 7 commits into from Jul 27, 2019
Next

Don't access a static just for its size and alignment

  • Loading branch information...
oli-obk committed Jul 25, 2019
commit b75dfa8a2bac745d7d09212e3e28cb4f0bc28fdf
@@ -535,6 +535,19 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
id: AllocId,
liveness: AllocCheck,
) -> InterpResult<'static, (Size, Align)> {
// Allocations of `static` items
// 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);
This conversation was marked as resolved by RalfJung

This comment has been minimized.

Copy link
@RalfJung

RalfJung Jul 25, 2019

Member

Isn't it very expensive to always try to get this global lock?

Also a comment should explain why this is done first.

This comment has been minimized.

Copy link
@RalfJung

RalfJung Jul 25, 2019

Member

Maybe a better ordering here is to first lookup directly in the local alloc_map (that will cover the vast majority of cases), then ask the global map, and only then proceed with get.

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();
return Ok((layout.size, layout.align.abi));
}
_ => {}
}
// Regular allocations.
if let Ok(alloc) = self.get(id) {
return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align));
@@ -548,20 +561,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
Ok((Size::ZERO, Align::from_bytes(1).unwrap()))
};
}
// Foreign 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)) => {
assert!(self.tcx.is_foreign_item(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();
return Ok((layout.size, layout.align.abi));
}
_ => {}
}
// The rest must be dead.
if let AllocCheck::MaybeDead = liveness {
// Deallocated pointers are allowed, we should be able to find
@@ -0,0 +1,11 @@
// check-pass

struct Foo {
foo: Option<&'static Foo>
}

static FOO: Foo = Foo {
foo: Some(&FOO),
};

fn main() {}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.