math/bits: panic when y<=hi in Div

Explicitly check for divide-by-zero/overflow and panic with the appropriate
runtime error.  The additional checks have basically no effect on performance
since the branch is easily predicted.

name     old time/op  new time/op  delta
Div-4    53.9ns ± 1%  53.0ns ± 1%  -1.59%  (p=0.016 n=4+5)
Div32-4  17.9ns ± 0%  18.4ns ± 0%  +2.56%  (p=0.008 n=5+5)
Div64-4  53.5ns ± 0%  53.3ns ± 0%    ~     (p=0.095 n=5+5)

Updates #28316

Change-Id: I36297ee9946cbbc57fefb44d1730283b049ecf57
Reviewed-on: https://go-review.googlesource.com/c/144377
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go
index bf447029b887ba9954c95a7639e62e4eddfde659..4654a8d9ed6f99d62a8089ac0925b62fe706681b 100644 (file)
--- a/src/go/build/deps_test.go
+++ b/src/go/build/deps_test.go
@@ -62,7 +62,7 @@ var pkgDeps = map[string][]string{
        // L1 adds simple functions and strings processing,
        // but not Unicode tables.
        "math":          {"internal/cpu", "unsafe"},
-       "math/bits":     {},
+       "math/bits":     {"unsafe"},
        "math/cmplx":    {"math"},
        "math/rand":     {"L0", "math"},
        "strconv":       {"L0", "unicode/utf8", "math", "math/bits"},

diff --git a/src/math/bits/bits.go b/src/math/bits/bits.go
index 58cf52d2a74582d427267ccadeb6288d453e1848..fbf49661573915a612db17511ce38782917bce80 100644 (file)
--- a/src/math/bits/bits.go
+++ b/src/math/bits/bits.go
@@ -8,6 +8,8 @@
 // functions for the predeclared unsigned integer types.
 package bits
 
+import _ "unsafe" // for go:linkname
+
 const uintSize = 32 << (^uint(0) >> 32 & 1) // 32 or 64
 
 // UintSize is the size of a uint in bits.
@@ -469,6 +471,9 @@ func Div(hi, lo, y uint) (quo, rem uint) {
 // hi must be < y otherwise the behavior is undefined (the quotient
 // won't fit into quo).
 func Div32(hi, lo, y uint32) (quo, rem uint32) {
+       if y != 0 && y <= hi {
+               panic(overflowError)
+       }
        z := uint64(hi)<<32 | uint64(lo)
        quo, rem = uint32(z/uint64(y)), uint32(z%uint64(y))
        return
@@ -484,8 +489,11 @@ func Div64(hi, lo, y uint64) (quo, rem uint64) {
                two32  = 1 << 32
                mask32 = two32 - 1
        )
-       if hi >= y {
-               return 1<<64 - 1, 1<<64 - 1
+       if y == 0 {
+               panic(divideError)
+       }
+       if y <= hi {
+               panic(overflowError)
        }
 
        s := uint(LeadingZeros64(y))
@@ -522,3 +530,9 @@ func Div64(hi, lo, y uint64) (quo, rem uint64) {
 
        return q1*two32 + q0, (un21*two32 + un0 - q0*y) >> s
 }
+
+//go:linkname overflowError runtime.overflowError
+var overflowError error
+
+//go:linkname divideError runtime.divideError
+var divideError error

diff --git a/src/math/bits/bits_test.go b/src/math/bits/bits_test.go
index 0bd52bee773b00e10ef24e66035cd81bcc5de2dd..1ec5107ae1294191d32cf68d2b15ca9d0ee18035 100644 (file)
--- a/src/math/bits/bits_test.go
+++ b/src/math/bits/bits_test.go
@@ -6,6 +6,7 @@ package bits_test
 
 import (
        . "math/bits"
+       "runtime"
        "testing"
        "unsafe"
 )
@@ -875,6 +876,89 @@ func TestMulDiv64(t *testing.T) {
        }
 }
 
+const (
+       divZeroError  = "runtime error: integer divide by zero"
+       overflowError = "runtime error: integer overflow"
+)
+
+func TestDivPanicOverflow(t *testing.T) {
+       // Expect a panic
+       defer func() {
+               if err := recover(); err == nil {
+                       t.Error("Div should have panicked when y<=hi")
+               } else if e, ok := err.(runtime.Error); !ok || e.Error() != overflowError {
+                       t.Errorf("Div expected panic: %q, got: %q ", overflowError, e.Error())
+               }
+       }()
+       q, r := Div(1, 0, 1)
+       t.Errorf("undefined q, r = %v, %v calculated when Div should have panicked", q, r)
+}
+
+func TestDiv32PanicOverflow(t *testing.T) {
+       // Expect a panic
+       defer func() {
+               if err := recover(); err == nil {
+                       t.Error("Div32 should have panicked when y<=hi")
+               } else if e, ok := err.(runtime.Error); !ok || e.Error() != overflowError {
+                       t.Errorf("Div32 expected panic: %q, got: %q ", overflowError, e.Error())
+               }
+       }()
+       q, r := Div32(1, 0, 1)
+       t.Errorf("undefined q, r = %v, %v calculated when Div32 should have panicked", q, r)
+}
+
+func TestDiv64PanicOverflow(t *testing.T) {
+       // Expect a panic
+       defer func() {
+               if err := recover(); err == nil {
+                       t.Error("Div64 should have panicked when y<=hi")
+               } else if e, ok := err.(runtime.Error); !ok || e.Error() != overflowError {
+                       t.Errorf("Div64 expected panic: %q, got: %q ", overflowError, e.Error())
+               }
+       }()
+       q, r := Div64(1, 0, 1)
+       t.Errorf("undefined q, r = %v, %v calculated when Div64 should have panicked", q, r)
+}
+
+func TestDivPanicZero(t *testing.T) {
+       // Expect a panic
+       defer func() {
+               if err := recover(); err == nil {
+                       t.Error("Div should have panicked when y==0")
+               } else if e, ok := err.(runtime.Error); !ok || e.Error() != divZeroError {
+                       t.Errorf("Div expected panic: %q, got: %q ", divZeroError, e.Error())
+               }
+       }()
+       q, r := Div(1, 1, 0)
+       t.Errorf("undefined q, r = %v, %v calculated when Div should have panicked", q, r)
+}
+
+func TestDiv32PanicZero(t *testing.T) {
+       // Expect a panic
+       defer func() {
+               if err := recover(); err == nil {
+                       t.Error("Div32 should have panicked when y==0")
+               } else if e, ok := err.(runtime.Error); !ok || e.Error() != divZeroError {
+                       t.Errorf("Div32 expected panic: %q, got: %q ", divZeroError, e.Error())
+               }
+       }()
+       q, r := Div32(1, 1, 0)
+       t.Errorf("undefined q, r = %v, %v calculated when Div32 should have panicked", q, r)
+}
+
+func TestDiv64PanicZero(t *testing.T) {
+       // Expect a panic
+       defer func() {
+               if err := recover(); err == nil {
+                       t.Error("Div64 should have panicked when y==0")
+               } else if e, ok := err.(runtime.Error); !ok || e.Error() != divZeroError {
+                       t.Errorf("Div64 expected panic: %q, got: %q ", divZeroError, e.Error())
+               }
+       }()
+       q, r := Div64(1, 1, 0)
+       t.Errorf("undefined q, r = %v, %v calculated when Div64 should have panicked", q, r)
+}
+
 func BenchmarkAdd(b *testing.B) {
        var z, c uint
        for i := 0; i < b.N; i++ {