Skip to content

Commit

Permalink
runtime: use explicit NOFRAME on linux/amd64
Browse files Browse the repository at this point in the history
This CL marks some linux assembly functions as NOFRAME to avoid relying
on the implicit amd64 NOFRAME heuristic, where NOSPLIT functions
without stack were also marked as NOFRAME.

Updates #58378

Change-Id: I7792cff4f6e539bfa56c02868f2965088ca1975a
Reviewed-on: https://go-review.googlesource.com/c/go/+/466316
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
  • Loading branch information
qmuntal committed Feb 22, 2023
1 parent 133e0bc commit 521d261
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 12 deletions.
10 changes: 8 additions & 2 deletions src/cmd/internal/obj/x86/obj6.go
Original file line number Diff line number Diff line change
Expand Up @@ -611,17 +611,23 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) {
}
}

var usefpheuristic bool
switch ctxt.Headtype {
case objabi.Hwindows, objabi.Hdarwin, objabi.Hlinux:
default:
usefpheuristic = true
}

var bpsize int
if ctxt.Arch.Family == sys.AMD64 &&
!p.From.Sym.NoFrame() && // (1) below
!(autoffset == 0 && p.From.Sym.NoSplit() && ctxt.Headtype != objabi.Hwindows && ctxt.Headtype != objabi.Hdarwin) && // (2) below
!(autoffset == 0 && p.From.Sym.NoSplit() && usefpheuristic) && // (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.
// Windows and Darwin do not use this heuristic anymore.
// 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
Expand Down
5 changes: 3 additions & 2 deletions src/cmd/link/internal/ld/testdata/stackcheck/main.s
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// license that can be found in the LICENSE file.

#define NOSPLIT 7
#define NOFRAME 512

TEXT ·asmMain(SB),0,$0-0
CALL ·startSelf(SB)
Expand Down Expand Up @@ -32,9 +33,9 @@ TEXT ·chainEnd(SB),NOSPLIT,$1000-0 // Should be reported twice
RET

// Test reporting of rootless recursion
TEXT ·startRec(SB),NOSPLIT,$0-0
TEXT ·startRec(SB),NOSPLIT|NOFRAME,$0-0
CALL ·startRec0(SB)
RET
TEXT ·startRec0(SB),NOSPLIT,$0-0
TEXT ·startRec0(SB),NOSPLIT|NOFRAME,$0-0
CALL ·startRec(SB)
RET
14 changes: 6 additions & 8 deletions src/runtime/sys_linux_amd64.s
Original file line number Diff line number Diff line change
Expand Up @@ -325,16 +325,14 @@ TEXT runtime·sigfwd(SB),NOSPLIT,$0-32
MOVL sig+8(FP), DI
MOVQ info+16(FP), SI
MOVQ ctx+24(FP), DX
PUSHQ BP
MOVQ SP, BP
MOVQ SP, BX // callee-saved
ANDQ $~15, SP // alignment for x86_64 ABI
CALL AX
MOVQ BP, SP
POPQ BP
MOVQ BX, SP
RET

// Called using C ABI.
TEXT runtime·sigtramp(SB),NOSPLIT|TOPFRAME,$0
TEXT runtime·sigtramp(SB),NOSPLIT|TOPFRAME|NOFRAME,$0
// Transition from C ABI to Go ABI.
PUSH_REGS_HOST_TO_ABI0()

Expand All @@ -359,7 +357,7 @@ TEXT runtime·sigtramp(SB),NOSPLIT|TOPFRAME,$0
RET

// Called using C ABI.
TEXT runtime·sigprofNonGoWrapper<>(SB),NOSPLIT,$0
TEXT runtime·sigprofNonGoWrapper<>(SB),NOSPLIT|NOFRAME,$0
// Transition from C ABI to Go ABI.
PUSH_REGS_HOST_TO_ABI0()

Expand Down Expand Up @@ -556,7 +554,7 @@ TEXT runtime·futex(SB),NOSPLIT,$0
RET

// int32 clone(int32 flags, void *stk, M *mp, G *gp, void (*fn)(void));
TEXT runtime·clone(SB),NOSPLIT,$0
TEXT runtime·clone(SB),NOSPLIT|NOFRAME,$0
MOVL flags+0(FP), DI
MOVQ stk+8(FP), SI
MOVQ $0, DX
Expand Down Expand Up @@ -620,7 +618,7 @@ nog2:
SYSCALL
JMP -3(PC) // keep exiting

TEXT runtime·sigaltstack(SB),NOSPLIT,$-8
TEXT runtime·sigaltstack(SB),NOSPLIT,$0
MOVQ new+0(FP), DI
MOVQ old+8(FP), SI
MOVQ $SYS_sigaltstack, AX
Expand Down

0 comments on commit 521d261

Please sign in to comment.