Skip to content
New issue

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

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

Already on GitHub? # to your account

Using grow to shrink can cause corruption. #149

Closed
ehuss opened this issue Jun 6, 2019 · 1 comment · Fixed by #152
Closed

Using grow to shrink can cause corruption. #149

ehuss opened this issue Jun 6, 2019 · 1 comment · Fixed by #152
Labels

Comments

@ehuss
Copy link
Contributor

ehuss commented Jun 6, 2019

If grow is given a size that is within the inline size after a SmallVec has been spilled, the resulting value is corrupted.

let mut v: SmallVec<[u8; 4]> = SmallVec::new();
v.push(1);
v.push(2);
v.push(3);
v.push(4);
// Cause it to spill.
v.push(5);
assert!(v.spilled());
v.clear();
// Shrink to inline.
v.grow(2);
// This should crash with 'entered unreachable code'.
println!("after grow {:?} len={} cap={}", v, v.len(), v.capacity());

This is admittedly unusual, most code would use shrink_to_fit.

I think the following should fix it.

diff --git a/lib.rs b/lib.rs
index 5af32ba..3767e3b 100644
--- a/lib.rs
+++ b/lib.rs
@@ -654,6 +654,7 @@ impl<A: Array> SmallVec<A> {
                 }
                 self.data = SmallVecData::from_inline(mem::uninitialized());
                 ptr::copy_nonoverlapping(ptr, self.data.inline_mut().ptr_mut(), len);
+                self.capacity = len;
             } else if new_cap != cap {
                 let mut vec = Vec::with_capacity(new_cap);
                 let new_alloc = vec.as_mut_ptr();
@jdm jdm added the bug label Jun 6, 2019
@jdm
Copy link
Member

jdm commented Jun 6, 2019

Would you like to make a pull request and add a unit test?

bors-servo pushed a commit that referenced this issue Jun 7, 2019
Fix using `grow` to shrink to inline.

Using `grow` on a spilled SmallVec to shrink to an unspilled SmallVec would result in a corrupted structure, as `capacity` was not updated.

Fixes #149

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-smallvec/152)
<!-- Reviewable:end -->
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants