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.

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

AVX stream intrinsics signatures are incorrect, maybe unsound #575

Closed
gnzlbg opened this issue Oct 3, 2018 · 4 comments
Closed

AVX stream intrinsics signatures are incorrect, maybe unsound #575

gnzlbg opened this issue Oct 3, 2018 · 4 comments

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Oct 3, 2018

In the intel intrinsics guide, the signature is:

void _mm256_stream_si256 (__m256i * mem_addr, __m256i a)

but in Rust the signature is:

pub unsafe fn _mm256_stream_si256(mem_addr: *const __m256i, a: __m256i)

Note that the mem_addr pointer is const in Rust, but not in C. Since the only purpose of this function is to write through this pointer, it is probably unsound for it to take a const pointer.

EDIT: _mm256_stream_pd and _mm256_stream_ps also have this same issue. The SSE _mm_stream_ps, _mm_stream_pd, and _mm_stream_pi intrinsics correctly use *mut here.

A tangential bug is that the intrinsic verification failed hard here, since the C intrinsics do not use const pointers, it would have been safer to require these intrinsics to use mut pointers.

@gnzlbg gnzlbg changed the title _mm256_stream_si256 signature incorrect, maybe unsound AVX stream intrinsic signatures are incorrect, maybe unsound Oct 3, 2018
@gnzlbg gnzlbg changed the title AVX stream intrinsic signatures are incorrect, maybe unsound AVX stream intrinsics signatures are incorrect, maybe unsound Oct 3, 2018
gnzlbg added a commit to gnzlbg/stdsimd that referenced this issue Oct 3, 2018
@alexcrichton
Copy link
Member

Oh dear sounds bad! It may be the case though we can fix this without practical breakage. Want to send a PR and we can crater?

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Oct 5, 2018

Will do.

@RalfJung
Copy link
Member

Notice that *const T and *mut T are equivalent in terms of UB, so this is not -- on its own -- unsound.

However, it could lead to people using &T on the call site, and that would be unsound.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jan 20, 2019

Yes, we should just have had a single pointer type :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants