From ead5d1e316873a63471de31f3d70f97aeb7969f5 Mon Sep 17 00:00:00 2001 From: Brian Kessler Date: Tue, 23 Oct 2018 20:20:44 -0600 Subject: [PATCH] math/bits: panic when y<=hi in Div MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Keith Randall --- src/go/build/deps_test.go | 2 +- src/math/bits/bits.go | 18 +++++++- src/math/bits/bits_test.go | 84 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 3 deletions(-) diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index bf447029b8..4654a8d9ed 100644 --- 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 58cf52d2a7..fbf4966157 100644 --- 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 0bd52bee77..1ec5107ae1 100644 --- 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++ { -- 2.48.1