go/analysis/passes/nilness: degenerate nil condition checker

This check uses the control-flow graph and SSA value graph to detect
problems such as:

	p := &v
	...
	if p != nil { // tautological condition
	}

and:

	if p == nil {
		print(*p) // nil dereference
	}

(It was originally developed within Google's Go analysis framework and
can now be published in a form useful to all analysis drivers.)

This CL also includes buildssa, an Analyzer that constructs SSA for
later analysis passes but does not report diagnostics or facts of its
own.

Change-Id: I27bc4eea10d71d958685a403234879112c21f433
Reviewed-on: https://go-review.googlesource.com/c/142698
Reviewed-by: Michael Matloob <matloob@golang.org>

go/analysis/passes/buildssa/buildssa.go

@@ -0,0 +1,117 @@
+// Copyright 2018 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 buildssa defines an Analyzer that constructs the SSA
+// representation of an error-free package and returns the set of all
+// functions within it. It does not report any diagnostics itself but
+// may be used as an input to other analyzers.
+//
+// THIS INTERFACE IS EXPERIMENTAL AND MAY BE SUBJECT TO INCOMPATIBLE CHANGE.
+package buildssa
+
+import (
+	"go/ast"
+	"go/types"
+	"reflect"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/ssa"
+)
+
+var Analyzer = &analysis.Analyzer{
+	Name:       "buildssa",
+	Doc:        "build SSA-form IR for later passes",
+	Run:        run,
+	ResultType: reflect.TypeOf(new(SSA)),
+}
+
+// SSA provides SSA-form intermediate representation for all the
+// non-blank source functions in the current package.
+type SSA struct {
+	Pkg      *ssa.Package
+	SrcFuncs []*ssa.Function
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	// Plundered from ssautil.BuildPackage.
+
+	// We must create a new Program for each Package because the
+	// analysis API provides no place to hang a Program shared by
+	// all Packages. Consequently, SSA Packages and Functions do not
+	// have a canonical representation across an analysis session of
+	// multiple packages. This is unlikely to be a problem in
+	// practice because the analysis API essentially forces all
+	// packages to be analysed independently, so any given call to
+	// Analysis.Run on a package will see only SSA objects belonging
+	// to a single Program.
+
+	// Some Analyzers may need GlobalDebug, in which case we'll have
+	// to set it globally, but let's wait till we need it.
+	mode := ssa.BuilderMode(0)
+
+	prog := ssa.NewProgram(pass.Fset, mode)
+
+	// Create SSA packages for all imports.
+	// Order is not significant.
+	created := make(map[*types.Package]bool)
+	var createAll func(pkgs []*types.Package)
+	createAll = func(pkgs []*types.Package) {
+		for _, p := range pkgs {
+			if !created[p] {
+				created[p] = true
+				prog.CreatePackage(p, nil, nil, true)
+				createAll(p.Imports())
+			}
+		}
+	}
+	createAll(pass.Pkg.Imports())
+
+	// Create and build the primary package.
+	ssapkg := prog.CreatePackage(pass.Pkg, pass.Files, pass.TypesInfo, false)
+	ssapkg.Build()
+
+	// Compute list of source functions, including literals,
+	// in source order.
+	var funcs []*ssa.Function
+	for _, f := range pass.Files {
+		for _, decl := range f.Decls {
+			if fdecl, ok := decl.(*ast.FuncDecl); ok {
+
+				// SSA will not build a Function
+				// for a FuncDecl named blank.
+				// That's arguably too strict but
+				// relaxing it would break uniqueness of
+				// names of package members.
+				if fdecl.Name.Name == "_" {
+					continue
+				}
+
+				// (init functions have distinct Func
+				// objects named "init" and distinct
+				// ssa.Functions named "init#1", ...)
+
+				fn := pass.TypesInfo.Defs[fdecl.Name].(*types.Func)
+				if fn == nil {
+					panic(fn)
+				}
+
+				f := ssapkg.Prog.FuncValue(fn)
+				if f == nil {
+					panic(fn)
+				}
+
+				var addAnons func(f *ssa.Function)
+				addAnons = func(f *ssa.Function) {
+					funcs = append(funcs, f)
+					for _, anon := range f.AnonFuncs {
+						addAnons(anon)
+					}
+				}
+				addAnons(f)
+			}
+		}
+	}
+
+	return &SSA{Pkg: ssapkg, SrcFuncs: funcs}, nil
+}

