From c96e94e69d5b29787589385d93eb4452af05c6f3 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Wed, 30 Nov 2016 16:15:32 -0800 Subject: [PATCH] cmd/compile: generate frame pointers for otherwise frameless functions 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 TryBot-Result: Gobot Gobot Reviewed-by: Austin Clements --- src/cmd/compile/internal/gc/pgen.go | 3 +++ src/cmd/compile/internal/gc/subr.go | 2 ++ src/cmd/compile/internal/gc/syntax.go | 1 + src/cmd/internal/obj/x86/obj6.go | 24 ++++++++++++++++++++---- 4 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go index acea790498..643ba79d63 100644 --- a/src/cmd/compile/internal/gc/pgen.go +++ b/src/cmd/compile/internal/gc/pgen.go @@ -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 } diff --git a/src/cmd/compile/internal/gc/subr.go b/src/cmd/compile/internal/gc/subr.go index a53ba1fffc..9b9a3f1210 100644 --- a/src/cmd/compile/internal/gc/subr.go +++ b/src/cmd/compile/internal/gc/subr.go @@ -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) diff --git a/src/cmd/compile/internal/gc/syntax.go b/src/cmd/compile/internal/gc/syntax.go index 8b06d3aba8..8848bb5955 100644 --- a/src/cmd/compile/internal/gc/syntax.go +++ b/src/cmd/compile/internal/gc/syntax.go @@ -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 diff --git a/src/cmd/internal/obj/x86/obj6.go b/src/cmd/internal/obj/x86/obj6.go index 102d8c3c4f..eb6f867ca7 100644 --- a/src/cmd/internal/obj/x86/obj6.go +++ b/src/cmd/internal/obj/x86/obj6.go @@ -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) -- 2.50.0