From 594f1d79da1eebb2acb674b686d98b4fc1ef165f Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Thu, 30 Jun 2016 16:42:09 +0200 Subject: [PATCH 1/2] optimize all ZST allocations into one single allocation --- src/interpreter/terminator.rs | 4 ++-- src/memory.rs | 45 +++++++++++++++++++++++++++++++---- tests/run-pass/zst.rs | 8 +++++++ 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/src/interpreter/terminator.rs b/src/interpreter/terminator.rs index a9bb1fa531..8f5ec0e1b5 100644 --- a/src/interpreter/terminator.rs +++ b/src/interpreter/terminator.rs @@ -423,8 +423,8 @@ impl<'a, 'tcx> EvalContext<'a, 'tcx> { "__rust_reallocate" => { let ptr = self.memory.read_ptr(args[0])?; let size = self.memory.read_usize(args[2])?; - self.memory.reallocate(ptr, size as usize)?; - self.memory.write_ptr(dest, ptr)?; + let new_ptr = self.memory.reallocate(ptr, size as usize)?; + self.memory.write_ptr(dest, new_ptr)?; } "memcmp" => { diff --git a/src/memory.rs b/src/memory.rs index 64c105a4e7..25612c435e 100644 --- a/src/memory.rs +++ b/src/memory.rs @@ -39,8 +39,18 @@ pub struct Pointer { impl Pointer { pub fn offset(self, i: isize) -> Self { + // FIXME: prevent offsetting ZST ptrs in tracing mode Pointer { offset: (self.offset as isize + i) as usize, ..self } } + pub fn points_to_zst(&self) -> bool { + self.alloc_id.0 == 0 + } + fn zst_ptr() -> Self { + Pointer { + alloc_id: AllocId(0), + offset: 0, + } + } } #[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)] @@ -68,13 +78,25 @@ pub struct Memory<'a, 'tcx> { impl<'a, 'tcx> Memory<'a, 'tcx> { pub fn new(layout: &'a TargetDataLayout) -> Self { - Memory { + let mut mem = Memory { alloc_map: HashMap::new(), functions: HashMap::new(), function_alloc_cache: HashMap::new(), - next_id: AllocId(0), + next_id: AllocId(1), layout: layout, - } + }; + // alloc id 0 is reserved for ZSTs, this is an optimization to prevent ZST + // (e.g. function pointers, (), [], ...) from requiring memory + let alloc = Allocation { + bytes: Vec::new(), + relocations: BTreeMap::new(), + undef_mask: UndefMask::new(0), + }; + mem.alloc_map.insert(AllocId(0), alloc); + // check that additional zst allocs work + debug_assert!(mem.allocate(0).points_to_zst()); + debug_assert!(mem.get(AllocId(0)).is_ok()); + mem } pub fn allocations<'b>(&'b self) -> ::std::collections::hash_map::Iter<'b, AllocId, Allocation> { @@ -105,6 +127,9 @@ impl<'a, 'tcx> Memory<'a, 'tcx> { } pub fn allocate(&mut self, size: usize) -> Pointer { + if size == 0 { + return Pointer::zst_ptr(); + } let alloc = Allocation { bytes: vec![0; size], relocations: BTreeMap::new(), @@ -121,7 +146,14 @@ impl<'a, 'tcx> Memory<'a, 'tcx> { // TODO(solson): Track which allocations were returned from __rust_allocate and report an error // when reallocating/deallocating any others. - pub fn reallocate(&mut self, ptr: Pointer, new_size: usize) -> EvalResult<'tcx, ()> { + pub fn reallocate(&mut self, ptr: Pointer, new_size: usize) -> EvalResult<'tcx, Pointer> { + if ptr.points_to_zst() { + if new_size != 0 { + return Ok(self.allocate(new_size)); + } else { + return Ok(ptr); + } + } if ptr.offset != 0 { // TODO(solson): Report error about non-__rust_allocate'd pointer. return Err(EvalError::Unimplemented(format!("bad pointer offset: {}", ptr.offset))); @@ -141,11 +173,14 @@ impl<'a, 'tcx> Memory<'a, 'tcx> { alloc.undef_mask.truncate(new_size); } - Ok(()) + Ok(ptr) } // TODO(solson): See comment on `reallocate`. pub fn deallocate(&mut self, ptr: Pointer) -> EvalResult<'tcx, ()> { + if ptr.points_to_zst() { + return Ok(()); + } if ptr.offset != 0 { // TODO(solson): Report error about non-__rust_allocate'd pointer. return Err(EvalError::Unimplemented(format!("bad pointer offset: {}", ptr.offset))); diff --git a/tests/run-pass/zst.rs b/tests/run-pass/zst.rs index db98e96930..4ebb2001e7 100644 --- a/tests/run-pass/zst.rs +++ b/tests/run-pass/zst.rs @@ -1,3 +1,9 @@ +// the following flag prevents this test from running on the host machine +// this should only be run on miri, because rust doesn't (yet?) optimize ZSTs of different types +// into the same memory location +// ignore-test + + #[derive(PartialEq, Debug)] struct A; @@ -13,4 +19,6 @@ fn use_zst() -> A { fn main() { assert_eq!(zst_ret(), A); assert_eq!(use_zst(), A); + assert_eq!(&A as *const A as *const (), &() as *const _); + assert_eq!(&A as *const A, &A as *const A); } From 3d9588332f82a085586b0a847715ff3835986861 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 1 Jul 2016 13:08:19 +0200 Subject: [PATCH 2/2] address comments --- src/memory.rs | 23 +++++++++++------------ tests/compile-fail/out_of_bounds_read.rs | 2 +- tests/compile-fail/out_of_bounds_read2.rs | 5 +++++ 3 files changed, 17 insertions(+), 13 deletions(-) create mode 100644 tests/compile-fail/out_of_bounds_read2.rs diff --git a/src/memory.rs b/src/memory.rs index 25612c435e..2f5b4470cf 100644 --- a/src/memory.rs +++ b/src/memory.rs @@ -39,7 +39,6 @@ pub struct Pointer { impl Pointer { pub fn offset(self, i: isize) -> Self { - // FIXME: prevent offsetting ZST ptrs in tracing mode Pointer { offset: (self.offset as isize + i) as usize, ..self } } pub fn points_to_zst(&self) -> bool { @@ -47,7 +46,7 @@ impl Pointer { } fn zst_ptr() -> Self { Pointer { - alloc_id: AllocId(0), + alloc_id: ZST_ALLOC_ID, offset: 0, } } @@ -76,6 +75,8 @@ pub struct Memory<'a, 'tcx> { pub layout: &'a TargetDataLayout, } +const ZST_ALLOC_ID: AllocId = AllocId(0); + impl<'a, 'tcx> Memory<'a, 'tcx> { pub fn new(layout: &'a TargetDataLayout) -> Self { let mut mem = Memory { @@ -86,16 +87,16 @@ impl<'a, 'tcx> Memory<'a, 'tcx> { layout: layout, }; // alloc id 0 is reserved for ZSTs, this is an optimization to prevent ZST - // (e.g. function pointers, (), [], ...) from requiring memory + // (e.g. function items, (), [], ...) from requiring memory let alloc = Allocation { bytes: Vec::new(), relocations: BTreeMap::new(), undef_mask: UndefMask::new(0), }; - mem.alloc_map.insert(AllocId(0), alloc); + mem.alloc_map.insert(ZST_ALLOC_ID, alloc); // check that additional zst allocs work debug_assert!(mem.allocate(0).points_to_zst()); - debug_assert!(mem.get(AllocId(0)).is_ok()); + debug_assert!(mem.get(ZST_ALLOC_ID).is_ok()); mem } @@ -147,17 +148,13 @@ impl<'a, 'tcx> Memory<'a, 'tcx> { // TODO(solson): Track which allocations were returned from __rust_allocate and report an error // when reallocating/deallocating any others. pub fn reallocate(&mut self, ptr: Pointer, new_size: usize) -> EvalResult<'tcx, Pointer> { - if ptr.points_to_zst() { - if new_size != 0 { - return Ok(self.allocate(new_size)); - } else { - return Ok(ptr); - } - } if ptr.offset != 0 { // TODO(solson): Report error about non-__rust_allocate'd pointer. return Err(EvalError::Unimplemented(format!("bad pointer offset: {}", ptr.offset))); } + if ptr.points_to_zst() { + return Ok(self.allocate(new_size)); + } let size = self.get_mut(ptr.alloc_id)?.bytes.len(); @@ -187,10 +184,12 @@ impl<'a, 'tcx> Memory<'a, 'tcx> { } if self.alloc_map.remove(&ptr.alloc_id).is_none() { + debug!("deallocated a pointer twice: {}", ptr.alloc_id); // TODO(solson): Report error about erroneous free. This is blocked on properly tracking // already-dropped state since this if-statement is entered even in safe code without // it. } + debug!("deallocated : {}", ptr.alloc_id); Ok(()) } diff --git a/tests/compile-fail/out_of_bounds_read.rs b/tests/compile-fail/out_of_bounds_read.rs index fef6ff66c3..f6a305840c 100644 --- a/tests/compile-fail/out_of_bounds_read.rs +++ b/tests/compile-fail/out_of_bounds_read.rs @@ -1,5 +1,5 @@ fn main() { let v: Vec = vec![1, 2]; - let x = unsafe { *v.get_unchecked(5) }; //~ ERROR: memory access of 5..6 outside bounds of allocation 31 which has size 2 + let x = unsafe { *v.get_unchecked(5) }; //~ ERROR: which has size 2 panic!("this should never print: {}", x); } diff --git a/tests/compile-fail/out_of_bounds_read2.rs b/tests/compile-fail/out_of_bounds_read2.rs new file mode 100644 index 0000000000..5509a8346e --- /dev/null +++ b/tests/compile-fail/out_of_bounds_read2.rs @@ -0,0 +1,5 @@ +fn main() { + let v: Vec = vec![1, 2]; + let x = unsafe { *v.get_unchecked(5) }; //~ ERROR: memory access of 5..6 outside bounds of allocation + panic!("this should never print: {}", x); +}