go/analysis/passes/buildssa/buildssa_test.go

@@ -0,0 +1,29 @@
+// Copyright 2018 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 buildssa_test
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+	"golang.org/x/tools/go/analysis/passes/buildssa"
+)
+
+func Test(t *testing.T) {
+	testdata := analysistest.TestData()
+	result := analysistest.Run(t, testdata, buildssa.Analyzer, "a")[0].Result
+
+	ssainfo := result.(*buildssa.SSA)
+	got := fmt.Sprint(ssainfo.SrcFuncs)
+	want := `[a.Fib (a.T).fib]`
+	if got != want {
+		t.Errorf("SSA.SrcFuncs = %s, want %s", got, want)
+		for _, f := range ssainfo.SrcFuncs {
+			f.WriteTo(os.Stderr)
+		}
+	}
+}

go/analysis/passes/buildssa/testdata/src/a/a.go

@@ -0,0 +1,16 @@
+package a
+
+func Fib(x int) int {
+	if x < 2 {
+		return x
+	}
+	return Fib(x-1) + Fib(x-2)
+}
+
+type T int
+
+func (T) fib(x int) int { return Fib(x) }
+
+func _() {
+	print("hi")
+}

go/analysis/passes/nilness/cmd/nilness/main.go

@@ -0,0 +1,10 @@
+// The nilness command applies the golang.org/x/tools/go/analysis/passes/nilness
+// analysis to the specified packages of Go source code.
+package main
+
+import (
+	"golang.org/x/tools/go/analysis/passes/nilness"
+	"golang.org/x/tools/go/analysis/singlechecker"
+)
+
+func main() { singlechecker.Main(nilness.Analyzer) }

go/analysis/passes/nilness/nilness.go

