diff --git a/staticcheck/analysis.go b/staticcheck/analysis.go index 75df1e120..354734494 100644 --- a/staticcheck/analysis.go +++ b/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, diff --git a/staticcheck/doc.go b/staticcheck/doc.go index 319ea2707..08f47ffec 100644 --- a/staticcheck/doc.go +++ b/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 + }`, + }, } diff --git a/staticcheck/lint.go b/staticcheck/lint.go index 2ccfb9e6f..73d46049d 100644 --- a/staticcheck/lint.go +++ b/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 +} diff --git a/staticcheck/lint_test.go b/staticcheck/lint_test.go index 7e7fd592d..a628b2dca 100644 --- a/staticcheck/lint_test.go +++ b/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) diff --git a/staticcheck/testdata/src/CheckStaticBitShift/CheckStaticBitShift.go b/staticcheck/testdata/src/CheckStaticBitShift/CheckStaticBitShift.go new file mode 100644 index 000000000..efa583308 --- /dev/null +++ b/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 +}