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.

Sign up for GitHub

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

Jump to bottom

runtime: SetCgoTraceback drops frames from tracebacks #29034

Closed
mpx opened this issue Nov 30, 2018 · 7 comments
Closed

runtime: SetCgoTraceback drops frames from tracebacks #29034

mpx opened this issue Nov 30, 2018 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@mpx
Copy link
Contributor

mpx commented Nov 30, 2018

go version devel +2140975ebd Thu Nov 29 22:23:02 2018 +0000 linux/amd64

Using runtime.SetCgoTraceback along with github.com/ianlancetaylor/cgosymbolizer to record profiles that span Go and C code causes the bottom frames from tracebacks to be lost. Each additional C function on the stack causes another function frame to be lost from the bottom of the traceback.

Example:

package main

import (
    "os"
    "runtime/pprof"

    _ "github.com/ianlancetaylor/cgosymbolizer"
)

/*
#cgo CFLAGS: -O0

void run() {
    int i;
    for (i = 0; i < 1000000000; i++);
}
*/
import "C"

func main() {
    f, _ := os.Create("cpu.prof")
    defer f.Close()
    pprof.StartCPUProfile(f)
    C.run()
    pprof.StopCPUProfile()
    f.Close()
}

Running with go build && ./example && go tool pprof -traces cpu.prof gives the following:

run   
_cgo_e4e719602ade_Cfunc_run
runtime.asmcgocall
runtime.cgocall
main._Cfunc_run

main.main and runtime.main have been lost from the bottom of the trace.
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/151917 mentions this issue: runtime: fix CGO traceback frame count

@mpx
Copy link
Contributor Author

mpx commented Nov 30, 2018

Cc @ianlancetaylor

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/151918 mentions this issue: cmd/cgo: add test for lost frames via SetCgoTraceback

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 30, 2018
@andybons
Copy link
Member

@mpx is this a regression from 1.11?

@andybons andybons added this to the Go1.12 milestone Nov 30, 2018
@mpx
Copy link
Contributor Author

mpx commented Nov 30, 2018

No, I originally found the bug in 1.11.

@andybons andybons modified the milestones: Go1.12, Unplanned Nov 30, 2018
@mpx
Copy link
Contributor Author

mpx commented Dec 1, 2018

Without this fix CGO profiles can be subtly confusing/unhelpful since frames are lost. The bug and the fix are fairly trivial:

var stk [32]uintptr
cgoOff := copy(stk[:], cStk)
n := copy(stk[cgoOff:], goStk)
// Missing fix: n += cgoOff
frames := stk[:n]

I understand the release is getting close and the need to reduce change risk, but could this fix please be considered for 1.12? Is this bug/fix trivial enough to make it in?

@ianlancetaylor
Copy link
Member

Yes, we should get this into 1.12.

It doesn't need to block the beta release, though.

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

No branches or pull requests
4 participants
@andybons @mpx @ianlancetaylor @gopherbot