Skip to content

floating point to integer casts can cause undefined behaviour #10184

Closed
#71269
Closed
floating point to integer casts can cause undefined behaviour#10184
#71269
Labels
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.C-bugCategory: This is a bug.I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessP-mediumMedium priorityT-langRelevant to the language team

Description

@thestinger
thestinger
opened on Oct 31, 2013

Status as of 2020-04-18

We intend to stabilize the saturating-float-casts behavior for as, and have stabilized unsafe library functions that handle the previous behavior. See #71269 for the latest discussion on that stabilization process.

Status as of 2018-11-05

A flag has been implemented in the compiler, -Zsaturating-float-casts, which will cause all float to integer casts have "saturating" behavior where if it's out of bounds it's clamped to the nearest bound. A call for benchmarking of this change went out awhile ago. Results, while positive in many projects, are quite negative for some projects and indicates that we're not done here.

The next steps are figuring out how to recover performance for these cases:

  • One option is to take today's as cast behavior (which is UB in some cases) and add unsafe functions for the relevant types and such.
  • Another is to wait for LLVM to add a freeze concept which means that we get a garbage bit pattern, but it's at least not UB
  • Another is to implement casts via inline assembly in LLVM IR, as the current codegen is not heavily optimized.

Old status

UPDATE (by @nikomatsakis): After much discussion, we've got the rudiments of a plan for how to address this problem. But we need some help with actually investigating the performance impact and working out the final details!

ORIGINAL ISSUE FOLLOWS:

If the value cannot fit in ty2, the results are undefined.

1.04E+17 as u8

Activity

  1. brson commented on Oct 31, 2013

    @brson
    brson
    on Oct 31, 2013
    Contributor

    Nominating

  2. pnkfelix commented on Nov 7, 2013

    @pnkfelix
    pnkfelix
    on Nov 7, 2013
    Contributor

    accepted for P-high, same reasoning as #10183

  3. pcwalton commented on Nov 22, 2013

    @pcwalton
    pcwalton
    on Nov 22, 2013
    Contributor

    I don't think this is backwards incompatible at a language level. It will not cause code that was working OK to stop working. Nominating.

  4. pnkfelix commented on Dec 19, 2013

    @pnkfelix
    pnkfelix
    on Dec 19, 2013
    Contributor

    changing to P-high, same reasoning as #10183

  5. nrc commented on Sep 12, 2014

    @nrc
    nrc
    on Sep 12, 2014
    Member

    How do we propose to solve this and #10185? Since whether behaviour is defined or not depends on the dynamic value of the number being cast, it seems the only solution is to insert dynamic checks. We seem to agree we do not want to do that for arithmetic overflow, are we happy to do it for cast overflow?

  6. pcwalton commented on Sep 12, 2014

    @pcwalton
    pcwalton
    on Sep 12, 2014
    Contributor

    We could add an intrinsic to LLVM that performs a "safe conversion". @zwarich may have other ideas.

  7. zwarich commented on Sep 12, 2014

    @zwarich
    zwarich
    on Sep 12, 2014

    AFAIK the only solution at the moment is to use the target-specific intrinsics. That's what JavaScriptCore does, at least according to someone I asked.

  8. pcwalton commented on Sep 12, 2014

    @pcwalton
    pcwalton
    on Sep 12, 2014
    Contributor

    Oh, that's easy enough then.

  9. nrc commented on Apr 23, 2015

    @nrc
    nrc
    on Apr 23, 2015
    Member

    ping @pnkfelix is this covered by the new overflow checking stuff?

  10. bluss commented on May 7, 2015

    @bluss
    bluss
    on May 7, 2015
    Contributor

    These casts are not checked by rustc with debug assertions.

  11. Aatch commented on Sep 13, 2015

    @Aatch
    Aatch
    on Sep 13, 2015
    Contributor

    I'm happy to handle this, but I need a concrete solution. I personally think that it should be checked along with overflowing integer arithmetic, as it's a very similar issue. I don't really mind what we do though.

    Note that this issue is currently causing an ICE when used in certain constant expressions.

  12. bluss commented on Sep 13, 2015

    @bluss
    bluss
    on Sep 13, 2015
    Contributor

    This allows violating memory safety in safe rust, example from this forum post:

    Undefs, huh? Undefs are fun. They tend to propagate. After a few minutes of wrangling..

    #[inline(never)]
pub fn f(ary: &[u8; 5]) -> &[u8] {
    let idx = 1e100f64 as usize;
    &ary[idx..]
}

fn main() {
    println!("{}", f(&[1; 5])[0xdeadbeef]);
}

    segfaults on my system (latest nightly) with -O.

  13. steveklabnik
    added
    I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness
    on Oct 8, 2015

  14. steveklabnik commented on Oct 8, 2015

    @steveklabnik
    steveklabnik
    on Oct 8, 2015
    Contributor

    Marking with I-unsound given the violation of memory safety in safe rust.

  15. steveklabnik commented on Oct 29, 2015

    @steveklabnik
    steveklabnik
    on Oct 29, 2015
    Contributor

    @bluss , this does not segfualt for me, just gives an assertion error. untagging since i was the one who added it

  16. 262 remaining items

  17. Mark-Simulacrum
    removed
    I-nominated
    E-help-wantedCall for participation: Help is requested to fix this issue.
    E-needs-mentor
    WG-compiler-middle
    on Apr 18, 2020

  18. federicomenaquintero commented on Apr 22, 2020

    @federicomenaquintero
    federicomenaquintero
    on Apr 22, 2020
    Contributor

    Are there plans for checked conversions? Something like fn i32::checked_from(f64) -> Result<i32, DoesntFit>?

  19. kennytm commented on Apr 22, 2020

    @kennytm
    kennytm
    on Apr 22, 2020
    Member

    You'll need to consider what should i32::checked_from(4.5) return.

  20. Dylan-DPC-zz
    added 2 commits that reference this issue on May 5, 2020

    Rollup merge of rust-lang#71269 - Mark-Simulacrum:sat-float-casts, r=…

    e2911f9

    Rollup merge of rust-lang#71269 - Mark-Simulacrum:sat-float-casts, r=…

    14d608f
  21. bors
    closed this as completedin 14d608fon May 6, 2020
  22. lopopolo
    added a commit that references this issue on Sep 25, 2022

    Panic if NaN is passed to `Duration` constructors

    9516a7e
  23. flip1995
    added a commit that references this issue on Jan 12, 2023

    Auto merge of rust-lang#10184 - robertbastian:master, r=xFrednet

    decaba9
  24. waywardmonkeys
    added a commit that references this issue on Dec 11, 2024

    Rely on defined saturation behavior for f32 to u8 casts

    6ed9e77
  25. github-merge-queue
    added a commit that references this issue on Dec 11, 2024

    Rely on defined saturation behavior for f32 to u8 casts (#87)

    9a7727a
  26. mandeep
    added a commit that references this issue on Apr 24, 2026

    Add clamp function due to rust-lang/rust#10184

    0c321ee
  27. U007D
    added a commit that references this issue on Aug 21, 2026

    Merge rust-lang#10188

    92ce768
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

    A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.C-bugCategory: This is a bug.I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessP-mediumMedium priorityT-langRelevant to the language team

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions