Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/go/callgraph/cha: does not recognise reflect.ValueOf(f).Call() with args #24196

Closed
mvdan opened this issue Mar 1, 2018 · 3 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@mvdan
Copy link
Member

mvdan commented Mar 1, 2018

$ cat f.go
package main

import "reflect"

func Using(f interface{}) {
        v := reflect.ValueOf(f)
        v.Call([]reflect.Value{
                reflect.ValueOf(3),
        })
}

func Used(i int) {
        println("used")
}

func main() {
        Using(Used)
}
$ go run f.go
used
$ callgraph -algo cha f.go | grep Used
runtime.sysUsed --static-126:11-->      runtime.madvise
(*runtime.mheap).allocSpanLocked        --static-866:10-->      runtime.sysUsed
$ callgraph -algo rta f.go | grep Used
runtime.sysUsed --static-126:11-->      runtime.madvise
(*runtime.mheap).allocSpanLocked        --static-866:10-->      runtime.sysUsed

I would expect at least one edge towards Used, in both algorithms, since they both support reflection as far as I can tell.

They both give some edges (although somewhat confusing ones) if no args are used:

$ cat f.go
package main

import "reflect"

func Using(f interface{}) {
        v := reflect.ValueOf(f)
        v.Call(nil)
}

func Used() {
        println("used")
}

func main() {
        Using(Used)
}
$ callgraph -algo cha f.go | grep Used
runtime.sysUsed --static-126:11-->      runtime.madvise
(*runtime.mheap).allocSpanLocked        --static-866:10-->      runtime.sysUsed
runtime.clearpools      --dynamic-2126:14-->    main.Used
runtime.mstart1 --dynamic-1222:5-->     main.Used
runtime.recv    --dynamic-585:9-->      main.Used
runtime.send    --dynamic-287:9-->      main.Used
(*sync.Once).Do --dynamic-44:4-->       main.Used
$ callgraph -algo rta f.go | grep Used
runtime.clearpools      --dynamic-2126:14-->    main.Used
runtime.sysUsed --static-126:11-->      runtime.madvise
(*runtime.mheap).allocSpanLocked        --static-866:10-->      runtime.sysUsed
runtime.mstart1 --dynamic-1222:5-->     main.Used

/cc @alandonovan

@mvdan mvdan added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 1, 2018
@gopherbot gopherbot added this to the Unreleased milestone Mar 1, 2018
@mvdan
Copy link
Member Author

mvdan commented Apr 26, 2018

Friendly ping @alandonovan - in case you are accumulating another stack of callgraph issues to look at :)

If this indeed sounds like a bug to you, and you can give a few pointers, I can also look into writing a fix.

@alandonovan
Copy link
Contributor

I would expect at least one edge towards Used, in both algorithms, since they both support reflection as far as I can tell.

The only call-graph construction algorithm that attempts to represent edges arising from reflective calls is pointer analysis (-algo=pta), and it must be enabled by a flag that increases memory usage and running time substantially and still has known bugs and functional gaps.

The rta algorithm (rapid-type-analysis) conservatively models the ability of reflection to derive one type from another and to instantiate those types, but it does not model reflective calls. For example, in this program:

type T struct{}
func (*T) f {}

type I interface{ f() }

func g(T) {}

func main() {
    fmt.Println(g)
    var i I = ...
    i.f() // may call (*T).f
}

the call fmt.Println(g) causes the type of g to be stored in an interface. The analysis conservatively assumes that the Println function could obtain g's type, extract its parameter type T, and call reflect.New on that type, causing T to become instantiated. The analysis then assumes the call i.f() might dispatch to (*T).f because T is instantiated and *T satisfies I.

Because RTA does not model individual values, only types, if it were to handle reflect.Call it would effectively add an edge to every single address-taken function and method, which would be useless, so it doesn't.

So, working as intended.

@mvdan
Copy link
Member Author

mvdan commented May 5, 2018

That makes sense - thanks for the thorough explanation!

@golang golang locked and limited conversation to collaborators May 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants