Skip to content

Miri: handling of SNaN inputs in f*::pow operations #142514

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 69 additions & 27 deletions src/tools/miri/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ mod simd;
use std::ops::Neg;

use rand::Rng;
use rand::rngs::StdRng;
use rustc_abi::Size;
use rustc_apfloat::ieee::{IeeeFloat, Semantics};
use rustc_apfloat::{self, Float, Round};
Expand Down Expand Up @@ -191,7 +192,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let [f] = check_intrinsic_arg_count(args)?;
let f = this.read_scalar(f)?.to_f32()?;

let res = fixed_float_value(intrinsic_name, &[f]).unwrap_or_else(||{
let res = fixed_float_value(this, intrinsic_name, &[f]).unwrap_or_else(||{
// Using host floats (but it's fine, these operations do not have
// guaranteed precision).
let host = f.to_host();
Expand Down Expand Up @@ -235,7 +236,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let [f] = check_intrinsic_arg_count(args)?;
let f = this.read_scalar(f)?.to_f64()?;

let res = fixed_float_value(intrinsic_name, &[f]).unwrap_or_else(||{
let res = fixed_float_value(this, intrinsic_name, &[f]).unwrap_or_else(||{
// Using host floats (but it's fine, these operations do not have
// guaranteed precision).
let host = f.to_host();
Expand Down Expand Up @@ -324,7 +325,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let f1 = this.read_scalar(f1)?.to_f32()?;
let f2 = this.read_scalar(f2)?.to_f32()?;

let res = fixed_float_value(intrinsic_name, &[f1, f2]).unwrap_or_else(|| {
let res = fixed_float_value(this, intrinsic_name, &[f1, f2]).unwrap_or_else(|| {
// Using host floats (but it's fine, this operation does not have guaranteed precision).
let res = f1.to_host().powf(f2.to_host()).to_soft();

Expand All @@ -342,7 +343,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let f1 = this.read_scalar(f1)?.to_f64()?;
let f2 = this.read_scalar(f2)?.to_f64()?;

let res = fixed_float_value(intrinsic_name, &[f1, f2]).unwrap_or_else(|| {
let res = fixed_float_value(this, intrinsic_name, &[f1, f2]).unwrap_or_else(|| {
// Using host floats (but it's fine, this operation does not have guaranteed precision).
let res = f1.to_host().powf(f2.to_host()).to_soft();

Expand All @@ -361,7 +362,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let f = this.read_scalar(f)?.to_f32()?;
let i = this.read_scalar(i)?.to_i32()?;

let res = fixed_powi_float_value(f, i).unwrap_or_else(|| {
let res = fixed_powi_float_value(this, f, i).unwrap_or_else(|| {
// Using host floats (but it's fine, this operation does not have guaranteed precision).
let res = f.to_host().powi(i).to_soft();

Expand All @@ -379,7 +380,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
let f = this.read_scalar(f)?.to_f64()?;
let i = this.read_scalar(i)?.to_i32()?;

let res = fixed_powi_float_value(f, i).unwrap_or_else(|| {
let res = fixed_powi_float_value(this, f, i).unwrap_or_else(|| {
// Using host floats (but it's fine, this operation does not have guaranteed precision).
let res = f.to_host().powi(i).to_soft();

Expand Down Expand Up @@ -501,56 +502,97 @@ fn apply_random_float_error_to_imm<'tcx>(
interp_ok(ImmTy::from_scalar_int(res, val.layout))
}

/// Returns either a SNaN or a QNaN, with a randomly generated payload.
fn random_nan<S: Semantics>(rng: &mut StdRng) -> IeeeFloat<S> {
if rng.random() {
IeeeFloat::<S>::snan(Some(rng.random()))
} else {
IeeeFloat::<S>::qnan(Some(rng.random()))
}
}

/// For the intrinsics:
/// - sinf32, sinf64
/// - cosf32, cosf64
/// - expf32, expf64, exp2f32, exp2f64
/// - logf32, logf64, log2f32, log2f64, log10f32, log10f64
/// - powf32, powf64
///
/// # Return
///
/// Returns `Some(output)` if the `intrinsic` results in a defined fixed `output` specified in the C standard
/// (specifically, C23 annex F.10) when given `args` as arguments. Outputs that are unaffected by a relative error
/// (such as INF and zero) are not handled here, they are assumed to be handled by the underlying
/// implementation. Returns `None` if no specific value is guaranteed.
///
/// # Note
///
/// For `powf*` operations of the form:
///
/// - `(SNaN)^(±0)`
/// - `1^(SNaN)`
///
/// The result is implementation-defined:
/// - musl returns for both `1.0`
/// - glibc returns for both `NaN`
///
/// This discrepancy exists because SNaN handling is not consistently defined across platforms,
/// and the C standard leaves behavior for SNaNs unspecified.
///
/// Miri chooses to adhere to both implementations and returns either one of them non-deterministically.
fn fixed_float_value<S: Semantics>(
ecx: &mut MiriInterpCx<'_>,
intrinsic_name: &str,
args: &[IeeeFloat<S>],
) -> Option<IeeeFloat<S>> {
let one = IeeeFloat::<S>::one();
match (intrinsic_name, args) {
Some(match (intrinsic_name, args) {
// cos(+- 0) = 1
("cosf32" | "cosf64", [input]) if input.is_zero() => Some(one),
("cosf32" | "cosf64", [input]) if input.is_zero() => one,

// e^0 = 1
("expf32" | "expf64" | "exp2f32" | "exp2f64", [input]) if input.is_zero() => Some(one),

// 1^y = 1 for any y, even a NaN.
("powf32" | "powf64", [base, _]) if *base == one => Some(one),
("expf32" | "expf64" | "exp2f32" | "exp2f64", [input]) if input.is_zero() => one,

// (-1)^(±INF) = 1
("powf32" | "powf64", [base, exp]) if *base == -one && exp.is_infinite() => Some(one),
("powf32" | "powf64", [base, exp]) if *base == -one && exp.is_infinite() => one,

// FIXME(#4286): The C ecosystem is inconsistent with handling sNaN's, some return 1 others propogate
// the NaN. We should return either 1 or the NaN non-deterministically here.
// But for now, just handle them all the same.
// x^(±0) = 1 for any x, even a NaN
("powf32" | "powf64", [_, exp]) if exp.is_zero() => Some(one),
// 1^y = 1 for any y, even a NaN, *but* not a SNaN
("powf32" | "powf64", [base, exp]) if *base == one => {
let rng = ecx.machine.rng.get_mut();
// Handle both the musl and glibc cases non-deterministically.
if !exp.is_signaling() || rng.random() { one } else { random_nan(rng) }
}

// x^(±0) = 1 for any x, even a NaN, *but* not a SNaN
("powf32" | "powf64", [base, exp]) if exp.is_zero() => {
let rng = ecx.machine.rng.get_mut();
// Handle both the musl and glibc cases non-deterministically.
if !base.is_signaling() || rng.random() { one } else { random_nan(rng) }
}

// There are a lot of cases for fixed outputs according to the C Standard, but these are mainly INF or zero
// which are not affected by the applied error.
_ => None,
}
_ => return None,
})
}

/// Returns `Some(output)` if `powi` (called `pown` in C) results in a fixed value specified in the C standard
/// (specifically, C23 annex F.10.4.6) when doing `base^exp`. Otherwise, returns `None`.
fn fixed_powi_float_value<S: Semantics>(base: IeeeFloat<S>, exp: i32) -> Option<IeeeFloat<S>> {
match (base.category(), exp) {
// x^0 = 1, if x is not a Signaling NaN
// FIXME(#4286): The C ecosystem is inconsistent with handling sNaN's, some return 1 others propogate
// the NaN. We should return either 1 or the NaN non-deterministically here.
// But for now, just handle them all the same.
(_, 0) => Some(IeeeFloat::<S>::one()),
// TODO: I'm not sure what I should document here about pown(1, SNaN) since musl and glibc do the same and the C standard is explicit here.
fn fixed_powi_float_value<S: Semantics>(
ecx: &mut MiriInterpCx<'_>,
base: IeeeFloat<S>,
exp: i32,
) -> Option<IeeeFloat<S>> {
match exp {
0 => {
let one = IeeeFloat::<S>::one();
let rng = ecx.machine.rng.get_mut();
Some(
// Handle both the musl and glibc powf cases non-deterministically.
if !base.is_signaling() || rng.random() { one } else { random_nan(rng) },
)
}

_ => None,
}
Expand Down
44 changes: 33 additions & 11 deletions src/tools/miri/tests/pass/float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1067,17 +1067,39 @@ pub fn libm() {
assert_eq!((-1f32).powf(f32::NEG_INFINITY), 1.0);
assert_eq!((-1f64).powf(f64::NEG_INFINITY), 1.0);

// For pow (powf in rust) the C standard says:
// x^0 = 1 for all x even a sNaN
// FIXME(#4286): this does not match the behavior of all implementations.
assert_eq!(SNAN_F32.powf(0.0), 1.0);
assert_eq!(SNAN_F64.powf(0.0), 1.0);

// For pown (powi in rust) the C standard says:
// x^0 = 1 for all x even a sNaN
// FIXME(#4286): this does not match the behavior of all implementations.
assert_eq!(SNAN_F32.powi(0), 1.0);
assert_eq!(SNAN_F64.powi(0), 1.0);
// Makes sure an operations returns both `1` and a `NaN` randomly.
macro_rules! test_snan_nondet {
($pow_op:expr) => {{
let mut nan_seen = false;
let mut one_seen = false;

for _ in 0..64 {
let res = $pow_op;
nan_seen |= res.is_nan();
one_seen |= res == 1.0;

// little speedup
if nan_seen && one_seen { break; };
}

let op_as_str = stringify!($pow_op);

assert!(nan_seen && one_seen, "{} should return both `NaN` or `1.0` randomly", op_as_str);
}};
}

// x^(SNaN) = (1 | NaN)
test_snan_nondet!(f32::powf(SNAN_F32, 0.0));
test_snan_nondet!(f64::powf(SNAN_F64, 0.0));

// 1^(SNaN) = (1 | NaN)
test_snan_nondet!(f32::powf(1.0, SNAN_F32));
test_snan_nondet!(f64::powf(1.0, SNAN_F64));

// same as powf (keep it consistent):
// x^(SNaN) = (1 | NaN)
test_snan_nondet!(f32::powi(SNAN_F32, 0));
test_snan_nondet!(f64::powi(SNAN_F64, 0));

assert_eq!(0f32.powi(10), 0.0);
assert_eq!(0f64.powi(100), 0.0);
Expand Down
Loading