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 error: found bad pointer in Go heap (incorrect use of unsafe or cgo?) [1.14 backport] #37968

Closed
gopherbot opened this issue Mar 20, 2020 · 4 comments
Closed

runtime: fatal error: found bad pointer in Go heap (incorrect use of unsafe or cgo?) [1.14 backport] #37968

gopherbot opened this issue Mar 20, 2020 · 4 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.14.2

Comments

@gopherbot
Copy link
Contributor

@ianlancetaylor requested issue #37688 to be considered for backport to the next 1.14 minor release.

@gopherbot Please open a backport to 1.14. This problem causes program crashes with no workaround.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Mar 20, 2020
@gopherbot gopherbot mentioned this issue Mar 20, 2020
Closed
@gopherbot gopherbot added this to the Go1.14.2 milestone Mar 20, 2020
@cagedmantis
Copy link
Contributor

Approved. This is a critical issue without a workaround.

@cagedmantis cagedmantis added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Mar 20, 2020
@danscales
Copy link
Contributor

I checked in the fix for #37688 at
825ae71 , https://go-review.googlesource.com/c/go/+/224581

Thanks for cherry-picking to go 1.14.2!

@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/225279 mentions this issue: [release-branch.go1.14] runtime: fix code so defer record is not added to g0 defer list during panic

@gopherbot
Copy link
Contributor Author

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

gopherbot pushed a commit that referenced this issue Mar 24, 2020
@danscales @ianlancetaylor
[release-branch.go1.14] runtime: fix code so defer record is not adde…
f75a45c 
…d to g0 defer list during panic

newdefer() actually adds the new defer to the current g's defer chain. That
happens even if we are on the system stack, in which case the g will be the g0
stack. For open-coded defers, we call newdefer() (only during panic processing)
while on the system stack, so the new defer is unintentionally added to the
g0._defer defer list. The code later correctly adds the defer to the user g's
defer list.

The g0._defer list is never used. However, that pointer on the g0._defer list can
keep a defer struct alive that is intended to be garbage-collected (smaller defers
use a defer pool, but larger-sized defer records are just GC'ed). freedefer() does
not zero out pointers when it intends that a defer become garbage-collected. So,
we can have the pointers in a defer that is held alive by g0._defer become invalid
(in particular d.link). This is the cause of the bad pointer bug in this issue

The fix is to change newdefer (only used in two places) to not add the new defer
to the gp._defer list. We just do it after the call with the correct gp pointer.
(As mentioned above, this code was already there after the newdefer in
addOneOpenDeferFrame.) That ensures that defers will be correctly
garbage-collected and eliminate the bad pointer.

This fix definitely fixes the original repro. I added a test and tried hard to
reproduce the bug (based on the original repro code), but awasn't actually able to
cause the bug. However, the test is still an interesting mix of heap-allocated,
stack-allocated, and open-coded defers.

For #37688
Fixes #37968

Fixes #37688

Change-Id: I1a481b9d9e9b9ba4e8726ef718a1f4512a2d6faf
Reviewed-on: https://go-review.googlesource.com/c/go/+/224581
Run-TryBot: Dan Scales <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
(cherry picked from commit 825ae71)
Reviewed-on: https://go-review.googlesource.com/c/go/+/225279
Run-TryBot: Ian Lance Taylor <[email protected]>
Reviewed-by: Dan Scales <[email protected]>
@Roasbeef Roasbeef mentioned this issue Mar 26, 2020
Closed
@golang golang locked and limited conversation to collaborators Mar 24, 2021
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.14.2
Development

No branches or pull requests
3 participants
@cagedmantis @danscales @gopherbot