From b75dfa8a2bac745d7d09212e3e28cb4f0bc28fdf Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Thu, 25 Jul 2019 19:29:48 +0200
Subject: [PATCH 1/7] Don't access a static just for its size and alignment
---
src/librustc_mir/interpret/memory.rs | 27 ++++++++++++------------
src/test/ui/consts/static-cycle-error.rs | 11 ++++++++++
2 files changed, 24 insertions(+), 14 deletions(-)
create mode 100644 src/test/ui/consts/static-cycle-error.rs
diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
index 3f2a76a77be36..674ae29070644 100644
--- a/src/librustc_mir/interpret/memory.rs
+++ b/src/librustc_mir/interpret/memory.rs
@@ -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);
+ 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
diff --git a/src/test/ui/consts/static-cycle-error.rs b/src/test/ui/consts/static-cycle-error.rs
new file mode 100644
index 0000000000000..9ce050aae2181
--- /dev/null
+++ b/src/test/ui/consts/static-cycle-error.rs
@@ -0,0 +1,11 @@
+// check-pass
+
+struct Foo {
+ foo: Option<&'static Foo>
+}
+
+static FOO: Foo = Foo {
+ foo: Some(&FOO),
+};
+
+fn main() {}
From d9ac0c67ed5a3ea7d708894a4636a6e83c5aec49 Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Thu, 25 Jul 2019 23:53:05 +0200
Subject: [PATCH 2/7] Rewrite `get_size_and_align` so it doesn't duplicate work
---
src/librustc_mir/interpret/memory.rs | 69 ++++++++++++++--------------
1 file changed, 35 insertions(+), 34 deletions(-)
diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
index 674ae29070644..c8e09ca4a1a47 100644
--- a/src/librustc_mir/interpret/memory.rs
+++ b/src/librustc_mir/interpret/memory.rs
@@ -535,40 +535,41 @@ 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);
- 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));
- }
- // Function pointers.
- 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()))
- };
- }
- // The rest must be dead.
- 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)
+ let alloc_or_size_align = self.alloc_map.get_or(id, || -> Result<_, InterpResult<'static, (Size, Align)>> {
+ // 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);
+ Err(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)) =>
+ // this duplicates the logic on the `match alloc_or_size_align`, but due to the
+ // API of `get_or` there's no way around that.
+ 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)
+ },
+ })
+ });
+ match alloc_or_size_align {
+ Ok((_, alloc)) => Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)),
+ Err(done) => done,
}
}
From 34e7a3cc4dcb7ddd404b9566047a78d1e234f137 Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Fri, 26 Jul 2019 08:10:09 +0200
Subject: [PATCH 3/7] Fix tidy
---
src/librustc_mir/interpret/memory.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
index c8e09ca4a1a47..4ac5920c60e1a 100644
--- a/src/librustc_mir/interpret/memory.rs
+++ b/src/librustc_mir/interpret/memory.rs
@@ -535,7 +535,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
id: AllocId,
liveness: AllocCheck,
) -> InterpResult<'static, (Size, Align)> {
- let alloc_or_size_align = self.alloc_map.get_or(id, || -> Result<_, InterpResult<'static, (Size, Align)>> {
+ let alloc_or_size_align = self.alloc_map.get_or(id, || {
// 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);
From 3bc1d01bb9d81fa3682186d8ace06becaa8cac8c Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Fri, 26 Jul 2019 10:34:54 +0200
Subject: [PATCH 4/7] Clear up `get_size_and_align`
---
src/librustc_mir/interpret/memory.rs | 71 +++++++++++++++-------------
1 file changed, 37 insertions(+), 34 deletions(-)
diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
index 4ac5920c60e1a..9140c90bed376 100644
--- a/src/librustc_mir/interpret/memory.rs
+++ b/src/librustc_mir/interpret/memory.rs
@@ -535,41 +535,44 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
id: AllocId,
liveness: AllocCheck,
) -> InterpResult<'static, (Size, Align)> {
- let alloc_or_size_align = self.alloc_map.get_or(id, || {
- // 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);
- Err(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)) =>
- // this duplicates the logic on the `match alloc_or_size_align`, but due to the
- // API of `get_or` there's no way around that.
- 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)
- },
- })
- });
- match alloc_or_size_align {
+ // 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(done) => done,
+ Err(()) => {
+ // 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();
+ return 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.
+ return err!(DerefFunctionPointer);
+ } else {
+ return Ok((Size::ZERO, Align::from_bytes(1).unwrap()));
+ },
+ // The rest must be dead.
+ None => return 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 796e7a8d7ca1b06e07112579cdd74acc7b1c102b Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Fri, 26 Jul 2019 17:10:21 +0200
Subject: [PATCH 5/7] Address review comments
---
src/librustc_mir/interpret/memory.rs | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
index 9140c90bed376..934fa7f6f877b 100644
--- a/src/librustc_mir/interpret/memory.rs
+++ b/src/librustc_mir/interpret/memory.rs
@@ -541,6 +541,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
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);
@@ -549,20 +551,22 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
// 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));
+ 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.
- return err!(DerefFunctionPointer);
- } else {
- return Ok((Size::ZERO, Align::from_bytes(1).unwrap()));
+ 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 => return if let AllocCheck::MaybeDead = liveness {
+ 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)
From 6e04ca7fb6865f1481a7b2e635fd67b586dc0216 Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Fri, 26 Jul 2019 17:43:49 +0200
Subject: [PATCH 6/7] Update src/librustc_mir/interpret/memory.rs
Co-Authored-By: Ralf Jung <post@ralfj.de>
---
src/librustc_mir/interpret/memory.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
index 934fa7f6f877b..1981a239a1196 100644
--- a/src/librustc_mir/interpret/memory.rs
+++ b/src/librustc_mir/interpret/memory.rs
@@ -548,7 +548,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
let alloc = self.tcx.alloc_map.lock().get(id);
match alloc {
Some(GlobalAlloc::Static(did)) => {
- // Use size and align of the type
+ // 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))
From 0cd71678e17973ed40f898101d01588bf6f6757a Mon Sep 17 00:00:00 2001
From: Oliver Scherer <github35764891676564198441@oli-obk.de>
Date: Fri, 26 Jul 2019 17:44:11 +0200
Subject: [PATCH 7/7] Update src/librustc_mir/interpret/memory.rs
Co-Authored-By: Ralf Jung <post@ralfj.de>
---
src/librustc_mir/interpret/memory.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs
index 1981a239a1196..4575784ac3703 100644
--- a/src/librustc_mir/interpret/memory.rs
+++ b/src/librustc_mir/interpret/memory.rs
@@ -555,7 +555,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
},
Some(GlobalAlloc::Memory(alloc)) =>
// Need to duplicate the logic here, because the global allocations have
- // different associated types than the interpreter-local ones
+ // 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 {