From 2299ac6bf31d4fe46b7fb3523771921212e31558 Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Wed, 9 Oct 2013 12:41:55 -0400 Subject: [PATCH] go.tools/pointer: make sole callsite available to intrinsics in non-shared contours. This information can be used to specialize such calls, e.g. - report location of unsound calls (done for reflect.NewAt) - exploit argument information (done for constant 'dir' parameter to reflect.ChanOf) + tests. R=crawshaw CC=golang-dev https://golang.org/cl/14517046 --- pointer/callgraph.go | 7 +-- pointer/gen.go | 82 +++++++++++++++------------------ pointer/intrinsics.go | 2 + pointer/reflect.go | 41 +++++++++++++---- pointer/testdata/chanreflect.go | 14 ++---- pointer/testdata/reflect.go | 4 +- 6 files changed, 83 insertions(+), 67 deletions(-) diff --git a/pointer/callgraph.go b/pointer/callgraph.go index d209b98b..b5cedfab 100644 --- a/pointer/callgraph.go +++ b/pointer/callgraph.go @@ -34,9 +34,10 @@ func (g *cgraph) Root() call.GraphNode { // cgnode implements call.GraphNode. type cgnode struct { - fn *ssa.Function - obj nodeid // start of this contour's object block - sites []*callsite // ordered list of callsites within this function + fn *ssa.Function + obj nodeid // start of this contour's object block + sites []*callsite // ordered list of callsites within this function + callersite *callsite // where called from, if known; nil for shared contours } func (n *cgnode) Func() *ssa.Function { diff --git a/pointer/gen.go b/pointer/gen.go index 154cde6b..8c82cf37 100644 --- a/pointer/gen.go +++ b/pointer/gen.go @@ -115,18 +115,21 @@ func (a *analysis) endObject(obj nodeid, cgn *cgnode, data interface{}) *object return o } -// makeFunctionObject creates and returns a new function object for -// fn, and returns the id of its first node. It also enqueues fn for -// subsequent constraint generation. +// makeFunctionObject creates and returns a new function object +// (contour) for fn, and returns the id of its first node. It also +// enqueues fn for subsequent constraint generation. // -func (a *analysis) makeFunctionObject(fn *ssa.Function) nodeid { +// For a context-sensitive contour, callersite identifies the sole +// callsite; for shared contours, caller is nil. +// +func (a *analysis) makeFunctionObject(fn *ssa.Function, callersite *callsite) nodeid { if a.log != nil { fmt.Fprintf(a.log, "\t---- makeFunctionObject %s\n", fn) } // obj is the function object (identity, params, results). obj := a.nextNode() - cgn := a.makeCGNode(fn, obj) + cgn := a.makeCGNode(fn, obj, callersite) sig := fn.Signature a.addOneNode(sig, "func.cgnode", nil) // (scalar with Signature type) if recv := sig.Recv(); recv != nil { @@ -568,18 +571,15 @@ func (a *analysis) shouldUseContext(fn *ssa.Function) bool { return true } -// genStaticCall generates constraints for a statically dispatched -// function call. It returns a node whose pts() will be the set of -// possible call targets (in this case, a singleton). -// -func (a *analysis) genStaticCall(call *ssa.CallCommon, result nodeid) nodeid { +// genStaticCall generates constraints for a statically dispatched function call. +func (a *analysis) genStaticCall(caller *cgnode, site *callsite, call *ssa.CallCommon, result nodeid) { // Ascertain the context (contour/CGNode) for a particular call. var obj nodeid fn := call.StaticCallee() if a.shouldUseContext(fn) { - obj = a.makeFunctionObject(fn) // new contour for this call + obj = a.makeFunctionObject(fn, site) // new contour } else { - obj = a.objectNode(nil, fn) // ordinary (shared) contour + obj = a.objectNode(nil, fn) // shared contour } sig := call.Signature() @@ -609,13 +609,12 @@ func (a *analysis) genStaticCall(call *ssa.CallCommon, result nodeid) nodeid { a.copy(result, a.funcResults(obj), a.sizeof(sig.Results())) } - return targets + // pts(targets) will be the (singleton) set of possible call targets. + site.targets = targets } // genDynamicCall generates constraints for a dynamic function call. -// It returns a node whose pts() will be the set of possible call targets. -// -func (a *analysis) genDynamicCall(caller *cgnode, call *ssa.CallCommon, result nodeid) nodeid { +func (a *analysis) genDynamicCall(caller *cgnode, site *callsite, call *ssa.CallCommon, result nodeid) { fn := a.valueNode(call.Value) sig := call.Signature() @@ -632,22 +631,24 @@ func (a *analysis) genDynamicCall(caller *cgnode, call *ssa.CallCommon, result n if result != 0 { a.genLoad(caller, result, call.Value, offset, a.sizeof(sig.Results())) } - return fn + + // pts(targets) will be the (singleton) set of possible call targets. + site.targets = fn } // genInvoke generates constraints for a dynamic method invocation. -// It returns a node whose pts() will be the set of possible call targets. -// -func (a *analysis) genInvoke(call *ssa.CallCommon, result nodeid) nodeid { +func (a *analysis) genInvoke(caller *cgnode, site *callsite, call *ssa.CallCommon, result nodeid) { if call.Value.Type() == a.reflectType { - return a.genInvokeReflectType(call, result) + a.genInvokeReflectType(caller, site, call, result) + return } sig := call.Signature() // Allocate a contiguous targets/params/results block for this call. block := a.nextNode() - targets := a.addOneNode(sig, "invoke.targets", nil) + // pts(targets) will be the set of possible call targets + site.targets = a.addOneNode(sig, "invoke.targets", nil) p := a.addNodes(sig.Params(), "invoke.params") r := a.addNodes(sig.Results(), "invoke.results") @@ -666,8 +667,6 @@ func (a *analysis) genInvoke(call *ssa.CallCommon, result nodeid) nodeid { // edges from the caller's P/R block to the callee's // P/R block for each discovered call target. a.addConstraint(&invokeConstraint{call.Method, a.valueNode(call.Value), block}) - - return targets } // genInvokeReflectType is a specialization of genInvoke where the @@ -684,10 +683,7 @@ func (a *analysis) genInvoke(call *ssa.CallCommon, result nodeid) nodeid { // as this: // rt.(*reflect.rtype).F() // -// It returns a node whose pts() will be the (singleton) set of -// possible call targets. -// -func (a *analysis) genInvokeReflectType(call *ssa.CallCommon, result nodeid) nodeid { +func (a *analysis) genInvokeReflectType(caller *cgnode, site *callsite, call *ssa.CallCommon, result nodeid) { // Unpack receiver into rtype rtype := a.addOneNode(a.reflectRtypePtr, "rtype.recv", nil) recv := a.valueNode(call.Value) @@ -697,7 +693,10 @@ func (a *analysis) genInvokeReflectType(call *ssa.CallCommon, result nodeid) nod meth := a.reflectRtypePtr.MethodSet().Lookup(call.Method.Pkg(), call.Method.Name()) fn := a.prog.Method(meth) - obj := a.makeFunctionObject(fn) // new contour for this call + obj := a.makeFunctionObject(fn, site) // new contour for this call + + // pts(targets) will be the (singleton) set of possible call targets. + site.targets = obj // From now on, it's essentially a static call, but little is // gained by factoring together the code for both cases. @@ -723,8 +722,6 @@ func (a *analysis) genInvokeReflectType(call *ssa.CallCommon, result nodeid) nod if result != 0 { a.copy(result, a.funcResults(obj), a.sizeof(sig.Results())) } - - return obj } // genCall generates contraints for call instruction instr. @@ -742,22 +739,19 @@ func (a *analysis) genCall(caller *cgnode, instr ssa.CallInstruction) { result = a.valueNode(v) } - // The node whose pts(·) will contain all targets of the call. - var targets nodeid + site := &callsite{instr: instr} + switch { case call.StaticCallee() != nil: - targets = a.genStaticCall(call, result) + a.genStaticCall(caller, site, call, result) case call.IsInvoke(): - targets = a.genInvoke(call, result) + a.genInvoke(caller, site, call, result) default: - targets = a.genDynamicCall(caller, call, result) + a.genDynamicCall(caller, site, call, result) } - site := &callsite{ - targets: targets, - instr: instr, - } caller.sites = append(caller.sites, site) + if a.log != nil { fmt.Fprintf(a.log, "\t%s to targets %s from %s\n", site, site.targets, caller) } @@ -791,7 +785,7 @@ func (a *analysis) objectNode(cgn *cgnode, v ssa.Value) nodeid { a.endObject(obj, nil, v) case *ssa.Function: - obj = a.makeFunctionObject(v) + obj = a.makeFunctionObject(v, nil) case *ssa.Const: if t, ok := v.Type().Underlying().(*types.Slice); ok && !v.IsNil() { @@ -1075,8 +1069,8 @@ func (a *analysis) genInstr(cgn *cgnode, instr ssa.Instruction) { } } -func (a *analysis) makeCGNode(fn *ssa.Function, obj nodeid) *cgnode { - cgn := &cgnode{fn: fn, obj: obj} +func (a *analysis) makeCGNode(fn *ssa.Function, obj nodeid, callersite *callsite) *cgnode { + cgn := &cgnode{fn: fn, obj: obj, callersite: callersite} a.cgnodes = append(a.cgnodes, cgn) return cgn } @@ -1090,7 +1084,7 @@ func (a *analysis) genRootCalls() *cgnode { r.Prog = a.prog // hack. r.Enclosing = r // hack, so Function.String() doesn't crash r.String() // (asserts that it doesn't crash) - root := a.makeCGNode(r, 0) + root := a.makeCGNode(r, 0, nil) // For each main package, call main.init(), main.main(). for _, mainPkg := range a.config.Mains { diff --git a/pointer/intrinsics.go b/pointer/intrinsics.go index 917701f2..762429e0 100644 --- a/pointer/intrinsics.go +++ b/pointer/intrinsics.go @@ -21,6 +21,8 @@ import ( // Instances of 'intrinsic' generate analysis constraints for calls to // intrinsic functions. +// Implementations may exploit information from the calling site +// via cgn.callersite; for shared contours this is nil. type intrinsic func(a *analysis, cgn *cgnode) // Initialized in explicit init() to defeat (spurious) initialization diff --git a/pointer/reflect.go b/pointer/reflect.go index 591bc613..b305ae39 100644 --- a/pointer/reflect.go +++ b/pointer/reflect.go @@ -22,8 +22,11 @@ package pointer import ( "fmt" "go/ast" + "reflect" + "code.google.com/p/go.tools/go/exact" "code.google.com/p/go.tools/go/types" + "code.google.com/p/go.tools/ssa" ) // -------------------- (reflect.Value) -------------------- @@ -362,11 +365,12 @@ func ext۰reflect۰Copy(a *analysis, cgn *cgnode) {} // ---------- func ChanOf(ChanDir, Type) Type ---------- -// result = ChanOf(_, t) +// result = ChanOf(dir, t) type reflectChanOfConstraint struct { cgn *cgnode t nodeid // (ptr) result nodeid + dirs []ast.ChanDir } func (c *reflectChanOfConstraint) String() string { @@ -381,9 +385,8 @@ func (c *reflectChanOfConstraint) solve(a *analysis, _ *node, delta nodeset) { changed := false for tObj := range delta { T := a.rtypeTaggedValue(tObj) - // TODO(adonovan): use only the channel direction - // provided at the callsite, if constant. - for _, dir := range []ast.ChanDir{1, 2, 3} { + + for _, dir := range c.dirs { if a.addLabel(c.result, a.makeRtype(types.NewChan(dir, T))) { changed = true } @@ -394,12 +397,34 @@ func (c *reflectChanOfConstraint) solve(a *analysis, _ *node, delta nodeset) { } } +// dirMap maps reflect.ChanDir to the set of channel types generated by ChanOf. +var dirMap = [...][]ast.ChanDir{ + 0: {ast.RECV, ast.SEND, ast.RECV | ast.SEND}, // unknown + reflect.RecvDir: {ast.RECV}, + reflect.SendDir: {ast.SEND}, + reflect.BothDir: {ast.RECV | ast.SEND}, +} + func ext۰reflect۰ChanOf(a *analysis, cgn *cgnode) { + // If we have access to the callsite, + // and the channel argument is a constant (as is usual), + // only generate the requested direction. + var dir reflect.ChanDir // unknown + if site := cgn.callersite; site != nil { + if c, ok := site.instr.Common().Args[0].(*ssa.Const); ok { + v, _ := exact.Int64Val(c.Value) + if 0 <= v && v <= int64(reflect.BothDir) { + dir = reflect.ChanDir(v) + } + } + } + params := a.funcParams(cgn.obj) a.addConstraint(&reflectChanOfConstraint{ cgn: cgn, t: params + 1, result: a.funcResults(cgn.obj), + dirs: dirMap[dir], }) } @@ -617,10 +642,10 @@ func ext۰reflect۰New(a *analysis, cgn *cgnode) { func ext۰reflect۰NewAt(a *analysis, cgn *cgnode) { ext۰reflect۰New(a, cgn) - // TODO(adonovan): make it easier to report errors of this form, - // which includes the callsite: - // a.warnf("unsound: main.reflectNewAt contains a reflect.NewAt() call") - a.warnf(cgn.Func().Pos(), "unsound: reflect.NewAt() call") + // TODO(adonovan): also report dynamic calls to unsound intrinsics. + if site := cgn.callersite; site != nil { + a.warnf(site.pos(), "unsound: %s contains a reflect.NewAt() call", site.instr.Parent()) + } } func ext۰reflect۰PtrTo(a *analysis, cgn *cgnode) {} diff --git a/pointer/testdata/chanreflect.go b/pointer/testdata/chanreflect.go index 9f68e841..5f3b1138 100644 --- a/pointer/testdata/chanreflect.go +++ b/pointer/testdata/chanreflect.go @@ -26,29 +26,25 @@ func chanreflect2() { print(r.Interface().(*int)) // @pointsto main.b } -// TODO(adonovan): the analysis can't yet take advantage of the -// ChanOf(dir) parameter so the results are less precise than they -// should be: all three directions are returned. - func chanOfRecv() { // MakeChan(<-chan) is a no-op. t := reflect.ChanOf(reflect.RecvDir, reflect.TypeOf(&a)) - print(reflect.Zero(t).Interface()) // @types <-chan *int | chan<- *int | chan *int + print(reflect.Zero(t).Interface()) // @types <-chan *int print(reflect.MakeChan(t, 0).Interface().(<-chan *int)) // @pointsto - print(reflect.MakeChan(t, 0).Interface().(chan *int)) // @pointsto + print(reflect.MakeChan(t, 0).Interface().(chan *int)) // @pointsto } func chanOfSend() { // MakeChan(chan<-) is a no-op. t := reflect.ChanOf(reflect.SendDir, reflect.TypeOf(&a)) - print(reflect.Zero(t).Interface()) // @types <-chan *int | chan<- *int | chan *int + print(reflect.Zero(t).Interface()) // @types chan<- *int print(reflect.MakeChan(t, 0).Interface().(chan<- *int)) // @pointsto - print(reflect.MakeChan(t, 0).Interface().(chan *int)) // @pointsto + print(reflect.MakeChan(t, 0).Interface().(chan *int)) // @pointsto } func chanOfBoth() { t := reflect.ChanOf(reflect.BothDir, reflect.TypeOf(&a)) - print(reflect.Zero(t).Interface()) // @types <-chan *int | chan<- *int | chan *int + print(reflect.Zero(t).Interface()) // @types chan *int ch := reflect.MakeChan(t, 0) print(ch.Interface().(chan *int)) // @pointsto ch.Send(reflect.ValueOf(&b)) diff --git a/pointer/testdata/reflect.go b/pointer/testdata/reflect.go index 306f7649..ad494c98 100644 --- a/pointer/testdata/reflect.go +++ b/pointer/testdata/reflect.go @@ -21,9 +21,7 @@ func reflectNewAt() { print(reflect.NewAt(reflect.TypeOf(3), unsafe.Pointer(&x)).Interface()) // @types *int } -// TODO(adonovan): report the location of the caller, not NewAt. -// #warning "unsound: main.reflectNewAt contains a reflect.NewAt.. call" -// @warning "unsound: reflect.NewAt.. call" +// @warning "unsound: main.reflectNewAt contains a reflect.NewAt.. call" func reflectTypeOf() { t := reflect.TypeOf(3)