]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: implement shifts by signed amounts
authorKeith Randall <keithr@alum.mit.edu>
Sun, 20 Jan 2019 18:52:11 +0000 (10:52 -0800)
committerKeith Randall <khr@golang.org>
Fri, 15 Feb 2019 23:13:09 +0000 (23:13 +0000)
Allow shifts by signed amounts. Panic if the shift amount is negative.

TODO: We end up doing two compares per shift, see Ian's comment
https://github.com/golang/go/issues/19113#issuecomment-443241799 that
we could do it with a single comparison in the normal case.

The prove pass mostly handles this code well. For instance, it removes the
<0 check for cases like this:
    if s >= 0 { _ = x << s }
    _ = x << len(a)

This case isn't handled well yet:
    _ = x << (y & 0xf)
I'll do followon CLs for unhandled cases as needed.

Update #19113

R=go1.13

Change-Id: I839a5933d94b54ab04deb9dd5149f32c51c90fa1
Reviewed-on: https://go-review.googlesource.com/c/158719
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
src/cmd/compile/internal/gc/builtin.go
src/cmd/compile/internal/gc/builtin/runtime.go
src/cmd/compile/internal/gc/go.go
src/cmd/compile/internal/gc/ssa.go
src/cmd/compile/internal/gc/typecheck.go
src/cmd/compile/internal/ssa/rewrite.go
src/cmd/internal/obj/x86/obj6.go
src/runtime/panic.go
test/fixedbugs/bug073.go
test/fixedbugs/issue19113.go [new file with mode: 0644]

