From 0f0bbfd77bf2de5c3a4bc5cc6126522f42ef47c7 Mon Sep 17 00:00:00 2001
From: Zvonimir Pavlinovic <zpavlinovic@google.com>
Date: Tue, 25 Jan 2022 10:53:53 -0800
Subject: [PATCH] go/callgraph/vta: avoid panic on missing function definitions

For golang/go#50670

Change-Id: I86d4aee70584cf255f0972e56726b29555120e6d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/380795
Reviewed-by: Guodong Li <guodongli@google.com>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Tim King <taking@google.com>
---
 go/callgraph/vta/graph.go            | 15 ++++++++++++
 go/callgraph/vta/testdata/src/d/d.go | 13 +++++++++++
 go/callgraph/vta/testdata/src/t/t.go |  8 +++++++
 go/callgraph/vta/vta_test.go         | 35 ++++++++++++++++++++++++++++
 4 files changed, 71 insertions(+)
 create mode 100644 go/callgraph/vta/testdata/src/d/d.go
 create mode 100644 go/callgraph/vta/testdata/src/t/t.go

diff --git a/go/callgraph/vta/graph.go b/go/callgraph/vta/graph.go
index f846418f1c..d734d1bb6d 100644
--- a/go/callgraph/vta/graph.go
+++ b/go/callgraph/vta/graph.go
@@ -573,6 +573,13 @@ func (b *builder) call(c ssa.CallInstruction) {
 }
 
 func addArgumentFlows(b *builder, c ssa.CallInstruction, f *ssa.Function) {
+	// When f has no paremeters (including receiver), there is no type
+	// flow here. Also, f's body and parameters might be missing, such
+	// as when vta is used within the golang.org/x/tools/go/analysis
+	// framework (see github.com/golang/go/issues/50670).
+	if len(f.Params) == 0 {
+		return
+	}
 	cc := c.Common()
 	// When c is an unresolved method call (cc.Method != nil), cc.Value contains
 	// the receiver object rather than cc.Args[0].
@@ -585,6 +592,14 @@ func addArgumentFlows(b *builder, c ssa.CallInstruction, f *ssa.Function) {
 		offset = 1
 	}
 	for i, v := range cc.Args {
+		// Parameters of f might not be available, as in the case
+		// when vta is used within the golang.org/x/tools/go/analysis
+		// framework (see github.com/golang/go/issues/50670).
+		//
+		// TODO: investigate other cases of missing body and parameters
+		if len(f.Params) <= i+offset {
+			return
+		}
 		b.addInFlowAliasEdges(b.nodeFromVal(f.Params[i+offset]), b.nodeFromVal(v))
 	}
 }
diff --git a/go/callgraph/vta/testdata/src/d/d.go b/go/callgraph/vta/testdata/src/d/d.go
new file mode 100644
index 0000000000..eedcc3a886
--- /dev/null
+++ b/go/callgraph/vta/testdata/src/d/d.go
@@ -0,0 +1,13 @@
+package d
+
+func D(i int) int {
+	return i + 1
+}
+
+type Data struct {
+	V int
+}
+
+func (d Data) Do() int {
+	return d.V - 1
+}
diff --git a/go/callgraph/vta/testdata/src/t/t.go b/go/callgraph/vta/testdata/src/t/t.go
new file mode 100644
index 0000000000..55a700259d
--- /dev/null
+++ b/go/callgraph/vta/testdata/src/t/t.go
@@ -0,0 +1,8 @@
+package t
+
+import "d"
+
+func t(i int) int {
+	data := d.Data{V: i}
+	return d.D(i) + data.Do()
+}
diff --git a/go/callgraph/vta/vta_test.go b/go/callgraph/vta/vta_test.go
index 6ae7a44f31..33ceaf9091 100644
--- a/go/callgraph/vta/vta_test.go
+++ b/go/callgraph/vta/vta_test.go
@@ -7,6 +7,9 @@ package vta
 import (
 	"testing"
 
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/analysistest"
+	"golang.org/x/tools/go/analysis/passes/buildssa"
 	"golang.org/x/tools/go/callgraph/cha"
 	"golang.org/x/tools/go/ssa"
 	"golang.org/x/tools/go/ssa/ssautil"
@@ -76,3 +79,35 @@ func TestVTAProgVsFuncSet(t *testing.T) {
 		t.Errorf("pruned callgraph %v should contain %v", got, want)
 	}
 }
+
+// TestVTAPanicMissingDefinitions tests if VTA gracefully handles the case
+// where VTA panics when a definition of a function or method is not
+// available, which can happen when using analysis package. A successful
+// test simply does not panic.
+func TestVTAPanicMissingDefinitions(t *testing.T) {
+	run := func(pass *analysis.Pass) (interface{}, error) {
+		s := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA)
+		CallGraph(ssautil.AllFunctions(s.Pkg.Prog), cha.CallGraph(s.Pkg.Prog))
+		return nil, nil
+	}
+
+	analyzer := &analysis.Analyzer{
+		Name: "test",
+		Doc:  "test",
+		Run:  run,
+		Requires: []*analysis.Analyzer{
+			buildssa.Analyzer,
+		},
+	}
+
+	testdata := analysistest.TestData()
+	res := analysistest.Run(t, testdata, analyzer, "t", "d")
+	if len(res) != 2 {
+		t.Errorf("want analysis results for 2 packages; got %v", len(res))
+	}
+	for _, r := range res {
+		if r.Err != nil {
+			t.Errorf("want no error for package %v; got %v", r.Pass.Pkg.Path(), r.Err)
+		}
+	}
+}