Skip to content

Commit 63ad2b5

Browse files
cuonglmgopherbot
authored andcommitted
[release-branch.go1.20] cmd/compile: do not report division by error during typecheck
types2 have already errored about any spec-required overflows, and division by zero. CL 469595 unintentionally fixed typecheck not to error about overflows, but zero division is still be checked during tcArith. This causes unsafe operations with variable size failed to compile, instead of raising runtime error. This CL also making change to typecheck.EvalConst, to stop evaluating literal shifts or binary operators where {over,under}flows can happen. For go1.21, typecheck.EvalConst is removed entirely, but that change is too large to backport. See discussion in CL 501735 for more details. Fixes #60675 Change-Id: I7bea2821099556835c920713397f7c5d8a4025ac Reviewed-on: https://go-review.googlesource.com/c/go/+/501735 Auto-Submit: Cuong Manh Le <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Keith Randall <[email protected]> Run-TryBot: Cuong Manh Le <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Cherry Mui <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/503855 Auto-Submit: Dmitri Shuralyov <[email protected]>
1 parent 95f377d commit 63ad2b5

File tree

3 files changed

+52
-43
lines changed

3 files changed

+52
-43
lines changed

src/cmd/compile/internal/typecheck/const.go

Lines changed: 2 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -367,29 +367,7 @@ func EvalConst(n ir.Node) ir.Node {
367367
}
368368

369369
case ir.OADD, ir.OSUB, ir.OMUL, ir.ODIV, ir.OMOD, ir.OOR, ir.OXOR, ir.OAND, ir.OANDNOT:
370-
n := n.(*ir.BinaryExpr)
371-
nl, nr := n.X, n.Y
372-
if nl.Op() == ir.OLITERAL && nr.Op() == ir.OLITERAL {
373-
rval := nr.Val()
374-
375-
// check for divisor underflow in complex division (see issue 20227)
376-
if n.Op() == ir.ODIV && n.Type().IsComplex() && constant.Sign(square(constant.Real(rval))) == 0 && constant.Sign(square(constant.Imag(rval))) == 0 {
377-
base.Errorf("complex division by zero")
378-
n.SetType(nil)
379-
return n
380-
}
381-
if (n.Op() == ir.ODIV || n.Op() == ir.OMOD) && constant.Sign(rval) == 0 {
382-
base.Errorf("division by zero")
383-
n.SetType(nil)
384-
return n
385-
}
386-
387-
tok := tokenForOp[n.Op()]
388-
if n.Op() == ir.ODIV && n.Type().IsInteger() {
389-
tok = token.QUO_ASSIGN // integer division
390-
}
391-
return OrigConst(n, constant.BinaryOp(nl.Val(), tok, rval))
392-
}
370+
return n
393371

394372
case ir.OOROR, ir.OANDAND:
395373
n := n.(*ir.LogicalExpr)
@@ -406,19 +384,7 @@ func EvalConst(n ir.Node) ir.Node {
406384
}
407385

408386
case ir.OLSH, ir.ORSH:
409-
n := n.(*ir.BinaryExpr)
410-
nl, nr := n.X, n.Y
411-
if nl.Op() == ir.OLITERAL && nr.Op() == ir.OLITERAL {
412-
// shiftBound from go/types; "so we can express smallestFloat64" (see issue #44057)
413-
const shiftBound = 1023 - 1 + 52
414-
s, ok := constant.Uint64Val(nr.Val())
415-
if !ok || s > shiftBound {
416-
base.Errorf("invalid shift count %v", nr)
417-
n.SetType(nil)
418-
break
419-
}
420-
return OrigConst(n, constant.Shift(toint(nl.Val()), tokenForOp[n.Op()], uint(s)))
421-
}
387+
return n
422388

423389
case ir.OCONV, ir.ORUNESTR:
424390
n := n.(*ir.ConvExpr)

src/cmd/compile/internal/typecheck/expr.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -184,13 +184,6 @@ func tcArith(n ir.Node, op ir.Op, l, r ir.Node) (ir.Node, ir.Node, *types.Type)
184184
}
185185
}
186186

187-
if (op == ir.ODIV || op == ir.OMOD) && ir.IsConst(r, constant.Int) {
188-
if constant.Sign(r.Val()) == 0 {
189-
base.Errorf("division by zero")
190-
return l, r, nil
191-
}
192-
}
193-
194187
return l, r, t
195188
}
196189

test/fixedbugs/issue60601.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// run
2+
3+
// Copyright 2023 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package main
8+
9+
import (
10+
"strings"
11+
"unsafe"
12+
)
13+
14+
func shift[T any]() int64 {
15+
return 1 << unsafe.Sizeof(*new(T))
16+
}
17+
18+
func div[T any]() uintptr {
19+
return 1 / unsafe.Sizeof(*new(T))
20+
}
21+
22+
func add[T any]() int64 {
23+
return 1<<63 - 1 + int64(unsafe.Sizeof(*new(T)))
24+
}
25+
26+
func main() {
27+
shift[[62]byte]()
28+
shift[[63]byte]()
29+
shift[[64]byte]()
30+
shift[[100]byte]()
31+
shift[[1e6]byte]()
32+
33+
add[[1]byte]()
34+
shouldPanic("divide by zero", func() { div[[0]byte]() })
35+
}
36+
37+
func shouldPanic(str string, f func()) {
38+
defer func() {
39+
err := recover()
40+
if err == nil {
41+
panic("did not panic")
42+
}
43+
s := err.(error).Error()
44+
if !strings.Contains(s, str) {
45+
panic("got panic " + s + ", want " + str)
46+
}
47+
}()
48+
49+
f()
50+
}

0 commit comments

Comments
 (0)