arduano / simdeez

easy simd
MIT License
332 stars 25 forks source link

Load and store methods use &T but read/write out of bounds #47

Open nico-abram opened 2 years ago

nico-abram commented 2 years ago

The following methods:

Take a &T (Ex. a &f32) and read out of bounds to load a vector.

The following methods:

Are the same but stores which write out of bounds of a &mut T.

It is not entirely clear, but if you see the discussion in https://github.com/rust-lang/unsafe-code-guidelines/issues/134 and https://github.com/rust-lang/unsafe-code-guidelines/issues/134 , it looks like reading/writing past the end of a &T might be UB.

Also, a reference has an alignment requirement, which may be stricter than wanted. The alignment requirement is that the &T ptr is aligned to align_of::(), which is not enough to use the aligned load/write variants, but can certainly be more strict than needed for the unaligned loads.

I propose switching to raw pointers. This avoids the possibility of Undefined Behaviour from reading past the end of the reference and the unnecessary alignment requirements on the unaligned variants. Code like this from the example in the readme:

            let xv1 = S::loadu_ps(&x1[0]);
            let yv1 = S::loadu_ps(&y1[0]);
            let xv2 = S::loadu_ps(&x2[0]);
            let yv2 = S::loadu_ps(&y2[0]);

Would still compile after that change, since a &T coerces to *const T, but it would remain UB because that pointer has the same provenance/tag, but it could now be fixed by using the correct pointer:

            let xv1 = S::loadu_ps(x1.as_ptr());
            let yv1 = S::loadu_ps(y1.as_ptr());
            let xv2 = S::loadu_ps(x2.as_ptr());
            let yv2 = S::loadu_ps(y2.as_ptr());