From 01f99b4e9540f34b44e13b25f6dd04b82ac952d9 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Tue, 11 Aug 2020 13:07:35 -0700 Subject: [PATCH] cmd/compile: mark DUFFZERO/DUFFCOPY as async unsafe MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit These operations are async unsafe on architectures that use frame pointers. The reason is they rely on data being safe when stored below the stack pointer. They do: 45da69: 48 89 6c 24 f0 mov %rbp,-0x10(%rsp) 45da6e: 48 8d 6c 24 f0 lea -0x10(%rsp),%rbp 45da73: e8 7d d0 ff ff callq 45aaf5 45da78: 48 8b 6d 00 mov 0x0(%rbp),%rbp This dance ensures that inside duffzero, it looks like there is a proper frame pointer set up, so that stack walkbacks work correctly if the kernel samples during duffzero. However, this instruction sequence depends on data not being clobbered even though it is below the stack pointer. If there is an async interrupt at any of those last 3 instructions, and the interrupt decides to insert a call to asyncPreempt, then the saved frame pointer on the stack gets clobbered. The last instruction above then restores junk to the frame pointer. To prevent this, mark these instructions as async unsafe. (The body of duffzero is already async unsafe, as it is in package runtime.) Change-Id: I5562e82f9f5bd2fb543dcf2b6b9133d87ff83032 Reviewed-on: https://go-review.googlesource.com/c/go/+/248261 Run-TryBot: Keith Randall TryBot-Result: Gobot Gobot Reviewed-by: Martin Möhrmann Reviewed-by: Cherry Zhang --- src/cmd/compile/internal/ssa/gen/AMD64Ops.go | 2 ++ src/cmd/compile/internal/ssa/gen/ARM64Ops.go | 2 ++ src/cmd/compile/internal/ssa/opGen.go | 4 ++++ 3 files changed, 8 insertions(+) diff --git a/src/cmd/compile/internal/ssa/gen/AMD64Ops.go b/src/cmd/compile/internal/ssa/gen/AMD64Ops.go index a3b29049df..e6d66957dd 100644 --- a/src/cmd/compile/internal/ssa/gen/AMD64Ops.go +++ b/src/cmd/compile/internal/ssa/gen/AMD64Ops.go @@ -748,6 +748,7 @@ func init() { clobbers: buildReg("DI"), }, faultOnNilArg0: true, + unsafePoint: true, // FP maintenance around DUFFCOPY can be clobbered by interrupts }, {name: "MOVOconst", reg: regInfo{nil, 0, []regMask{fp}}, typ: "Int128", aux: "Int128", rematerializeable: true}, @@ -786,6 +787,7 @@ func init() { clobberFlags: true, faultOnNilArg0: true, faultOnNilArg1: true, + unsafePoint: true, // FP maintenance around DUFFCOPY can be clobbered by interrupts }, // arg0 = destination pointer diff --git a/src/cmd/compile/internal/ssa/gen/ARM64Ops.go b/src/cmd/compile/internal/ssa/gen/ARM64Ops.go index b402e35ea6..2424e67e20 100644 --- a/src/cmd/compile/internal/ssa/gen/ARM64Ops.go +++ b/src/cmd/compile/internal/ssa/gen/ARM64Ops.go @@ -507,6 +507,7 @@ func init() { clobbers: buildReg("R20 R30"), }, faultOnNilArg0: true, + unsafePoint: true, // FP maintenance around DUFFZERO can be clobbered by interrupts }, // large zeroing @@ -547,6 +548,7 @@ func init() { }, faultOnNilArg0: true, faultOnNilArg1: true, + unsafePoint: true, // FP maintenance around DUFFCOPY can be clobbered by interrupts }, // large move diff --git a/src/cmd/compile/internal/ssa/opGen.go b/src/cmd/compile/internal/ssa/opGen.go index 9efa1bfcc4..408c855dbd 100644 --- a/src/cmd/compile/internal/ssa/opGen.go +++ b/src/cmd/compile/internal/ssa/opGen.go @@ -13119,6 +13119,7 @@ var opcodeTable = [...]opInfo{ auxType: auxInt64, argLen: 3, faultOnNilArg0: true, + unsafePoint: true, reg: regInfo{ inputs: []inputInfo{ {0, 128}, // DI @@ -13196,6 +13197,7 @@ var opcodeTable = [...]opInfo{ clobberFlags: true, faultOnNilArg0: true, faultOnNilArg1: true, + unsafePoint: true, reg: regInfo{ inputs: []inputInfo{ {0, 128}, // DI @@ -20734,6 +20736,7 @@ var opcodeTable = [...]opInfo{ auxType: auxInt64, argLen: 2, faultOnNilArg0: true, + unsafePoint: true, reg: regInfo{ inputs: []inputInfo{ {0, 1048576}, // R20 @@ -20760,6 +20763,7 @@ var opcodeTable = [...]opInfo{ argLen: 3, faultOnNilArg0: true, faultOnNilArg1: true, + unsafePoint: true, reg: regInfo{ inputs: []inputInfo{ {0, 2097152}, // R21 -- 2.50.0