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 <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>

diff --git a/src/cmd/compile/internal/gc/pgen.go b/src/cmd/compile/internal/gc/pgen.go
index acea790498d2f28524066612ba23fe549c3156fc..643ba79d631036923b4228094c8fad1624f101dd 100644 (file)
--- 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 a53ba1fffc1bce839f4d1c15f47f01b90bf6c676..9b9a3f121015d86fa746647b255ed19ab632e13d 100644 (file)
--- 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 8b06d3aba8dd68215ed1eaf4c459f5a26eac7bac..8848bb5955d50cf9f0e7ec026215c74b3a072b55 100644 (file)
--- 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 102d8c3c4f91d24cb522ab5028d5bf6f4dcd2efe..eb6f867ca7e4a188b22d08aac6d635876641f2e6 100644 (file)
--- 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)