Browse Source

Fix broken assertion

The first assertion in Bookkeeper::push was broken due to executing when the vector is empty triggering an out-of-bound panic.
ticki 8 years ago
parent
commit
ddad677c34
6 changed files with 17 additions and 43 deletions
  1. 4 2
      src/block.rs
  2. 3 2
      src/bookkeeper.rs
  3. 1 1
      src/brk.rs
  4. 1 2
      src/lib.rs
  5. 5 22
      src/vec.rs
  6. 3 14
      src/write.rs

+ 4 - 2
src/block.rs

@@ -3,12 +3,13 @@
 //! Blocks are the main unit for the memory bookkeeping. A block is a simple construct with a
 //! Blocks are the main unit for the memory bookkeeping. A block is a simple construct with a
 //! `Pointer` pointer and a size. Occupied (non-free) blocks are represented by a zero-sized block.
 //! `Pointer` pointer and a size. Occupied (non-free) blocks are represented by a zero-sized block.
 
 
+// TODO check the allow(cast_possible_wrap)s again.
+
 use prelude::*;
 use prelude::*;
 
 
 use {sys, fail};
 use {sys, fail};
 
 
 use core::{ptr, cmp, mem, fmt};
 use core::{ptr, cmp, mem, fmt};
-use core::convert::TryInto;
 
 
 /// A contiguous memory block.
 /// A contiguous memory block.
 ///
 ///
@@ -46,11 +47,12 @@ impl Block {
 
 
     /// BRK allocate a block.
     /// BRK allocate a block.
     #[inline]
     #[inline]
+    #[allow(cast_possible_wrap)]
     pub fn brk(size: usize) -> Block {
     pub fn brk(size: usize) -> Block {
         Block {
         Block {
             size: size,
             size: size,
             ptr: unsafe {
             ptr: unsafe {
-                Pointer::new(sys::sbrk(size.try_into().unwrap()).unwrap_or_else(|()| fail::oom()))
+                Pointer::new(sys::sbrk(size as isize).unwrap_or_else(|()| fail::oom()))
             },
             },
         }
         }
     }
     }

+ 3 - 2
src/bookkeeper.rs

@@ -578,7 +578,8 @@ pub trait Allocator: ops::DerefMut<Target = Bookkeeper> {
         log!(self.pool;self.pool.len(), "Pushing {:?}.", block);
         log!(self.pool;self.pool.len(), "Pushing {:?}.", block);
 
 
         // Some assertions...
         // Some assertions...
-        debug_assert!(&block <= self.pool.last().unwrap(), "Pushing will make the list unsorted.");
+        debug_assert!(self.pool.is_empty() || &block <= self.pool.last().unwrap(), "Pushing will \
+                      make the list unsorted.");
 
 
         // Short-circuit in case on empty block.
         // Short-circuit in case on empty block.
         if !block.is_empty() {
         if !block.is_empty() {
@@ -752,7 +753,7 @@ pub trait Allocator: ops::DerefMut<Target = Bookkeeper> {
         // Logging.
         // Logging.
         log!(self.pool;ind, "Removing block.");
         log!(self.pool;ind, "Removing block.");
 
 
-        if ind == self.pool.len() - 1 {
+        if ind + 1 == self.pool.len() {
             let res = self.pool[ind].pop();
             let res = self.pool[ind].pop();
             // Make sure there are no trailing empty blocks.
             // Make sure there are no trailing empty blocks.
             let new_len = self.pool.len() - self.pool.iter().rev().take_while(|x| x.is_empty()).count();
             let new_len = self.pool.len() - self.pool.iter().rev().take_while(|x| x.is_empty()).count();

+ 1 - 1
src/brk.rs

@@ -23,7 +23,7 @@ fn canonicalize_space(min: usize) -> usize {
     /// The minimum size to be BRK'd.
     /// The minimum size to be BRK'd.
     const BRK_MIN: usize = 1024;
     const BRK_MIN: usize = 1024;
     /// The maximal amount of _extra_ elements.
     /// The maximal amount of _extra_ elements.
-    const BRK_MAX_EXTRA: usize = 4 * 65536;
+    const BRK_MAX_EXTRA: usize = 65536;
 
 
     let res = cmp::max(BRK_MIN, min + cmp::min(BRK_MULTIPLIER * min, BRK_MAX_EXTRA));
     let res = cmp::max(BRK_MIN, min + cmp::min(BRK_MULTIPLIER * min, BRK_MAX_EXTRA));
 
 

+ 1 - 2
src/lib.rs

@@ -16,8 +16,7 @@
 #![no_std]
 #![no_std]
 
 
 #![feature(allocator, const_fn, core_intrinsics, stmt_expr_attributes, drop_types_in_const,
 #![feature(allocator, const_fn, core_intrinsics, stmt_expr_attributes, drop_types_in_const,
-           nonzero, optin_builtin_traits, type_ascription, question_mark, try_from, thread_local,
-           linkage)]
+           nonzero, optin_builtin_traits, type_ascription, question_mark, thread_local, linkage)]
 #![warn(missing_docs, cast_precision_loss, cast_sign_loss, cast_possible_wrap,
 #![warn(missing_docs, cast_precision_loss, cast_sign_loss, cast_possible_wrap,
         cast_possible_truncation, filter_map, if_not_else, items_after_statements,
         cast_possible_truncation, filter_map, if_not_else, items_after_statements,
         invalid_upcast_comparisons, mutex_integer, nonminimal_bool, shadow_same, shadow_unrelated,
         invalid_upcast_comparisons, mutex_integer, nonminimal_bool, shadow_same, shadow_unrelated,

+ 5 - 22
src/vec.rs

@@ -32,9 +32,9 @@ impl<T: Leak> Vec<T> {
     #[inline]
     #[inline]
     pub unsafe fn from_raw_parts(block: Block, len: usize) -> Vec<T> {
     pub unsafe fn from_raw_parts(block: Block, len: usize) -> Vec<T> {
         Vec {
         Vec {
-            cap: block.size() / mem::size_of::<T>(),
-            ptr: Pointer::new(*Pointer::from(block) as *mut T),
             len: len,
             len: len,
+            cap: block.size() / mem::size_of::<T>(),
+            ptr: Pointer::from(block).cast(),
         }
         }
     }
     }
 
 
@@ -45,8 +45,7 @@ impl<T: Leak> Vec<T> {
     ///
     ///
     /// # Panics
     /// # Panics
     ///
     ///
-    /// This panics if the vector is bigger than the block. Additional checks might be done in
-    /// debug mode.
+    /// This panics if the vector is bigger than the block.
     pub fn refill(&mut self, block: Block) -> Block {
     pub fn refill(&mut self, block: Block) -> Block {
         // Calculate the new capacity.
         // Calculate the new capacity.
         let new_cap = block.size() / mem::size_of::<T>();
         let new_cap = block.size() / mem::size_of::<T>();
@@ -55,8 +54,6 @@ impl<T: Leak> Vec<T> {
         assert!(self.len <= new_cap, "Block not large enough to cover the vector.");
         assert!(self.len <= new_cap, "Block not large enough to cover the vector.");
         assert!(block.aligned_to(mem::align_of::<T>()), "Block not aligned.");
         assert!(block.aligned_to(mem::align_of::<T>()), "Block not aligned.");
 
 
-        self.check(&block);
-
         let old = mem::replace(self, Vec::default());
         let old = mem::replace(self, Vec::default());
 
 
         // Update the fields of `self`.
         // Update the fields of `self`.
@@ -130,20 +127,6 @@ impl<T: Leak> Vec<T> {
 
 
         self.len = len;
         self.len = len;
     }
     }
-
-    /// Check the validity of a block with respect to the vector.
-    ///
-    /// Blocks not passing this checks might lead to logic errors when used as buffer for the
-    /// vector.
-    ///
-    /// This is a NO-OP in release mode.
-    #[inline]
-    fn check(&self, block: &Block) {
-        debug_assert!(block.size() % mem::size_of::<T>() == 0, "The size of T does not divide the \
-                      block's size.");
-        debug_assert!(self.len <= block.size() / mem::size_of::<T>(), "Block not large enough to \
-                      cover the vector.");
-    }
 }
 }
 
 
 // TODO remove this in favour of `derive` when rust-lang/rust#35263 is fixed.
 // TODO remove this in favour of `derive` when rust-lang/rust#35263 is fixed.
@@ -170,7 +153,7 @@ impl<T: Leak> ops::Deref for Vec<T> {
     #[inline]
     #[inline]
     fn deref(&self) -> &[T] {
     fn deref(&self) -> &[T] {
         unsafe {
         unsafe {
-            slice::from_raw_parts(*self.ptr as *const _, self.len)
+            slice::from_raw_parts(*self.ptr as *const T, self.len)
         }
         }
     }
     }
 }
 }
@@ -179,7 +162,7 @@ impl<T: Leak> ops::DerefMut for Vec<T> {
     #[inline]
     #[inline]
     fn deref_mut(&mut self) -> &mut [T] {
     fn deref_mut(&mut self) -> &mut [T] {
         unsafe {
         unsafe {
-            slice::from_raw_parts_mut(*self.ptr as *mut _, self.len)
+            slice::from_raw_parts_mut(*self.ptr as *mut T, self.len)
         }
         }
     }
     }
 }
 }

+ 3 - 14
src/write.rs

@@ -41,20 +41,9 @@ impl fmt::Write for Writer {
 /// allows for aborting, non-allocating panics when running the tests.
 /// allows for aborting, non-allocating panics when running the tests.
 #[macro_export]
 #[macro_export]
 macro_rules! assert {
 macro_rules! assert {
-    ($e:expr) => {{
-        use write;
-
-        use core::intrinsics;
-        use core::fmt::Write;
-
-        if !$e {
-            let _ = write!(write::Writer::stderr(), "assertion failed at {}:{}: {}", file!(),
-                           line!(), stringify!($e));
-
-            #[allow(unused_unsafe)]
-            unsafe { intrinsics::abort() }
-        }
-    }};
+    ($e:expr) => {
+        assert!($e, "No description.");
+    };
     ($e:expr, $( $arg:expr ),*) => {{
     ($e:expr, $( $arg:expr ),*) => {{
         use write;
         use write;