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.

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

math: pure go version of math.Sqrt is broken #13013

Closed
ghost opened this issue Oct 21, 2015 · 5 comments
Closed

math: pure go version of math.Sqrt is broken #13013

ghost opened this issue Oct 21, 2015 · 5 comments
Labels
FrozenDueToAge
Milestone
Go1.6

Comments

@ghost
Copy link

ghost commented Oct 21, 2015

If you pass math.Float64frombits(uint64(2)) to math.Sqrt on a machine that uses the software version (such as ppc64x https://golang.org/src/math/stubs_ppc64x.s#L87) it will end up in an infinite loop here https://golang.org/src/math/sqrt.go#L110

Unfortunatelly i don't have a ppc64x machine nearby. So I can't test this properly, however I did copy the library and manually call math.sqrt (the unexported, pure go version) with the argument and I have reproduced the problem.
@ianlancetaylor ianlancetaylor changed the title pure go version of math.Sqrt is broken math: pure go version of math.Sqrt is broken Oct 21, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Oct 21, 2015
@cespare
Copy link
Contributor

cespare commented Oct 22, 2015

Nice find, any subnormal with 0 LSB will hit this. Fixed in https://golang.org/cl/16158.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/16158 mentions this issue.

@binarycrusader
Copy link
Contributor

Should this fix also be applied to src/runtime/sqrt.go#L120 ?

https://github.com/golang/go/blob/master/src/runtime/sqrt.go#L120

@cespare
Copy link
Contributor

cespare commented Oct 23, 2015

@binarycrusader yeah, I'll submit another CL

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/16291 mentions this issue.

@minux minux mentioned this issue Oct 23, 2015
Closed
@davecheney davecheney mentioned this issue Oct 23, 2015
Closed
minux pushed a commit that referenced this issue Oct 23, 2015
@cespare @minux
runtime: copy sqrt normalization bugfix from math

Verified

This commit was signed with the committer’s verified signature.
kevinburke Kevin Burke
GPG key ID: 24B0EF06511BA263
Verified
Learn about vigilant mode
fb7178e 
This copies the change from CL 16158 (applied as
22d4c8b).

Updates #13013

Change-Id: Id7d02e63d92806f06a4e064a91b2fb6574fe385f
Reviewed-on: https://go-review.googlesource.com/16291
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Oct 24, 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.6
Development

No branches or pull requests
4 participants
@cespare @binarycrusader @ianlancetaylor @gopherbot