Skip to content

Commit

Permalink
internal/vulncheck: compute proper db names for generic functions
Browse files Browse the repository at this point in the history
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 <gobot@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Maceo Thompson <maceothompson@google.com>
  • Loading branch information
zpavlinovic committed Dec 13, 2023
1 parent 864243b commit 53a5385
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 6 deletions.
6 changes: 6 additions & 0 deletions internal/vulncheck/entries.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 15 additions & 6 deletions internal/vulncheck/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
80 changes: 80 additions & 0 deletions internal/vulncheck/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
}
}

0 comments on commit 53a5385

Please sign in to comment.