staticcheck: flag dubious bit shifting of fixed size integers

staticcheck/analysis.go

@@ -259,6 +259,10 @@ var Analyzers = lintutil.InitializeAnalyzers(Docs, map[string]*analysis.Analyzer
 	},
 	// Filtering generated code because it may include empty structs generated from data models.
 	"SA9005": makeCallCheckerAnalyzer(checkNoopMarshal, facts.Generated),
+	"SA9006": {
+		Run:      CheckStaticBitShift,
+		Requires: []*analysis.Analyzer{inspect.Analyzer},
+	},
 
 	"SA4022": {
 		Run:      CheckAddressIsNil,

staticcheck/doc.go

@@ -839,4 +839,27 @@ marshaling behavior, e.g. via MarshalJSON methods. It will also not
 flag empty structs.`,
 		Since: "2019.2",
 	},
+
+	"SA9006": {
+		Title: `Dubious bit shifting of a fixed size integer value`,
+		Text: `Bit shifting a value past its size will always clear the value.
+
+For instance:
+
+    v := int8(42)
+    v >>= 8
+
+will always result in 0.
+
+This check flags bit shifiting operations on fixed size integer values only.
+That is, int, uint and uintptr are never flagged to avoid potential false
+positives in somewhat exotic but valid bit twiddling tricks:
+
+    // Clear any value above 32 bits if integers are more than 32 bits.
+    func f(i int) int {
+        v := i >> 32
+        v = v << 32
+        return i-v
+    }`,
+	},
 }

staticcheck/lint.go

@@ -3817,3 +3817,57 @@ func CheckAddressIsNil(pass *analysis.Pass) (interface{}, error) {
 	code.Preorder(pass, fn, (*ast.BinaryExpr)(nil))
 	return nil, nil
 }
+
+var (
+	checkFixedLengthTypeShiftQ = pattern.MustParse(`
+		(Or
+			(AssignStmt _ (Or ">>=" "<<=") _)
+			(BinaryExpr _ (Or ">>" "<<") _))
+	`)
+)
+
+func CheckStaticBitShift(pass *analysis.Pass) (interface{}, error) {
+	isDubiousShift := func(x, y ast.Expr) (int64, int64, bool) {
+		typ, ok := pass.TypesInfo.TypeOf(x).(*types.Basic)
+		if !ok {
+			return 0, 0, false
+		}
+		switch typ.Kind() {
+		case types.Int8, types.Int16, types.Int32, types.Int64,
+			types.Uint8, types.Uint16, types.Uint32, types.Uint64:
+			// We're only interested in fixed–size types.
+		default:
+			return 0, 0, false
+		}
+
+		const bitsInByte = 8
+		typeBits := pass.TypesSizes.Sizeof(typ) * bitsInByte
+
+		shiftLength, ok := code.ExprToInt(pass, y)
+		if !ok {
+			return 0, 0, false
+		}
+
+		return typeBits, shiftLength, shiftLength >= typeBits
+	}
+
+	fn := func(node ast.Node) {
+		if _, ok := Match(pass, checkFixedLengthTypeShiftQ, node); !ok {
+			return
+		}
+
+		switch e := node.(type) {
+		case *ast.AssignStmt:
+			if size, shift, yes := isDubiousShift(e.Lhs[0], e.Rhs[0]); yes {
+				report.Report(pass, e, fmt.Sprintf("shifting %d-bit value by %d bits will always clear it", size, shift))
+			}
+		case *ast.BinaryExpr:
+			if size, shift, yes := isDubiousShift(e.X, e.Y); yes {
+				report.Report(pass, e, fmt.Sprintf("shifting %d-bit value by %d bits will always clear it", size, shift))
+			}
+		}
+	}
+	code.Preorder(pass, fn, (*ast.AssignStmt)(nil), (*ast.BinaryExpr)(nil))
+
+	return nil, nil
+}

staticcheck/lint_test.go

@@ -83,6 +83,7 @@ func TestAll(t *testing.T) {
 		"SA9003": {{Dir: "CheckEmptyBranch"}},
 		"SA9004": {{Dir: "CheckMissingEnumTypesInDeclaration"}},
 		"SA9005": {{Dir: "CheckNoopMarshal"}},
+		"SA9006": {{Dir: "CheckStaticBitShift"}},
 	}
 
 	testutil.Run(t, Analyzers, checks)

staticcheck/testdata/src/CheckStaticBitShift/CheckStaticBitShift.go

@@ -0,0 +1,87 @@
+package pkg
+
+// Partially copied from go vet's test suite.
+
+// 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-THIRD-PARTY file.
+
+func fn() {
+	var i8 int8
+	_ = i8 << 7
+	_ = (i8 + 1) << 8 // want `will always clear it`
+	_ = i8 << (7 + 1) // want `will always clear it`
+	_ = i8 >> 8       // want `will always clear it`
+	i8 <<= 8          // want `will always clear it`
+	i8 >>= 8          // want `will always clear it`
+
+	var i16 int16
+	_ = i16 << 15
+	_ = i16 << 16 // want `will always clear it`
+	_ = i16 >> 16 // want `will always clear it`
+	i16 <<= 16    // want `will always clear it`
+	i16 >>= 16    // want `will always clear it`
+
+	var i32 int32
+	_ = i32 << 31
+	_ = i32 << 32 // want `will always clear it`
+	_ = i32 >> 32 // want `will always clear it`
+	i32 <<= 32    // want `will always clear it`
+	i32 >>= 32    // want `will always clear it`
+
+	var i64 int64
+	_ = i64 << 63
+	_ = i64 << 64 // want `will always clear it`
+	_ = i64 >> 64 // want `will always clear it`
+	i64 <<= 64    // want `will always clear it`
+	i64 >>= 64    // want `will always clear it`
+
+	var u8 uint8
+	_ = u8 << 7
+	_ = u8 << 8 // want `will always clear it`
+	_ = u8 >> 8 // want `will always clear it`
+	u8 <<= 8    // want `will always clear it`
+	u8 >>= 8    // want `will always clear it`
+
+	var u16 uint16
+	_ = u16 << 15
+	_ = u16 << 16 // want `will always clear it`
+	_ = u16 >> 16 // want `will always clear it`
+	u16 <<= 16    // want `will always clear it`
+	u16 >>= 16    // want `will always clear it`
+
+	var u32 uint32
+	_ = u32 << 31
+	_ = u32 << 32 // want `will always clear it`
+	_ = u32 >> 32 // want `will always clear it`
+	u32 <<= 32    // want `will always clear it`
+	u32 >>= 32    // want `will always clear it`
+
+	var u64 uint64
+	_ = u64 << 63
+	_ = u64 << 64 // want `will always clear it`
+	_ = u64 >> 64 // want `will always clear it`
+	u64 <<= 64    // want `will always clear it`
+	u64 >>= 64    // want `will always clear it`
+	_ = u64 << u64
+}
+
+func fn1() {
+	var ui uint
+	_ = ui << 64
+	_ = ui >> 64
+	ui <<= 64
+	ui >>= 64
+
+	var uptr uintptr
+	_ = uptr << 64
+	_ = uptr >> 64
+	uptr <<= 64
+	uptr >>= 64
+
+	var i int
+	_ = i << 64
+	_ = i >> 64
+	i <<= 64
+	i >>= 64
+}