runtime: fix handling of SPWRITE functions in traceback

It is valid to see SPWRITE functions at the top of a GC stack traceback,
in the case where they self-preempted during the stack growth check
and haven't actually modified SP in a traceback-unfriendly manner yet.
The current check is therefore too aggressive.

isAsyncSafePoint is taking care of not async-preempting SPWRITE functions
because it doesn't async-preempt any assembly functions at all.
But perhaps it will in the future.

To keep a check that SPWRITE assembly functions are not async-preempted,
add one in preemptPark. Then relax the check in traceback to avoid
triggering on self-preempted SPWRITE functions.

The long and short of this is that the assembly we corrected in x/crypto
issue #44269 was incredibly dodgy but not technically incompatible with
the Go runtime. After this change, the original x/crypto assembly no longer
causes GC traceback crashes during "GOGC=1 go test -count=1000".
But we'll still leave the corrected assembly.

This also means that we don't need to worry about diagnosing SPWRITE
assembly functions that may exist in the wild. They will be skipped for
async preemption and no harm no foul.

Fixes #44269, which was open pending some kind of check for
bad SPWRITE functions in the wild. (No longer needed.)

Change-Id: I6000197b62812bbd2cd92da28eab422634cf75a8
Reviewed-on: https://go-review.googlesource.com/c/go/+/317669
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>

1 files changed, 2 insertions, 0 deletions

diff --git a/src/runtime/preempt.go b/src/runtime/preempt.go
index 372185266f..1d5aae1363 100644
--- a/src/runtime/preempt.go
+++ b/src/runtime/preempt.go
@@ -413,6 +413,8 @@ func isAsyncSafePoint(gp *g, pc, sp, lr uintptr) (bool, uintptr) {
 		//
 		// TODO: Are there cases that are safe but don't have a
 		// locals pointer map, like empty frame functions?
+		// It might be possible to preempt any assembly functions
+		// except the ones that have funcFlag_SPWRITE set in f.flag.
 		return false, 0
 	}
 	name := funcname(f)