]> Cypherpunks.ru repositories - gostls13.git/commit
[dev.typeparams] runtime: handle d.link carefully when freeing a defer
authorAustin Clements <austin@google.com>
Wed, 4 Aug 2021 12:54:09 +0000 (08:54 -0400)
committerAustin Clements <austin@google.com>
Wed, 4 Aug 2021 15:51:58 +0000 (15:51 +0000)
commite590cb64f940b2d4996a6e7773c1b855be952632
tree8830538d5b7d69ceae2530ff50eb240dab8f8c83
parentd27a889119ce05b1faae29aa549887e86ce453df
[dev.typeparams] runtime: handle d.link carefully when freeing a defer

CL 339396 allowed stack copying on entry to and during freedefer, but
this introduced a subtle bug: if d is heap-allocated, and d.link
points to a stack-allocated defer, stack copying during freedefer can
briefly introduce a stale pointer, which the garbage collector can
discover and panic about. This happens because d has already been
unlinked from the defer chain when freedefer is called, so stack
copying won't update stack pointers in it.

Fix this by making freedefer nosplit again and immediately clearing
d.link.

This should fix the longtest builders, which currently fail on
GOMAXPROCS=2 runtime -cpu=1,2,4 -quick in the TestDeferHeapAndStack
test.

This seems like the simplest fix, but it just deals with the subtlety
rather than eliminating it. Really, every call site of freedefer (of
which there are surprisingly many) has hidden subtlety between
unlinking the defer and calling freedefer. We could consolidate the
subtlety into each call site by requiring that they unlink the defer
and set d.link to nil before calling freedefer. freedefer could check
this condition like it checks that various other fields have already
been zeroed. A more radical option is to replace freedefer with
"popDefer", which would both pop the defer off the link and take care
of freeing it. There would still be a brief moment of subtlety, but it
would be in one place, in popDefer. Annoyingly, *almost* every call to
freedefer just pops the defer from the head of the G's list, but
there's one place when handling open-coded defers where we have to
remove a defer from the middle of the list. I'm inclined to first fix
that subtlety by only expanding open-coded defer records when they're
at the head of the defer list, and then revisit the popDefer idea.

Change-Id: I3130d2542c01a421a5d60e8c31f5379263219627
Reviewed-on: https://go-review.googlesource.com/c/go/+/339730
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
src/runtime/panic.go
src/runtime/runtime2.go