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.

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

cmd/compile: truncates constants #11326

Closed
dvyukov opened this issue Jun 22, 2015 · 8 comments
Closed

cmd/compile: truncates constants #11326

dvyukov opened this issue Jun 22, 2015 · 8 comments

Comments

@dvyukov
Copy link
Member

dvyukov commented Jun 22, 2015

The following program:

package main
import "fmt"
func main() {
    var g = 1e81391777742999
    fmt.Println(g)
}

prints:

1e+151

That's a wrong answer.

go version devel +514014c Thu Jun 18 15:54:35 2015 +0200 linux/amd64

@griesemer griesemer assigned griesemer and unassigned rsc Jun 22, 2015
@griesemer griesemer added this to the Go1.5Maybe milestone Jun 22, 2015
@griesemer
Copy link
Contributor

This is related to a math/big bug. See issue #11341 .

Once issue #11341 is fixed, re-vendor math/big to src/cmd/compile/internal. The respective method call in the compiler is in mparith3.go:156 (function mpatoflt).

Depending on the math/big fix, the value may become +Inf (and then we're all done), or we may need to add a range check.

@rsc
Copy link
Contributor

rsc commented Jun 29, 2015

I am a bit worried about deferring to math/big on this. For example, the value should not become +Inf, since that does not mean anything inside the compiler. More generally, math/big's job is to do what is asked, and the compiler has the added requirement of not running forever. The two are different, so it makes sense for the compiler to have some checks of its own. I will send a CL rejecting numbers with too many exponent digits. That should defend against a whole class of problems.

@rsc rsc assigned rsc and unassigned griesemer Jun 29, 2015
@rsc rsc modified the milestones: Go1.5, Go1.5Maybe Jun 29, 2015
@gopherbot
Copy link

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

@rsc
Copy link
Contributor

rsc commented Jun 29, 2015

I'm going to cut the size of the accepted exponents in the gc compiler for Go 1.5 to no more than 8 digits. It's not perfect but it eliminates the simplest problems. We can revisit both correctness and performance for Go 1.6.

In particular, if I write:

var h = 1e2147483647

then I should get an error that that doesn't fit in a float64, but instead the error says a completely different number doesn't fit in the float64:

x.go:13: constant 0.93342e+536870911 overflows float64

@rsc rsc modified the milestones: Go1.6Early, Go1.5 Jun 29, 2015
@griesemer
Copy link
Contributor

@rsc sounds ok. This does appear very much like a math/big issue. Will investigate.

rsc added a commit that referenced this issue Jun 29, 2015
For #11326 (but not a fix).

Change-Id: Ic51814f5cd7357427c3fd990a5522775d05e7987
Reviewed-on: https://go-review.googlesource.com/11673
Reviewed-by: Robert Griesemer <gri@golang.org>
@griesemer
Copy link
Contributor

#11341 was fixed and addresses this issue as well. Cutting exponent size to 8 digits (mpatoflt in mparith3.go) is no longer needed.

However, Float.SetString appears to not accept a full 32bit exponent:

var g = 0.5e2147483647

leads to the error (after removing cutting the exponent size):

constant too large: 0.5e2147483647

implying that the constant internally turned into a (Float) infinity. Keep this open for now.

@griesemer griesemer assigned griesemer and unassigned rsc Aug 28, 2015
@griesemer
Copy link
Contributor

#11341 also fixed this issue.

The spec mentions ( http://golang.org/ref/spec#Constants ) as an implementation restriction

...
Represent floating-point constants, including the parts of a complex constant, with a mantissa of at least 256 bits and a signed exponent of at least 32 bits.
...

A 32-bit exponent corresponds to a maximum value of approx. 1e646456992, not 1e2147483646 (== 0.5e2147483647) -- the 32bits are for a binary (not decimal) exponent.

(This was also misunderstood in the test cases: test/fixedbugs/issue11326.go, test/fixedbugs/issue11326b.go)

@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Sep 22, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants