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.

Sign up for GitHub

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

Jump to bottom

runtime/trace: segfault in runtime.fpTracebackPCs during deferred call after recovering from panic [1.21 backport] #62046

Closed
gopherbot opened this issue Aug 15, 2023 · 7 comments
Closed

runtime/trace: segfault in runtime.fpTracebackPCs during deferred call after recovering from panic [1.21 backport] #62046

gopherbot opened this issue Aug 15, 2023 · 7 comments
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Milestone
Go1.21.1

Comments

@gopherbot
Copy link

@mknyszek requested issue #61766 to be considered for backport to the next 1.21 minor release.

@gopherbot Please open up a backport issue for Go 1.21.

This causes crashes that while rare (a fairly specific situation is required) are difficult to work around. The only option is turning off FP unwinding for the tracer, but that means a fairly severe regression in overheads, which can be a problem if you've already committed to tracing more frequently at scale. The fix is also small and fairly safe.

(This issue also does not apply to Go 1.20.)
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Aug 15, 2023
@gopherbot gopherbot mentioned this issue Aug 15, 2023
Closed
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Aug 15, 2023
@gopherbot gopherbot added this to the Go1.21.1 milestone Aug 15, 2023
@mknyszek
Copy link
Contributor

I said "fairly safe" but I meant "safe". I should not have qualified that.

@gopherbot
Copy link
Author

Change https://go.dev/cl/519656 mentions this issue: [release-branch.go1.21] runtime: restore caller's frame pointer when recovering from panic

@cagedmantis cagedmantis added the CherryPickApproved Used during the release process for point releases label Aug 23, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Aug 23, 2023
@cagedmantis
Copy link
Contributor

Approved as this is a serious issue without a workaround.

@joedian
Copy link

joedian commented Aug 28, 2023

@mknyszek can you please resolve the issues on the CL?

@mknyszek
Copy link
Contributor

On it. Sorry for the delay.

@gopherbot
Copy link
Author

Change https://go.dev/cl/523698 mentions this issue: [release-branch.go1.21] runtime: restore caller's frame pointer when recovering from panic

gopherbot pushed a commit that referenced this issue Aug 30, 2023
@nsrip-dd @gopherbot
[release-branch.go1.21] runtime: restore caller's frame pointer when …
8dc6ad1 
…recovering from panic

When recovering from a panic, restore the caller's frame pointer before
returning control to the caller. Otherwise, if the function proceeds to
run more deferred calls before returning, the deferred functions will
get invalid frame pointers pointing to an address lower in the stack.
This can cause frame pointer unwinding to crash, such as if an execution
trace event is recorded during the deferred call on architectures which
support frame pointer unwinding.

Original CL by Nick Ripley, includes fix from CL 523697, and includes a
test update from CL 524315.

This CL also deviates from the original fix by doing some extra
computation to figure out the fp from the sp, since we don't have the fp
immediately available to us in `recovery` on the Go 1.21 branch, and it
would probably be complicated to plumb that through its caller.

For #61766
Fixes #62046

Change-Id: I5a99ca4f909f6b6e209a330d595d1c99987d4359
Reviewed-on: https://go-review.googlesource.com/c/go/+/523698
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
@gopherbot
Copy link
Author

Closed by merging 8dc6ad1 to release-branch.go1.21.

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

No branches or pull requests
4 participants
@cagedmantis @mknyszek @gopherbot @joedian