mirror of
https://github.com/golang/go.git
synced 2025-12-08 06:10:04 +00:00
cmd/compile: do nil check before calling duff functions, on arm64 and amd64
On these platforms, we set up a frame pointer record below the current stack pointer, so when we're in duffcopy or duffzero, we get a reasonable traceback. See #73753. But because this frame pointer record is below SP, it is vulnerable. Anything that adds a new stack frame to the stack might clobber it. Which actually happens in #73748 on amd64. I have not yet come across a repro on arm64, but might as well be safe here. The only real situation this could happen is when duffzero or duffcopy is passed a nil pointer. So we can just avoid the problem by doing the nil check outside duffzero/duffcopy. That way we never add a frame below duffzero/duffcopy. (Most other ways to get a new frame below the current one, like async preempt or debugger-generated calls, don't apply to duffzero/duffcopy because they are runtime functions; we're not allowed to preempt there.) Longer term, we should stop putting stuff below SP. #73753 will include that as part of its remit. But that's not for 1.25, so we'll do the simple thing for 1.25 for this issue. Fixes #73748 Change-Id: I913c49ee46dcaee8fb439415a4531f7b59d0f612 Reviewed-on: https://go-review.googlesource.com/c/go/+/676916 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Keith Randall <khr@google.com>
This commit is contained in:
parent
6160fa59b6
commit
dbaa2d3e65
5 changed files with 92 additions and 34 deletions
|
|
@ -897,7 +897,7 @@ func init() {
|
|||
inputs: []regMask{buildReg("DI")},
|
||||
clobbers: buildReg("DI"),
|
||||
},
|
||||
faultOnNilArg0: true,
|
||||
//faultOnNilArg0: true, // Note: removed for 73748. TODO: reenable at some point
|
||||
unsafePoint: true, // FP maintenance around DUFFCOPY can be clobbered by interrupts
|
||||
},
|
||||
|
||||
|
|
@ -936,8 +936,8 @@ func init() {
|
|||
clobbers: buildReg("DI SI X0"), // uses X0 as a temporary
|
||||
},
|
||||
clobberFlags: true,
|
||||
faultOnNilArg0: true,
|
||||
faultOnNilArg1: true,
|
||||
//faultOnNilArg0: true, // Note: removed for 73748. TODO: reenable at some point
|
||||
//faultOnNilArg1: true,
|
||||
unsafePoint: true, // FP maintenance around DUFFCOPY can be clobbered by interrupts
|
||||
},
|
||||
|
||||
|
|
|
|||
|
|
@ -554,7 +554,7 @@ func init() {
|
|||
inputs: []regMask{buildReg("R20")},
|
||||
clobbers: buildReg("R16 R17 R20 R30"),
|
||||
},
|
||||
faultOnNilArg0: true,
|
||||
//faultOnNilArg0: true, // Note: removed for 73748. TODO: reenable at some point
|
||||
unsafePoint: true, // FP maintenance around DUFFZERO can be clobbered by interrupts
|
||||
},
|
||||
|
||||
|
|
@ -595,8 +595,8 @@ func init() {
|
|||
inputs: []regMask{buildReg("R21"), buildReg("R20")},
|
||||
clobbers: buildReg("R16 R17 R20 R21 R26 R30"),
|
||||
},
|
||||
faultOnNilArg0: true,
|
||||
faultOnNilArg1: true,
|
||||
//faultOnNilArg0: true, // Note: removed for 73748. TODO: reenable at some point
|
||||
//faultOnNilArg1: true,
|
||||
unsafePoint: true, // FP maintenance around DUFFCOPY can be clobbered by interrupts
|
||||
},
|
||||
|
||||
|
|
|
|||
|
|
@ -13877,7 +13877,6 @@ var opcodeTable = [...]opInfo{
|
|||
name: "DUFFZERO",
|
||||
auxType: auxInt64,
|
||||
argLen: 2,
|
||||
faultOnNilArg0: true,
|
||||
unsafePoint: true,
|
||||
reg: regInfo{
|
||||
inputs: []inputInfo{
|
||||
|
|
@ -13952,8 +13951,6 @@ var opcodeTable = [...]opInfo{
|
|||
auxType: auxInt64,
|
||||
argLen: 3,
|
||||
clobberFlags: true,
|
||||
faultOnNilArg0: true,
|
||||
faultOnNilArg1: true,
|
||||
unsafePoint: true,
|
||||
reg: regInfo{
|
||||
inputs: []inputInfo{
|
||||
|
|
@ -23042,7 +23039,6 @@ var opcodeTable = [...]opInfo{
|
|||
name: "DUFFZERO",
|
||||
auxType: auxInt64,
|
||||
argLen: 2,
|
||||
faultOnNilArg0: true,
|
||||
unsafePoint: true,
|
||||
reg: regInfo{
|
||||
inputs: []inputInfo{
|
||||
|
|
@ -23068,8 +23064,6 @@ var opcodeTable = [...]opInfo{
|
|||
name: "DUFFCOPY",
|
||||
auxType: auxInt64,
|
||||
argLen: 3,
|
||||
faultOnNilArg0: true,
|
||||
faultOnNilArg1: true,
|
||||
unsafePoint: true,
|
||||
reg: regInfo{
|
||||
inputs: []inputInfo{
|
||||
|
|
|
|||
32
test/fixedbugs/issue73748a.go
Normal file
32
test/fixedbugs/issue73748a.go
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
// run
|
||||
|
||||
// Copyright 2025 The Go Authors. All rights reserved.
|
||||
// Use of this source code is governed by a BSD-style
|
||||
// license that can be found in the LICENSE file.
|
||||
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
"runtime/trace"
|
||||
)
|
||||
|
||||
type T struct {
|
||||
a [16]int
|
||||
}
|
||||
|
||||
//go:noinline
|
||||
func f(x *T) {
|
||||
*x = T{}
|
||||
}
|
||||
|
||||
func main() {
|
||||
trace.Start(io.Discard)
|
||||
defer func() {
|
||||
recover()
|
||||
trace.Log(context.Background(), "a", "b")
|
||||
|
||||
}()
|
||||
f(nil)
|
||||
}
|
||||
32
test/fixedbugs/issue73748b.go
Normal file
32
test/fixedbugs/issue73748b.go
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
// run
|
||||
|
||||
// Copyright 2025 The Go Authors. All rights reserved.
|
||||
// Use of this source code is governed by a BSD-style
|
||||
// license that can be found in the LICENSE file.
|
||||
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
"runtime/trace"
|
||||
)
|
||||
|
||||
type T struct {
|
||||
a [16]int
|
||||
}
|
||||
|
||||
//go:noinline
|
||||
func f(x, y *T) {
|
||||
*x = *y
|
||||
}
|
||||
|
||||
func main() {
|
||||
trace.Start(io.Discard)
|
||||
defer func() {
|
||||
recover()
|
||||
trace.Log(context.Background(), "a", "b")
|
||||
|
||||
}()
|
||||
f(nil, nil)
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue