Skip to content

Commit

Permalink
cmd/compile: generate frame pointers for otherwise frameless functions
Browse files Browse the repository at this point in the history
func f() {
    g()
}

We mistakenly don't add a frame pointer for f.  This means f
isn't seen when walking the frame pointer linked list.  That
matters for kernel-gathered profiles, and is an impediment for
issues like #16638.

To fix, allocate a stack frame even for otherwise frameless functions
like f.  It is a bit tricky because we need to avoid some runtime
internals that really, really don't want one.

No test at the moment, as only kernel CPU profiles would catch it.
Tests will come with the implementation of #16638.

Fixes #18103

Change-Id: I411206cc9de4c8fdd265bee2e4fa61d161ad1847
Reviewed-on: https://go-review.googlesource.com/33754
Run-TryBot: Keith Randall <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Austin Clements <[email protected]>
  • Loading branch information
randall77 committed Dec 1, 2016
1 parent 7e5b2e0 commit c96e94e
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 4 deletions.
3 changes: 3 additions & 0 deletions src/cmd/compile/internal/gc/pgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,9 @@ func compile(fn *Node) {
if fn.Func.Wrapper {
ptxt.From3.Offset |= obj.WRAPPER
}
if fn.Func.NoFramePointer {
ptxt.From3.Offset |= obj.NOFRAME
}
if fn.Func.Needctxt {
ptxt.From3.Offset |= obj.NEEDCTXT
}
Expand Down
2 changes: 2 additions & 0 deletions src/cmd/compile/internal/gc/subr.go
Original file line number Diff line number Diff line change
Expand Up @@ -1810,6 +1810,8 @@ func genwrapper(rcvr *Type, method *Field, newnam *Sym, iface int) {
n := nod(ORETJMP, nil, nil)
n.Left = newname(methodsym(method.Sym, methodrcvr, 0))
fn.Nbody.Append(n)
// When tail-calling, we can't use a frame pointer.
fn.Func.NoFramePointer = true
} else {
fn.Func.Wrapper = true // ignore frame for panic+recover matching
call := nod(OCALL, dot, nil)
Expand Down
1 change: 1 addition & 0 deletions src/cmd/compile/internal/gc/syntax.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ type Func struct {
Needctxt bool // function uses context register (has closure variables)
ReflectMethod bool // function calls reflect.Type.Method or MethodByName
IsHiddenClosure bool
NoFramePointer bool // Must not use a frame pointer for this function
}

type Op uint8
Expand Down
24 changes: 20 additions & 4 deletions src/cmd/internal/obj/x86/obj6.go
Original file line number Diff line number Diff line change
Expand Up @@ -632,11 +632,27 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym) {
autoffset = 0
}

hasCall := false
for q := p; q != nil; q = q.Link {
if q.As == obj.ACALL || q.As == obj.ADUFFCOPY || q.As == obj.ADUFFZERO {
hasCall = true
break
}
}

var bpsize int
if p.Mode == 64 && ctxt.Framepointer_enabled && autoffset > 0 && p.From3.Offset&obj.NOFRAME == 0 {
// Make room for to save a base pointer. If autoffset == 0,
// this might do something special like a tail jump to
// another function, so in that case we omit this.
if p.Mode == 64 && ctxt.Framepointer_enabled &&
p.From3.Offset&obj.NOFRAME == 0 && // (1) below
!(autoffset == 0 && p.From3.Offset&obj.NOSPLIT != 0) && // (2) below
!(autoffset == 0 && !hasCall) { // (3) below
// Make room to save a base pointer.
// There are 2 cases we must avoid:
// 1) If noframe is set (which we do for functions which tail call).
// 2) Scary runtime internals which would be all messed up by frame pointers.
// We detect these using a heuristic: frameless nosplit functions.
// TODO: Maybe someday we label them all with NOFRAME and get rid of this heuristic.
// For performance, we also want to avoid:
// 3) Frameless leaf functions
bpsize = ctxt.Arch.PtrSize
autoffset += int32(bpsize)
p.To.Offset += int64(bpsize)
Expand Down

0 comments on commit c96e94e

Please sign in to comment.