go.tools/oracle: describe: query content of lvalues, not their address.

Background: some ssa.Values represent lvalues, e.g.
      var g = new(string)
the *ssa.Global g is a **string, the address of what users
think of as the global g.

Querying pts(g) returns a singleton containing the object g, a
*string.  What users really want to see is what that in turn
points to, i.e. the label for the call to new().

This change now lets users make "indirect" pointer queries,
i.e. for pts(*v) where v is an ssa.Value.  The oracle makes an
indirect query if the type of the ssa.Value differs from the
source expression type by a pointer, i.e. it's an lvalue.

In other words, we're hiding the fact that compilers (e.g. ssa) internally represent globals by their address.

+ Tests.

This serendipitously fixed an outstanding bug mentioned in the
describe.go

R=crawshaw
CC=golang-dev
https://golang.org/cl/13532043

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,43 +372,58 @@ 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
+	var ptrs []pointerResult
 
+	// Only run pointer analysis on pointerlike expression types.
+	if pointer.CanPoint(typ) {
 		// Determine the ssa.Value for the expression.
-	if id, ok := expr.(*ast.Ident); ok {
+		var value ssa.Value
+		if obj != nil {
 			// 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?
+			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())
 
-	// Don't run pointer analysis on non-pointerlike types.
+			ptrs, ptaErr = describePointer(o, value, indirect)
-	if value != nil && !pointer.CanPoint(value.Type()) {
+		}
-		value = nil
 	}
 
-	// Run pointer analysis of the selected SSA value.
+	return &describeValueResult{
-	var ptrs []pointerResult
+		expr:     expr,
-	if value != nil {
+		typ:      typ,
+		constVal: constVal,
+		obj:      obj,
+		ptaErr:   ptaErr,
+		ptrs:     ptrs,
+	}, 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)
 
-		o.config.QueryValues = map[ssa.Value][]pointer.Pointer{value: nil}
+	// 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.QueryValues[value]
+	pointers := o.config.QueryResults[v]
 	if pointers == nil {
-			ptaErr = fmt.Errorf("PTA did not encounter this expression (dead code?)")
+		return nil, fmt.Errorf("PTA did not encounter this expression (dead code?)")
 	}
 	pts := pointer.PointsToCombined(pointers)
 
-		if _, ok := value.Type().Underlying().(*types.Interface); ok {
+	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{}) {
@@ -415,22 +437,10 @@ func describeValue(o *oracle, path []ast.Node) (*describeValueResult, error) {
 		// Show labels for other expressions.
 		labels := pts.Labels()
 		sort.Sort(byPosAndString(labels)) // to ensure determinism
-			ptrs = append(ptrs, pointerResult{value.Type(), labels})
+		ptrs = append(ptrs, pointerResult{v.Type(), labels})
-		}
 	}
 	sort.Sort(byTypeString(ptrs)) // to ensure determinism
-
+	return ptrs, nil
-	typ := o.queryPkgInfo.TypeOf(expr)
-	constVal := o.queryPkgInfo.ValueOf(expr)
-
-	return &describeValueResult{
-		expr:     expr,
-		typ:      typ,
-		constVal: constVal,
-		obj:      obj,
-		ptaErr:   ptaErr,
-		ptrs:     ptrs,
-	}, nil
 }
 
 type pointerResult struct {

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)

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"
@@ -28,6 +30,7 @@ func main() { // @describe func-def-main "main"
 		_ = d // @describe ref-lexical-d "d"
 	}
 	_ = 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"

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

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

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
+	// The client populates QueryValues[v] for each ssa.Value v
-	// ssa.Value v of interest.  The pointer analysis will
+	// of interest.
-	// populate the corresponding map value when it creates the
+	//
-	// pointer variable for v.  Upon completion the client can
+	// The boolean (Indirect) indicates whether to compute the
-	// inspect the map for the results.
+	// 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
+	// context-sensitively, the corresponding QueryResults slice
-	// multiple Pointers, one per distinct context.
+	// may have multiple Pointers, one per distinct context.  Use
-	// Use PointsToCombined to merge them.
+	// PointsToCombined to merge them.
 	//
-	// TODO(adonovan): separate the keys set (input) from the
+	// TODO(adonovan): refactor the API: separate all results of
-	// key/value associations (result) and perhaps return the
+	// Analyze() into a dedicated Result struct.
-	// latter from Analyze().
 	//
-	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

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 indirect, ok := a.config.QueryValues[v]; ok {
-	if ptrs, ok := qv[v]; ok {
+		if indirect {
-		qv[v] = append(ptrs, ptr{a, id})
+			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})
 	}
 }