From 59c864756201f50c737208490a686f22a2033e17 Mon Sep 17 00:00:00 2001 From: Matt Jibson Date: Fri, 29 Aug 2014 11:17:01 -0700 Subject: [PATCH] go.tools/cmd/vet: detect suspicious shifts LGTM=josharian, dsymonds, r R=golang-codereviews, josharian, minux, dsymonds, dave, axwalk, adg, r CC=golang-codereviews https://golang.org/cl/134780043 --- cmd/vet/doc.go | 6 +++ cmd/vet/shift.go | 83 +++++++++++++++++++++++++++++++++++++++ cmd/vet/testdata/shift.go | 78 ++++++++++++++++++++++++++++++++++++ 3 files changed, 167 insertions(+) create mode 100644 cmd/vet/shift.go create mode 100644 cmd/vet/testdata/shift.go diff --git a/cmd/vet/doc.go b/cmd/vet/doc.go index b610b401..e90a8b87 100644 --- a/cmd/vet/doc.go +++ b/cmd/vet/doc.go @@ -151,6 +151,12 @@ there is a uintptr-typed word in memory that holds a pointer value, because that word will be invisible to stack copying and to the garbage collector. +Shifts + +Flag: -shift + +Shifts equal to or longer than the variable's length. + Other flags These flags configure the behavior of vet: diff --git a/cmd/vet/shift.go b/cmd/vet/shift.go new file mode 100644 index 00000000..1f9f804a --- /dev/null +++ b/cmd/vet/shift.go @@ -0,0 +1,83 @@ +// Copyright 2014 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. + +/* +This file contains the code to check for suspicious shifts. +*/ + +package main + +import ( + "go/ast" + "go/token" + + "code.google.com/p/go.tools/go/exact" + "code.google.com/p/go.tools/go/types" +) + +func init() { + register("shift", + "check for useless shifts", + checkShift, + binaryExpr, assignStmt) +} + +func checkShift(f *File, node ast.Node) { + switch node := node.(type) { + case *ast.BinaryExpr: + if node.Op == token.SHL || node.Op == token.SHR { + checkLongShift(f, node, node.X, node.Y) + } + case *ast.AssignStmt: + if len(node.Lhs) != 1 || len(node.Rhs) != 1 { + return + } + if node.Tok == token.SHL_ASSIGN || node.Tok == token.SHR_ASSIGN { + checkLongShift(f, node, node.Lhs[0], node.Rhs[0]) + } + } +} + +// checkLongShift checks if shift or shift-assign operations shift by more than +// the length of the underlying variable. +func checkLongShift(f *File, node ast.Node, x, y ast.Expr) { + v := f.pkg.types[y].Value + if v == nil { + return + } + amt, ok := exact.Int64Val(v) + if !ok { + return + } + t := f.pkg.types[x].Type + if t == nil { + return + } + b, ok := t.Underlying().(*types.Basic) + if !ok { + return + } + var size int64 + var msg string + switch b.Kind() { + case types.Uint8, types.Int8: + size = 8 + case types.Uint16, types.Int16: + size = 16 + case types.Uint32, types.Int32: + size = 32 + case types.Uint64, types.Int64: + size = 64 + case types.Int, types.Uint, types.Uintptr: + // These types may be as small as 32 bits, but no smaller. + size = 32 + msg = "might be " + default: + return + } + if amt >= size { + ident := f.gofmt(x) + f.Badf(node.Pos(), "%s %stoo small for shift of %d", ident, msg, amt) + } +} diff --git a/cmd/vet/testdata/shift.go b/cmd/vet/testdata/shift.go new file mode 100644 index 00000000..6624f09c --- /dev/null +++ b/cmd/vet/testdata/shift.go @@ -0,0 +1,78 @@ +// Copyright 2014 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. + +// This file contains tests for the suspicious shift checker. + +package testdata + +func ShiftTest() { + var i8 int8 + _ = i8 << 7 + _ = (i8 + 1) << 8 // ERROR "\(i8 \+ 1\) too small for shift of 8" + _ = i8 << (7 + 1) // ERROR "i8 too small for shift of 8" + _ = i8 >> 8 // ERROR "i8 too small for shift of 8" + i8 <<= 8 // ERROR "i8 too small for shift of 8" + i8 >>= 8 // ERROR "i8 too small for shift of 8" + var i16 int16 + _ = i16 << 15 + _ = i16 << 16 // ERROR "i16 too small for shift of 16" + _ = i16 >> 16 // ERROR "i16 too small for shift of 16" + i16 <<= 16 // ERROR "i16 too small for shift of 16" + i16 >>= 16 // ERROR "i16 too small for shift of 16" + var i32 int32 + _ = i32 << 31 + _ = i32 << 32 // ERROR "i32 too small for shift of 32" + _ = i32 >> 32 // ERROR "i32 too small for shift of 32" + i32 <<= 32 // ERROR "i32 too small for shift of 32" + i32 >>= 32 // ERROR "i32 too small for shift of 32" + var i64 int64 + _ = i64 << 63 + _ = i64 << 64 // ERROR "i64 too small for shift of 64" + _ = i64 >> 64 // ERROR "i64 too small for shift of 64" + i64 <<= 64 // ERROR "i64 too small for shift of 64" + i64 >>= 64 // ERROR "i64 too small for shift of 64" + var u8 uint8 + _ = u8 << 7 + _ = u8 << 8 // ERROR "u8 too small for shift of 8" + _ = u8 >> 8 // ERROR "u8 too small for shift of 8" + u8 <<= 8 // ERROR "u8 too small for shift of 8" + u8 >>= 8 // ERROR "u8 too small for shift of 8" + var u16 uint16 + _ = u16 << 15 + _ = u16 << 16 // ERROR "u16 too small for shift of 16" + _ = u16 >> 16 // ERROR "u16 too small for shift of 16" + u16 <<= 16 // ERROR "u16 too small for shift of 16" + u16 >>= 16 // ERROR "u16 too small for shift of 16" + var u32 uint32 + _ = u32 << 31 + _ = u32 << 32 // ERROR "u32 too small for shift of 32" + _ = u32 >> 32 // ERROR "u32 too small for shift of 32" + u32 <<= 32 // ERROR "u32 too small for shift of 32" + u32 >>= 32 // ERROR "u32 too small for shift of 32" + var u64 uint64 + _ = u64 << 63 + _ = u64 << 64 // ERROR "u64 too small for shift of 64" + _ = u64 >> 64 // ERROR "u64 too small for shift of 64" + u64 <<= 64 // ERROR "u64 too small for shift of 64" + u64 >>= 64 // ERROR "u64 too small for shift of 64" + _ = u64 << u64 // Non-constant shifts should succeed. + var i int + _ = i << 31 + _ = i << 32 // ERROR "i might be too small for shift of 32" + _ = i >> 32 // ERROR "i might be too small for shift of 32" + i <<= 32 // ERROR "i might be too small for shift of 32" + i >>= 32 // ERROR "i might be too small for shift of 32" + var u uint + _ = u << 31 + _ = u << 32 // ERROR "u might be too small for shift of 32" + _ = u >> 32 // ERROR "u might be too small for shift of 32" + u <<= 32 // ERROR "u might be too small for shift of 32" + u >>= 32 // ERROR "u might be too small for shift of 32" + var p uintptr + _ = p << 31 + _ = p << 32 // ERROR "p might be too small for shift of 32" + _ = p >> 32 // ERROR "p might be too small for shift of 32" + p <<= 32 // ERROR "p might be too small for shift of 32" + p >>= 32 // ERROR "p might be too small for shift of 32" +}