Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime/trace: frame pointer unwinding crash on arm64 during async preemption [1.21 backport] #65449

Closed
gopherbot opened this issue Feb 2, 2024 · 5 comments
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Milestone

Comments

@gopherbot
Copy link

@prattmic requested issue #63830 to be considered for backport to the next 1.21 minor release.

@gopherbot Please backport to 1.21. This causes crashes with tracing enabled on arm64. Though the FP code has been broken longer, only in 1.21 did tracing start to depend on the frame pointer.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Feb 2, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 2, 2024
@gopherbot gopherbot added this to the Go1.21.7 milestone Feb 2, 2024
@gopherbot
Copy link
Author

Change https://go.dev/cl/560735 mentions this issue: [release-branch.go1.21] cmd/internal/obj/arm64: fix frame pointer restore in epilogue

@prattmic
Copy link
Member

prattmic commented Feb 2, 2024

There is technically a workaround here: setting GODEBUG=tracefpunwindoff=1 will disable use of frame pointers with tracing entirely. However, because these crashes are effectively random every single program that enables tracing on arm64 should set this flag to be safe.

Therefore in my opinion we should either backport this fix, or backport a change to set GODEBUG=tracefpunwindoff=1 by default on arm64.

@prattmic
Copy link
Member

prattmic commented Feb 2, 2024

@mknyszek
Copy link
Contributor

mknyszek commented Feb 2, 2024

The fix was merged in October, so I feel fairly confident that it's safe at this point. It's also pretty small. I'm inclined to approve this.

@mknyszek mknyszek added the CherryPickApproved Used during the release process for point releases label Feb 2, 2024
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Feb 2, 2024
gopherbot pushed a commit that referenced this issue Feb 2, 2024
…tore in epilogue

For leaf but nonzero-frame functions.

Currently we're not restoring it properly. We also need to restore
it before popping the stack frame, so that the frame won't get
clobbered by a signal handler in the meantime.

For #63830
Fixes #65449

Needs a test, but I'm not at all sure how we would actually do that. Leaving for inspiration.

Change-Id: I273a25f2a838f05a959c810145cccc5428eaf164
Reviewed-on: https://go-review.googlesource.com/c/go/+/538635
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Eric Fang <eric.fang@arm.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit c9888bd)
Reviewed-on: https://go-review.googlesource.com/c/go/+/560735
TryBot-Bypass: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
@gopherbot
Copy link
Author

Closed by merging 2fdad8a to release-branch.go1.21.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
None yet
Development

No branches or pull requests

3 participants