servo: Merge #18789 - Assert more things in hashtables (from bholley:more_hashmap_asserts); r=Manishearth

https://bugzilla.mozilla.org/show_bug.cgi?id=1406815

Source-Repo: https://github.com/servo/servo
Source-Revision: 49376c9e544c6b3f0d340dff0450ad3267219428

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : bd5bd5b614114c1e46c1446a0534cc7e48e84d91

servo/components/hashglobe/src/table.rs

@@ -43,7 +43,7 @@ struct TaggedHashUintPtr(Unique<HashUint>);
 impl TaggedHashUintPtr {
     #[inline]
     unsafe fn new(ptr: *mut HashUint) -> Self {
-        debug_assert!(ptr as usize & 1 == 0 || ptr as usize == EMPTY as usize);
+        assert!(ptr as usize & 1 == 0 || ptr as usize == EMPTY as usize);
         TaggedHashUintPtr(Unique::new_unchecked(ptr))
     }
 
@@ -117,6 +117,7 @@ pub struct RawTable<K, V> {
     capacity_mask: usize,
     size: usize,
     hashes: TaggedHashUintPtr,
+    bytes_allocated: usize,
 
     // Because K/V do not appear directly in any of the types in the struct,
     // inform rustc that in fact instances of K and V are reachable from here.
@@ -245,6 +246,15 @@ impl<K, V> RawBucket<K, V> {
     unsafe fn hash_pair(&self) -> (*mut HashUint, *mut (K, V)) {
         (self.hash(), self.pair())
     }
+
+    fn assert_bounds(&self, bytes_allocated: usize) {
+        let base = self.hash_start as *mut u8;
+        let (h, p) = unsafe { self.hash_pair() };
+        assert!((h as *mut u8) < (p as *mut u8), "HashMap Corruption - hash offset not below pair offset");
+        let end = unsafe { p.offset(1) } as *mut u8;
+        assert!(end > base, "HashMap Corruption - end={:?}, base={:?}", end, base);
+        assert!(end <= unsafe { base.offset(bytes_allocated as isize) }, "HashMap Corruption - end={:?}, base={:?}", end, base);
+    }
 }
 
 // Buckets hold references to the table.
@@ -348,8 +358,7 @@ impl<K, V, M: Deref<Target = RawTable<K, V>>> Bucket<K, V, M> {
     pub fn at_index(table: M, ib_index: usize) -> Bucket<K, V, M> {
         // if capacity is 0, then the RawBucket will be populated with bogus pointers.
         // This is an uncommon case though, so avoid it in release builds.
-        debug_assert!(table.capacity() > 0,
-                      "Table should have capacity at this point");
+        assert!(table.capacity() > 0, "HashMap Corruption - Table should have capacity at this point");
         let ib_index = ib_index & table.capacity_mask;
         Bucket {
             raw: table.raw_bucket_at(ib_index),
@@ -422,11 +431,13 @@ impl<K, V, M: Deref<Target = RawTable<K, V>>> Bucket<K, V, M> {
     /// Modifies the bucket in place to make it point to the next slot.
     pub fn next(&mut self) {
         self.raw.idx = self.raw.idx.wrapping_add(1) & self.table.capacity_mask;
+        self.raw.assert_bounds(self.table.bytes_allocated);
     }
 
     /// Modifies the bucket in place to make it point to the previous slot.
     pub fn prev(&mut self) {
         self.raw.idx = self.raw.idx.wrapping_sub(1) & self.table.capacity_mask;
+        self.raw.assert_bounds(self.table.bytes_allocated);
     }
 }
 
@@ -549,7 +560,7 @@ impl<'t, K, V> FullBucket<K, V, &'t mut RawTable<K, V>> {
     /// This works similarly to `put`, building an `EmptyBucket` out of the
     /// taken bucket.
     pub fn take(self) -> (EmptyBucket<K, V, &'t mut RawTable<K, V>>, K, V) {
-        self.table.size -= 1;
+        self.table.size = self.table.size.checked_sub(1).unwrap();
 
         unsafe {
             *self.raw.hash() = EMPTY_BUCKET;
@@ -664,7 +675,7 @@ impl<K, V, M> GapThenFull<K, V, M>
 /// Panics if `target_alignment` is not a power of two.
 #[inline]
 fn round_up_to_next(unrounded: usize, target_alignment: usize) -> usize {
-    assert!(target_alignment.is_power_of_two());
+    assert!(target_alignment.is_power_of_two(), "HashMap Corruption - alignment not power of two");
     (unrounded + target_alignment - 1) & !(target_alignment - 1)
 }
 
@@ -734,9 +745,11 @@ impl<K, V> RawTable<K, V> {
                 size: 0,
                 capacity_mask: capacity.wrapping_sub(1),
                 hashes: TaggedHashUintPtr::new(EMPTY as *mut HashUint),
+                bytes_allocated: 0,
                 marker: marker::PhantomData,
             });
         }
+        assert!(capacity.is_power_of_two(), "HashMap Corruption - capacity not power of two");
 
         // No need for `checked_mul` before a more restrictive check performed
         // later in this method.
@@ -788,11 +801,13 @@ impl<K, V> RawTable<K, V> {
         ptr::write_bytes(buffer, 0xe7, size);
 
         let hashes = buffer.offset(hash_offset as isize) as *mut HashUint;
+        assert!(hashes as *mut u8 == buffer, "HashMap Corruption - Nonzero hash_offset");
 
         Ok(RawTable {
             capacity_mask: capacity.wrapping_sub(1),
             size: 0,
             hashes: TaggedHashUintPtr::new(hashes),
+            bytes_allocated: size,
             marker: marker::PhantomData,
         })
     }
@@ -803,17 +818,23 @@ impl<K, V> RawTable<K, V> {
 
         let (pairs_offset, _, oflo) =
             calculate_offsets(hashes_size, pairs_size, align_of::<(K, V)>());
-        debug_assert!(!oflo, "capacity overflow");
+        assert!(!oflo, "HashMap Corruption - capacity overflow");
+        assert!(pairs_offset as isize > 0, "HashMap Corruption - pairs offset={}", pairs_offset);
+        assert!(index as isize >= 0, "HashMap Corruption - index={}", index);
+        assert!(index < self.capacity(), "HashMap Corruption - index={}", index);
 
         let buffer = self.hashes.ptr() as *mut u8;
-        unsafe {
+        let bucket = unsafe {
             RawBucket {
                 hash_start: buffer as *mut HashUint,
                 pair_start: buffer.offset(pairs_offset as isize) as *const (K, V),
                 idx: index,
                 _marker: marker::PhantomData,
             }
-        }
+        };
+
+        bucket.assert_bounds(self.bytes_allocated);
+        bucket
     }
 
     /// Returns a raw pointer to the table's buffer.
@@ -845,8 +866,9 @@ impl<K, V> RawTable<K, V> {
 
     fn raw_buckets(&self) -> RawBuckets<K, V> {
         RawBuckets {
-            raw: self.raw_bucket_at(0),
+            raw: if self.capacity() == 0 { None } else { Some(self.raw_bucket_at(0)) },
             elems_left: self.size,
+            bytes_allocated: self.bytes_allocated,
             marker: marker::PhantomData,
         }
     }
@@ -865,12 +887,13 @@ impl<K, V> RawTable<K, V> {
     }
 
     pub fn into_iter(self) -> IntoIter<K, V> {
-        let RawBuckets { raw, elems_left, .. } = self.raw_buckets();
+        let RawBuckets { raw, elems_left, bytes_allocated, .. } = self.raw_buckets();
         // Replace the marker regardless of lifetime bounds on parameters.
         IntoIter {
             iter: RawBuckets {
                 raw,
                 elems_left,
+                bytes_allocated,
                 marker: marker::PhantomData,
             },
             table: self,
@@ -878,12 +901,13 @@ impl<K, V> RawTable<K, V> {
     }
 
     pub fn drain(&mut self) -> Drain<K, V> {
-        let RawBuckets { raw, elems_left, .. } = self.raw_buckets();
+        let RawBuckets { raw, elems_left, bytes_allocated, .. } = self.raw_buckets();
         // Replace the marker regardless of lifetime bounds on parameters.
         Drain {
             iter: RawBuckets {
                 raw,
                 elems_left,
+                bytes_allocated,
                 marker: marker::PhantomData,
             },
             table: Shared::from(self),
@@ -895,17 +919,21 @@ impl<K, V> RawTable<K, V> {
     /// state and should only be used for dropping the table's remaining
     /// entries. It's used in the implementation of Drop.
     unsafe fn rev_drop_buckets(&mut self) {
-        // initialize the raw bucket past the end of the table
-        let mut raw = self.raw_bucket_at(self.capacity());
         let mut elems_left = self.size;
-
-        while elems_left != 0 {
-            raw.idx -= 1;
-
+        if elems_left == 0 {
+            return;
+        }
+        let mut raw = self.raw_bucket_at(self.capacity() - 1);
+        loop {
             if *raw.hash() != EMPTY_BUCKET {
-                elems_left -= 1;
                 ptr::drop_in_place(raw.pair());
+                elems_left = elems_left.checked_sub(1).unwrap();
+                if elems_left == 0 {
+                    return;
+                }
             }
+            raw.idx = raw.idx.checked_sub(1).unwrap();
+            raw.assert_bounds(self.bytes_allocated);
         }
     }
 
@@ -923,8 +951,11 @@ impl<K, V> RawTable<K, V> {
 /// A raw iterator. The basis for some other iterators in this module. Although
 /// this interface is safe, it's not used outside this module.
 struct RawBuckets<'a, K, V> {
-    raw: RawBucket<K, V>,
+    // We use an Option here to avoid ever constructing a RawBucket for
+    // invalid memory.
+    raw: Option<RawBucket<K, V>>,
     elems_left: usize,
+    bytes_allocated: usize,
 
     // Strictly speaking, this should be &'a (K,V), but that would
     // require that K:'a, and we often use RawBuckets<'static...> for
@@ -940,6 +971,7 @@ impl<'a, K, V> Clone for RawBuckets<'a, K, V> {
         RawBuckets {
             raw: self.raw,
             elems_left: self.elems_left,
+            bytes_allocated: self.bytes_allocated,
             marker: marker::PhantomData,
         }
     }
@@ -956,12 +988,17 @@ impl<'a, K, V> Iterator for RawBuckets<'a, K, V> {
 
         loop {
             unsafe {
-                let item = self.raw;
-                self.raw.idx += 1;
+                let item = self.raw.unwrap();
                 if *item.hash() != EMPTY_BUCKET {
-                    self.elems_left -= 1;
+                    self.elems_left = self.elems_left.checked_sub(1).unwrap();
+                    if self.elems_left != 0 {
+                        self.raw.as_mut().unwrap().idx += 1;
+                        self.raw.as_ref().unwrap().assert_bounds(self.bytes_allocated);
+                    }
                     return Some(item);
                 }
+                self.raw.as_mut().unwrap().idx += 1;
+                self.raw.as_ref().unwrap().assert_bounds(self.bytes_allocated);
             }
         }
     }
@@ -1096,7 +1133,7 @@ impl<K, V> Iterator for IntoIter<K, V> {
 
     fn next(&mut self) -> Option<(SafeHash, K, V)> {
         self.iter.next().map(|raw| {
-            self.table.size -= 1;
+            self.table.size = self.table.size.checked_sub(1).unwrap();
             unsafe {
                 let (k, v) = ptr::read(raw.pair());
                 (SafeHash { hash: *raw.hash() }, k, v)
@@ -1122,7 +1159,7 @@ impl<'a, K, V> Iterator for Drain<'a, K, V> {
     fn next(&mut self) -> Option<(SafeHash, K, V)> {
         self.iter.next().map(|raw| {
             unsafe {
-                self.table.as_mut().size -= 1;
+                self.table.as_mut().size = self.table.as_mut().size.checked_sub(1).unwrap();
                 let (k, v) = ptr::read(raw.pair());
                 (SafeHash { hash: ptr::replace(&mut *raw.hash(), EMPTY_BUCKET) }, k, v)
             }
@@ -1151,18 +1188,28 @@ impl<K: Clone, V: Clone> Clone for RawTable<K, V> {
         unsafe {
             let cap = self.capacity();
             let mut new_ht = RawTable::new_uninitialized(cap);
+            if cap == 0 {
+                return new_ht;
+            }
 
             let mut new_buckets = new_ht.raw_bucket_at(0);
             let mut buckets = self.raw_bucket_at(0);
-            while buckets.idx < cap {
+            loop {
                 *new_buckets.hash() = *buckets.hash();
                 if *new_buckets.hash() != EMPTY_BUCKET {
                     let pair_ptr = buckets.pair();
                     let kv = ((*pair_ptr).0.clone(), (*pair_ptr).1.clone());
                     ptr::write(new_buckets.pair(), kv);
                 }
+
+                if buckets.idx == cap - 1 {
+                    break;
+                }
+
                 buckets.idx += 1;
+                buckets.assert_bounds(self.bytes_allocated);
                 new_buckets.idx += 1;
+                new_buckets.assert_bounds(new_ht.bytes_allocated);
             }
 
             new_ht.size = self.size();
@@ -1201,7 +1248,7 @@ impl<K, V> Drop for RawTable<K, V> {
                                                           pairs_size,
                                                           align_of::<(K, V)>());
 
-        debug_assert!(!oflo, "should be impossible");
+        assert!(!oflo, "HashMap Corruption - should be impossible");
 
         unsafe {
             dealloc(self.hashes.ptr() as *mut u8, align);