From 53a5385d13db1bd7d996dadc96f3e49d290e7b78 Mon Sep 17 00:00:00 2001 From: Zvonimir Pavlinovic Date: Wed, 6 Dec 2023 21:47:37 +0000 Subject: [PATCH] internal/vulncheck: compute proper db names for generic functions These should not include the type parameter/argument. Updates golang/go#63535 Change-Id: Iaae0e587d365f7e26e2361c9814fa1d288d8ad86 Reviewed-on: https://go-review.googlesource.com/c/vuln/+/548016 TryBot-Result: Gopher Robot LUCI-TryBot-Result: Go LUCI Run-TryBot: Zvonimir Pavlinovic Reviewed-by: Maceo Thompson --- internal/vulncheck/entries.go | 6 +++ internal/vulncheck/utils.go | 21 ++++++--- internal/vulncheck/utils_test.go | 80 ++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 6 deletions(-) diff --git a/internal/vulncheck/entries.go b/internal/vulncheck/entries.go index 834d5901..3d46f6ca 100644 --- a/internal/vulncheck/entries.go +++ b/internal/vulncheck/entries.go @@ -10,6 +10,12 @@ import ( "golang.org/x/tools/go/ssa" ) +// entryPoints returns functions of topPackages considered entry +// points of govulncheck analysis: main, inits, and exported methods +// and functions. +// +// TODO(https://go.dev/issue/57221): currently, entry functions +// that are generics are not considered an entry point. func entryPoints(topPackages []*ssa.Package) []*ssa.Function { var entries []*ssa.Function for _, pkg := range topPackages { diff --git a/internal/vulncheck/utils.go b/internal/vulncheck/utils.go index 28610913..7817e984 100644 --- a/internal/vulncheck/utils.go +++ b/internal/vulncheck/utils.go @@ -28,7 +28,6 @@ import ( // the ssa program encapsulating the packages and top level // ssa packages corresponding to pkgs. func buildSSA(pkgs []*packages.Package, fset *token.FileSet) (*ssa.Program, []*ssa.Package) { - // TODO(https://go.dev/issue/57221): what about entry functions that are generics? prog := ssa.NewProgram(fset, ssa.InstantiateGenerics) imports := make(map[*packages.Package]*ssa.Package) @@ -112,15 +111,17 @@ func dbTypeFormat(t types.Type) string { // dbFuncName computes a function name consistent with the namings used in vulnerability // databases. Effectively, a qualified name of a function local to its enclosing package. -// If a receiver is a pointer, this information is not encoded in the resulting name. The -// name of anonymous functions is simply "". The function names are unique subject to the -// enclosing package, but not globally. +// If a receiver is a pointer, this information is not encoded in the resulting name. If +// a function has type argument/parameter, this information is omitted. The name of +// anonymous functions is simply "". The function names are unique subject to the enclosing +// package, but not globally. // // Examples: // // func (a A) foo (...) {...} -> A.foo // func foo(...) {...} -> foo // func (b *B) bar (...) {...} -> B.bar +// func (c C[T]) do(...) {...} -> C.do func dbFuncName(f *ssa.Function) string { selectBound := func(f *ssa.Function) types.Type { // If f is a "bound" function introduced by ssa for a given type, return the type. @@ -153,9 +154,17 @@ func dbFuncName(f *ssa.Function) string { } if qprefix == "" { - return f.Name() + return funcName(f) } - return qprefix + "." + f.Name() + return qprefix + "." + funcName(f) +} + +// funcName returns the name of the ssa function f. +// It is f.Name() without additional type argument +// information in case of generics. +func funcName(f *ssa.Function) string { + n, _, _ := strings.Cut(f.Name(), "[") + return n } // dbTypesFuncName is dbFuncName defined over *types.Func. diff --git a/internal/vulncheck/utils_test.go b/internal/vulncheck/utils_test.go index 879cfc48..dc22bc52 100644 --- a/internal/vulncheck/utils_test.go +++ b/internal/vulncheck/utils_test.go @@ -5,8 +5,12 @@ package vulncheck import ( + "path" "testing" + "github.com/google/go-cmp/cmp" + "golang.org/x/tools/go/packages/packagestest" + "golang.org/x/tools/go/ssa/ssautil" "golang.org/x/vuln/internal/osv" ) @@ -184,3 +188,79 @@ func TestFixedVersion(t *testing.T) { }) } } + +func TestDbSymbolName(t *testing.T) { + e := packagestest.Export(t, packagestest.Modules, []packagestest.Module{ + { + Name: "golang.org/package", + Files: map[string]interface{}{ + "x/x.go": ` + package x + + func Foo() { + // needed for ssautil.Allfunctions + x := a{} + x.Do() + x.NotDo() + b := B[a]{} + b.P() + b.Q(x) + Z[a]() + } + + func bar() {} + + type a struct{} + + func (x a) Do() {} + func (x *a) NotDo() { + } + + type B[T any] struct{} + + func (b *B[T]) P() {} + func (b B[T]) Q(t T) {} + + func Z[T any]() {} + `}, + }, + }) + defer e.Cleanup() + + graph := NewPackageGraph("go1.18") + pkgs, err := graph.LoadPackages(e.Config, nil, []string{path.Join(e.Temp(), "package/x")}) + if err != nil { + t.Fatal(err) + } + + want := map[string]bool{ + "init": true, + "bar": true, + "B.P": true, + "B.Q": true, + "a.Do": true, + "a.NotDo": true, + "Foo": true, + "Z": true, + } + + // test dbFuncName + prog, _ := buildSSA(pkgs, pkgs[0].Fset) + got := make(map[string]bool) + for f := range ssautil.AllFunctions(prog) { + got[dbFuncName(f)] = true + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("(-want;got+): %s", diff) + } + + // test dbTypesFuncName + delete(want, "init") // init does not appear in types.Package + got = make(map[string]bool) + for _, s := range allSymbols(pkgs[0].Types) { + got[s] = true + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("(-want;got+): %s", diff) + } +}