Skip to content

Commit 9f57cfb

Browse files
committed
Handle overflows in capacities more
I was mentally applying the rule that capacity can never exceed isize::MAX as a precondition BUT this is the code necessary for enforcing that! In fixing this I broke the fact that this code was subtly relying on that overflow to allow usize::MAX ZSTs to be allocated (by only allocating space for the header). ZSTs previously actually worked fine, as the garbage overflowed value was always wiped out by multiplying by 0 to get the array size. Now we need to handle it more explicitly.
1 parent bdbee2b commit 9f57cfb

File tree

1 file changed

+41
-3
lines changed

1 file changed

+41
-3
lines changed

src/lib.rs

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ use std::alloc::*;
147147
use std::borrow::*;
148148
use std::cmp::*;
149149
use std::convert::TryFrom;
150+
use std::convert::TryInto;
150151
use std::hash::*;
151152
use std::iter::FromIterator;
152153
use std::marker::PhantomData;
@@ -345,12 +346,18 @@ fn alloc_size<T>(cap: usize) -> usize {
345346
//
346347
// We turn everything into isizes here so that we can catch isize::MAX overflow,
347348
// we never want to allow allocations larger than that!
348-
let cap = cap as isize;
349349
let header_size = mem::size_of::<Header>() as isize;
350-
let elem_size = mem::size_of::<T>() as isize;
351350
let padding = padding::<T>() as isize;
352351

353-
let data_size = elem_size.checked_mul(cap).expect("capacity overflow");
352+
let data_size = if mem::size_of::<T>() == 0 {
353+
// If we're allocating an array for ZSTs we need a header/padding but no actual
354+
// space for items, so we don't care about the capacity that was requested!
355+
0
356+
} else {
357+
let cap: isize = cap.try_into().expect("capacity overflow");
358+
let elem_size = mem::size_of::<T>() as isize;
359+
elem_size.checked_mul(cap).expect("capacity overflow")
360+
};
354361

355362
let final_size = data_size
356363
.checked_add(header_size + padding)
@@ -4242,4 +4249,35 @@ mod std_tests {
42424249
vec.set_len(1);
42434250
}
42444251
}
4252+
4253+
#[test]
4254+
#[should_panic(expected = "capacity overflow")]
4255+
fn test_capacity_overflow_header_too_big() {
4256+
let vec: ThinVec<u8> = ThinVec::with_capacity(isize::MAX as usize - 2);
4257+
assert!(vec.capacity() > 0);
4258+
}
4259+
#[test]
4260+
#[should_panic(expected = "capacity overflow")]
4261+
fn test_capacity_overflow_cap_too_big() {
4262+
let vec: ThinVec<u8> = ThinVec::with_capacity(isize::MAX as usize + 1);
4263+
assert!(vec.capacity() > 0);
4264+
}
4265+
#[test]
4266+
#[should_panic(expected = "capacity overflow")]
4267+
fn test_capacity_overflow_size_mul1() {
4268+
let vec: ThinVec<u16> = ThinVec::with_capacity(isize::MAX as usize + 1);
4269+
assert!(vec.capacity() > 0);
4270+
}
4271+
#[test]
4272+
#[should_panic(expected = "capacity overflow")]
4273+
fn test_capacity_overflow_size_mul2() {
4274+
let vec: ThinVec<u16> = ThinVec::with_capacity(isize::MAX as usize / 2 + 1);
4275+
assert!(vec.capacity() > 0);
4276+
}
4277+
#[test]
4278+
#[should_panic(expected = "capacity overflow")]
4279+
fn test_capacity_overflow_cap_really_isnt_isize() {
4280+
let vec: ThinVec<u8> = ThinVec::with_capacity(isize::MAX as usize);
4281+
assert!(vec.capacity() > 0);
4282+
}
42454283
}

0 commit comments

Comments
 (0)