Skip to content

Commit d40ffa1

Browse files
committed
Auto merge of #405 - JustForFun88:find_insert_slot, r=Amanieu
Doc `RawTableInner::find_insert_slot` and also make it unsafe Made this function unsafe, as it is possible to use the index returned by this function unsafely. Here are two functions that confirm the documentation I made: ```rust #[test] fn test_infinite_loop() { use ::alloc::vec::Vec; let mut table: RawTable<u16> = RawTable::with_capacity(20); assert_eq!(table.capacity(), 28); assert_eq!(table.buckets(), 32); for i in 0..table.buckets() { unsafe { let bucket_index = table.table.find_insert_slot(i as u64); table.table.set_ctrl_h2(bucket_index, i as u64); table.table.items += 1; table.bucket(bucket_index).write(i as u16); } } let vec = unsafe { table.iter().map(|x| x.read()).collect::<Vec<_>>() }; assert_eq!(vec, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31]); // this never return let _bucket_index = unsafe { table.table.find_insert_slot(32) }; } #[test] fn test_undefined_behavior() { use ::alloc::vec::Vec; let mut table: RawTable<u16> = RawTable::with_capacity(3); assert_eq!(table.capacity(), 3); assert_eq!(table.buckets(), 4); for i in 0..3 { table.insert(i, i as u16, |i| *i as u64); } assert_eq!(table.table.items, 3); assert_eq!(table.table.growth_left, 0); let bucket_index = unsafe { table.table.find_insert_slot(3_u64) }; assert_eq!(bucket_index, 3); unsafe { table.table.set_ctrl_h2(bucket_index, 3_u64); table.table.items += 1; table.bucket(bucket_index).write(3_u16); } let vec = unsafe { table.iter().map(|x| x.read()).collect::<Vec<_>>() }; assert_eq!(vec, [0, 1, 2, 3]); let bucket_index = unsafe { table.table.find_insert_slot(4_u64) }; assert_eq!(bucket_index, 4); } ```
2 parents 408bd9d + 1605442 commit d40ffa1

File tree

1 file changed

+51
-2
lines changed

1 file changed

+51
-2
lines changed

src/raw/mod.rs

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1586,16 +1586,48 @@ impl<A: Allocator + Clone> RawTableInner<A> {
15861586
}
15871587

15881588
/// Searches for an empty or deleted bucket which is suitable for inserting
1589-
/// a new element.
1589+
/// a new element, returning the `index` for the new [`Bucket`].
15901590
///
1591-
/// There must be at least 1 empty bucket in the table.
1591+
/// This function does not make any changes to the `data` part of the table, or any
1592+
/// changes to the `items` or `growth_left` field of the table.
1593+
///
1594+
/// The table must have at least 1 empty or deleted `bucket`, otherwise this function
1595+
/// will never return (will go into an infinite loop) for tables larger than the group
1596+
/// width, or return an index outside of the table indices range if the table is less
1597+
/// than the group width.
1598+
///
1599+
/// # Note
1600+
///
1601+
/// Calling this function is always safe, but attempting to write data at
1602+
/// the index returned by this function when the table is less than the group width
1603+
/// and if there was not at least one empty bucket in the table will cause immediate
1604+
/// [`undefined behavior`]. This is because in this case the function will return
1605+
/// `self.bucket_mask + 1` as an index due to the trailing EMPTY control bytes outside
1606+
/// the table range.
1607+
///
1608+
/// [`undefined behavior`]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
15921609
#[inline]
15931610
fn find_insert_slot(&self, hash: u64) -> usize {
15941611
let mut probe_seq = self.probe_seq(hash);
15951612
loop {
1613+
// SAFETY:
1614+
// * `ProbeSeq.pos` cannot be greater than `self.bucket_mask = self.buckets() - 1`
1615+
// of the table due to masking with `self.bucket_mask` and also because mumber of
1616+
// buckets is a power of two (see comment for masking below).
1617+
//
1618+
// * Even if `ProbeSeq.pos` returns `position == self.bucket_mask`, it is safe to
1619+
// call `Group::load` due to the extended control bytes range, which is
1620+
// `self.bucket_mask + 1 + Group::WIDTH` (in fact, this means that the last control
1621+
// byte will never be read for the allocated table);
1622+
//
1623+
// * Also, even if `RawTableInner` is not already allocated, `ProbeSeq.pos` will
1624+
// always return "0" (zero), so Group::load will read unaligned `Group::static_empty()`
1625+
// bytes, which is safe (see RawTableInner::new_in).
15961626
unsafe {
15971627
let group = Group::load(self.ctrl(probe_seq.pos));
15981628
if let Some(bit) = group.match_empty_or_deleted().lowest_set_bit() {
1629+
// This is the same as `(probe_seq.pos + bit) % self.buckets()` because the number
1630+
// of buckets is a power of two, and `self.bucket_mask = self.buckets() - 1`.
15991631
let result = (probe_seq.pos + bit) & self.bucket_mask;
16001632

16011633
// In tables smaller than the group width, trailing control
@@ -1607,9 +1639,26 @@ impl<A: Allocator + Clone> RawTableInner<A> {
16071639
// table. This second scan is guaranteed to find an empty
16081640
// slot (due to the load factor) before hitting the trailing
16091641
// control bytes (containing EMPTY).
1642+
//
1643+
// SAFETY: The `result` is guaranteed to be in range `0..self.bucket_mask`
1644+
// due to masking with `self.bucket_mask`
16101645
if unlikely(self.is_bucket_full(result)) {
16111646
debug_assert!(self.bucket_mask < Group::WIDTH);
16121647
debug_assert_ne!(probe_seq.pos, 0);
1648+
// SAFETY:
1649+
//
1650+
// * We are in range and `ptr = self.ctrl(0)` are valid for reads
1651+
// and properly aligned, because the table is already allocated
1652+
// (see `TableLayout::calculate_layout_for` and `ptr::read`);
1653+
//
1654+
// * For tables larger than the group width, we will never end up in the given
1655+
// branch, since `(probe_seq.pos + bit) & self.bucket_mask` cannot return a
1656+
// full bucket index. For tables smaller than the group width, calling the
1657+
// `lowest_set_bit_nonzero` function (when `nightly` feature enabled) is also
1658+
// safe, as the trailing control bytes outside the range of the table are filled
1659+
// with EMPTY bytes, so this second scan either finds an empty slot (due to the
1660+
// load factor) or hits the trailing control bytes (containing EMPTY). See
1661+
// `intrinsics::cttz_nonzero` for more information.
16131662
return Group::load_aligned(self.ctrl(0))
16141663
.match_empty_or_deleted()
16151664
.lowest_set_bit_nonzero();

0 commit comments

Comments
 (0)