go.tools/pointer: use new callgraph API.
Also: pointer.Analyze now returns a pointer.Result object,
containing the callgraph and the results of ssa.Value queries.
The oracle has been updated to use the new call and pointer APIs.
R=crawshaw, gri
CC=golang-dev
https://golang.org/cl/13915043
diff --git a/pointer/TODO b/pointer/TODO
index d5f12bc..10afd49 100644
--- a/pointer/TODO
+++ b/pointer/TODO
@@ -32,12 +32,6 @@
dannyb recommends sparse bitmap.
API:
-- Rely less on callbacks and more on a 'result' type
- returned by Analyze().
-- Abstract the callgraph into a pure interface so that
- we can provide other implementations in future (e.g. RTA-based).
- Also provide the option to eliminate context-sensitivity
- in a callgraph to yield a smaller (less precise) callgraph.
- Some optimisations (e.g. LE, PE) may change the API.
Think about them sooner rather than later.
- Eliminate Print probe now that we can query specific ssa.Values.
diff --git a/pointer/analysis.go b/pointer/analysis.go
index b825bb1..d645365 100644
--- a/pointer/analysis.go
+++ b/pointer/analysis.go
@@ -173,13 +173,14 @@
nodes []*node // indexed by nodeid
flattenMemo map[types.Type][]*fieldInfo // memoization of flatten()
constraints []constraint // set of constraints
- callsites []*callsite // all callsites
+ cgnodes []*cgnode // all cgnodes
genq []*cgnode // queue of functions to generate constraints for
intrinsics map[*ssa.Function]intrinsic // non-nil values are summaries for intrinsic fns
funcObj map[*ssa.Function]nodeid // default function object for each func
probes map[*ssa.CallCommon]nodeid // maps call to print() to argument variable
valNode map[ssa.Value]nodeid // node for each ssa.Value
work worklist // solver's worklist
+ queries map[ssa.Value][]Pointer // same as Results.Queries
// Reflection:
hasher typemap.Hasher // cache of type hashes
@@ -229,7 +230,7 @@
// Analyze runs the pointer analysis with the scope and options
// specified by config, and returns the (synthetic) root of the callgraph.
//
-func Analyze(config *Config) CallGraphNode {
+func Analyze(config *Config) *Result {
a := &analysis{
config: config,
log: config.Log,
@@ -241,6 +242,7 @@
funcObj: make(map[*ssa.Function]nodeid),
probes: make(map[*ssa.CallCommon]nodeid),
work: makeMapWorklist(),
+ queries: make(map[ssa.Value][]Pointer),
}
if reflect := a.prog.ImportedPackage("reflect"); reflect != nil {
@@ -265,7 +267,7 @@
fmt.Fprintln(a.log, "======== NEW ANALYSIS ========")
}
- root := a.generate()
+ a.generate()
//a.optimize()
@@ -280,32 +282,33 @@
}
}
- // Notify the client of the callsites if they're interested.
- if CallSite := a.config.CallSite; CallSite != nil {
- for _, site := range a.callsites {
- CallSite(site)
- }
- }
+ // Visit discovered call graph.
+ for _, caller := range a.cgnodes {
+ for _, site := range caller.sites {
+ for nid := range a.nodes[site.targets].pts {
+ callee := a.nodes[nid].obj.cgn
- Call := a.config.Call
- for _, site := range a.callsites {
- for nid := range a.nodes[site.targets].pts {
- cgn := a.nodes[nid].obj.cgn
+ if a.config.BuildCallGraph {
+ site.callees = append(site.callees, callee)
+ }
- // Notify the client of the call graph, if
- // they're interested.
- if Call != nil {
- Call(site, cgn)
- }
-
- // Warn about calls to non-intrinsic external functions.
-
- if fn := cgn.fn; fn.Blocks == nil && a.findIntrinsic(fn) == nil {
- a.warnf(site.Pos(), "unsound call to unknown intrinsic: %s", fn)
- a.warnf(fn.Pos(), " (declared here)")
+ // TODO(adonovan): de-dup these messages.
+ // Warn about calls to non-intrinsic external functions.
+ if fn := callee.fn; fn.Blocks == nil && a.findIntrinsic(fn) == nil {
+ a.warnf(site.pos(), "unsound call to unknown intrinsic: %s", fn)
+ a.warnf(fn.Pos(), " (declared here)")
+ }
}
}
}
- return root
+ var callgraph *cgraph
+ if a.config.BuildCallGraph {
+ callgraph = &cgraph{a.cgnodes}
+ }
+
+ return &Result{
+ CallGraph: callgraph,
+ Queries: a.queries,
+ }
}
diff --git a/pointer/api.go b/pointer/api.go
index f1f4e47..3867b2a 100644
--- a/pointer/api.go
+++ b/pointer/api.go
@@ -9,6 +9,7 @@
"go/token"
"io"
+ "code.google.com/p/go.tools/call"
"code.google.com/p/go.tools/go/types/typemap"
"code.google.com/p/go.tools/ssa"
)
@@ -27,31 +28,12 @@
// has not yet been reduced by presolver optimisation.
Reflection bool
+ // BuildCallGraph determines whether to construct a callgraph.
+ // If enabled, the graph will be available in Result.CallGraph.
+ BuildCallGraph bool
+
// -------- Optional callbacks invoked by the analysis --------
- // Call is invoked for each discovered call-graph edge. The
- // call-graph is a multigraph over CallGraphNodes with edges
- // labelled by the CallSite that gives rise to the edge.
- // (The caller node is available as site.Caller())
- //
- // Clients that wish to construct a call graph may provide
- // CallGraph.AddEdge here.
- //
- // The callgraph may be context-sensitive, i.e. it may
- // distinguish separate calls to the same function depending
- // on the context.
- //
- Call func(site CallSite, callee CallGraphNode)
-
- // CallSite is invoked for each call-site encountered in the
- // program.
- //
- // The callgraph may be context-sensitive, i.e. it may
- // distinguish separate calls to the same function depending
- // on the context.
- //
- CallSite func(site CallSite)
-
// Warn is invoked for each warning encountered by the analysis,
// e.g. unknown external function, unsound use of unsafe.Pointer.
// pos may be zero if the position is not known.
@@ -71,8 +53,8 @@
//
Print func(site *ssa.CallCommon, p Pointer)
- // The client populates QueryValues[v] for each ssa.Value v
- // of interest.
+ // The client populates Queries[v] for each ssa.Value v of
+ // interest.
//
// The boolean (Indirect) indicates whether to compute the
// points-to set for v (false) or *v (true): the latter is
@@ -80,20 +62,16 @@
// lvalues, e.g. an *ssa.Global.
//
// The pointer analysis will populate the corresponding
- // QueryResults value when it creates the pointer variable
- // for v or *v. Upon completion the client can inspect the
+ // Results.Queries value when it creates the pointer variable
+ // for v or *v. Upon completion the client can inspect that
// map for the results.
//
// If a Value belongs to a function that the analysis treats
- // context-sensitively, the corresponding QueryResults slice
+ // context-sensitively, the corresponding Results.Queries slice
// may have multiple Pointers, one per distinct context. Use
// PointsToCombined to merge them.
//
- // TODO(adonovan): refactor the API: separate all results of
- // Analyze() into a dedicated Result struct.
- //
- QueryValues map[ssa.Value]Indirect
- QueryResults map[ssa.Value][]Pointer
+ Queries map[ssa.Value]Indirect
// -------- Other configuration options --------
@@ -111,10 +89,19 @@
panic("empty scope")
}
+// A Result contains the results of a pointer analysis.
+//
+// See Config for how to request the various Result components.
+//
+type Result struct {
+ CallGraph call.Graph // discovered call graph
+ Queries map[ssa.Value][]Pointer // points-to sets for queried ssa.Values
+}
+
// A Pointer is an equivalence class of pointerlike values.
//
// TODO(adonovan): add a method
-// Context() CallGraphNode
+// Context() call.GraphNode
// for pointers corresponding to local variables,
//
type Pointer interface {
diff --git a/pointer/callgraph.go b/pointer/callgraph.go
index af328a9..e3d8bce 100644
--- a/pointer/callgraph.go
+++ b/pointer/callgraph.go
@@ -4,116 +4,93 @@
package pointer
+// This file defines our implementation of the call.Graph API.
+
import (
"fmt"
"go/token"
+ "code.google.com/p/go.tools/call"
"code.google.com/p/go.tools/ssa"
)
-// TODO(adonovan): move the CallGraph, CallGraphNode, CallSite types
-// into a separate package 'callgraph', and make them pure interfaces
-// capable of supporting several implementations (context-sensitive
-// and insensitive PTA, RTA, etc).
-
-// ---------- CallGraphNode ----------
-
-// A CallGraphNode is a context-sensitive representation of a node in
-// the callgraph. In other words, there may be multiple nodes
-// representing a single *Function, depending on the contexts in which
-// it is called. The identity of the node is therefore important.
-//
-type CallGraphNode interface {
- Func() *ssa.Function // the function this node represents
- String() string // diagnostic description of this callgraph node
+// cgraph implements call.Graph.
+type cgraph struct {
+ nodes []*cgnode
}
+func (g *cgraph) Nodes() []call.GraphNode {
+ nodes := make([]call.GraphNode, len(g.nodes))
+ for i, node := range g.nodes {
+ nodes[i] = node
+ }
+ return nodes
+}
+
+func (g *cgraph) Root() call.GraphNode {
+ return g.nodes[0]
+}
+
+// cgnode implements call.GraphNode.
type cgnode struct {
- fn *ssa.Function
- obj nodeid // start of this contour's object block
+ fn *ssa.Function
+ obj nodeid // start of this contour's object block
+ sites []*callsite // ordered list of callsites within this function
}
func (n *cgnode) Func() *ssa.Function {
return n.fn
}
+func (n *cgnode) Sites() []ssa.CallInstruction {
+ sites := make([]ssa.CallInstruction, len(n.sites))
+ for i, site := range n.sites {
+ sites[i] = site.instr
+ }
+ return sites
+}
+
+func (n *cgnode) Edges() []call.Edge {
+ var numEdges int
+ for _, site := range n.sites {
+ numEdges += len(site.callees)
+ }
+ edges := make([]call.Edge, 0, numEdges)
+
+ for _, site := range n.sites {
+ for _, callee := range site.callees {
+ edges = append(edges, call.Edge{Caller: n, Site: site.instr, Callee: callee})
+ }
+ }
+ return edges
+}
+
func (n *cgnode) String() string {
return fmt.Sprintf("cg%d:%s", n.obj, n.fn)
}
-// ---------- CallSite ----------
-
-// A CallSite is a context-sensitive representation of a function call
-// site in the program.
-//
-type CallSite interface {
- Caller() CallGraphNode // the enclosing context of this call
- Pos() token.Pos // source position; token.NoPos for synthetic calls
- Description() string // UI description of call kind; see (*ssa.CallCommon).Description
- String() string // diagnostic description of this callsite
-}
-
-// A callsite represents a single function or method callsite within a
-// function. callsites never represent calls to built-ins; they are
-// handled as intrinsics.
+// A callsite represents a single call site within a cgnode;
+// it is implicitly context-sensitive.
+// callsites never represent calls to built-ins;
+// they are handled as intrinsics.
//
type callsite struct {
- caller *cgnode // the origin of the call
targets nodeid // pts(targets) contains identities of all called functions.
- instr ssa.CallInstruction // optional call instruction; provides IsInvoke, position, etc.
- pos token.Pos // position, if instr == nil, i.e. synthetic callsites.
+ instr ssa.CallInstruction // the call instruction; nil for synthetic/intrinsic
+ callees []*cgnode // unordered set of callees of this site
}
-// Caller returns the node in the callgraph from which this call originated.
-func (c *callsite) Caller() CallGraphNode {
- return c.caller
-}
-
-// Description returns a description of this kind of call, in the
-// manner of ssa.CallCommon.Description().
-//
-func (c *callsite) Description() string {
+func (c *callsite) String() string {
if c.instr != nil {
return c.instr.Common().Description()
}
return "synthetic function call"
}
-// Pos returns the source position of this callsite, or token.NoPos if implicit.
-func (c *callsite) Pos() token.Pos {
+// pos returns the source position of this callsite, or token.NoPos if implicit.
+func (c *callsite) pos() token.Pos {
if c.instr != nil {
return c.instr.Pos()
}
- return c.pos
-}
-
-func (c *callsite) String() string {
- // TODO(adonovan): provide more info, e.g. target of static
- // call, arguments, location.
- return c.Description()
-}
-
-// ---------- CallGraph ----------
-
-// CallGraph is a forward directed graph of functions labelled by an
-// arbitrary site within the caller.
-//
-// CallGraph.AddEdge may be used as the Context.Call callback for
-// clients that wish to construct a call graph.
-//
-// TODO(adonovan): this is just a starting point. Add options to
-// control whether we record no callsite, an arbitrary callsite, or
-// all callsites for a given graph edge. Also, this could live in
-// another package since it's just a client utility.
-//
-type CallGraph map[CallGraphNode]map[CallGraphNode]CallSite
-
-func (cg CallGraph) AddEdge(site CallSite, callee CallGraphNode) {
- caller := site.Caller()
- callees := cg[caller]
- if callees == nil {
- callees = make(map[CallGraphNode]CallSite)
- cg[caller] = callees
- }
- callees[callee] = site // save an arbitrary site
+ return token.NoPos
}
diff --git a/pointer/example_test.go b/pointer/example_test.go
index fae6195..bd86818a 100644
--- a/pointer/example_test.go
+++ b/pointer/example_test.go
@@ -10,6 +10,7 @@
"go/parser"
"sort"
+ "code.google.com/p/go.tools/call"
"code.google.com/p/go.tools/importer"
"code.google.com/p/go.tools/pointer"
"code.google.com/p/go.tools/ssa"
@@ -66,34 +67,23 @@
prog.BuildAll()
// Run the pointer analysis and build the complete callgraph.
- callgraph := make(pointer.CallGraph)
config := &pointer.Config{
- Mains: []*ssa.Package{mainPkg},
- Call: callgraph.AddEdge,
+ Mains: []*ssa.Package{mainPkg},
+ BuildCallGraph: true,
}
- root := pointer.Analyze(config)
+ result := pointer.Analyze(config)
- // Visit callgraph in depth-first order.
- //
- // There may be multiple nodes for the
- // same function due to context sensitivity.
- var edges []string // call edges originating from the main package.
- seen := make(map[pointer.CallGraphNode]bool)
- var visit func(cgn pointer.CallGraphNode)
- visit = func(cgn pointer.CallGraphNode) {
- if seen[cgn] {
- return // already seen
+ // Find edges originating from the main package.
+ // By converting to strings, we de-duplicate nodes
+ // representing the same function due to context sensitivity.
+ var edges []string
+ call.GraphVisitEdges(result.CallGraph, func(edge call.Edge) error {
+ caller := edge.Caller.Func()
+ if caller.Pkg == mainPkg {
+ edges = append(edges, fmt.Sprint(caller, " --> ", edge.Callee.Func()))
}
- seen[cgn] = true
- caller := cgn.Func()
- for callee := range callgraph[cgn] {
- if caller.Pkg == mainPkg {
- edges = append(edges, fmt.Sprint(caller, " --> ", callee.Func()))
- }
- visit(callee)
- }
- }
- visit(root)
+ return nil
+ })
// Print the edges in sorted order.
sort.Strings(edges)
diff --git a/pointer/gen.go b/pointer/gen.go
index 62fb103..cebeff5 100644
--- a/pointer/gen.go
+++ b/pointer/gen.go
@@ -72,18 +72,13 @@
}
// Record the (v, id) relation if the client has queried v.
- if indirect, ok := a.config.QueryValues[v]; ok {
+ if indirect, ok := a.config.Queries[v]; ok {
if indirect {
tmp := a.addNodes(v.Type(), "query.indirect")
a.load(tmp, id, a.sizeof(v.Type()))
id = tmp
}
- ptrs := a.config.QueryResults
- if ptrs == nil {
- ptrs = make(map[ssa.Value][]Pointer)
- a.config.QueryResults = ptrs
- }
- ptrs[v] = append(ptrs[v], ptr{a, id})
+ a.queries[v] = append(a.queries[v], ptr{a, id})
}
}
@@ -125,7 +120,7 @@
// obj is the function object (identity, params, results).
obj := a.nextNode()
- cgn := &cgnode{fn: fn, obj: obj}
+ cgn := a.makeCGNode(fn, obj)
sig := fn.Signature
a.addOneNode(sig, "func.cgnode", nil) // (scalar with Signature type)
if recv := sig.Recv(); recv != nil {
@@ -849,15 +844,12 @@
}
site := &callsite{
- caller: caller,
targets: targets,
instr: instr,
- pos: instr.Pos(),
}
- a.callsites = append(a.callsites, site)
+ caller.sites = append(caller.sites, site)
if a.log != nil {
- fmt.Fprintf(a.log, "\t%s to targets %s from %s\n",
- site.Description(), site.targets, site.caller)
+ fmt.Fprintf(a.log, "\t%s to targets %s from %s\n", site, site.targets, caller)
}
}
@@ -1061,6 +1053,12 @@
}
}
+func (a *analysis) makeCGNode(fn *ssa.Function, obj nodeid) *cgnode {
+ cgn := &cgnode{fn: fn, obj: obj}
+ a.cgnodes = append(a.cgnodes, cgn)
+ return cgn
+}
+
// genRootCalls generates the synthetic root of the callgraph and the
// initial calls from it to the analysis scope, such as main, a test
// or a library.
@@ -1070,7 +1068,7 @@
r.Prog = a.prog // hack.
r.Enclosing = r // hack, so Function.String() doesn't crash
r.String() // (asserts that it doesn't crash)
- root := &cgnode{fn: r}
+ root := a.makeCGNode(r, 0)
// For each main package, call main.init(), main.main().
for _, mainPkg := range a.config.Mains {
@@ -1080,11 +1078,8 @@
}
targets := a.addOneNode(main.Signature, "root.targets", nil)
- site := &callsite{
- caller: root,
- targets: targets,
- }
- a.callsites = append(a.callsites, site)
+ site := &callsite{targets: targets}
+ root.sites = append(root.sites, site)
for _, fn := range [2]*ssa.Function{mainPkg.Func("init"), main} {
if a.log != nil {
fmt.Fprintf(a.log, "\troot call to %s:\n", fn)
diff --git a/pointer/labels.go b/pointer/labels.go
index 6f33f1f..215d24c 100644
--- a/pointer/labels.go
+++ b/pointer/labels.go
@@ -9,6 +9,7 @@
"go/token"
"strings"
+ "code.google.com/p/go.tools/call"
"code.google.com/p/go.tools/go/types"
"code.google.com/p/go.tools/ssa"
)
@@ -46,7 +47,7 @@
// Context returns the analytic context in which this label's object was allocated,
// or nil for global objects: global, const, and shared contours for functions.
//
-func (l Label) Context() CallGraphNode {
+func (l Label) Context() call.GraphNode {
return l.obj.cgn
}
diff --git a/pointer/pointer_test.go b/pointer/pointer_test.go
index 8799e71..7f4a500 100644
--- a/pointer/pointer_test.go
+++ b/pointer/pointer_test.go
@@ -10,6 +10,7 @@
import (
"bytes"
+ "errors"
"fmt"
"go/build"
"go/parser"
@@ -21,6 +22,7 @@
"strings"
"testing"
+ "code.google.com/p/go.tools/call"
"code.google.com/p/go.tools/go/types"
"code.google.com/p/go.tools/go/types/typemap"
"code.google.com/p/go.tools/importer"
@@ -286,24 +288,22 @@
var warnings []string
var log bytes.Buffer
- callgraph := make(pointer.CallGraph)
-
// Run the analysis.
config := &pointer.Config{
- Reflection: true,
- Mains: []*ssa.Package{ptrmain},
- Log: &log,
+ Reflection: true,
+ BuildCallGraph: true,
+ Mains: []*ssa.Package{ptrmain},
+ Log: &log,
Print: func(site *ssa.CallCommon, p pointer.Pointer) {
probes = append(probes, probe{site, p})
},
- Call: callgraph.AddEdge,
Warn: func(pos token.Pos, format string, args ...interface{}) {
msg := fmt.Sprintf(format, args...)
fmt.Printf("%s: warning: %s\n", prog.Fset.Position(pos), msg)
warnings = append(warnings, msg)
},
}
- pointer.Analyze(config)
+ result := pointer.Analyze(config)
// Print the log is there was an error or a panic.
complete := false
@@ -341,7 +341,7 @@
}
case "calls":
- if !checkCallsExpectation(prog, e, callgraph) {
+ if !checkCallsExpectation(prog, e, result.CallGraph) {
ok = false
}
@@ -463,29 +463,33 @@
e.errorf("interface may additionally contain these types: %s", surplus.KeysString())
}
return ok
- return false
}
-func checkCallsExpectation(prog *ssa.Program, e *expectation, callgraph pointer.CallGraph) bool {
- // TODO(adonovan): this is inefficient and not robust against
- // typos. Better to convert strings to *Functions during
- // expectation parsing (somehow).
- for caller, callees := range callgraph {
- if caller.Func().String() == e.args[0] {
- found := make(map[string]struct{})
- for callee := range callees {
- s := callee.Func().String()
- found[s] = struct{}{}
- if s == e.args[1] {
- return true // expectation satisfied
- }
+var errOK = errors.New("OK")
+
+func checkCallsExpectation(prog *ssa.Program, e *expectation, callgraph call.Graph) bool {
+ found := make(map[string]struct{})
+ err := call.GraphVisitEdges(callgraph, func(edge call.Edge) error {
+ // Name-based matching is inefficient but it allows us to
+ // match functions whose names that would not appear in an
+ // index ("<root>") or which are not unique ("func@1.2").
+ if edge.Caller.Func().String() == e.args[0] {
+ calleeStr := edge.Callee.Func().String()
+ if calleeStr == e.args[1] {
+ return errOK // expectation satisified; stop the search
}
- e.errorf("found no call from %s to %s, but only to %s",
- e.args[0], e.args[1], join(found))
- return false
+ found[calleeStr] = struct{}{}
}
+ return nil
+ })
+ if err == errOK {
+ return true
}
- e.errorf("didn't find any calls from %s", e.args[0])
+ if len(found) == 0 {
+ e.errorf("didn't find any calls from %s", e.args[0])
+ }
+ e.errorf("found no call from %s to %s, but only to %s",
+ e.args[0], e.args[1], join(found))
return false
}