index 04f4cbfd5852d2df95e6bb04b8aad72f4cf2e586..f32fcd675d84cb26a0215318a7f70aff1f8eb970 100644 (file)
@@ -13,6 +13,7 @@ var runtimeDecls = [...]struct {
        {"panicindex", funcTag, 5},
        {"panicslice", funcTag, 5},
        {"panicdivide", funcTag, 5},
+       {"panicshift", funcTag, 5},
        {"panicmakeslicelen", funcTag, 5},
        {"throwinit", funcTag, 5},
        {"panicwrap", funcTag, 5},
index fc879badb2b7eb36e95d359c136f2016392fe7ca..210881a6e99011c5c3633d8a587a16f3af7c48f6 100644 (file)
@@ -18,6 +18,7 @@ func newobject(typ *byte) *any
 func panicindex()
 func panicslice()
 func panicdivide()
+func panicshift()
 func panicmakeslicelen()
 func throwinit()
 func panicwrap()
index 376637ba9adcabeb21e0528e14f32a09f1587908..2213d8d9b8bf7a39aaa7152483d17db268f45b1c 100644 (file)
@@ -296,6 +296,7 @@ var (
        msanwrite,
        newproc,
        panicdivide,
+       panicshift,
        panicdottypeE,
        panicdottypeI,
        panicindex,
index e20137669aaac831cc8b9e8eba1781e985dbb68e..6ddc9fba7a2ada9754d1502a4f9f88eb857285ed 100644 (file)
@@ -84,6 +84,7 @@ func initssaconfig() {
        panicnildottype = sysfunc("panicnildottype")
        panicoverflow = sysfunc("panicoverflow")
        panicslice = sysfunc("panicslice")
+       panicshift = sysfunc("panicshift")
        raceread = sysfunc("raceread")
        racereadrange = sysfunc("racereadrange")
        racewrite = sysfunc("racewrite")
@@ -2128,7 +2129,13 @@ func (s *state) expr(n *Node) *ssa.Value {
        case OLSH, ORSH:
                a := s.expr(n.Left)
                b := s.expr(n.Right)
-               return s.newValue2(s.ssaShiftOp(n.Op, n.Type, n.Right.Type), a.Type, a, b)
+               bt := b.Type
+               if bt.IsSigned() {
+                       cmp := s.newValue2(s.ssaOp(OGE, bt), types.Types[TBOOL], b, s.zeroVal(bt))
+                       s.check(cmp, panicshift)
+                       bt = bt.ToUnsigned()
+               }
+               return s.newValue2(s.ssaShiftOp(n.Op, n.Type, bt), a.Type, a, b)
        case OANDAND, OOROR:
                // To implement OANDAND (and OOROR), we introduce a
                // new temporary variable to hold the result. The
index 4fc1c5c73c582f3c361518d36b85b281ebc471d8..63e0d782733a521afcd446cb35418c8746626cbc 100644 (file)
@@ -660,8 +660,8 @@ func typecheck1(n *Node, top int) (res *Node) {
                        r = defaultlit(r, types.Types[TUINT])
                        n.Right = r
                        t := r.Type
-                       if !t.IsInteger() || t.IsSigned() {
-                               yyerror("invalid operation: %v (shift count type %v, must be unsigned integer)", n, r.Type)
+                       if !t.IsInteger() {
+                               yyerror("invalid operation: %v (shift count type %v, must be integer)", n, r.Type)
                                n.Type = nil
                                return n
                        }
index a154249371a29117e4f429645895e07d9aa0fc65..6edb593df97ae670e786506b2743e92d7368b789 100644 (file)
@@ -1115,7 +1115,8 @@ func needRaceCleanup(sym interface{}, v *Value) bool {
                        case OpStaticCall:
                                switch v.Aux.(fmt.Stringer).String() {
                                case "runtime.racefuncenter", "runtime.racefuncexit", "runtime.panicindex",
-                                       "runtime.panicslice", "runtime.panicdivide", "runtime.panicwrap":
+                                       "runtime.panicslice", "runtime.panicdivide", "runtime.panicwrap",
+                                       "runtime.panicshift":
                                // Check for racefuncenter will encounter racefuncexit and vice versa.
                                // Allow calls to panic*
                                default:
index babfd38ad235148522e5031b7cfed3203c03d779..a6931e8441828e275979349e08b88144c8639290 100644 (file)
@@ -968,7 +968,7 @@ func isZeroArgRuntimeCall(s *obj.LSym) bool {
                return false
        }
        switch s.Name {
-       case "runtime.panicindex", "runtime.panicslice", "runtime.panicdivide", "runtime.panicwrap":
+       case "runtime.panicindex", "runtime.panicslice", "runtime.panicdivide", "runtime.panicwrap", "runtime.panicshift":
                return true
        }
        return false
index bb83be4715d3a450b6f289aee9f2e5dfefb13532..59916dd5e5c15a1167ae433c949d8c33be1cdb92 100644 (file)
@@ -23,16 +23,16 @@ func panicCheckMalloc(err error) {
 
 var indexError = error(errorString("index out of range"))
 
-// The panicindex, panicslice, and panicdivide functions are called by
+// The panic{index,slice,divide,shift} functions are called by
 // code generated by the compiler for out of bounds index expressions,
-// out of bounds slice expressions, and division by zero. The
-// panicdivide (again), panicoverflow, panicfloat, and panicmem
+// out of bounds slice expressions, division by zero, and shift by negative.
+// The panicdivide (again), panicoverflow, panicfloat, and panicmem
 // functions are called by the signal handler when a signal occurs
 // indicating the respective problem.
 //
-// Since panicindex and panicslice are never called directly, and
+// Since panic{index,slice,shift} are never called directly, and
 // since the runtime package should never have an out of bounds slice
-// or array reference, if we see those functions called from the
+// or array reference or negative shift, if we see those functions called from the
 // runtime package we turn the panic into a throw. That will dump the
 // entire runtime stack for easier debugging.
 
@@ -68,6 +68,16 @@ func panicoverflow() {
        panic(overflowError)
 }
 
+var shiftError = error(errorString("negative shift amount"))
+
+func panicshift() {
+       if hasPrefix(funcname(findfunc(getcallerpc())), "runtime.") {
+               throw(string(shiftError.(errorString)))
+       }
+       panicCheckMalloc(shiftError)
+       panic(shiftError)
+}
+
 var floatError = error(errorString("floating point error"))
 
 func panicfloat() {
index 49b47ae464915450aeedda0a32ede85b96e831a4..f3605b37cfe6df8d99182679c7a1077a93e7a127 100644 (file)
@@ -1,4 +1,4 @@
-// errorcheck
+// compile
 
 // Copyright 2009 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
@@ -7,8 +7,8 @@
 package main
 
 func main() {
-       var s int = 0;
-       var x int = 0;
-       x = x << s;  // ERROR "illegal|inval|shift"
-       x = x >> s;  // ERROR "illegal|inval|shift"
+       var s int = 0
+       var x int = 0
+       x = x << s // as of 1.13, these are ok
+       x = x >> s // as of 1.13, these are ok
 }
diff --git a/test/fixedbugs/issue19113.go b/test/fixedbugs/issue19113.go
new file mode 100644 (file)
index 0000000..5e01dde
--- /dev/null
@@ -0,0 +1,108 @@
+// run
+
+// Copyright 2019 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import "reflect"
+
+var tests = []interface{}{
+       func(x int, s int) int {
+               return x << s
+       },
+       func(x int, s int64) int {
+               return x << s
+       },
+       func(x int, s int32) int {
+               return x << s
+       },
+       func(x int, s int16) int {
+               return x << s
+       },
+       func(x int, s int8) int {
+               return x << s
+       },
+       func(x int, s int) int {
+               return x >> s
+       },
+       func(x int, s int64) int {
+               return x >> s
+       },
+       func(x int, s int32) int {
+               return x >> s
+       },
+       func(x int, s int16) int {
+               return x >> s
+       },
+       func(x int, s int8) int {
+               return x >> s
+       },
+       func(x uint, s int) uint {
+               return x << s
+       },
+       func(x uint, s int64) uint {
+               return x << s
+       },
+       func(x uint, s int32) uint {
+               return x << s
+       },
+       func(x uint, s int16) uint {
+               return x << s
+       },
+       func(x uint, s int8) uint {
+               return x << s
+       },
+       func(x uint, s int) uint {
+               return x >> s
+       },
+       func(x uint, s int64) uint {
+               return x >> s
+       },
+       func(x uint, s int32) uint {
+               return x >> s
+       },
+       func(x uint, s int16) uint {
+               return x >> s
+       },
+       func(x uint, s int8) uint {
+               return x >> s
+       },
+}
+
+func main() {
+       for _, t := range tests {
+               runTest(reflect.ValueOf(t))
+       }
+}
+
+func runTest(f reflect.Value) {
+       xt := f.Type().In(0)
+       st := f.Type().In(1)
+
+       for _, x := range []int{1, 0, -1} {
+               for _, s := range []int{-99, -64, -63, -32, -31, -16, -15, -8, -7, -1, 0, 1, 7, 8, 15, 16, 31, 32, 63, 64, 99} {
+                       args := []reflect.Value{
+                               reflect.ValueOf(x).Convert(xt),
+                               reflect.ValueOf(s).Convert(st),
+                       }
+                       if s < 0 {
+                               shouldPanic(func() {
+                                       f.Call(args)
+                               })
+                       } else {
+                               f.Call(args) // should not panic
+                       }
+               }
+       }
+}
+
+func shouldPanic(f func()) {
+       defer func() {
+               if recover() == nil {
+                       panic("did not panic")
+               }
+       }()
+       f()
+}