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: does not diagnose constant division by zero #11674

Closed
dvyukov opened this issue Jul 11, 2015 · 4 comments
Closed

cmd/compile: does not diagnose constant division by zero #11674

dvyukov opened this issue Jul 11, 2015 · 4 comments

Comments

@dvyukov
Copy link
Member

dvyukov commented Jul 11, 2015

gc produces an error for c and d, but not for a and b:

package a
var a = complex64(0)/1e-1000
var b = complex64(0)/1e-47
var c = float32(0)/1e-1000
var d = float32(0)/1e-47
go.go:4: division by zero
go.go:5: division by zero

gotype produces errors for all 4 variables.

go version devel +9b04852 Sat Jul 11 00:08:50 2015 +0000 linux/amd64

@dvyukov
Copy link
Member Author

dvyukov commented Jul 11, 2015

@griesemer

@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Jul 11, 2015
@griesemer
Copy link
Contributor

The spec is not super-clear, but this appears to be a compiler bug: The untyped constants (1e-47, 1e-1000) are converted to float32/complex64 values before the operation, and the conversion results in silent underflow to 0, hence the div-by-zero error.

For comparison, in

package main
const _ = int8(-128) + 128

( http://play.golang.org/p/YrUx9sSX3w ) the compiler does exactly that: it converts the untyped 128 into an int8 which is not possible and thus returns an error.

@ALTree
Copy link
Member

ALTree commented Nov 15, 2015

Analysis: in convlit1 function inside const.go. What happens for a float constant is that the node is converted to float using

n.SetVal(Val{truncfltlit(n.Val().U.(*Mpflt), t)})

Note the call to truncfltlit, that truncates a float literal to float32 or float64 precision. For the above snippet, both the denominators get truncated to 0. After that we enter the evconst function, and the following check fails

mpcmpfltc(rv.U.(*Mpflt), 0) == 0

here, because rv is 0, and the compiler calls Yyerror("division by zero").

For complex values, the compiler executes

n.SetVal(tocplx(n.Val()))

unfortunately, the tocplx function does not truncate float values. It just does

mpmovefltflt(&c.Real, v.U.(*Mpflt))

i.e. it moves the value of the real part to c. For that reason evconst actually gets 1e-1000, so the following check

mpcmpfltc(&rv.U.(*Mpcplx).Real, 0) == 0 && mpcmpfltc(&rv.U.(*Mpcplx).Imag, 0) == 0

here passes, because rv (the real part) in not zero, and the

Yyerror("complex division by zero")

lines is not called.

I have a simple patch for this (we just need to call truncfltlit after the tocplx call in convlit1) that passes all.bash. I will email it if nobody else already have a CL for this.

@rsc rsc modified the milestones: Unplanned, Go1.6 Dec 5, 2015
@gopherbot
Copy link

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

lparth pushed a commit to lparth/go that referenced this issue Apr 13, 2017
When casting an ideal to complex{64,128}, for example during the
evaluation of

  var a = complex64(0) / 1e-50

we want the compiler to report a division-by-zero error if a divisor
would be zero after the cast.

We already do this for floats; for example

  var b = float32(0) / 1e-50

generates a 'division by zero' error at compile time (because
float32(1e-50) is zero, and the cast is done before performing the
division).

There's no such check in the path for complex{64,128} expressions, and
no cast is performed before the division in the evaluation of

  var a = complex64(0) / 1e-50

which compiles just fine.

This patch changes the convlit1 function so that complex ideals
components (real and imag) are correctly truncated to float{32,64}
when doing an ideal -> complex{64, 128} cast.

Fixes golang#11674

Change-Id: Ic5f8ee3c8cfe4c3bb0621481792c96511723d151
Reviewed-on: https://go-review.googlesource.com/37891
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
@golang golang locked and limited conversation to collaborators Apr 12, 2018
@rsc rsc removed their assignment Jun 23, 2022
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

6 participants