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

NaN argument to the mod operator (%) causes unbounded recursion #862

Closed
mch opened this Issue May 4, 2017 · 3 comments

Comments

Projects
None yet
4 participants
@mch

mch commented May 4, 2017

Running this simple program causes a JavaScript exception:

module Main exposing (..)

import Html exposing (Html, text, div)


main : Html a
main =
    let
        nanFloat : Float
        nanFloat = sqrt -1
        
        nanInt : Int
        nanInt = ceiling nanFloat
        
        recursionHere : Int
        recursionHere = 6 % nanInt

    in
        div [] 
            [ div [] [ text (toString recursionHere) ]
            ]

Resulting error:

Uncaught RangeError: Maximum call stack size exceeded
    at mod (VM98 javascript:1117)
    at mod (VM98 javascript:1124)
    at mod (VM98 javascript:1124)
    at mod (VM98 javascript:1124)
    ...

On Ellie: https://ellie-app.com/36bpc9T7v5ba1/5

The implementation should check for NaN in the arguments, and likely return NaN. In addition, it should not be possible to get an Int containing a value like Nan or Infinity.

Browser: Chrome 58.0.3029.96 (64-bit)
OS: macOS 10.12.4
Elm: 0.18.0

@process-bot

This comment has been minimized.

Show comment
Hide comment
@process-bot

process-bot May 4, 2017

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

process-bot commented May 4, 2017

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

@astlouisf

This comment has been minimized.

Show comment
Hide comment
@astlouisf

astlouisf May 6, 2017

Judging by the type signature of both isNaN and isInfinite the problem rather lies in the fact that you were able to get a NaN typed as an Int.

One possible fix would be to add a check to round, floor, ceiling and truncate.

However, some research led me to this disccussion on ghc's trac. Of particular interest is tristes_tigres suggestion to separate rounding and conversion functions, to avoid the performance hit of checking undefined values.

This could be implemented with the addition of a toInt function (or fromFloat to avoid a possible collision with String.toInt). The new type signatures would look like:

round: Float -> Float
ceiling: Float -> Float
floor: Float -> Float
truncate: Float -> Float
toInt: Float -> Int
toFloat: Int -> Float  -- unchanged

Current uses of round, ceiling, floor and truncate would need to be composed with toInt

Whatever seems more appropriate, I would be willing to tackle this issue

astlouisf commented May 6, 2017

Judging by the type signature of both isNaN and isInfinite the problem rather lies in the fact that you were able to get a NaN typed as an Int.

One possible fix would be to add a check to round, floor, ceiling and truncate.

However, some research led me to this disccussion on ghc's trac. Of particular interest is tristes_tigres suggestion to separate rounding and conversion functions, to avoid the performance hit of checking undefined values.

This could be implemented with the addition of a toInt function (or fromFloat to avoid a possible collision with String.toInt). The new type signatures would look like:

round: Float -> Float
ceiling: Float -> Float
floor: Float -> Float
truncate: Float -> Float
toInt: Float -> Int
toFloat: Int -> Float  -- unchanged

Current uses of round, ceiling, floor and truncate would need to be composed with toInt

Whatever seems more appropriate, I would be willing to tackle this issue

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Mar 7, 2018

Member

Tracking this in #721. Gathering all the JS math oddities may help clarify how to address it in a coherent way that does not have really troubling performance implications.

Member

evancz commented Mar 7, 2018

Tracking this in #721. Gathering all the JS math oddities may help clarify how to address it in a coherent way that does not have really troubling performance implications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment