diff --git a/oracle/describe.go b/oracle/describe.go index 162085d3..c355a766 100644 --- a/oracle/describe.go +++ b/oracle/describe.go @@ -409,16 +409,24 @@ func describeValue(o *Oracle, qpos *QueryPos, path []ast.Node) (*describeValueRe }, nil } -// describePointer runs the pointer analysis of the selected SSA value. -func describePointer(o *Oracle, v ssa.Value, indirect bool) (ptrs []pointerResult, err error) { +// describePointer runs the pointer analysis of the selected SSA value or address. +func describePointer(o *Oracle, v ssa.Value, isAddr bool) (ptrs []pointerResult, err error) { buildSSA(o) - // TODO(adonovan): don't run indirect pointer analysis on non-ptr-ptrlike types. - o.config.Queries = map[ssa.Value]pointer.Indirect{v: pointer.Indirect(indirect)} + if isAddr { + o.config.AddIndirectQuery(v) + } else { + o.config.AddQuery(v) + } ptares := ptrAnalysis(o) // Combine the PT sets from all contexts. - pointers := ptares.Queries[v] + var pointers []pointer.Pointer + if isAddr { + pointers = ptares.IndirectQueries[v] + } else { + pointers = ptares.Queries[v] + } if pointers == nil { return nil, fmt.Errorf("PTA did not encounter this expression (dead code?)") } diff --git a/oracle/peers.go b/oracle/peers.go index 822c8db5..3ec50c21 100644 --- a/oracle/peers.go +++ b/oracle/peers.go @@ -60,11 +60,11 @@ func peers(o *Oracle, qpos *QueryPos) (queryResult, error) { // ignore both directionality and type names. queryType := queryOp.ch.Type() queryElemType := queryType.Underlying().(*types.Chan).Elem() - channels := map[ssa.Value]pointer.Indirect{queryOp.ch: false} + o.config.AddQuery(queryOp.ch) i := 0 for _, op := range ops { if types.IsIdentical(op.ch.Type().Underlying().(*types.Chan).Elem(), queryElemType) { - channels[op.ch] = false + o.config.AddQuery(op.ch) ops[i] = op i++ } @@ -72,7 +72,6 @@ func peers(o *Oracle, qpos *QueryPos) (queryResult, error) { ops = ops[:i] // Run the pointer analysis. - o.config.Queries = channels ptares := ptrAnalysis(o) // Combine the PT sets from all contexts. diff --git a/pointer/analysis.go b/pointer/analysis.go index 0f21424d..4602ba58 100644 --- a/pointer/analysis.go +++ b/pointer/analysis.go @@ -262,7 +262,8 @@ func Analyze(config *Config) *Result { probes: make(map[*ssa.CallCommon]nodeid), work: makeMapWorklist(), result: &Result{ - Queries: make(map[ssa.Value][]Pointer), + Queries: make(map[ssa.Value][]Pointer), + IndirectQueries: make(map[ssa.Value][]Pointer), }, } diff --git a/pointer/api.go b/pointer/api.go index 10103b94..5f294aa0 100644 --- a/pointer/api.go +++ b/pointer/api.go @@ -5,6 +5,7 @@ package pointer import ( + "bytes" "fmt" "go/token" "io" @@ -43,32 +44,54 @@ type Config struct { // Print func(site *ssa.CallCommon, p Pointer) - // The client populates Queries[v] for each ssa.Value v of - // interest. + // The client populates Queries[v] or IndirectQueries[v] + // for each ssa.Value v of interest, to request that the + // points-to sets pts(v) or pts(*v) be computed. If the + // client needs both points-to sets, v may appear in both + // maps. // - // The boolean (Indirect) indicates whether to compute the - // points-to set for v (false) or *v (true): the latter is - // typically wanted for Values corresponding to source-level - // lvalues, e.g. an *ssa.Global. + // (IndirectQueries is typically used for Values corresponding + // to source-level lvalues, e.g. an *ssa.Global.) // - // The pointer analysis will populate the corresponding - // Results.Queries value when it creates the pointer variable - // for v or *v. Upon completion the client can inspect that - // map for the results. + // The analysis populates the corresponding + // Results.{Indirect,}Queries map 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 Results.Queries slice - // may have multiple Pointers, one per distinct context. Use - // PointsToCombined to merge them. + // context-sensitively, the corresponding Results.{Indirect,}Queries + // slice may have multiple Pointers, one per distinct context. + // Use PointsToCombined to merge them. // - Queries map[ssa.Value]Indirect + // TODO(adonovan): this API doesn't scale well for batch tools + // that want to dump the entire solution. + // + // TODO(adonovan): need we distinguish contexts? Current + // clients always combine them. + // + Queries map[ssa.Value]struct{} + IndirectQueries map[ssa.Value]struct{} // If Log is non-nil, log messages are written to it. // Logging is extremely verbose. Log io.Writer } -type Indirect bool // map[ssa.Value]Indirect is not a set +// AddQuery adds v to Config.Queries. +func (c *Config) AddQuery(v ssa.Value) { + if c.Queries == nil { + c.Queries = make(map[ssa.Value]struct{}) + } + c.Queries[v] = struct{}{} +} + +// AddQuery adds v to Config.IndirectQueries. +func (c *Config) AddIndirectQuery(v ssa.Value) { + if c.IndirectQueries == nil { + c.IndirectQueries = make(map[ssa.Value]struct{}) + } + c.IndirectQueries[v] = struct{}{} +} func (c *Config) prog() *ssa.Program { for _, main := range c.Mains { @@ -87,9 +110,10 @@ type Warning struct { // 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 - Warnings []Warning // warnings of unsoundness + CallGraph call.Graph // discovered call graph + Queries map[ssa.Value][]Pointer // pts(v) for each v in Config.Queries. + IndirectQueries map[ssa.Value][]Pointer // pts(*v) for each v in Config.IndirectQueries. + Warnings []Warning // warnings of unsoundness } // A Pointer is an equivalence class of pointerlike values. @@ -163,6 +187,18 @@ type ptset struct { pts nodeset } +func (s ptset) String() string { + var buf bytes.Buffer + fmt.Fprintf(&buf, "[") + sep := "" + for l := range s.pts { + fmt.Fprintf(&buf, "%s%s", sep, s.a.labelFor(l)) + sep = ", " + } + fmt.Fprintf(&buf, "]") + return buf.String() +} + func (s ptset) Labels() []*Label { var labels []*Label for l := range s.pts { diff --git a/pointer/gen.go b/pointer/gen.go index 297ad633..502fa825 100644 --- a/pointer/gen.go +++ b/pointer/gen.go @@ -80,15 +80,17 @@ func (a *analysis) setValueNode(v ssa.Value, id nodeid, cgn *cgnode) { fmt.Fprintf(a.log, "\tval[%s] = n%d (%T)\n", v.Name(), id, v) } - // Record the (v, id) relation if the client has queried v. - if indirect, ok := a.config.Queries[v]; ok { - if indirect { - tmp := a.addNodes(v.Type(), "query.indirect") - a.genLoad(cgn, tmp, v, 0, a.sizeof(v.Type())) - id = tmp - } + // Record the (v, id) relation if the client has queried pts(v). + if _, ok := a.config.Queries[v]; ok { a.result.Queries[v] = append(a.result.Queries[v], ptr{a, cgn, id}) } + + // Record the (*v, id) relation if the client has queried pts(*v). + if _, ok := a.config.IndirectQueries[v]; ok { + indirect := a.addNodes(v.Type(), "query.indirect") + a.genLoad(cgn, indirect, v, 0, a.sizeof(v.Type())) + a.result.IndirectQueries[v] = append(a.result.IndirectQueries[v], ptr{a, cgn, indirect}) + } } // endObject marks the end of a sequence of calls to addNodes denoting diff --git a/pointer/labels.go b/pointer/labels.go index 10c8ecda..9c6ce114 100644 --- a/pointer/labels.go +++ b/pointer/labels.go @@ -18,15 +18,15 @@ import ( // channel, 'func', slice or interface. Labels include: // // Labels include: -// - functions +// - functions // - globals // - tagged objects, representing interfaces and reflect.Values -// - arrays created by literals (e.g. []byte("foo")) and conversions ([]byte(s)) -// - stack- and heap-allocated variables (including composite literals) -// - channels, maps and arrays created by make() -// - instrinsic or reflective operations that allocate (e.g. append, reflect.New) -// - instrinsic objects, e.g. the initial array behind os.Args. -// - and their subelements, e.g. "alloc.y[*].z" +// - arrays created by conversions (e.g. []byte("foo"), []byte(s)) +// - stack- and heap-allocated variables (including composite literals) +// - channels, maps and arrays created by make() +// - instrinsic or reflective operations that allocate (e.g. append, reflect.New) +// - instrinsic objects, e.g. the initial array behind os.Args. +// - and their subelements, e.g. "alloc.y[*].z" // // Labels are so varied that they defy good generalizations; // some have no value, no callgraph node, or no position.