go/analysis/passes/ctrlflow: an Analyzer that builds CFGs

The ctrlflow Analyzer builds a control-flow graph (see
golang.org/x/tools/go/cfg) for each named and unnamed function in the
package.

It computes for each function whether it can never return, either
because the function is an intrinsic that stops the thread (e.g.
os.Exit), or because control never reaches a return statement, or
because the function inevitably calls another function that never
returns.  For each such function it exports a noReturn fact.

This change also:
- adds 'inspect', another Analyzer that builds an optimized AST
  traversal table for use by nearly every other Analyzer.
- changes analysistest.Run to return the analysis result to enable
  further testing.
  (This required changing it to analyze one package at a time,
  which is no less efficient, and is the typical case.)

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

go/analysis/analysistest/analysistest.go

@@ -60,8 +60,8 @@ type Testing interface {
 	Errorf(format string, args ...interface{})
 }
 
-// Run applies an analysis to each named package.
-// It loads each package from the specified GOPATH-style project
+// Run applies an analysis to the named package.
+// It loads the package from the specified GOPATH-style project
 // directory using golang.org/x/tools/go/packages, runs the analysis on
 // it, and checks that each the analysis emits the expected diagnostics
 // and facts specified by the contents of '// want ...' comments in the
@@ -81,7 +81,8 @@ type Testing interface {
 //
 //	func panicf(format string, args interface{}) { // want panicf:"printfWrapper"
 //
-// Package facts are specified by the name "package".
+// Package facts are specified by the name "package" and appear on
+// line 1 of the first source file of the package.
 //
 // A single 'want' comment may contain a mixture of diagnostic and fact
 // expectations, including multiple facts about the same object:
@@ -93,25 +94,25 @@ type Testing interface {
 //
 // You may wish to call this function from within a (*testing.T).Run
 // subtest to ensure that errors have adequate contextual description.
-func Run(t Testing, dir string, a *analysis.Analyzer, pkgnames ...string) {
-	if pkgnames == nil {
-		t.Errorf("Run: no packages")
+//
+// Run returns the pass and the result of the Analyzer's Run function,
+// or (nil, nil) if loading or analysis failed.
+func Run(t Testing, dir string, a *analysis.Analyzer, pkgname string) (*analysis.Pass, interface{}) {
+	pkg, err := loadPackage(dir, pkgname)
+	if err != nil {
+		t.Errorf("loading %s: %v", pkgname, err)
+		return nil, nil
 	}
-	for _, pkgname := range pkgnames {
-		pkg, err := loadPackage(dir, pkgname)
-		if err != nil {
-			t.Errorf("loading %s: %v", pkgname, err)
-			continue
-		}
 
-		pass, diagnostics, facts, err := checker.Analyze(pkg, a)
-		if err != nil {
-			t.Errorf("analyzing %s: %v", pkgname, err)
-			continue
-		}
-
-		check(t, dir, pass, diagnostics, facts)
+	pass, diagnostics, facts, result, err := checker.Analyze(pkg, a)
+	if err != nil {
+		t.Errorf("analyzing %s: %v", pkgname, err)
+		return nil, nil
 	}
+
+	check(t, dir, pass, diagnostics, facts)
+
+	return pass, result
 }
 
 // loadPackage loads the specified package (from source, with

go/analysis/internal/checker/checker.go

@@ -157,7 +157,7 @@ func load(patterns []string, allSyntax bool) ([]*packages.Package, error) {
 // have a nil key.
 //
 // It is exposed for use in testing.
-func Analyze(pkg *packages.Package, a *analysis.Analyzer) (*analysis.Pass, []analysis.Diagnostic, map[types.Object][]analysis.Fact, error) {
+func Analyze(pkg *packages.Package, a *analysis.Analyzer) (*analysis.Pass, []analysis.Diagnostic, map[types.Object][]analysis.Fact, interface{}, error) {
 	act := analyze([]*packages.Package{pkg}, []*analysis.Analyzer{a})[0]
 
 	facts := make(map[types.Object][]analysis.Fact)
@@ -172,7 +172,7 @@ func Analyze(pkg *packages.Package, a *analysis.Analyzer) (*analysis.Pass, []ana
 		}
 	}
 
-	return act.pass, act.diagnostics, facts, act.err
+	return act.pass, act.diagnostics, facts, act.result, act.err
 }
 
 func analyze(pkgs []*packages.Package, analyzers []*analysis.Analyzer) []*action {

go/analysis/passes/ctrlflow/ctrlflow.go

@@ -0,0 +1,225 @@
+// 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 ctrlflow is an analysis that provides a syntactic
+// control-flow graph (CFG) for the body of a function.
+// It records whether a function cannot return.
+// By itself, it does not report any diagnostics.
+package ctrlflow
+
+import (
+	"go/ast"
+	"go/types"
+	"log"
+	"reflect"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+	"golang.org/x/tools/go/cfg"
+	"golang.org/x/tools/go/types/typeutil"
+)
+
+var Analyzer = &analysis.Analyzer{
+	Name:       "ctrlflow",
+	Doc:        "build a control-flow graph",
+	Run:        run,
+	ResultType: reflect.TypeOf(new(CFGs)),
+	FactTypes:  []analysis.Fact{new(noReturn)},
+	Requires:   []*analysis.Analyzer{inspect.Analyzer},
+}
+
+// noReturn is a fact indicating that a function does not return.
+type noReturn struct{}
+
+func (*noReturn) AFact() {}
+
+func (*noReturn) String() string { return "noReturn" }
+
+// A CFGs holds the control-flow graphs
+// for all the functions of the current package.
+type CFGs struct {
+	defs      map[*ast.Ident]types.Object // from Pass.TypesInfo.Defs
+	funcDecls map[*types.Func]*declInfo
+	funcLits  map[*ast.FuncLit]*litInfo
+	pass      *analysis.Pass // transient; nil after construction
+}
+
+// CFGs has two maps: funcDecls for named functions and funcLits for
+// unnamed ones. Unlike funcLits, the funcDecls map is not keyed by its
+// syntax node, *ast.FuncDecl, because callMayReturn needs to do a
+// look-up by *types.Func, and you can get from an *ast.FuncDecl to a
+// *types.Func but not the other way.
+
+type declInfo struct {
+	decl     *ast.FuncDecl
+	cfg      *cfg.CFG // iff decl.Body != nil
+	started  bool     // to break cycles
+	noReturn bool
+}
+
+type litInfo struct {
+	cfg      *cfg.CFG
+	noReturn bool
+}
+
+// FuncDecl returns the control-flow graph for a named function.
+// It returns nil if decl.Body==nil.
+func (c *CFGs) FuncDecl(decl *ast.FuncDecl) *cfg.CFG {
+	if decl.Body == nil {
+		return nil
+	}
+	fn := c.defs[decl.Name].(*types.Func)
+	return c.funcDecls[fn].cfg
+}
+
+// FuncLit returns the control-flow graph for a literal function.
+func (c *CFGs) FuncLit(lit *ast.FuncLit) *cfg.CFG {
+	return c.funcLits[lit].cfg
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+
+	// Because CFG construction consumes and produces noReturn
+	// facts, CFGs for exported FuncDecls must be built before 'run'
+	// returns; we cannot construct them lazily.
+	// (We could build CFGs for FuncLits lazily,
+	// but the benefit is marginal.)
+
+	// Pass 1. Map types.Funcs to ast.FuncDecls in this package.
+	funcDecls := make(map[*types.Func]*declInfo) // functions and methods
+	funcLits := make(map[*ast.FuncLit]*litInfo)
+
+	var decls []*types.Func // keys(funcDecls), in order
+	var lits []*ast.FuncLit // keys(funcLits), in order
+
+	nodeFilter := []ast.Node{
+		(*ast.FuncDecl)(nil),
+		(*ast.FuncLit)(nil),
+	}
+	inspect.Preorder(nodeFilter, func(n ast.Node) {
+		switch n := n.(type) {
+		case *ast.FuncDecl:
+			fn := pass.TypesInfo.Defs[n.Name].(*types.Func)
+			funcDecls[fn] = &declInfo{decl: n}
+			decls = append(decls, fn)
+
+		case *ast.FuncLit:
+			funcLits[n] = new(litInfo)
+			lits = append(lits, n)
+		}
+	})
+
+	c := &CFGs{
+		defs:      pass.TypesInfo.Defs,
+		funcDecls: funcDecls,
+		funcLits:  funcLits,
+		pass:      pass,
+	}
+
+	// Pass 2. Build CFGs.
+
+	// Build CFGs for named functions.
+	// Cycles in the static call graph are broken
+	// arbitrarily but deterministically.
+	// We create noReturn facts as discovered.
+	for _, fn := range decls {
+		c.buildDecl(fn, funcDecls[fn])
+	}
+
+	// Build CFGs for literal functions.
+	// These aren't relevant to facts (since they aren't named)
+	// but are required for the CFGs.FuncLit API.
+	for _, lit := range lits {
+		li := funcLits[lit]
+		if li.cfg == nil {
+			li.cfg = cfg.New(lit.Body, c.callMayReturn)
+			if !hasReachableReturn(li.cfg) {
+				li.noReturn = true
+			}
+		}
+	}
+
+	// All CFGs are now built.
+	c.pass = nil
+
+	return c, nil
+}
+
+// di.cfg may be nil on return.
+func (c *CFGs) buildDecl(fn *types.Func, di *declInfo) {
+	// buildDecl may call itself recursively for the same function,
+	// because cfg.New is passed the callMayReturn method, which
+	// builds the CFG of the callee, leading to recursion.
+	// The buildDecl call tree thus resembles the static call graph.
+	// We mark each node when we start working on it to break cycles.
+
+	if !di.started { // break cycle
+		di.started = true
+
+		if isIntrinsicNoReturn(fn) {
+			di.noReturn = true
+		}
+		if di.decl.Body != nil {
+			di.cfg = cfg.New(di.decl.Body, c.callMayReturn)
+			if !hasReachableReturn(di.cfg) {
+				di.noReturn = true
+			}
+		}
+		if di.noReturn {
+			c.pass.ExportObjectFact(fn, new(noReturn))
+		}
+
+		// debugging
+		if false {
+			log.Printf("CFG for %s:\n%s (noreturn=%t)\n", fn, di.cfg.Format(c.pass.Fset), di.noReturn)
+		}
+	}
+}
+
+// callMayReturn reports whether the called function may return.
+// It is passed to the CFG builder.
+func (c *CFGs) callMayReturn(call *ast.CallExpr) (r bool) {
+	if id, ok := call.Fun.(*ast.Ident); ok && c.pass.TypesInfo.Uses[id] == panicBuiltin {
+		return false // panic never returns
+	}
+
+	// Is this a static call?
+	fn := typeutil.StaticCallee(c.pass.TypesInfo, call)
+	if fn == nil {
+		return true // callee not statically known; be conservative
+	}
+
+	// Function or method declared in this package?
+	if di, ok := c.funcDecls[fn]; ok {
+		c.buildDecl(fn, di)
+		return !di.noReturn
+	}
+
+	// Not declared in this package.
+	// Is there a fact from another package?
+	return !c.pass.ImportObjectFact(fn, new(noReturn))
+}
+
+var panicBuiltin = types.Universe.Lookup("panic").(*types.Builtin)
+
+func hasReachableReturn(g *cfg.CFG) bool {
+	for _, b := range g.Blocks {
+		if b.Live && b.Return() != nil {
+			return true
+		}
+	}
+	return false
+}
+
+// isIntrinsicNoReturn reports whether a function intrinsically never
+// returns because it stops execution of the calling thread.
+// It is the base case in the recursion.
+func isIntrinsicNoReturn(fn *types.Func) bool {
+	// Add functions here as the need arises, but don't allocate memory.
+	path, name := fn.Pkg().Path(), fn.Name()
+	return path == "syscall" && (name == "Exit" || name == "ExitProcess" || name == "ExitThread") ||
+		path == "runtime" && name == "Goexit"
+}

go/analysis/passes/ctrlflow/ctrlflow_test.go

@@ -0,0 +1,31 @@
+package ctrlflow_test
+
+import (
+	"go/ast"
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+	"golang.org/x/tools/go/analysis/passes/ctrlflow"
+)
+
+func Test(t *testing.T) {
+	testdata := analysistest.TestData()
+
+	// load testdata/src/a/a.go
+	pass, result := analysistest.Run(t, testdata, ctrlflow.Analyzer, "a")
+
+	// Perform a minimal smoke test on
+	// the result (CFG) computed by ctrlflow.
+	if result != nil {
+		cfgs := result.(*ctrlflow.CFGs)
+
+		for _, decl := range pass.Files[0].Decls {
+			if decl, ok := decl.(*ast.FuncDecl); ok && decl.Body != nil {
+				if cfgs.FuncDecl(decl) == nil {
+					t.Errorf("%s: no CFG for func %s",
+						pass.Fset.Position(decl.Pos()), decl.Name.Name)
+				}
+			}
+		}
+	}
+}

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

@@ -0,0 +1,99 @@
+package a
+
+// This file tests facts produced by ctrlflow.
+
+import (
+	"log"
+	"os"
+	"runtime"
+	"syscall"
+	"testing"
+)
+
+var cond bool
+
+func a() { // want a:"noReturn"
+	if cond {
+		b()
+	} else {
+		for {
+		}
+	}
+}
+
+func b() { // want b:"noReturn"
+	select {}
+}
+
+func f(x int) { // no fact here
+	switch x {
+	case 0:
+		os.Exit(0)
+	case 1:
+		panic(0)
+	}
+	// default case returns
+}
+
+type T int
+
+func (T) method1() { // want method1:"noReturn"
+	a()
+}
+
+func (T) method2() { // (may return)
+	if cond {
+		a()
+	}
+}
+
+// Checking for the noreturn fact associated with F ensures that
+// ctrlflow proved each of the listed functions was "noReturn".
+//
+func standardFunctions(x int) { // want standardFunctions:"noReturn"
+	t := new(testing.T)
+	switch x {
+	case 0:
+		t.FailNow()
+	case 1:
+		t.Fatal()
+	case 2:
+		t.Fatalf("")
+	case 3:
+		t.Skip()
+	case 4:
+		t.SkipNow()
+	case 5:
+		t.Skipf("")
+	case 6:
+		log.Fatal()
+	case 7:
+		log.Fatalf("")
+	case 8:
+		log.Fatalln()
+	case 9:
+		os.Exit(0)
+	case 10:
+		syscall.Exit(0)
+	case 11:
+		runtime.Goexit()
+	case 12:
+		log.Panic()
+	case 13:
+		log.Panicln()
+	case 14:
+		log.Panicf("")
+	default:
+		panic(0)
+	}
+}
+
+// False positives are possible.
+// This function is marked noReturn but in fact returns.
+//
+func spurious() { // want spurious:"noReturn"
+	defer func() { recover() }()
+	panic(nil)
+}
+
+func noBody()

go/analysis/passes/findcall/findcall_test.go

@@ -51,7 +51,5 @@ func main() {
 // multiple variants of a single scenario.
 func TestFromFileSystem(t *testing.T) {
 	testdata := analysistest.TestData()
-	analysistest.Run(t, testdata, findcall.Analyzer,
-		"a", // loads testdata/src/a/a.go.
-	)
+	analysistest.Run(t, testdata, findcall.Analyzer, "a") // loads testdata/src/a/a.go.
 }

go/analysis/passes/inspect/inspect.go

@@ -0,0 +1,45 @@
+// 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 inspect is an analysis that provides an AST inspector
+// (golang.org/x/tools/go/ast/inspect.Inspect) for the syntax trees of a
+// package. It is only a building block for other analyzers.
+//
+// Example of use in another analysis:
+//
+//	import "golang.org/x/tools/go/analysis/passes/inspect"
+//
+//	var Analyzer = &analysis.Analyzer{
+//		...
+//		Requires:       reflect.TypeOf(new(inspect.Analyzer)),
+//	}
+//
+// 	func run(pass *analysis.Pass) (interface{}, error) {
+// 		inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+// 		inspect.Preorder(nil, func(n ast.Node) {
+// 			...
+// 		})
+// 		return nil
+// 	}
+//
+package inspect
+
+import (
+	"reflect"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/ast/inspector"
+)
+
+var Analyzer = &analysis.Analyzer{
+	Name:             "inspect",
+	Doc:              "optimize AST traversal for later passes",
+	Run:              run,
+	RunDespiteErrors: true,
+	ResultType:       reflect.TypeOf(new(inspector.Inspector)),
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	return inspector.New(pass.Files), nil
+}

go/analysis/passes/pkgfact/pkgfact_test.go

@@ -9,7 +9,5 @@ import (
 
 func Test(t *testing.T) {
 	testdata := analysistest.TestData()
-	analysistest.Run(t, testdata, pkgfact.Analyzer,
-		"c", // loads testdata/src/c/c.go.
-	)
+	analysistest.Run(t, testdata, pkgfact.Analyzer, "c") // load testdata/src/c/c.go
 }