go/ssa: exploit "noreturn" info to prune spurious CFG edges

This CL causes the buildssa analyzer to exploit interprocedural
"noreturn" information (computed by the ctrlflow analyzer)
when building SSA. This allows it to prune infeasible control-flow
edges after a call that cannot return, such as t.Fatal or log.Fatal,
making the dominator tree deeper and improving the precision of
downstream analysis such as nilness and yield.

For example, nilness can now report cases such as this:

	f, err := os.Open("")
	if err != nil {
		log.Fatal(err) // can't return
	}
	if err != nil { // "impossible condition"
		log.Fatal(err)
	}
	f.Close()

and it now reports 26 (was 20) diagnostics in std.

Similarly, gopls' yield analyzer is now silent where
before it reported a false positive here:

	return func(yield func(int) bool) {
		for {
			if !yield(55) { // "yield may be called again after returning false"
				runtime.Goexit()
			}
		}
	}

Various "internal" hooks were required to expose the three
hidden accessor functions, which are logically equivalent
to these potential future API additions:

 (cfg.CFG).NoReturn() bool
 (*ctrlflow.CFGs).NoReturn(*types.Func) bool
 (*ssa.Program).SetNoReturn(func (*types.Func) bool)

See golang/go#76161 for the proposal.

Change-Id: Ic91572a19f062babd520762dfe6d5749e12d8cd0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/716920
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Markus Kusano <kusano@google.com>

Author: adonovan

cmd/ssadump/main.go

@@ -132,28 +132,30 @@ func doMain() error {
 		return fmt.Errorf("packages contain errors")
 	}
 
