Skip to content

math: Max(NaN, x) may return x instead of NaN (depending on the bit representation of NaN) #9919

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.

Sign up for GitHub

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

Jump to bottom
Closed
barnex opened this issue Feb 18, 2015 · 3 comments
Closed

math: Max(NaN, x) may return x instead of NaN (depending on the bit representation of NaN) #9919

barnex opened this issue Feb 18, 2015 · 3 comments
Labels
FrozenDueToAge
Milestone
Go1.5

Comments

@barnex
Copy link

barnex commented Feb 18, 2015

The following code: http://play.golang.org/p/uvj_pmygbE prints math.Max(NaN, 0), where NaN is obtained by either math.NaN() or a/a with a := 0.0.

In the second case, the result is 0 instead of the expected NaN.

This issue is shared by math.Min. However, math.IsNaN works fine for both NaN's.

Tested with go 1.4.2 on linux amd64 and with go 1.4.1 in the playground on nacl amd64p32.
@mikioh mikioh changed the title math.Max(NaN, x) may return x instead of NaN (depending on the bit representation of NaN) math: Max(NaN, x) may return x instead of NaN (depending on the bit representation of NaN) Feb 18, 2015
@barnex
Copy link
Author

barnex commented Feb 18, 2015

The issue can be traced back to math/dim_amd64.s. Here, checking for NaN is done by comparing the number with 0x7FF8000000000001. Instead, we need a check that works for all NaN bit patterns.

@bradfitz bradfitz added this to the Go1.5 milestone Feb 18, 2015
@cldorian
Copy link
Contributor

The bit representation of NaN was changed (https://go.googlesource.com/go/+/6e9506a7b45958665c3f48deecc8555f3ee2c42b%5E!/src/pkg/math/bits.go) to match gcc for a reason. I think the problem may be that 0/0 is generating the wrong bit pattern for NaN. That said, changing the function to do a mask and compare for NaN would be more general. I'll take a look.

@cldorian
Copy link
Contributor

The bit pattern starting 0xFFF8000... comes from the x64 DIVSD instruction. I've submitted change 5553.

@minux minux closed this as completed in ec92af6 Feb 23, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.5
Development

No branches or pull requests
5 participants
@bradfitz @barnex @griesemer @gopherbot @cldorian