From 8db0cacb796c0ebb7ef17973fe1e7a407f1e0b83 Mon Sep 17 00:00:00 2001 From: Zvonimir Pavlinovic Date: Thu, 7 Dec 2023 15:27:16 +0000 Subject: [PATCH] internal/symbols: compute proper db names for generic functions These should not include the type parameter/argument. Fixes golang/go#63535 Change-Id: I08a5929825d569fb3104f084ea55766ba6f5542e Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/548195 Run-TryBot: Zvonimir Pavlinovic LUCI-TryBot-Result: Go LUCI TryBot-Result: Gopher Robot Reviewed-by: Tatiana Bradley --- internal/symbols/exported_functions.go | 21 +++-------------- internal/symbols/patched_functions.go | 26 +++++++++++++++------- internal/symbols/patched_functions_test.go | 10 ++++++++- internal/symbols/utils.go | 20 ++++++++++++----- 4 files changed, 45 insertions(+), 32 deletions(-) diff --git a/internal/symbols/exported_functions.go b/internal/symbols/exported_functions.go index 91775002..e2108767 100644 --- a/internal/symbols/exported_functions.go +++ b/internal/symbols/exported_functions.go @@ -16,15 +16,14 @@ import ( "golang.org/x/exp/slices" "golang.org/x/tools/go/packages" - "golang.org/x/tools/go/ssa" "golang.org/x/vulndb/internal/derrors" "golang.org/x/vulndb/internal/osvutils" "golang.org/x/vulndb/internal/report" "golang.org/x/vulndb/internal/version" ) -// Exported returns a set of vulnerable symbols exported -// by a package p from the module m. +// Exported returns a set of vulnerable symbols, in the vuln +// db format, exported by a package p from the module m. func Exported(m *report.Module, p *report.Package, errlog *log.Logger) (_ []string, err error) { defer derrors.Wrap(&err, "Exported(%q, %q)", m.Module, p.Package) @@ -175,22 +174,8 @@ func exportedFunctions(pkg *packages.Package, m *report.Module) (_ map[string]bo names := map[string]bool{} for _, e := range entries { if pkgPath(e) == pkg.PkgPath { - names[ssaSymbolName(e)] = true + names[dbFuncName(e)] = true } } return names, nil } - -func ssaSymbolName(fn *ssa.Function) string { - recv := fn.Signature.Recv() - if recv == nil { - return fn.Name() - } - recvType := recv.Type().String() - // Remove package path from type. - i := strings.LastIndexByte(recvType, '.') - if i < 0 { - return recvType + "." + fn.Name() - } - return recvType[i+1:] + "." + fn.Name() -} diff --git a/internal/symbols/patched_functions.go b/internal/symbols/patched_functions.go index d1658dcd..fa3c85ce 100644 --- a/internal/symbols/patched_functions.go +++ b/internal/symbols/patched_functions.go @@ -364,17 +364,27 @@ func astSymbolName(f *ast.FuncDecl) string { return "" // sanity } + // unpackIdent assumes e is of the form id or id[...] + // and then returns id. Otherwise, returns "". + unpackIdent := func(e ast.Expr) string { + switch xv := e.(type) { + case *ast.Ident: + return xv.Name + case *ast.IndexExpr: + if si, ok := xv.X.(*ast.Ident); ok { + return si.Name + } + } + return "" + } + + // supported receiver type snames are id, *id, id[...], and *id[...]. t := "" switch xv := field.Type.(type) { case *ast.StarExpr: - if si, ok := xv.X.(*ast.Ident); ok { - t = si.Name - } - case *ast.Ident: - t = xv.Name - case *ast.IndexExpr: - // TODO(#63535): cover index instructions stemming from generics - return "" + t = unpackIdent(xv.X) + case *ast.Ident, *ast.IndexExpr: + t = unpackIdent(xv) default: panic(fmt.Sprintf("astSymbolName: unexpected receiver type: %v\n", reflect.TypeOf(field.Type))) } diff --git a/internal/symbols/patched_functions_test.go b/internal/symbols/patched_functions_test.go index 31af7544..194b3b43 100644 --- a/internal/symbols/patched_functions_test.go +++ b/internal/symbols/patched_functions_test.go @@ -198,6 +198,14 @@ func (a A) Do() {} type B struct {} func (b *B) Do() {} + +type C[T any] struct { + t T +} +func (c C[T]) Do() {} +func (c *C[T]) Bar() {} + +func Go[X any]() {} ` fset := token.NewFileSet() // positions are relative to fset f, err := parser.ParseFile(fset, "src.go", src, 0) @@ -212,7 +220,7 @@ func (b *B) Do() {} } } sort.Strings(got) - want := []string{"A.Do", "B.Do", "Foo"} + want := []string{"A.Do", "B.Do", "C.Bar", "C.Do", "Foo", "Go"} if diff := cmp.Diff(want, got); diff != "" { t.Errorf("(-got, want+):\n%s", diff) } diff --git a/internal/symbols/utils.go b/internal/symbols/utils.go index d27a0223..be41b196 100644 --- a/internal/symbols/utils.go +++ b/internal/symbols/utils.go @@ -179,15 +179,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. @@ -220,9 +222,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 } // memberFuncs returns functions associated with the `member`: