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

Ensure f32 deserialized from f64 and vice versa preserve NaN sign #2637

Merged
merged 3 commits into from
Oct 26, 2023

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Oct 26, 2023

f32 as f64 and f64 as f32, if the input is NaN, produces an output NaN with nondeterministic sign. This leads to bugs like TOML +nan and -nan and YAML .nan being deserialized to f32 with the wrong sign nondeterministically because they're manipulated as f64 internally by the library's Value type.

See rust-lang/miri#3139.

Comment on lines +190 to +193
#[cfg(any(no_float_copysign, not(feature = "std")))]
{
Ok(v as Self::Value)
}
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 produces nondeterministic wrong values if serde is built with "std" feature disabled. rust-lang/rust#50145

I'm not sure whether we can manually implement a correct copysign without going through the LLVM copysign intrinsics. If that were possible, I presume libcore would already have done it?

@dtolnay dtolnay merged commit 11c2917 into serde-rs:master Oct 26, 2023
15 checks passed
@dtolnay dtolnay deleted the nansign branch October 26, 2023 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant