tree 96aab5ed6739dacbd3b3749db53a2f5cb8e86acb
parent 9483a0bc23904af80e47aaa8cf1239b3012246d2
author Austin Clements <austin@google.com> 1516305485 -0500
committer Austin Clements <austin@google.com> 1516738126 +0000

runtime: never allocate during an unrecoverable panic

Currently, startpanic_m (which prepares for an unrecoverable panic)
goes out of its way to make it possible to allocate during panic
handling by allocating an mcache if there isn't one.

However, this is both potentially dangerous and unnecessary.
Allocating an mcache is a generally complex thing to do in an already
precarious situation. Specifically, it requires obtaining the heap
lock, and there's evidence that this may be able to deadlock (#23360).
However, it's also unnecessary because we never allocate from the
unrecoverable panic path.

This didn't use to be the case. The call to allocmcache was introduced
long ago, in CL 7388043, where it was in preparation for separating Ms
and Ps and potentially running an M without an mcache. At the time,
after calling startpanic, the runtime could call String and Error
methods on panicked values, which could do anything including
allocating. That was generally unsafe even at the time, and CL 19792
fixed this be pre-printing panic messages before calling startpanic.
As a result, we now no longer allocate after calling startpanic.

This CL not only removes the allocmcache call, but goes a step further
to explicitly disallow any allocation during unrecoverable panic
handling, even in situations where it might be safe. This way, if
panic handling ever does an allocation that would be unsafe in unusual
circumstances, we'll know even if it happens during normal
circumstances.

This would help with debugging #23360, since the deadlock in
allocmcache is currently masking the real failure.

Beyond all.bash, I manually tested this change by adding panics at
various points in early runtime init, signal handling, and the
scheduler to check unusual panic situations.

Change-Id: I85df21e2b4b20c6faf1f13fae266c9339eebc061
Reviewed-on: https://go-review.googlesource.com/88835
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>