Skip to content

AVX stream intrinsics signatures are incorrect, maybe unsound #575

Description

@gnzlbg

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.

Activity

  1. changed the title [-]_mm256_stream_si256 signature incorrect, maybe unsound[/-] [+]AVX stream intrinsic signatures are incorrect, maybe unsound[/+] on Oct 3, 2018
  2. changed the title [-]AVX stream intrinsic signatures are incorrect, maybe unsound[/-] [+]AVX stream intrinsics signatures are incorrect, maybe unsound[/+] on Oct 3, 2018
  3. added a commit that references this issue on Oct 3, 2018
    07fcc60
  4. alexcrichton commented on Oct 3, 2018

    @alexcrichton
    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?

  5. gnzlbg commented on Oct 5, 2018

    @gnzlbg
    ContributorAuthor

    Will do.

  6. added a commit that references this issue on Oct 5, 2018
    deea87e
  7. added a commit that references this issue on Nov 2, 2018
    0309be1
  8. RalfJung commented on Jan 20, 2019

    @RalfJung
    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.

  9. gnzlbg commented on Jan 20, 2019

    @gnzlbg
    ContributorAuthor

    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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions