Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

builtin: panic on trying to grow arrays with capacity bigger than 2^31, instead of overflowing a.cap (partial fix for #21918) #21947

Merged
merged 4 commits into from
Jul 28, 2024
Merged

builtin: panic on trying to grow arrays with capacity bigger than 2^31, instead of overflowing a.cap (partial fix for #21918) #21947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
29fbd14
builtin: panic on trying to grow arrays with capacity bigger than 2^3…
spytheman Jul 27, 2024
70f8527
builtin: do more overflow checks on .len and .cap fields for common a…
spytheman Jul 27, 2024
f7436c3
fix `v -cc musl-gcc vlib/builtin/array_test.v` (the array.drop test)
spytheman Jul 27, 2024
6dd3ead
fix msvc failure
spytheman Jul 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 60 additions & 39 deletions vlib/builtin/array.v
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,18 @@ fn (mut a array) ensure_cap(required int) {
if a.flags.has(.nogrow) {
panic('array.ensure_cap: array with the flag `.nogrow` cannot grow in size, array required new size: ${required}')
}
mut cap := if a.cap > 0 { a.cap } else { 2 }
mut cap := if a.cap > 0 { i64(a.cap) } else { i64(2) }
for required > cap {
cap *= 2
}
if cap > max_int {
if a.cap < max_int {
// limit the capacity, since bigger values, will overflow the 32bit integer used to store it
cap = max_int
} else {
panic('array.ensure_cap: array needs to grow to cap = ${cap}, which is > 2^31')
}
}
new_size := u64(cap) * u64(a.element_size)
new_data := unsafe { malloc(__at_least_one(new_size)) }
if a.data != unsafe { nil } {
Expand All @@ -199,7 +207,7 @@ fn (mut a array) ensure_cap(required int) {
}
a.data = new_data
a.offset = 0
a.cap = cap
a.cap = int(cap)
}

// repeat returns a new array with the given array elements repeated given times.
Expand Down Expand Up @@ -270,10 +278,11 @@ pub fn (a array) repeat_to_depth(count int, depth int) array {
// c.insert(0, [1, 2]) // c now is [[1, 2], [3, 4]]
// ```
pub fn (mut a array) insert(i int, val voidptr) {
$if !no_bounds_checking {
if i < 0 || i > a.len {
panic('array.insert: index out of range (i == ${i}, a.len == ${a.len})')
}
if i < 0 || i > a.len {
panic('array.insert: index out of range (i == ${i}, a.len == ${a.len})')
}
if a.len == max_int {
panic('array.insert: a.len reached max_int')
}
if a.len >= a.cap {
a.ensure_cap(a.len + 1)
Expand All @@ -289,19 +298,21 @@ pub fn (mut a array) insert(i int, val voidptr) {
// into an the array beginning at `i`.
@[unsafe]
fn (mut a array) insert_many(i int, val voidptr, size int) {
$if !no_bounds_checking {
if i < 0 || i > a.len {
panic('array.insert_many: index out of range (i == ${i}, a.len == ${a.len})')
}
if i < 0 || i > a.len {
panic('array.insert_many: index out of range (i == ${i}, a.len == ${a.len})')
}
new_len := i64(a.len) + i64(size)
if new_len > max_int {
panic('array.insert_many: a.len = ${new_len} will exceed max_int')
}
a.ensure_cap(a.len + size)
a.ensure_cap(int(new_len))
elem_size := a.element_size
unsafe {
iptr := a.get_unsafe(i)
vmemmove(a.get_unsafe(i + size), iptr, u64(a.len - i) * u64(elem_size))
vmemcpy(iptr, val, u64(size) * u64(elem_size))
}
a.len += size
a.len = int(new_len)
}

// prepend prepends one or more elements to an array.
Expand Down Expand Up @@ -348,11 +359,9 @@ pub fn (mut a array) delete(i int) {
// dump(b) // b: [1, 2, 3, 4, 5, 6, 7, 8, 9] // `b` is still the same
// ```
pub fn (mut a array) delete_many(i int, size int) {
$if !no_bounds_checking {
if i < 0 || i + size > a.len {
endidx := if size > 1 { '..${i + size}' } else { '' }
panic('array.delete: index out of range (i == ${i}${endidx}, a.len == ${a.len})')
}
if i < 0 || i64(i) + i64(size) > i64(a.len) {
endidx := if size > 1 { '..${i + size}' } else { '' }
panic('array.delete: index out of range (i == ${i}${endidx}, a.len == ${a.len})')
}
if a.flags.all(.noshrink | .noslices) {
unsafe {
Expand Down Expand Up @@ -465,21 +474,17 @@ fn (a array) get_with_check(i int) voidptr {
// However, `a[0]` returns an error object
// so it can be handled with an `or` block.
pub fn (a array) first() voidptr {
$if !no_bounds_checking {
if a.len == 0 {
panic('array.first: array is empty')
}
if a.len == 0 {
panic('array.first: array is empty')
}
return a.data
}

// last returns the last element of the `array`.
// If the `array` is empty, this will panic.
pub fn (a array) last() voidptr {
$if !no_bounds_checking {
if a.len == 0 {
panic('array.last: array is empty')
}
if a.len == 0 {
panic('array.last: array is empty')
}
unsafe {
return &u8(a.data) + u64(a.len - 1) * u64(a.element_size)
Expand All @@ -503,10 +508,8 @@ pub fn (a array) last() voidptr {
// ```
pub fn (mut a array) pop() voidptr {
// in a sense, this is the opposite of `a << x`
$if !no_bounds_checking {
if a.len == 0 {
panic('array.pop: array is empty')
}
if a.len == 0 {
panic('array.pop: array is empty')
}
new_len := a.len - 1
last_elem := unsafe { &u8(a.data) + u64(new_len) * u64(a.element_size) }
Expand All @@ -521,11 +524,8 @@ pub fn (mut a array) pop() voidptr {
// If the array is empty, this will panic.
// See also: [trim](#array.trim)
pub fn (mut a array) delete_last() {
// copy pasting code for performance
$if !no_bounds_checking {
if a.len == 0 {
panic('array.pop: array is empty')
}
if a.len == 0 {
panic('array.delete_last: array is empty')
}
a.len--
}
Expand Down Expand Up @@ -676,6 +676,12 @@ fn (mut a array) set(i int, val voidptr) {
}

fn (mut a array) push(val voidptr) {
if a.len < 0 {
panic('array.push: negative len')
}
if a.len >= max_int {
panic('array.push: len bigger than max_int')
}
if a.len >= a.cap {
a.ensure_cap(a.len + 1)
}
Expand All @@ -690,7 +696,14 @@ pub fn (mut a3 array) push_many(val voidptr, size int) {
if size <= 0 || val == unsafe { nil } {
return
}
a3.ensure_cap(a3.len + size)
new_len := i64(a3.len) + i64(size)
if new_len > max_int {
// string interpolation also uses <<; avoid it, use a fixed string for the panic
panic('array.push_many: new len exceeds max_int')
}
if new_len >= a3.cap {
a3.ensure_cap(int(new_len))
}
if a3.data == val && a3.data != 0 {
// handle `arr << arr`
copy := a3.clone()
Expand All @@ -702,7 +715,7 @@ pub fn (mut a3 array) push_many(val voidptr, size int) {
unsafe { vmemcpy(&u8(a3.data) + u64(a3.element_size) * u64(a3.len), val, u64(a3.element_size) * u64(size)) }
}
}
a3.len += size
a3.len = int(new_len)
}

// reverse_in_place reverses existing array data, modifying original array.
Expand Down Expand Up @@ -976,7 +989,11 @@ pub fn copy(mut dst []u8, src []u8) int {
// Internally, it does this by copying the entire array to
// a new memory location (creating a clone).
pub fn (mut a array) grow_cap(amount int) {
a.ensure_cap(a.cap + amount)
new_cap := i64(amount) + i64(a.cap)
if new_cap > max_int {
panic('array.grow_cap: new capacity ${new_cap} will exceed max_int')
}
a.ensure_cap(int(new_cap))
}

// grow_len ensures that an array has a.len + amount of length
Expand All @@ -985,8 +1002,12 @@ pub fn (mut a array) grow_cap(amount int) {
// is already large enough.
@[unsafe]
pub fn (mut a array) grow_len(amount int) {
a.ensure_cap(a.len + amount)
a.len += amount
new_len := i64(amount) + i64(a.len)
if new_len > max_int {
panic('array.grow_len: new len ${new_len} will exceed max_int')
}
a.ensure_cap(int(new_len))
a.len = int(new_len)
}

// pointers returns a new array, where each element
Expand Down
70 changes: 50 additions & 20 deletions vlib/builtin/array_d_gcboehm_opt.v
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,18 @@ fn (mut a array) ensure_cap_noscan(required int) {
if a.flags.has(.nogrow) {
panic('array.ensure_cap_noscan: array with the flag `.nogrow` cannot grow in size, array required new size: ${required}')
}
mut cap := if a.cap > 0 { a.cap } else { 2 }
mut cap := if a.cap > 0 { i64(a.cap) } else { i64(2) }
for required > cap {
cap *= 2
}
if cap > max_int {
if a.cap < max_int {
// limit the capacity, since bigger values, will overflow the 32bit integer used to store it
cap = max_int
} else {
panic('array.ensure_cap_noscan: array needs to grow to cap = ${cap}, which is > 2^31')
}
}
new_size := u64(cap) * u64(a.element_size)
new_data := vcalloc_noscan(new_size)
if a.data != unsafe { nil } {
Expand All @@ -107,7 +115,7 @@ fn (mut a array) ensure_cap_noscan(required int) {
}
a.data = new_data
a.offset = 0
a.cap = cap
a.cap = int(cap)
}

// repeat returns a new array with the given array elements repeated given times.
Expand Down Expand Up @@ -151,10 +159,11 @@ fn (a array) repeat_to_depth_noscan(count int, depth int) array {

// insert inserts a value in the array at index `i`
fn (mut a array) insert_noscan(i int, val voidptr) {
$if !no_bounds_checking {
if i < 0 || i > a.len {
panic('array.insert: index out of range (i == ${i}, a.len == ${a.len})')
}
if i < 0 || i > a.len {
panic('array.insert_noscan: index out of range (i == ${i}, a.len == ${a.len})')
}
if a.len == max_int {
panic('array.insert_noscan: a.len reached max_int')
}
a.ensure_cap_noscan(a.len + 1)
unsafe {
Expand All @@ -167,10 +176,12 @@ fn (mut a array) insert_noscan(i int, val voidptr) {
// insert_many inserts many values into the array from index `i`.
@[unsafe]
fn (mut a array) insert_many_noscan(i int, val voidptr, size int) {
$if !no_bounds_checking {
if i < 0 || i > a.len {
panic('array.insert_many: index out of range (i == ${i}, a.len == ${a.len})')
}
if i < 0 || i > a.len {
panic('array.insert_many: index out of range (i == ${i}, a.len == ${a.len})')
}
new_len := i64(a.len) + i64(size)
if new_len > max_int {
panic('array.insert_many_noscan: a.len = ${new_len} will exceed max_int')
}
a.ensure_cap_noscan(a.len + size)
elem_size := a.element_size
Expand All @@ -196,10 +207,8 @@ fn (mut a array) prepend_many_noscan(val voidptr, size int) {
// pop returns the last element of the array, and removes it.
fn (mut a array) pop_noscan() voidptr {
// in a sense, this is the opposite of `a << x`
$if !no_bounds_checking {
if a.len == 0 {
panic('array.pop: array is empty')
}
if a.len == 0 {
panic('array.pop: array is empty')
}
new_len := a.len - 1
last_elem := unsafe { &u8(a.data) + u64(new_len) * u64(a.element_size) }
Expand Down Expand Up @@ -247,7 +256,15 @@ fn (a &array) clone_to_depth_noscan(depth int) array {
}

fn (mut a array) push_noscan(val voidptr) {
a.ensure_cap_noscan(a.len + 1)
if a.len < 0 {
panic('array.push_noscan: negative len')
}
if a.len >= max_int {
panic('array.push_noscan: len bigger than max_int')
}
if a.len >= a.cap {
a.ensure_cap_noscan(a.len + 1)
}
unsafe { vmemcpy(&u8(a.data) + u64(a.element_size) * u64(a.len), val, a.element_size) }
a.len++
}
Expand All @@ -256,9 +273,14 @@ fn (mut a array) push_noscan(val voidptr) {
// `val` is array.data and user facing usage is `a << [1,2,3]`
@[unsafe]
fn (mut a3 array) push_many_noscan(val voidptr, size int) {
if size <= 0 || val == unsafe { nil } {
if size == 0 || val == unsafe { nil } {
return
}
new_len := i64(a3.len) + i64(size)
if new_len > max_int {
// string interpolation also uses <<; avoid it, use a fixed string for the panic
panic('array.push_many_noscan: new len exceeds max_int')
}
if a3.data == val && a3.data != 0 {
// handle `arr << arr`
copy := a3.clone()
Expand All @@ -272,7 +294,7 @@ fn (mut a3 array) push_many_noscan(val voidptr, size int) {
unsafe { vmemcpy(a3.get_unsafe(a3.len), val, u64(a3.element_size) * u64(size)) }
}
}
a3.len += size
a3.len = int(new_len)
}

// reverse returns a new array with the elements of the original array in reverse order.
Expand All @@ -294,12 +316,20 @@ fn (a array) reverse_noscan() array {

// grow_cap grows the array's capacity by `amount` elements.
fn (mut a array) grow_cap_noscan(amount int) {
a.ensure_cap_noscan(a.cap + amount)
new_cap := i64(amount) + i64(a.cap)
if new_cap > max_int {
panic('array.grow_cap: new capacity ${new_cap} will exceed max_int')
}
a.ensure_cap_noscan(int(new_cap))
}

// grow_len ensures that an array has a.len + amount of length
@[unsafe]
fn (mut a array) grow_len_noscan(amount int) {
a.ensure_cap_noscan(a.len + amount)
a.len += amount
new_len := i64(amount) + i64(a.len)
if new_len > max_int {
panic('array.grow_len: new len ${new_len} will exceed max_int')
}
a.ensure_cap_noscan(int(new_len))
a.len = int(new_len)
}
4 changes: 2 additions & 2 deletions vlib/builtin/int.v
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ pub const max_i16 = i16(32767)
pub const min_i32 = i32(-2147483648)
pub const max_i32 = i32(2147483647)

pub const min_int = min_i32
pub const max_int = max_i32
pub const min_int = int(-2147483648)
pub const max_int = int(2147483647)

// -9223372036854775808 is wrong, because C compilers parse literal values
// without sign first, and 9223372036854775808 overflows i64, hence the
Expand Down
Loading