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: crash on 1.14 with unexpected return pc, fatal error: unknown caller pc [1.14 backport] #37782

Closed
gopherbot opened this issue Mar 10, 2020 · 5 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone

Comments

@gopherbot
Copy link

@danscales requested issue #37664 to be considered for backport to the next 1.14 minor release.

@gopherbot please consider this for backport to 1.14, it's a regression (and the fix is quite simple).

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Mar 10, 2020
@gopherbot gopherbot added this to the Go1.14.1 milestone Mar 10, 2020
@danscales
Copy link
Contributor

This is a regression in defer behavior for 1.14 for programs that do repeated panics, recovers, and re-panics (which could be because of a recursive, interpreter-like function). The actual fix is fairly simple, so it would be good to get this into 1.14.1.

@danscales
Copy link
Contributor

Fix (submitted for 1.15) is here: https://go-review.googlesource.com/c/go/+/222420

@dmitshur
Copy link
Contributor

dmitshur commented Mar 10, 2020

Approving as this is a serious issue without a viable workaround. /cc @toothrot @cagedmantis

@dmitshur dmitshur added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Mar 10, 2020
@gopherbot
Copy link
Author

Change https://golang.org/cl/222818 mentions this issue: [release-branch.go1.14] runtime: fix problem with repeated panic/recover/re-panics and open-coded defers

@gopherbot
Copy link
Author

Closed by merging fd85ff5 to release-branch.go1.14.

gopherbot pushed a commit that referenced this issue Mar 11, 2020
…ver/re-panics and open-coded defers

In the open-code defer implementation, we add defer struct entries to the defer
chain on-the-fly at panic time to represent stack frames that contain open-coded
defers. This allows us to process non-open-coded and open-coded defers in the
correct order. Also, we need somewhere to be able to store the 'started' state of
open-coded defers. However, if a recover succeeds, defers will now be processed
inline again (unless another panic happens). Any defer entry representing a frame
with open-coded defers will become stale once we run the corresponding defers
inline and exit the associated stack frame. So, we need to remove all entries for
open-coded defers at recover time.

The current code was only removing the top-most open-coded defer from the defer
chain during recovery. However, with recursive functions that do repeated
panic-recover-repanic, multiple stale entries can accumulate on the chain. So, we
just adjust the loop to process the entire chain. Since this is at panic/recover
case, it is fine to scan through the entire chain (which should usually have few
elements in it, since most defers are open-coded).

The added test fails with a SEGV without the fix, because it tries to run a stale
open-code defer entry (and the stack has changed).

Updates #37664.
Fixes #37782.

Change-Id: I8e3da5d610b5e607411451b66881dea887f7484d
Reviewed-on: https://go-review.googlesource.com/c/go/+/222420
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit fae87a2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/222818
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
@golang golang locked and limited conversation to collaborators Mar 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

3 participants