Skip to content

ArrayDeque with a size of 0 panics #31

Description

@Takashiidobe

There's a couple bugs I found for empty ArrayDeques.

First, this panics even though it should pass:

#[test]
fn zero_capacity_arraydeque() {
    let deque: ArrayDeque<i32, 0> = ArrayDeque::new();

    let count = deque.iter().count();
    assert_eq!(count, 0);
}

If you run this code which is basically the same test for Vec, it will pass because having a Vec with 0 capacity is fine, it just does nothing basically:

use std::collections::VecDeque;

#[test]
fn zero_capacity_vec() {
    let empty: VecDeque<i32> = VecDeque::new();
    
    let count = empty.iter().count();
    assert_eq!(count, 0);
}

This happens in drop as well.

#[test]
fn test_bug1_zero_capacity_drop_compare_vecdeque() {
    {
        let _vec_deque: VecDeque<i32> = VecDeque::with_capacity(0);
    }

    {
        let _array_deque: ArrayDeque<i32, 0> = ArrayDeque::new();
        println!("This prints before drop");
    }
}

Which prints out the "This prints before drop" before panicking.
Lots of operations for arraydeque just panic when you try them on a 0 capacity ArrayDeque when they shouldn't.

I believe this is because wrap_add and wrap_sub use modulo to wrap indexes without checking for size, so when CAP == 0, any operation causes a panic because the length is 0, so you get division by 0.

In the stdlib, VecDeque handles this by special casing the 0 capacity side by having a different branch when the type is a ZST:

For example, wrap_add is defined like this:

#[inline]
fn wrap_add(&self, idx: usize, addend: usize) -> usize {
    wrap_index(idx.wrapping_add(addend), self.capacity())
}

Where capacity looks like this, so it doesn't run into the same bug.

pub fn capacity(&self) -> usize {
    if T::IS_ZST { usize::MAX } else { self.buf.capacity() }
}

Otherwise just making sure capacity is non-zero should be good enough:

#[inline]
fn wrap_add(index: usize, addend: usize, capacity: usize) -> usize {
    debug_assert!(addend <= capacity);
    (index + addend) % capacity.max(1)
}

#[inline]
fn wrap_sub(index: usize, subtrahend: usize, capacity: usize) -> usize {
    debug_assert!(subtrahend <= capacity);
    (index + capacity - subtrahend) % capacity.max(1)
}

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions