math: handle int64 overflows for odd integer exponents in Pow(-0, y) #57494
Conversation
…in Pow(-0, y) The existing implementation does a float64 to int64 comversion in order to check whether the number is odd, however it does not check for overflows. If an overflow occurs, the result is implementation-defined and while it happens to work on amd64 and i386, it produces an incorrect result on arm64 and possibly other architectures. This change fixes that and also avoids calling isOddInt altogether if the base is +0, because it's unnecessary. (I was considering avoiding the extra check if runtime.GOARCH is "amd64" or "i386", but I can't see this pattern being used anywhere outside the tests. And having separate files with build tags just for isOddInt() seems like an overkill) Fixes golang#57465
|
This PR (HEAD: 9a6d458) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/459815 to see it. Tip: You can toggle comments from me using the |
|
Message from Keith Randall: Patch Set 1: (2 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/459815. |
|
This PR (HEAD: 3bfbd85) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/go/+/459815 to see it. Tip: You can toggle comments from me using the |
|
Message from Dmitry Panov: Patch Set 2: (3 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/459815. |
|
Message from Keith Randall: Patch Set 2: Code-Review+2 TryBot-Bypass+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/459815. |
The existing implementation does a float64 to int64 conversion in order to check whether the number is odd, however it does not check for overflows. If an overflow occurs, the result is implementation-defined and while it happens to work on amd64 and i386, it produces an incorrect result on arm64 and possibly other architectures.
This change fixes that and also avoids calling isOddInt altogether if the base is +0, because it's unnecessary.
(I was considering avoiding the extra check if runtime.GOARCH is "amd64" or "i386", but I can't see this pattern being used anywhere outside the tests. And having separate files with build tags just for isOddInt() seems like an overkill)
Fixes #57465