@@ -0,0 +1,271 @@
+// Copyright 2018 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 nilness inspects the control-flow graph of an SSA function
+// and reports errors such as nil pointer dereferences and degenerate
+// nil pointer comparisons.
+package nilness
+
+import (
+	"fmt"
+	"go/token"
+	"go/types"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/buildssa"
+	"golang.org/x/tools/go/ssa"
+)
+
+const Doc = `check for redundant or impossible nil comparisons
+
+The nilness checker inspects the control-flow graph of each function in
+a package and reports nil pointer dereferences and degenerate nil
+pointers. A degenerate comparison is of the form x==nil or x!=nil where x
+is statically known to be nil or non-nil. These are often a mistake,
+especially in control flow related to errors.
+
+This check reports conditions such as:
+
+	if f == nil { // impossible condition (f is a function)
+	}
+
+and:
+
+	p := &v
+	...
+	if p != nil { // tautological condition
+	}
+
+and:
+
+	if p == nil {
+		print(*p) // nil dereference
+	}
+`
+
+var Analyzer = &analysis.Analyzer{
+	Name:     "nilness",
+	Doc:      Doc,
+	Run:      run,
+	Requires: []*analysis.Analyzer{buildssa.Analyzer},
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	ssainput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
+	for _, fn := range ssainput.SrcFuncs {
+		runFunc(pass, fn)
+	}
+	return nil, nil
+}
+
+func runFunc(pass *analysis.Pass, fn *ssa.Function) {
+	reportf := func(category string, pos token.Pos, format string, args ...interface{}) {
+		pass.Report(analysis.Diagnostic{
+			Pos:      pos,
+			Category: category,
+			Message:  fmt.Sprintf(format, args...),
+		})
+	}
+
+	// notNil reports an error if v is provably nil.
+	notNil := func(stack []fact, instr ssa.Instruction, v ssa.Value, descr string) {
+		if nilnessOf(stack, v) == isnil {
+			reportf("nilderef", instr.Pos(), "nil dereference in "+descr)
+		}
+	}
+
+	// visit visits reachable blocks of the CFG in dominance order,
+	// maintaining a stack of dominating nilness facts.
+	//
+	// By traversing the dom tree, we can pop facts off the stack as
+	// soon as we've visited a subtree.  Had we traversed the CFG,
+	// we would need to retain the set of facts for each block.
+	seen := make([]bool, len(fn.Blocks)) // seen[i] means visit should ignore block i
+	var visit func(b *ssa.BasicBlock, stack []fact)
+	visit = func(b *ssa.BasicBlock, stack []fact) {
+		if seen[b.Index] {
+			return
+		}
+		seen[b.Index] = true
+
+		// Report nil dereferences.
+		for _, instr := range b.Instrs {
+			switch instr := instr.(type) {
+			case ssa.CallInstruction:
+				notNil(stack, instr, instr.Common().Value,
+					instr.Common().Description())
+			case *ssa.FieldAddr:
+				notNil(stack, instr, instr.X, "field selection")
+			case *ssa.IndexAddr:
+				notNil(stack, instr, instr.X, "index operation")
+			case *ssa.MapUpdate:
+				notNil(stack, instr, instr.Map, "map update")
+			case *ssa.Slice:
+				// A nilcheck occurs in ptr[:] iff ptr is a pointer to an array.
+				if _, ok := instr.X.Type().Underlying().(*types.Pointer); ok {
+					notNil(stack, instr, instr.X, "slice operation")
+				}
+			case *ssa.Store:
+				notNil(stack, instr, instr.Addr, "store")
+			case *ssa.TypeAssert:
+				notNil(stack, instr, instr.X, "type assertion")
+			case *ssa.UnOp:
+				if instr.Op == token.MUL { // *X
+					notNil(stack, instr, instr.X, "load")
+				}
+			}
+		}
+
+		// For nil comparison blocks, report an error if the condition
+		// is degenerate, and push a nilness fact on the stack when
+		// visiting its true and false successor blocks.
+		if binop, tsucc, fsucc := eq(b); binop != nil {
+			xnil := nilnessOf(stack, binop.X)
+			ynil := nilnessOf(stack, binop.Y)
+
+			if ynil != unknown && xnil != unknown && (xnil == isnil || ynil == isnil) {
+				// Degenerate condition:
+				// the nilness of both operands is known,
+				// and at least one of them is nil.
+				var adj string
+				if (xnil == ynil) == (binop.Op == token.EQL) {
+					adj = "tautological"
+				} else {
+					adj = "impossible"
+				}
+				reportf("cond", binop.Pos(), "%s condition: %s %s %s", adj, xnil, binop.Op, ynil)
+
+				// If tsucc's or fsucc's sole incoming edge is impossible,
+				// it is unreachable.  Prune traversal of it and
+				// all the blocks it dominates.
+				// (We could be more precise with full dataflow
+				// analysis of control-flow joins.)
+				var skip *ssa.BasicBlock
+				if xnil == ynil {
+					skip = fsucc
+				} else {
+					skip = tsucc
+				}
+				for _, d := range b.Dominees() {
+					if d == skip && len(d.Preds) == 1 {
+						continue
+					}
+					visit(d, stack)
+				}
+				return
+			}
+
+			// "if x == nil" or "if nil == y" condition; x, y are unknown.
+			if xnil == isnil || ynil == isnil {
+				var f fact
+				if xnil == isnil {
+					// x is nil, y is unknown:
+					// t successor learns y is nil.
+					f = fact{binop.Y, isnil}
+				} else {
+					// x is nil, y is unknown:
+					// t successor learns x is nil.
+					f = fact{binop.X, isnil}
+				}
+
+				for _, d := range b.Dominees() {
+					// Successor blocks learn a fact
+					// only at non-critical edges.
+					// (We could do be more precise with full dataflow
+					// analysis of control-flow joins.)
+					s := stack
+					if len(d.Preds) == 1 {
+						if d == tsucc {
+							s = append(s, f)
+						} else if d == fsucc {
+							s = append(s, f.negate())
+						}
+					}
+					visit(d, s)
+				}
+				return
+			}
+		}
+
+		for _, d := range b.Dominees() {
+			visit(d, stack)
+		}
+	}
+
+	// Visit the entry block.  No need to visit fn.Recover.
+	if fn.Blocks != nil {
+		visit(fn.Blocks[0], make([]fact, 0, 20)) // 20 is plenty
+	}
+}
+
+// A fact records that a block is dominated
+// by the condition v == nil or v != nil.
+type fact struct {
+	value   ssa.Value
+	nilness nilness
+}
+
+func (f fact) negate() fact { return fact{f.value, -f.nilness} }
+
+type nilness int
+
+const (
+	isnonnil         = -1
+	unknown  nilness = 0
+	isnil            = 1
+)
+
+var nilnessStrings = []string{"non-nil", "unknown", "nil"}
+
+func (n nilness) String() string { return nilnessStrings[n+1] }
+
+// nilnessOf reports whether v is definitely nil, definitely not nil,
+// or unknown given the dominating stack of facts.
+func nilnessOf(stack []fact, v ssa.Value) nilness {
+	// Is value intrinsically nil or non-nil?
+	switch v := v.(type) {
+	case *ssa.Alloc,
+		*ssa.FieldAddr,
+		*ssa.FreeVar,
+		*ssa.Function,
+		*ssa.Global,
+		*ssa.IndexAddr,
+		*ssa.MakeChan,
+		*ssa.MakeClosure,
+		*ssa.MakeInterface,
+		*ssa.MakeMap,
+		*ssa.MakeSlice:
+		return isnonnil
+	case *ssa.Const:
+		if v.IsNil() {
+			return isnil
+		} else {
+			return isnonnil
+		}
+	}
+
+	// Search dominating control-flow facts.
+	for _, f := range stack {
+		if f.value == v {
+			return f.nilness
+		}
+	}
+	return unknown
+}
+
+// If b ends with an equality comparison, eq returns the operation and
+// its true (equal) and false (not equal) successors.
+func eq(b *ssa.BasicBlock) (op *ssa.BinOp, tsucc, fsucc *ssa.BasicBlock) {
+	if If, ok := b.Instrs[len(b.Instrs)-1].(*ssa.If); ok {
+		if binop, ok := If.Cond.(*ssa.BinOp); ok {
+			switch binop.Op {
+			case token.EQL:
+				return binop, b.Succs[0], b.Succs[1]
+			case token.NEQ:
+				return binop, b.Succs[1], b.Succs[0]
+			}
+		}
+	}
+	return nil, nil, nil
+}

