Merge branch 'vet-loopclosure' of https://github.com/thepudds/xtools into vet-loopclosure

Author: thepudds

go/analysis/passes/buildssa/buildssa.go

@@ -48,8 +48,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 
 	// Some Analyzers may need GlobalDebug, in which case we'll have
 	// to set it globally, but let's wait till we need it.
-	// Monomorphize at least until type parameters are available.
-	mode := ssa.InstantiateGenerics
+	mode := ssa.BuilderMode(0)
 
 	prog := ssa.NewProgram(pass.Fset, mode)
 

go/analysis/passes/nilness/nilness.go

@@ -62,7 +62,6 @@ var Analyzer = &analysis.Analyzer{
 
 func run(pass *analysis.Pass) (interface{}, error) {
 	ssainput := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
-	// TODO(48525): ssainput.SrcFuncs is missing fn._Instances(). runFunc will be skipped.
 	for _, fn := range ssainput.SrcFuncs {
 		runFunc(pass, fn)
 	}
@@ -307,9 +306,9 @@ func nilnessOf(stack []fact, v ssa.Value) nilness {
 		return isnonnil
 	case *ssa.Const:
 		if v.IsNil() {
-			return isnil
+			return isnil // nil or zero value of a pointer-like type
 		} else {
-			return isnonnil
+			return unknown // non-pointer
 		}
 	}
 

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

@@ -209,3 +209,10 @@ func f13() {
 	var d *Y
 	print(d.value) // want "nil dereference in field selection"
 }
+
+func f14() {
+	var x struct{ f string }
+	if x == struct{ f string }{} { // we don't catch this tautology as we restrict to reference types
+		print(x)
+	}
+}

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

@@ -2,7 +2,7 @@ package c
 
 func instantiated[X any](x *X) int {
 	if x == nil {
-		print(*x) // not reported until _Instances are added to SrcFuncs
+		print(*x) // want "nil dereference in load"
 	}
 	return 1
 }

go/callgraph/callgraph.go

@@ -37,6 +37,8 @@ package callgraph // import "golang.org/x/tools/go/callgraph"
 // More generally, we could eliminate "uninteresting" nodes such as
 // nodes from packages we don't care about.
 
+// TODO(zpavlinovic): decide how callgraphs handle calls to and from generic function bodies.
+
 import (
 	"fmt"
 	"go/token"

go/callgraph/cha/cha.go

@@ -22,6 +22,8 @@
 // partial programs, such as libraries without a main or test function.
 package cha // import "golang.org/x/tools/go/callgraph/cha"
 
+// TODO(zpavlinovic): update CHA for how it handles generic function bodies.
+
 import (
 	"go/types"
 

go/callgraph/cha/testdata/generics.go

@@ -41,5 +41,9 @@ func f(h func(), g func(I), k func(A), a A, b B) {
 //   f --> instantiated[main.A]
 //   f --> instantiated[main.A]
 //   f --> instantiated[main.B]
+//   instantiated --> (*A).Foo
+//   instantiated --> (*B).Foo
+//   instantiated --> (A).Foo
+//   instantiated --> (B).Foo
 //   instantiated[main.A] --> (A).Foo
 //   instantiated[main.B] --> (B).Foo

go/callgraph/static/static.go

@@ -6,6 +6,8 @@
 // only static call edges.
 package static // import "golang.org/x/tools/go/callgraph/static"
 
+// TODO(zpavlinovic): update static for how it handles generic function bodies.
+
 import (
 	"golang.org/x/tools/go/callgraph"
 	"golang.org/x/tools/go/ssa"

go/callgraph/vta/utils.go

@@ -9,6 +9,7 @@ import (
 
 	"golang.org/x/tools/go/callgraph"
 	"golang.org/x/tools/go/ssa"
+	"golang.org/x/tools/internal/typeparams"
 )
 
 func canAlias(n1, n2 node) bool {
@@ -117,19 +118,27 @@ func functionUnderPtr(t types.Type) types.Type {
 }
 
 // sliceArrayElem returns the element type of type `t` that is
-// expected to be a (pointer to) array or slice, consistent with
+// expected to be a (pointer to) array, slice or string, consistent with
 // the ssa.Index and ssa.IndexAddr instructions. Panics otherwise.
 func sliceArrayElem(t types.Type) types.Type {
-	u := t.Underlying()
-
-	if p, ok := u.(*types.Pointer); ok {
-		u = p.Elem().Underlying()
-	}
-
-	if a, ok := u.(*types.Array); ok {
-		return a.Elem()
+	switch u := t.Underlying().(type) {
+	case *types.Pointer:
+		return u.Elem().Underlying().(*types.Array).Elem()
+	case *types.Array:
+		return u.Elem()
+	case *types.Slice:
+		return u.Elem()
+	case *types.Basic:
+		return types.Typ[types.Byte]
+	case *types.Interface: // type param.
+		terms, err := typeparams.InterfaceTermSet(u)
+		if err != nil || len(terms) == 0 {
+			panic(t)
+		}
+		return sliceArrayElem(terms[0].Type()) // Element types must match.
+	default:
+		panic(t)
 	}
-	return u.(*types.Slice).Elem()
 }
 
 // siteCallees computes a set of callees for call site `c` given program `callgraph`.

go/callgraph/vta/vta.go

@@ -54,6 +54,8 @@
 // reaching the node representing the call site to create a set of callees.
 package vta
 
+// TODO(zpavlinovic): update VTA for how it handles generic function bodies and instantiation wrappers.
+
 import (
 	"go/types"
 

go/pointer/analysis.go

@@ -16,6 +16,7 @@ import (
 	"runtime"
 	"runtime/debug"
 	"sort"
+	"strings"
 
 	"golang.org/x/tools/go/callgraph"
 	"golang.org/x/tools/go/ssa"
@@ -377,12 +378,27 @@ func (a *analysis) callEdge(caller *cgnode, site *callsite, calleeid nodeid) {
 		fmt.Fprintf(a.log, "\tcall edge %s -> %s\n", site, callee)
 	}
 
-	// Warn about calls to non-intrinsic external functions.
+	// Warn about calls to functions that are handled unsoundly.
 	// TODO(adonovan): de-dup these messages.
-	if fn := callee.fn; fn.Blocks == nil && a.findIntrinsic(fn) == nil {
+	fn := callee.fn
+
+	// Warn about calls to non-intrinsic external functions.
+	if fn.Blocks == nil && a.findIntrinsic(fn) == nil {
 		a.warnf(site.pos(), "unsound call to unknown intrinsic: %s", fn)
 		a.warnf(fn.Pos(), " (declared here)")
 	}
+
+	// Warn about calls to generic function bodies.
+	if fn.TypeParams().Len() > 0 && len(fn.TypeArgs()) == 0 {
+		a.warnf(site.pos(), "unsound call to generic function body: %s (build with ssa.InstantiateGenerics)", fn)
+		a.warnf(fn.Pos(), " (declared here)")
+	}
+
+	// Warn about calls to instantiation wrappers of generics functions.
+	if fn.Origin() != nil && strings.HasPrefix(fn.Synthetic, "instantiation wrapper ") {
+		a.warnf(site.pos(), "unsound call to instantiation wrapper of generic: %s (build with ssa.InstantiateGenerics)", fn)
+		a.warnf(fn.Pos(), " (declared here)")
+	}
 }
 
 // dumpSolution writes the PTS solution to the specified file.

go/pointer/api.go

@@ -28,7 +28,11 @@ type Config struct {
 	// dependencies of any main package may still affect the
 	// analysis result, because they contribute runtime types and
 	// thus methods.
+	//
 	// TODO(adonovan): investigate whether this is desirable.
+	//
+	// Calls to generic functions will be unsound unless packages
+	// are built using the ssa.InstantiateGenerics builder mode.
 	Mains []*ssa.Package
 
 	// Reflection determines whether to handle reflection

go/pointer/doc.go

@@ -358,6 +358,14 @@ A. Control-flow joins would merge interfaces ({T1}, {V1}) and ({T2},
 type-unsafe combination (T1,V2).  Treating the value and its concrete
 type as inseparable makes the analysis type-safe.)
 
+Type parameters:
+
+Type parameters are not directly supported by the analysis.
+Calls to generic functions will be left as if they had empty bodies.
+Users of the package are expected to use the ssa.InstantiateGenerics
+builder mode when building code that uses or depends on code
+containing generics.
+
 reflect.Value:
 
 A reflect.Value is modelled very similar to an interface{}, i.e. as

go/pointer/gen.go

@@ -14,9 +14,11 @@ import (
 	"fmt"
 	"go/token"
 	"go/types"
+	"strings"
 
 	"golang.org/x/tools/go/callgraph"
 	"golang.org/x/tools/go/ssa"
+	"golang.org/x/tools/internal/typeparams"
 )
 
 var (
@@ -978,7 +980,10 @@ func (a *analysis) genInstr(cgn *cgnode, instr ssa.Instruction) {
 			a.sizeof(instr.Type()))
 
 	case *ssa.Index:
-		a.copy(a.valueNode(instr), 1+a.valueNode(instr.X), a.sizeof(instr.Type()))
+		_, isstring := typeparams.CoreType(instr.X.Type()).(*types.Basic)
+		if !isstring {
+			a.copy(a.valueNode(instr), 1+a.valueNode(instr.X), a.sizeof(instr.Type()))
+		}
 
 	case *ssa.Select:
 		recv := a.valueOffsetNode(instr, 2) // instr : (index, recvOk, recv0, ... recv_n-1)
@@ -1202,6 +1207,19 @@ func (a *analysis) genFunc(cgn *cgnode) {
 		return
 	}
 
+	if fn.TypeParams().Len() > 0 && len(fn.TypeArgs()) == 0 {
+		// Body of generic function.
+		// We'll warn about calls to such functions at the end.
+		return
+	}
+
+	if strings.HasPrefix(fn.Synthetic, "instantiation wrapper ") {
+		// instantiation wrapper of a generic function.
+		// These may contain type coercions which are not currently supported.
+		// We'll warn about calls to such functions at the end.
+		return
+	}
+
 	if a.log != nil {
 		fmt.Fprintln(a.log, "; Creating nodes for local values")
 	}

go/pointer/pointer_test.go

@@ -240,9 +240,14 @@ func doOneInput(t *testing.T, input, fpath string) bool {
 	// Find all calls to the built-in print(x).  Analytically,
 	// print is a no-op, but it's a convenient hook for testing
 	// the PTS of an expression, so our tests use it.
+	// Exclude generic bodies as these should be dead code for pointer.
+	// Instance of generics are included.
 	probes := make(map[*ssa.CallCommon]bool)
 	for fn := range ssautil.AllFunctions(prog) {
-		// TODO(taking): Switch to a more principled check like fn.declaredPackage() == mainPkg if _Origin is exported.
+		if isGenericBody(fn) {
+			continue // skip generic bodies
+		}
+		// TODO(taking): Switch to a more principled check like fn.declaredPackage() == mainPkg if Origin is exported.
 		if fn.Pkg == mainpkg || (fn.Pkg == nil && mainFiles[prog.Fset.File(fn.Pos())]) {
 			for _, b := range fn.Blocks {
 				for _, instr := range b.Instrs {
@@ -656,6 +661,15 @@ func TestInput(t *testing.T) {
 	}
 }
 
+// isGenericBody returns true if fn is the body of a generic function.
+func isGenericBody(fn *ssa.Function) bool {
+	sig := fn.Signature
+	if typeparams.ForSignature(sig).Len() > 0 || typeparams.RecvTypeParams(sig).Len() > 0 {
+		return fn.Synthetic == ""
+	}
+	return false
+}
+
 // join joins the elements of multiset with " | "s.
 func join(set map[string]int) string {
 	var buf bytes.Buffer

go/pointer/reflect.go

@@ -1024,7 +1024,7 @@ func ext۰reflect۰ChanOf(a *analysis, cgn *cgnode) {
 	var dir reflect.ChanDir // unknown
 	if site := cgn.callersite; site != nil {
 		if c, ok := site.instr.Common().Args[0].(*ssa.Const); ok {
-			v, _ := constant.Int64Val(c.Value)
+			v := c.Int64()
 			if 0 <= v && v <= int64(reflect.BothDir) {
 				dir = reflect.ChanDir(v)
 			}
@@ -1751,8 +1751,7 @@ func ext۰reflect۰rtype۰InOut(a *analysis, cgn *cgnode, out bool) {
 	index := -1
 	if site := cgn.callersite; site != nil {
 		if c, ok := site.instr.Common().Args[0].(*ssa.Const); ok {
-			v, _ := constant.Int64Val(c.Value)
-			index = int(v)
+			index = int(c.Int64())
 		}
 	}
 	a.addConstraint(&rtypeInOutConstraint{

go/pointer/util.go

@@ -8,12 +8,13 @@ import (
 	"bytes"
 	"fmt"
 	"go/types"
-	exec "golang.org/x/sys/execabs"
 	"log"
 	"os"
 	"runtime"
 	"time"
 
+	exec "golang.org/x/sys/execabs"
+
 	"golang.org/x/tools/container/intsets"
 )
 
@@ -125,7 +126,7 @@ func (a *analysis) flatten(t types.Type) []*fieldInfo {
 				// Debuggability hack: don't remove
 				// the named type from interfaces as
 				// they're very verbose.
-				fl = append(fl, &fieldInfo{typ: t})
+				fl = append(fl, &fieldInfo{typ: t}) // t may be a type param
 			} else {
 				fl = a.flatten(u)
 			}

go/ssa/TODO

@@ -0,0 +1,16 @@
+-*- text -*-
+
+SSA Generics to-do list
+===========================
+
+DOCUMENTATION:
+- Read me for internals
+
+TYPE PARAMETERIZED GENERIC FUNCTIONS:
+- sanity.go updates.
+- Check source functions going to generics.
+- Tests, tests, tests...
+
+USAGE:
+- Back fill users for handling ssa.InstantiateGenerics being off.
+