diff options
| author | Keith Randall <khr@golang.org> | 2025-05-28 17:09:05 -0700 |
|---|---|---|
| committer | Keith Randall <khr@golang.org> | 2025-05-28 20:35:09 -0700 |
| commit | dbaa2d3e6525a29defdff16f354881a93974dd2e (patch) | |
| tree | 59377544427d2b9a8a344f32ec714a85b088c9cc /test | |
| parent | 6160fa59b6523e781db47eb1ee8f929398f2bb78 (diff) | |
| download | go-dbaa2d3e6525a29defdff16f354881a93974dd2e.tar.xz | |
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>
Diffstat (limited to 'test')
| -rw-r--r-- | test/fixedbugs/issue73748a.go | 32 | ||||
| -rw-r--r-- | test/fixedbugs/issue73748b.go | 32 |
2 files changed, 64 insertions, 0 deletions
diff --git a/test/fixedbugs/issue73748a.go b/test/fixedbugs/issue73748a.go new file mode 100644 index 0000000000..c8ac10c29c --- /dev/null +++ b/test/fixedbugs/issue73748a.go @@ -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) +} diff --git a/test/fixedbugs/issue73748b.go b/test/fixedbugs/issue73748b.go new file mode 100644 index 0000000000..ff094a9764 --- /dev/null +++ b/test/fixedbugs/issue73748b.go @@ -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) +} |
