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

Jump to bottom

runtime: gentraceback() dead loop on arm64 casued the process hang [1.18 backport] #53112

Closed
gopherbot opened this issue May 27, 2022 · 6 comments
Closed

runtime: gentraceback() dead loop on arm64 casued the process hang [1.18 backport] #53112

gopherbot opened this issue May 27, 2022 · 6 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.18.5

Comments

@gopherbot
Copy link

@cherrymui requested issue #52116 to be considered for backport to the next 1.18 minor release.

@gopherbot please backport this to previous releases. This is a runtime bug which can cause programs to hang or crash. Thanks.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label May 27, 2022
@gopherbot gopherbot mentioned this issue May 27, 2022
Closed
@gopherbot gopherbot added this to the Go1.18.3 milestone May 27, 2022
@gopherbot
Copy link
Author

Change https://go.dev/cl/408821 mentions this issue: [release-branch.go1.18] runtime: use saved LR when unwinding through morestack

@dmitshur dmitshur modified the milestones: Go1.18.3, Go1.18.4 Jun 1, 2022
@joedian joedian added the CherryPickApproved Used during the release process for point releases label Jun 22, 2022
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Jun 22, 2022
@EricMountain
Copy link

EricMountain commented Jul 7, 2022 •
edited

For several weeks now we have been running several applications in production with this patch back-ported on top of 1.18.1 and it has resolved our issues.

We're looking forward to upgrading to a 1.18 patch release with this fix.

Thanks!

@lizthegrey
Copy link

Likewise, Honeycomb has backported this patch into our go runtime directory on top of 1.18.3

@gopherbot gopherbot modified the milestones: Go1.18.4, Go1.18.5 Jul 12, 2022
@evanj
Copy link
Contributor

evanj commented Jul 13, 2022 •
edited

This missed the 1.18.4 release. Is there anything we can do to help make sure this lands as part of 1.18.5? It seems to me the problem was getting the change code reviewed? We would rather not have to maintain our own fork of the Go tools, and this is a pretty serious bug for ARM workloads. Thank you!

@Ivy-YinSu
Copy link

We are looking forward to this fix. Please let us know what we can do to help make sure this fix can be in the 1.18.5 release.
The cherry-pick missed the 1.18.4 release. /cc @felixge

@gopherbot
Copy link
Author

Closed by merging 12e00f6 to release-branch.go1.18.

gopherbot pushed a commit that referenced this issue Jul 22, 2022
@cherrymui @heschi
[release-branch.go1.18] runtime: use saved LR when unwinding through …
12e00f6 
…morestack

On LR machine, consider F calling G calling H, which grows stack.
The stack looks like
...
G's frame:
	... locals ...
	saved LR = return PC in F  <- SP points here at morestack
H's frame (to be created)

At morestack, we save
	gp.sched.pc = H's morestack call
	gp.sched.sp = H's entry SP (the arrow above)
	gp.sched.lr = return PC in G

Currently, when unwinding through morestack (if _TraceJumpStack
is set), we switch PC and SP but not LR. We then have
	frame.pc = H's morestack call
	frame.sp = H's entry SP (the arrow above)
As LR is not set, we load it from stack at *sp, so
	frame.lr = return PC in F
As the SP hasn't decremented at the morestack call,
	frame.fp = frame.sp = H's entry SP

Unwinding a frame, we have
	frame.pc = old frame.lr = return PC in F
	frame.sp = old frame.fp = H's entry SP a.k.a. G's SP
The PC and SP don't match. The unwinding will go off if F and G
have different frame sizes.

Fix this by preserving the LR when switching stack.

Also add code to detect infinite loop in unwinding.

TODO: add some test. I can reproduce the infinite loop (or throw
with added check) but the frequency is low.

Fixes #53112.
Updates #52116.

Change-Id: I6e1294f1c6e55f664c962767a1cf6c466a0c0eff
Reviewed-on: https://go-review.googlesource.com/c/go/+/400575
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Eric Fang <eric.fang@arm.com>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
(cherry picked from commit 74f0009)
Reviewed-on: https://go-review.googlesource.com/c/go/+/408821
Reviewed-by: Austin Clements <austin@google.com>
@golang golang locked and limited conversation to collaborators Jul 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Milestone
Go1.18.5
Development

No branches or pull requests
7 participants
@lizthegrey @evanj @dmitshur @EricMountain @gopherbot @joedian @Ivy-YinSu