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: "fatal: morestack on g0" on amd64 after upgrade to Go 1.21 [1.21 backport] #63209

Closed
gopherbot opened this issue Sep 25, 2023 · 6 comments
Closed

runtime: "fatal: morestack on g0" on amd64 after upgrade to Go 1.21 [1.21 backport] #63209

gopherbot opened this issue Sep 25, 2023 · 6 comments
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge
Milestone
Go1.21.7

Comments

@gopherbot
Copy link
Contributor

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

@gopherbot please backport to 1.21. This is a fatal crash with no workaround for applications using C coroutine libraries and calling into Go. The fix here is fairly complex, so this is a medium+ risk backport, but we don't have a simpler workaround to backport. That said, the fix has baked in post-submit for 2+ weeks without further issue.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Sep 25, 2023
@gopherbot gopherbot mentioned this issue Sep 25, 2023
Closed
@gopherbot gopherbot added this to the Go1.21.2 milestone Sep 25, 2023
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/530480 mentions this issue: [release-branch.go1.21] runtime: allow update of system stack bounds on callback from C thread

@prattmic prattmic modified the milestones: Go1.21.2, Go1.21.3 Oct 4, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Oct 9, 2023
@gopherbot gopherbot modified the milestones: Go1.21.3, Go1.21.4 Oct 10, 2023
@hajimehoshi hajimehoshi mentioned this issue Oct 17, 2023
Closed
@gopherbot gopherbot modified the milestones: Go1.21.4, Go1.21.5 Nov 7, 2023
@swiatekm swiatekm mentioned this issue Nov 8, 2023
Closed
@heschi
Copy link
Contributor

heschi commented Nov 15, 2023

What's the status on this one? We were waiting for any bugs to shake out on tip, I believe?

@gopherbot gopherbot modified the milestones: Go1.21.5, Go1.21.6 Dec 5, 2023
@swiatekm swiatekm mentioned this issue Dec 6, 2023
Closed
@prattmic prattmic added the CherryPickApproved Used during the release process for point releases label Dec 13, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Dec 13, 2023
@prattmic
Copy link
Member

This needs an extra backport: https://go.dev/cl/537695, but has received enough soaking now.

@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/549495 mentions this issue: [release-branch.go1.21] runtime: clear g0 stack bounds in dropm

@swiatekm swiatekm mentioned this issue Dec 14, 2023
Closed
@gopherbot gopherbot modified the milestones: Go1.21.6, Go1.21.7 Jan 9, 2024
gopherbot pushed a commit that referenced this issue Jan 10, 2024
@prattmic @gopherbot
[release-branch.go1.21] runtime: allow update of system stack bounds …
491c1e7 
…on callback from C thread

[This cherry-pick combines CL 527715, CL 527775, CL 527797, and
CL 529216.]

[This is a redo of CL 525455 with the test fixed on darwin by defining
_XOPEN_SOURCE, and disabled with android, musl, and openbsd, which do
not provide getcontext.]

Since CL 495855, Ms are cached for C threads calling into Go, including
the stack bounds of the system stack.

Some C libraries (e.g., coroutine libraries) do manual stack management
and may change stacks between calls to Go on the same thread.

Changing the stack if there is more Go up the stack would be
problematic. But if the calls are completely independent there is no
particular reason for Go to care about the changing stack boundary.

Thus, this CL allows the stack bounds to change in such cases. The
primary downside here (besides additional complexity) is that normal
systems that do not manipulate the stack may not notice unintentional
stack corruption as quickly as before.

Note that callbackUpdateSystemStack is written to be usable for the
initial setup in needm as well as updating the stack in cgocallbackg.

For #62440.
For #62130.
For #63209.

Change-Id: I0fe0134f865932bbaff1fc0da377c35c013bd768
Reviewed-on: https://go-review.googlesource.com/c/go/+/527715
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit 4f9fe6d)
(cherry picked from commit e8ba057)
(cherry picked from commit a843991)
(cherry picked from commit d110d7c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/530480
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Bypass: Michael Pratt <mpratt@google.com>
gopherbot pushed a commit that referenced this issue Jan 10, 2024
@prattmic @gopherbot
[release-branch.go1.21] runtime: clear g0 stack bounds in dropm
7e34c43 
After CL 527715, needm uses callbackUpdateSystemStack to set the stack
bounds for g0 on an M from the extra M list. Since
callbackUpdateSystemStack is also used for recursive cgocallback, it
does nothing if the stack is already in bounds.

Currently, the stack bounds in an extra M may contain stale bounds from
a previous thread that used this M and then returned it to the extra
list in dropm.

Typically a new thread will not have an overlapping stack with an old
thread, but because the old thread has exited there is a small chance
that the C memory allocator will allocate the new thread's stack
partially or fully overlapping with the old thread's stack.

If this occurs, then callbackUpdateSystemStack will not update the stack
bounds. If in addition, the overlap is partial such that SP on
cgocallback is close to the recorded stack lower bound, then Go may
quickly "overflow" the stack and crash with "morestack on g0".

Fix this by clearing the stack bounds in dropm, which ensures that
callbackUpdateSystemStack will unconditionally update the bounds in
needm.

For #62440.
Fixes #63209.

Change-Id: Ic9e2052c2090dd679ed716d1a23a86d66cbcada7
Reviewed-on: https://go-review.googlesource.com/c/go/+/537695
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Bypass: Michael Pratt <mpratt@google.com>
(cherry picked from commit 1af424c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/549495
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
@gopherbot
Copy link
Contributor Author

Closed by merging 7e34c43 to release-branch.go1.21.

@gopherbot

This comment was marked as duplicate.

Sign in to view
@swiatekm swiatekm mentioned this issue Jan 15, 2024
Closed
@golang golang locked and limited conversation to collaborators Jan 9, 2025
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 compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge
Projects
None yet
Milestone
Go1.21.7
Development

No branches or pull requests
3 participants
@prattmic @gopherbot @heschi