Skip to content

StaticVec::extend panics because of out of range memory access. #61

@Wasabi375

Description

@Wasabi375

The get_unchecked_mut call within drop_in_place sometimes panics.
When the capacity of the extended vector is not enough to hold all extra values extend_ex tries to drop the rest. However the calculation on what this rest is, is wrong. It should use the length of the iter variable and not the capacity (as it is currently doing).

The following patch fixes this:

diff --git a/src/trait_impls.rs b/src/trait_impls.rs
index 8e912f2..b127f6d 100644
--- a/src/trait_impls.rs
+++ b/src/trait_impls.rs
@@ -215,16 +215,21 @@ impl<T, const N1: usize, const N2: usize> ExtendEx<T, StaticVec<T, N1>> for Stat
         .as_ptr()
         .copy_to_nonoverlapping(self.mut_ptr_at_unchecked(old_length), added_length);
       self.set_len(old_length + added_length);
-      // Wrap the values in a MaybeUninit to inhibit their destructors (if any),
-      // then manually drop any excess ones. This is the same kind of "trick" as
-      // is used in `new_from_array`, as you may or may not have noticed.
-      let mut forgotten = MaybeUninit::new(iter);
-      ptr::drop_in_place(
-        forgotten
-          .assume_init_mut()
-          .as_mut_slice()
-          .get_unchecked_mut(N1.min(N2)..N1),
-      );
+
+      let drop_range = N1.min(N2)..iter.len().try_into().unwrap();
+      if !drop_range.is_empty() {
+          // Wrap the values in a MaybeUninit to inhibit their destructors (if any),
+          // then manually drop any excess ones. This is the same kind of "trick" as
+          // is used in `new_from_array`, as you may or may not have noticed.
+          let mut forgotten = MaybeUninit::new(iter);
+
+          ptr::drop_in_place(
+              forgotten
+                  .assume_init_mut()
+                  .as_mut_slice()
+                  .get_unchecked_mut(drop_range),
+          );
+      }
     }
   }

And here is a test to reproduce the error

#[cfg(test)]
mod test {
    use crate::StaticVec;

    #[test]
    fn test_extend_drop_no_panic() {
        let mut a = StaticVec::<u64, 6>::new();
        let mut b = StaticVec::<u64, 6>::new();

        a.extend_from_slice(&[1, 2]);
        b.extend_from_slice(&[3, 4, 5]);

        // This panics
        a.extend(b);

        assert_eq!(a, [1, 2, 3, 4, 5]);
    }
}

I would send a PR but #60 is blocking any real changes.

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