diff --git a/oracle/describe.go b/oracle/describe.go index 49ce9b9d..0a932459 100644 --- a/oracle/describe.go +++ b/oracle/describe.go @@ -342,10 +342,14 @@ func ssaValueForExpr(o *oracle, path []ast.Node) (ssa.Value, error) { func describeValue(o *oracle, path []ast.Node) (*describeValueResult, error) { var expr ast.Expr + var obj types.Object switch n := path[0].(type) { case *ast.ValueSpec: // ambiguous ValueSpec containing multiple names return nil, o.errorf(n, "multiple value specification") + case *ast.Ident: + obj = o.queryPkgInfo.ObjectOf(n) + expr = n case ast.Expr: expr = n default: @@ -353,6 +357,9 @@ func describeValue(o *oracle, path []ast.Node) (*describeValueResult, error) { return nil, o.errorf(n, "unexpected AST for expr: %T", n) } + typ := o.queryPkgInfo.TypeOf(expr) + constVal := o.queryPkgInfo.ValueOf(expr) + // From this point on, we cannot fail with an error. // Failure to run the pointer analysis will be reported later. // @@ -365,63 +372,31 @@ func describeValue(o *oracle, path []ast.Node) (*describeValueResult, error) { // - ok: Pointer has non-empty points-to set // ptaErr is non-nil only in the "error:" cases. - var value ssa.Value var ptaErr error - var obj types.Object - - // Determine the ssa.Value for the expression. - if id, ok := expr.(*ast.Ident); ok { - // def/ref of func/var/const object - obj = o.queryPkgInfo.ObjectOf(id) - value, ptaErr = ssaValueForIdent(o, obj, path) - } else { - // any other expression - if o.queryPkgInfo.ValueOf(expr) == nil { // non-constant? - value, ptaErr = ssaValueForExpr(o, path) - } - } - - // Don't run pointer analysis on non-pointerlike types. - if value != nil && !pointer.CanPoint(value.Type()) { - value = nil - } - - // Run pointer analysis of the selected SSA value. var ptrs []pointerResult - if value != nil { - buildSSA(o) - o.config.QueryValues = map[ssa.Value][]pointer.Pointer{value: nil} - ptrAnalysis(o) - - // Combine the PT sets from all contexts. - pointers := o.config.QueryValues[value] - if pointers == nil { - ptaErr = fmt.Errorf("PTA did not encounter this expression (dead code?)") - } - pts := pointer.PointsToCombined(pointers) - - if _, ok := value.Type().Underlying().(*types.Interface); ok { - // Show concrete types for interface expression. - if concs := pts.ConcreteTypes(); concs.Len() > 0 { - concs.Iterate(func(conc types.Type, pta interface{}) { - combined := pointer.PointsToCombined(pta.([]pointer.Pointer)) - labels := combined.Labels() - sort.Sort(byPosAndString(labels)) // to ensure determinism - ptrs = append(ptrs, pointerResult{conc, labels}) - }) - } + // Only run pointer analysis on pointerlike expression types. + if pointer.CanPoint(typ) { + // Determine the ssa.Value for the expression. + var value ssa.Value + if obj != nil { + // def/ref of func/var/const object + value, ptaErr = ssaValueForIdent(o, obj, path) } else { - // Show labels for other expressions. - labels := pts.Labels() - sort.Sort(byPosAndString(labels)) // to ensure determinism - ptrs = append(ptrs, pointerResult{value.Type(), labels}) + // any other expression + if o.queryPkgInfo.ValueOf(path[0].(ast.Expr)) == nil { // non-constant? + value, ptaErr = ssaValueForExpr(o, path) + } + } + if value != nil { + // TODO(adonovan): IsIdentical may be too strict; + // perhaps we need is-assignable or even + // has-same-underlying-representation? + indirect := types.IsIdentical(types.NewPointer(typ), value.Type()) + + ptrs, ptaErr = describePointer(o, value, indirect) } } - sort.Sort(byTypeString(ptrs)) // to ensure determinism - - typ := o.queryPkgInfo.TypeOf(expr) - constVal := o.queryPkgInfo.ValueOf(expr) return &describeValueResult{ expr: expr, @@ -433,6 +408,41 @@ func describeValue(o *oracle, path []ast.Node) (*describeValueResult, error) { }, nil } +// describePointer runs the pointer analysis of the selected SSA value. +func describePointer(o *oracle, v ssa.Value, indirect bool) (ptrs []pointerResult, err error) { + buildSSA(o) + + // TODO(adonovan): don't run indirect pointer analysis on non-ptr-ptrlike types. + o.config.QueryValues = map[ssa.Value]pointer.Indirect{v: pointer.Indirect(indirect)} + ptrAnalysis(o) + + // Combine the PT sets from all contexts. + pointers := o.config.QueryResults[v] + if pointers == nil { + return nil, fmt.Errorf("PTA did not encounter this expression (dead code?)") + } + pts := pointer.PointsToCombined(pointers) + + if _, ok := v.Type().Underlying().(*types.Interface); ok { + // Show concrete types for interface expression. + if concs := pts.ConcreteTypes(); concs.Len() > 0 { + concs.Iterate(func(conc types.Type, pta interface{}) { + combined := pointer.PointsToCombined(pta.([]pointer.Pointer)) + labels := combined.Labels() + sort.Sort(byPosAndString(labels)) // to ensure determinism + ptrs = append(ptrs, pointerResult{conc, labels}) + }) + } + } else { + // Show labels for other expressions. + labels := pts.Labels() + sort.Sort(byPosAndString(labels)) // to ensure determinism + ptrs = append(ptrs, pointerResult{v.Type(), labels}) + } + sort.Sort(byTypeString(ptrs)) // to ensure determinism + return ptrs, nil +} + type pointerResult struct { typ types.Type // type of the pointer (always concrete) labels []*pointer.Label diff --git a/oracle/peers.go b/oracle/peers.go index 07e807e3..23dd1bef 100644 --- a/oracle/peers.go +++ b/oracle/peers.go @@ -58,11 +58,11 @@ func peers(o *oracle) (queryResult, error) { // ignore both directionality and type names. queryType := queryOp.ch.Type() queryElemType := queryType.Underlying().(*types.Chan).Elem() - channels := map[ssa.Value][]pointer.Pointer{queryOp.ch: nil} + channels := map[ssa.Value]pointer.Indirect{queryOp.ch: false} i := 0 for _, op := range ops { if types.IsIdentical(op.ch.Type().Underlying().(*types.Chan).Elem(), queryElemType) { - channels[op.ch] = nil + channels[op.ch] = false ops[i] = op i++ } @@ -74,7 +74,7 @@ func peers(o *oracle) (queryResult, error) { ptrAnalysis(o) // Combine the PT sets from all contexts. - queryChanPts := pointer.PointsToCombined(channels[queryOp.ch]) + queryChanPts := pointer.PointsToCombined(o.config.QueryResults[queryOp.ch]) // Ascertain which make(chan) labels the query's channel can alias. var makes []token.Pos @@ -86,7 +86,7 @@ func peers(o *oracle) (queryResult, error) { // Ascertain which send/receive operations can alias the same make(chan) labels. var sends, receives []token.Pos for _, op := range ops { - for _, ptr := range o.config.QueryValues[op.ch] { + for _, ptr := range o.config.QueryResults[op.ch] { if ptr != nil && ptr.PointsTo().Intersects(queryChanPts) { if op.dir == ast.SEND { sends = append(sends, op.pos) diff --git a/oracle/testdata/src/main/describe.go b/oracle/testdata/src/main/describe.go index 52629170..91d2c168 100644 --- a/oracle/testdata/src/main/describe.go +++ b/oracle/testdata/src/main/describe.go @@ -12,6 +12,8 @@ const pi = 3.141 // @describe const-def-pi "pi" const pie = cake(pi) // @describe const-def-pie "pie" const _ = pi // @describe const-ref-pi "pi" +var global = new(string) // NB: ssa.Global is indirect, i.e. **string + func main() { // @describe func-def-main "main" // func objects _ = main // @describe func-ref-main "main" @@ -27,7 +29,8 @@ func main() { // @describe func-def-main "main" anon := func() { _ = d // @describe ref-lexical-d "d" } - _ = anon // @describe ref-anon "anon" + _ = anon // @describe ref-anon "anon" + _ = global // @describe ref-global "global" // SSA affords some local flow sensitivity. var a, b int @@ -49,7 +52,6 @@ func main() { // @describe func-def-main "main" print(real(1+2i) - 3) // @describe const-expr2 "real.*3" m := map[string]*int{"a": &a} - // TODO(adonovan): fix spurious error in map-lookup,ok result. mapval, _ := m["a"] // @describe map-lookup,ok "m..a.." _ = mapval // @describe mapval "mapval" _ = m // @describe m "m" diff --git a/oracle/testdata/src/main/describe.golden b/oracle/testdata/src/main/describe.golden index 70376e71..a58caefd 100644 --- a/oracle/testdata/src/main/describe.golden +++ b/oracle/testdata/src/main/describe.golden @@ -8,6 +8,7 @@ definition of package "describe" method (describe.I) f() type cake float64 func deadcode func() + var global *string func init func() var init$guard bool func main func() @@ -74,7 +75,13 @@ defined here reference to var anon func() defined here value may point to these labels: - func@27.10 + func@29.10 + +-------- @describe ref-global -------- +reference to var global *string +defined here +value may point to these labels: + new -------- @describe var-def-x-1 -------- definition of var x *int @@ -126,7 +133,6 @@ binary - operation of constant value -2 -------- @describe map-lookup,ok -------- index expression of type (*int, bool) -no points-to information: can't locate SSA Value for expression in describe.main -------- @describe mapval -------- reference to var mapval *int diff --git a/oracle/testdata/src/main/imports.golden b/oracle/testdata/src/main/imports.golden index b482fb4e..fdfef240 100644 --- a/oracle/testdata/src/main/imports.golden +++ b/oracle/testdata/src/main/imports.golden @@ -17,8 +17,6 @@ defined here -------- @describe ref-var -------- reference to var Var int defined here -value may point to these labels: - lib.Var -------- @describe ref-type -------- reference to type lib.Type diff --git a/pointer/api.go b/pointer/api.go index 37c9e9e5..4a8b9264 100644 --- a/pointer/api.go +++ b/pointer/api.go @@ -64,22 +64,29 @@ type Config struct { // Print func(site *ssa.CallCommon, p Pointer) - // The client populates QueryValues with {v, nil} for each - // ssa.Value v of interest. The pointer analysis will - // populate the corresponding map value when it creates the - // pointer variable for v. Upon completion the client can - // inspect the map for the results. + // The client populates QueryValues[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 + // typically wanted for Values corresponding to source-level + // 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 + // map for the results. // // If a Value belongs to a function that the analysis treats - // context-sensitively, the corresponding slice may have - // multiple Pointers, one per distinct context. - // Use PointsToCombined to merge them. + // context-sensitively, the corresponding QueryResults slice + // may have multiple Pointers, one per distinct context. Use + // PointsToCombined to merge them. // - // TODO(adonovan): separate the keys set (input) from the - // key/value associations (result) and perhaps return the - // latter from Analyze(). + // TODO(adonovan): refactor the API: separate all results of + // Analyze() into a dedicated Result struct. // - QueryValues map[ssa.Value][]Pointer + QueryValues map[ssa.Value]Indirect + QueryResults map[ssa.Value][]Pointer // -------- Other configuration options -------- @@ -88,6 +95,8 @@ type Config struct { Log io.Writer } +type Indirect bool // map[ssa.Value]Indirect is not a set + func (c *Config) prog() *ssa.Program { for _, main := range c.Mains { return main.Prog diff --git a/pointer/gen.go b/pointer/gen.go index 0d2e1bde..2c00053a 100644 --- a/pointer/gen.go +++ b/pointer/gen.go @@ -72,9 +72,18 @@ func (a *analysis) setValueNode(v ssa.Value, id nodeid) { } // Record the (v, id) relation if the client has queried v. - qv := a.config.QueryValues - if ptrs, ok := qv[v]; ok { - qv[v] = append(ptrs, ptr{a, id}) + if indirect, ok := a.config.QueryValues[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}) } }