-	// Turn on instantiating generics during build if the program will be run.
-	if *runFlag {
-		mode |= ssa.InstantiateGenerics
-	}
-
-	// Create SSA-form program representation.
-	prog, pkgs := ssautil.AllPackages(initial, mode)
-
-	for i, p := range pkgs {
-		if p == nil {
-			return fmt.Errorf("cannot build SSA for package %s", initial[i])
-		}
-	}
-
 	if !*runFlag {
-		// Build and display only the initial packages
-		// (and synthetic wrappers).
-		for _, p := range pkgs {
+		// Create (and display) SSA only for initial packages and wrappers.
+		_, pkgs := ssautil.Packages(initial, mode)
+		for i, p := range pkgs {
+			if p == nil {
+				return fmt.Errorf("cannot build SSA for package %s", initial[i])
+			}
 			p.Build()
 		}
 
 	} else {
+		// Create SSA for initial packages and all dependencies, instantiating generics.
+		mode |= ssa.InstantiateGenerics
+
+		// TODO(adonovan): opt: use noreturn analysis over transitive
+		// dependencies to prune spurious control flow graph edges.
+
+		prog, pkgs := ssautil.AllPackages(initial, mode)
+		for i, p := range pkgs {
+			if p == nil {
+				return fmt.Errorf("cannot build SSA for package %s", initial[i])
+			}
+		}
+
 		// Run the interpreter.
 		// Build SSA for all packages.
 		prog.Build()

go/analysis/passes/buildssa/buildssa.go

@@ -11,18 +11,28 @@ package buildssa
 import (
 	"go/ast"
 	"go/types"
+	"iter"
 	"reflect"
 
 	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/ctrlflow"
+	"golang.org/x/tools/go/analysis/passes/internal/ctrlflowinternal"
 	"golang.org/x/tools/go/ssa"
+	"golang.org/x/tools/internal/ssainternal"
 )
 
 var Analyzer = &analysis.Analyzer{
 	Name:       "buildssa",
 	Doc:        "build SSA-form IR for later passes",
 	URL:        "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/buildssa",
 	Run:        run,
-	ResultType: reflect.TypeOf(new(SSA)),
+	Requires:   []*analysis.Analyzer{ctrlflow.Analyzer},
+	ResultType: reflect.TypeFor[*SSA](),
+	// Do not add FactTypes here: SSA construction of P must not
+	// require SSA construction of all of P's dependencies.
+	// (That's why we enlist the cheaper ctrlflow pass to compute
+	// noreturn instead of having go/ssa + buildssa do it.)
+	FactTypes: nil,
 }
 
 // SSA provides SSA-form intermediate representation for all the
@@ -33,6 +43,8 @@ type SSA struct {
 }
 
 func run(pass *analysis.Pass) (any, error) {
+	cfgs := pass.ResultOf[ctrlflow.Analyzer].(*ctrlflow.CFGs)
+
 	// 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
@@ -49,6 +61,11 @@ func run(pass *analysis.Pass) (any, error) {
 
 	prog := ssa.NewProgram(pass.Fset, mode)
 
+	// Use the result of the ctrlflow analysis to improve the SSA CFG.
+	ssainternal.SetNoReturn(prog, func(fn *types.Func) bool {
+		return ctrlflowinternal.NoReturn(cfgs, fn)
+	})
+
 	// Create SSA packages for direct imports.
 	for _, p := range pass.Pkg.Imports() {
 		prog.CreatePackage(p, nil, nil, true)
@@ -61,34 +78,41 @@ func run(pass *analysis.Pass) (any, error) {
 	// 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 {
-				// (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)
-				}
+	for _, fn := range allFunctions(pass) {
+		// (init functions have distinct Func
+		// objects named "init" and distinct
+		// ssa.Functions named "init#1", ...)
 
-				f := ssapkg.Prog.FuncValue(fn)
-				if f == 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)
+		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
 }
+
+// allFunctions returns an iterator over all named functions.
+func allFunctions(pass *analysis.Pass) iter.Seq2[*ast.FuncDecl, *types.Func] {
+	return func(yield func(*ast.FuncDecl, *types.Func) bool) {
+		for _, file := range pass.Files {
+			for _, decl := range file.Decls {
+				if decl, ok := decl.(*ast.FuncDecl); ok {
+					fn := pass.TypesInfo.Defs[decl.Name].(*types.Func)
+					if !yield(decl, fn) {
+						return
+					}
+				}
+			}
+		}
+	}
+}

go/analysis/passes/ctrlflow/ctrlflow.go

@@ -16,17 +16,20 @@ import (
 
 	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/analysis/passes/internal/ctrlflowinternal"
 	"golang.org/x/tools/go/ast/inspector"
 	"golang.org/x/tools/go/cfg"
 	"golang.org/x/tools/go/types/typeutil"
+	"golang.org/x/tools/internal/cfginternal"
+	"golang.org/x/tools/internal/typesinternal"
 )
 
 var Analyzer = &analysis.Analyzer{
 	Name:       "ctrlflow",
 	Doc:        "build a control-flow graph",
 	URL:        "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/ctrlflow",
 	Run:        run,
-	ResultType: reflect.TypeOf(new(CFGs)),
+	ResultType: reflect.TypeFor[*CFGs](),
 	FactTypes:  []analysis.Fact{new(noReturn)},
 	Requires:   []*analysis.Analyzer{inspect.Analyzer},
 }
@@ -44,7 +47,20 @@ 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
+	noReturn  map[*types.Func]bool // functions lacking a reachable return statement
+	pass      *analysis.Pass       // transient; nil after construction
+}
+
+// TODO(adonovan): add (*CFGs).NoReturn to public API.
+func (c *CFGs) isNoReturn(fn *types.Func) bool {
+	return c.noReturn[fn]
+}
+
+func init() {
+	// Expose the hidden method to callers in x/tools.
+	ctrlflowinternal.NoReturn = func(c any, fn *types.Func) bool {
+		return c.(*CFGs).isNoReturn(fn)
+	}
 }
 
 // CFGs has two maps: funcDecls for named functions and funcLits for
@@ -54,15 +70,14 @@ type CFGs struct {
 // *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
+	decl    *ast.FuncDecl
+	cfg     *cfg.CFG // iff decl.Body != nil
+	started bool     // to break cycles
 }
 
 type litInfo struct {
 	cfg      *cfg.CFG
-	noReturn bool
+	noReturn bool // (currently unused)
 }
 
 // FuncDecl returns the control-flow graph for a named function.
@@ -118,6 +133,7 @@ func run(pass *analysis.Pass) (any, error) {
 		defs:      pass.TypesInfo.Defs,
 		funcDecls: funcDecls,
 		funcLits:  funcLits,
+		noReturn:  make(map[*types.Func]bool),
 		pass:      pass,
 	}
 
@@ -138,7 +154,7 @@ func run(pass *analysis.Pass) (any, error) {
 		li := funcLits[lit]
 		if li.cfg == nil {
 			li.cfg = cfg.New(lit.Body, c.callMayReturn)
-			if !hasReachableReturn(li.cfg) {
+			if cfginternal.IsNoReturn(li.cfg) {
 				li.noReturn = true
 			}
 		}
@@ -158,27 +174,28 @@ func (c *CFGs) buildDecl(fn *types.Func, di *declInfo) {
 	// 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 di.started {
+		return // 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))
-		}
+	noreturn := isIntrinsicNoReturn(fn)
 
-		// debugging
-		if false {
-			log.Printf("CFG for %s:\n%s (noreturn=%t)\n", fn, di.cfg.Format(c.pass.Fset), di.noReturn)
+	if di.decl.Body != nil {
+		di.cfg = cfg.New(di.decl.Body, c.callMayReturn)
+		if cfginternal.IsNoReturn(di.cfg) {
+			noreturn = true
 		}
 	}
+	if noreturn {
+		c.pass.ExportObjectFact(fn, new(noReturn))
+		c.noReturn[fn] = true
+	}
+
+	// debugging
+	if false {
+		log.Printf("CFG for %s:\n%s (noreturn=%t)\n", fn, di.cfg.Format(c.pass.Fset), noreturn)
+	}
 }
 
 // callMayReturn reports whether the called function may return.
@@ -201,31 +218,26 @@ func (c *CFGs) callMayReturn(call *ast.CallExpr) (r bool) {
 	// Function or method declared in this package?
 	if di, ok := c.funcDecls[fn]; ok {
 		c.buildDecl(fn, di)
-		return !di.noReturn
+		return !c.noReturn[fn]
 	}
 
 	// Not declared in this package.
 	// Is there a fact from another package?
-	return !c.pass.ImportObjectFact(fn, new(noReturn))
+	if c.pass.ImportObjectFact(fn, new(noReturn)) {
+		c.noReturn[fn] = true
+		return false
+	}
+
+	return true
 }
 
 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"
+	return typesinternal.IsFunctionNamed(fn, "syscall", "Exit", "ExitProcess", "ExitThread") ||
+		typesinternal.IsFunctionNamed(fn, "runtime", "Goexit")
 }