go/analysis/passes/nilness/nilness_test.go

@@ -0,0 +1,17 @@
+// Copyright 2018 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 nilness_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+	"golang.org/x/tools/go/analysis/passes/nilness"
+)
+
+func Test(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, nilness.Analyzer, "a")
+}

go/analysis/passes/nilness/testdata/src/a/a.go

@@ -0,0 +1,91 @@
+package a
+
+type X struct{ f, g int }
+
+func f(x, y *X) {
+	if x == nil {
+		print(x.f) // want "nil dereference in field selection"
+	} else {
+		print(x.f)
+	}
+
+	if x == nil {
+		if nil != y {
+			print(1)
+			panic(0)
+		}
+		x.f = 1 // want "nil dereference in field selection"
+		y.f = 1 // want "nil dereference in field selection"
+	}
+
+	var f func()
+	if f == nil { // want "tautological condition: nil == nil"
+		go f() // want "nil dereference in dynamic function call"
+	} else {
+		// This block is unreachable,
+		// so we don't report an error for the
+		// nil dereference in the call.
+		defer f()
+	}
+}
+
+func f2(ptr *[3]int, i interface{}) {
+	if ptr != nil {
+		print(ptr[:])
+		*ptr = [3]int{}
+		print(*ptr)
+	} else {
+		print(ptr[:])   // want "nil dereference in slice operation"
+		*ptr = [3]int{} // want "nil dereference in store"
+		print(*ptr)     // want "nil dereference in load"
+
+		if ptr != nil { // want "impossible condition: nil != nil"
+			// Dominated by ptr==nil and ptr!=nil,
+			// this block is unreachable.
+			// We do not report errors within it.
+			print(*ptr)
+		}
+	}
+
+	if i != nil {
+		print(i.(interface{ f() }))
+	} else {
+		print(i.(interface{ f() })) // want "nil dereference in type assertion"
+	}
+}
+
+func g() error
+
+func f3() error {
+	err := g()
+	if err != nil {
+		return err
+	}
+	if err != nil && err.Error() == "foo" { // want "impossible condition: nil != nil"
+		print(0)
+	}
+	ch := make(chan int)
+	if ch == nil { // want "impossible condition: non-nil == nil"
+		print(0)
+	}
+	if ch != nil { // want "tautological condition: non-nil != nil"
+		print(0)
+	}
+	return nil
+}
+
+func h(err error, b bool) {
+	if err != nil && b {
+		return
+	} else if err != nil {
+		panic(err)
+	}
+}
+
+func i(*int) error {
+	for {
+		if err := g(); err != nil {
+			return err
+		}
+	}
+}