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

Guarantees (or lack thereof) of floating-point arithmetic #123

Closed
gnzlbg opened this issue Apr 22, 2019 · 9 comments
Closed

Guarantees (or lack thereof) of floating-point arithmetic #123

gnzlbg opened this issue Apr 22, 2019 · 9 comments
Labels
A-floats Topic: concerns floating point operations/representations C-open-question Category: An open question that we should revisit S-pending-design Status: Resolving this issue requires addressing some open design questions

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Apr 22, 2019

Once we are finished with layout and validity we might want to write down what unsafe Rust code is allowed to assume about floating-point arithmetic with the f32 and f64 primitive types.

For example, does this code (playground) invoke undefined behavior?

fn foo(num: f32) -> f32 {
    ((num + 0.1) / 1.5e38) * 1.5e38
}

const A: f32 = 0.00000014156103134155273437500000000000000000000000;

fn main() {
    let expr =  foo(1.23456789) - 1.23456789 - 0.1;
    let idx = if expr == A {
        0
    } else {
        1
    };
    let a = [0];
    // With IEEE-754 arithmetic this is always safe:
    let r = unsafe { a.get_unchecked(idx) };
    println!("r = {}", r);
}

Currently, this code does not invoke UB on many targets, but on i586-unknown-linux-gnu in debug mode this code does an access out-of-bounds.

@gnzlbg gnzlbg added the C-open-question Category: An open question that we should revisit label Apr 22, 2019
@gnzlbg
Copy link
Contributor Author

gnzlbg commented Apr 23, 2019

IIUC, if we were to guarantee (like the reference might already do) that arithmetic on f32 and f64 follows IEEE-754 arithmetic, then AFAICT the example above never does an OOB access, and the i586-unknown-linux-gnu target has a bug.

The IEEE-754:2008 section 10.4 allows implementations to change the behavior of some of these operations for optimization purposes in an opt-in way, so a consequence of following the standard here would be that an implementation could add a flag that when enabled breaks the example above.

This is not as bad as it sounds. The example above would only be safe for certains values of these options (e.g. the defaults), and users can use cfg!(...) to make compilation fail if these assumptions are violated. That is, the example above might not always be safe, but adding a #![fp_options = "default"] (or something like that) to it would.

@joshtriplett
Copy link
Member

joshtriplett commented Apr 23, 2019

I would propose that the code sample above should always be considered unsafe, and one possible solution should be to not use get_unchecked. (Conversely, if it weren't unsafe then one codepath of it would always be dead.)

Also, the proposal as written relies on an assumption about what the defaults would be, which has not yet been determined (and isn't entirely clear from the limited degree that Rust currently specifies it). That's already the subject of an existing RFC. I don't think the answer to "is the above code safe" should be "it depends", it should be "yes" or "no". (And I think it should be "no".)

The above code should also be generating a lint about the use of == on a floating-point value, and if it isn't currently, it should be. I would similarly suggest a lint that catches any code doing a floating-point calculation then converting the result to an integer and using it in get_unchecked.

EDIT: Also, thank you for raising this issue; I definitely think it's worth evaluating, as an orthogonal question.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Apr 24, 2019

I don't think stating that the behavior of the example above is always undefined works very well without a more precise model of which exact fp operations are "deterministic" and which ones are not.

For example, if that's always UB, miri should be able to execute it and error properly, but how would it do that? If all fp arithmetic is treated as non-deterministic, then:

  • offsetting a pointer with something that was operated on using fp arithmetic can return multiple pointers, where only dereferencing one of them could be safe
  • branching on something that was operated on using fp arithmetic can take multiple branches, where only one might be safe to take

etc. We would need to track everything that participated in a fp operation, and when something like a pointer offset, a branch, etc. happens, error in miri. It might just be much simpler and almost as useful in practice to just forbid all fp arithmetic in miri and constant evaluation.

What would work is letting fp-arithmetic be implementation-defined, as long as it is deterministic (that's what IEEE-754 tries to do). Different targets can have different implementations (e.g. different libm's), but miri could implement those, and we could do fp arithmetic evaluation in miri. This means that whether the example above has UB or not is implementation defined, but you could run it under miri for a couple of implementations, and miri would let you know for which one the example does have UB.

If fp-arithmetic is allowed to depend on, e.g., the optimization level, miri would need to "somehow" perform the same optimizations that LLVM does to be able to replicate the arithmetic, which probably is not feasible, so this is something that might be worth constraining. This does not mean that an implementation wouldn't be allowed to perform any optimizations, but that miri has to be able to model them.

@scottmcm
Copy link
Member

The above code should also be generating a lint about the use of == on a floating-point value, and if it isn't currently, it should be.

I strongly disagree on this one. I've worked in codebases with such warnings before, and it's not helpful, resulting in just x >= 0 || x <= 0 instead, which doesn't improve anything.

After all, x >= 0 is no more correct on its own than x == 0 -- x could have been computed by a subtraction that's only below zero because of rounding -- so it amounts to a "you just can't use comparisons on f32" lint, which I don't think we should have in rustc.

I don't want lints that are, essentially, "Go read https://dl.acm.org/doi/10.1145/103162.103163 then allow this lint".

@programmerjake
Copy link
Member

I do think its useful to treat x86-32 without SSE2 as a legacy architecture so, even if it's slower on x86-32, we should have fp arithmetic use the precision and range and other properties of the fp types used, and not allow promoting intermediates to f80 unless explicitly annotated with fast-math or equivalent. In particular, there should not be a compiler flag that just enables fast-math everywhere, since library authors should be able to depend on fp semantics for unsafe code and for correctness.

@programmerjake
Copy link
Member

@RalfJung
Copy link
Member

RalfJung commented Jul 25, 2023

I recently wrote a pre-RFC that proposes concrete guarantees.

Under that pre-RFC, the program above is fine. The implementation for the target is just unfortunately broken.

@programmerjake

This comment was marked as resolved.

@JakobDegen JakobDegen added the S-pending-design Status: Resolving this issue requires addressing some open design questions label Aug 1, 2023
@RalfJung
Copy link
Member

RalfJung commented Aug 4, 2023

The implementation for the target is just unfortunately broken.

With rust-lang/rust#113053 having passed FCP, that is now clearly the case. So I think we can close this issue.

#237 tracks writing down our NaN semantics properly.

@RalfJung RalfJung closed this as completed Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-floats Topic: concerns floating point operations/representations C-open-question Category: An open question that we should revisit S-pending-design Status: Resolving this issue requires addressing some open design questions
Projects
None yet
Development

No branches or pull requests

6 participants