Skip to content

Commit

Permalink
Fix vec![x; n] with null raw fat pointer zeroing the pointer metadata
Browse files Browse the repository at this point in the history
rust-lang#49496 introduced specialization based on:

```
unsafe impl<T: ?Sized> IsZero for *mut T {
    fn is_zero(&self) -> bool {
        (*self).is_null()
    }
}
```

… to call `RawVec::with_capacity_zeroed` for creating `Vec<*mut T>`,
which is incorrect for fat pointers
since `<*mut T>::is_null` only looks at the data component.
That is, a fat pointer can be “null” without being made entirely of zero bits.

This commit fixes it by removing the `?Sized` bound on this impl
(and the corresponding `*const T` one).
This regresses `vec![x; n]` with `x` a null raw slice of length zero,
but that seems exceptionally uncommon.
(Vtable pointers are never null, so raw trait objects would not take
the fast path anyway.

An alternative to keep the `?Sized` bound
(or even generalize to `impl<U: Copy> IsZero for U`)
would be to cast to `&[u8]` of length `size_of::<U>()`,
but the optimizer seems not to be able to propagate alignment information
and sticks with comparing one byte at a time:

https://rust.godbolt.org/z/xQFkwL

----

Without the library change, the new test fails as follows:

```
---- vec::vec_macro_repeating_null_raw_fat_pointer stdout ----
[src/liballoc/tests/vec.rs:1301] ptr_metadata(raw_dyn) = 0x00005596ef95f9a8
[src/liballoc/tests/vec.rs:1306] ptr_metadata(vec[0]) = 0x0000000000000000
thread 'vec::vec_macro_repeating_null_raw_fat_pointer' panicked at 'assertion failed: vec[0] == null_raw_dyn', src/liballoc/tests/vec.rs:1307:5
```
  • Loading branch information
SimonSapin committed Sep 29, 2019
1 parent 0bbab7d commit ce60da4
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 2 deletions.
48 changes: 48 additions & 0 deletions src/liballoc/tests/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1281,3 +1281,51 @@ fn test_stable_push_pop() {
v.pop().unwrap();
assert_eq!(*v0, 13);
}

// https://github.com/rust-lang/rust/pull/49496 introduced specialization based on:
//
// ```
// unsafe impl<T: ?Sized> IsZero for *mut T {
// fn is_zero(&self) -> bool {
// (*self).is_null()
// }
// }
// ```
//
// … to call `RawVec::with_capacity_zeroed` for creating `Vec<*mut T>`,
// which is incorrect for fat pointers since `<*mut T>::is_null` only looks at the data component.
// That is, a fat pointer can be “null” without being made entirely of zero bits.
#[test]
fn vec_macro_repeating_null_raw_fat_pointer() {
let raw_dyn = &mut (|| ()) as &mut dyn Fn() as *mut dyn Fn();
let vtable = dbg!(ptr_metadata(raw_dyn));
let null_raw_dyn = ptr_from_raw_parts(std::ptr::null_mut(), vtable);
assert!(null_raw_dyn.is_null());

let vec = vec![null_raw_dyn; 1];
dbg!(ptr_metadata(vec[0]));
assert!(vec[0] == null_raw_dyn);

// Polyfill for https://github.com/rust-lang/rfcs/pull/2580

fn ptr_metadata(ptr: *mut dyn Fn()) -> *mut () {
unsafe {
std::mem::transmute::<*mut dyn Fn(), DynRepr>(ptr).vtable
}
}

fn ptr_from_raw_parts(data: *mut (), vtable: *mut()) -> *mut dyn Fn() {
unsafe {
std::mem::transmute::<DynRepr, *mut dyn Fn()>(DynRepr {
data,
vtable
})
}
}

#[repr(C)]
struct DynRepr {
data: *mut (),
vtable: *mut (),
}
}
4 changes: 2 additions & 2 deletions src/liballoc/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1734,14 +1734,14 @@ impl_is_zero!(char, |x| x == '\0');
impl_is_zero!(f32, |x: f32| x.to_bits() == 0);
impl_is_zero!(f64, |x: f64| x.to_bits() == 0);

unsafe impl<T: ?Sized> IsZero for *const T {
unsafe impl<T> IsZero for *const T {
#[inline]
fn is_zero(&self) -> bool {
(*self).is_null()
}
}

unsafe impl<T: ?Sized> IsZero for *mut T {
unsafe impl<T> IsZero for *mut T {
#[inline]
fn is_zero(&self) -> bool {
(*self).is_null()
Expand Down

0 comments on commit ce60da4

Please sign in to comment.