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

Rust does not let you observe or mutate the floating-point register in well-defined ways #1454

Merged
merged 4 commits into from
Sep 29, 2023

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Aug 8, 2023

Thanks @eduardosm for pointing me to these operations.
Cc @rust-lang/lang @rust-lang/opsem

@rustbot
Copy link
Collaborator

rustbot commented Aug 8, 2023

r? @Amanieu

(rustbot has picked a reviewer for you, use r? to override)

@@ -1401,6 +1410,16 @@ pub unsafe fn _mm_getcsr() -> u32 {
/// * The *denormals-are-zero mode flag* turns all numbers which would be
/// denormalized (exponent bits are all zeros) into zeros.
///
/// Note that modfying the masking flags, rounding mode, or denormals-are-zero mode flags leads to
/// **immediate Undefined Behavior**: Rust assumes that these are always in their default state and
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes this function basically impossible to use in a UB-free way. Should we deprecate it in favor of inline assembly blocks?

Copy link

@JakobDegen JakobDegen Aug 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I don't see why we wouldn't. As far as I can tell this function always either has no effect or is UB to call

Co-authored-by: Jacob Lifshay <[email protected]>
@RalfJung
Copy link
Member Author

Do other architectures have similar vendor intrinsics that would also need the same kind of comment?

@Amanieu
Copy link
Member

Amanieu commented Aug 13, 2023

There is some precedent for deprecating unsound intrinsics, see rust-lang/rust#51810.

Do other architectures have similar vendor intrinsics that would also need the same kind of comment?

At least not on ARM or RISC-V. I'm less familiar with MIPS/PPC.

@RalfJung
Copy link
Member Author

Ah that deprecated even the functions reading eflags, since they don't behave as expected -- even if they don't cause UB. So I made this deprecate both getcsr and setcsr.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 5, 2023
explain why we can mutate the FPU control word

This is usually not allowed (see rust-lang/stdarch#1454), but here we have a special case.
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Sep 6, 2023
explain why we can mutate the FPU control word

This is usually not allowed (see rust-lang/stdarch#1454), but here we have a special case.
@RalfJung
Copy link
Member Author

@Amanieu how do we proceed with this PR?

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

Successfully merging this pull request may close these issues.

6 participants