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: SetCgoTraceback drops frames from tracebacks #29034

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

Comments

Projects
None yet
4 participants
@mpx
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

This comment has been minimized.

gopherbot commented Nov 30, 2018

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

@mpx

This comment has been minimized.

Contributor

mpx commented Nov 30, 2018

@gopherbot

This comment has been minimized.

gopherbot commented Nov 30, 2018

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

@andybons

This comment has been minimized.

Member

andybons commented Nov 30, 2018

@mpx is this a regression from 1.11?

@andybons andybons added this to the Go1.12 milestone Nov 30, 2018

@mpx

This comment has been minimized.

Contributor

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

This comment has been minimized.

Contributor

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

This comment has been minimized.

Contributor

ianlancetaylor commented Dec 1, 2018

Yes, we should get this into 1.12.

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

@gopherbot gopherbot closed this in 0f0b108 Dec 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment