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

runtime: <autogenerated> elimination breaks some uses of `runtime.CallersFrames` #22231

Closed
aclements opened this issue Oct 12, 2017 · 8 comments

Comments

Projects
None yet
4 participants
@aclements
Copy link
Member

commented Oct 12, 2017

What version of Go are you using (go version)?

go version devel +67388e9866 Thu Oct 12 01:03:14 2017 +0000 linux/amd64

Does this issue reproduce with the latest release?

No. This was introduced on master after Go 1.9 by CL 45412, which fixed #16723.

What did you do?

https://play.golang.org/p/3rfhN6rO-_

What did you expect to see?

Prior to CL 45412, this would print the function information for the autogenerated wrapper I.M.

What did you see instead?

On master, Next returns no frames because the stack consists solely of an autogenerated wrapper, which we now hide from stack traces.

We started hiding autogenerated wrappers because, in any real stack trace, the preceding entry would be the actual method that the user thought they were calling. In this case, this logic fails because the stack is synthetic and consists solely of the wrapper.

Perhaps we shouldn't hide wrappers if they're the first entry in the stack trace.

There is a simple workaround. Code like this should be using runtime.FuncForPC anyway. But we shouldn't needlessly break things.

@aclements aclements added this to the Go1.10 milestone Oct 12, 2017

@aclements aclements self-assigned this Oct 12, 2017

@bcmills

This comment has been minimized.

Copy link
Member

commented Oct 12, 2017

I suspect this could interact badly with embedded methods too.

For example, the program in https://play.golang.org/p/xeyW_fDeo9 dereferences a nil pointer in an autogenerated method (see #18617), which might result in a panic trace showing no frames.

@bcmills

This comment has been minimized.

Copy link
Member

commented Oct 12, 2017

(And the workaround of using runtime.FuncForPC would not work for the nil-panic case.)

@aclements

This comment has been minimized.

Copy link
Member Author

commented Oct 13, 2017

That's a good example. Amusingly, when run on tip we get a larger stack trace because the first attempt doesn't get any frames, so we retry with runtime frames enabled. But that's clearly not ideal. :)

@s4l1h

This comment has been minimized.

Copy link

commented Oct 30, 2017

I have some issue latest version.

1.9.2 => called from # 1

devel => called from # 0

▶ tree
.
├── main.go
└── runtimetest
└── init.go

1 directory, 2 files

main.go

package main
import (
	"fmt"
	t "./runtimetest"
)
func main() {
	fmt.Println(t.File)
}

runtimetest/init.go


package runtimetest

import (
	"fmt"
	"runtime"
)

var File string

func init() {

	_, file, no, ok := runtime.Caller(1)
	if ok {
		fmt.Printf("called from %s#%d\n", file, no)
	}
	File = file
}

brew install go

▶ go version
go version go1.9.2 darwin/amd64
▶ go run main.go
called from <autogenerated>#1

brew install --HEAD go

▶ go version
go version devel +0153a4130d Mon Oct 30 19:55:02 2017 +0000 darwin/amd64
▶ go run main.go
called from #0
@aclements

This comment has been minimized.

Copy link
Member Author

commented Oct 30, 2017

@s4l1h, what's the issue? init doesn't have a caller, so arguably HEAD is giving the right answer and 1.9 wasn't.

@s4l1h

This comment has been minimized.

Copy link

commented Oct 30, 2017

@aclements
I want to get filename with runtime.
if change the caller skip param 1 to 0.
1.8rc3, 1.9.2 and head version work perfect.

runtime.Caller(1) => runtime.Caller(0)

https://github.com/akmyazilim/assetmanager/blob/master/example/modules/t1/init.go#L17

@aclements

This comment has been minimized.

Copy link
Member Author

commented Oct 31, 2017

if change the caller skip param 1 to 0.

Right. I'm still unclear on what you were expecting. Using Caller(1) in an init function is definitely depending on an implementation detail that could easily change for any number of reasons. If you want to find out information on the function that calls Caller(), you should use a skip of 0.

@gopherbot

This comment has been minimized.

Copy link

commented Nov 9, 2017

Change https://golang.org/cl/76770 mentions this issue: runtime: don't elide wrapper functions that call panic or at TOS

@gopherbot gopherbot closed this in 032678e Nov 13, 2017

@golang golang locked and limited conversation to collaborators Nov 13, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.