From: Michael Pratt <mpratt@google.com>
Date: Mon, 31 Jan 2022 21:37:40 +0000 (-0500)
Subject: runtime: regression test for issue 50936
X-Git-Tag: go1.18rc1~113
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=f9b761aa76d0cf439a0c996e1b7c06a0fe49314e;p=gostls13.git

runtime: regression test for issue 50936

Add a regression test for issue 50936 which coerces the runtime into
frequent execution of the cgocall dropg/execute curg assignment race by
making many concurrent cgo calls eligible for P retake by sysmon. This
results in no P during exitsyscall, at which point they will update curg
and will crash if SIGPROF arrives in between updating mp.curg and
mp.curg.m.

This test is conceptually similar to the basic cgo callback test in
aprof.go but with additional concurrency and a sleep in C.

On my machine this test fails ~5% of the time prior to CL 382079.

For #50936.

Change-Id: I21b6c7f2594f9a615a64580ef70a88b692505678
Reviewed-on: https://go-review.googlesource.com/c/go/+/382244
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
---

diff --git a/src/runtime/crash_cgo_test.go b/src/runtime/crash_cgo_test.go
index dc8f6a7148..8c250f72d6 100644
--- a/src/runtime/crash_cgo_test.go
+++ b/src/runtime/crash_cgo_test.go
@@ -216,6 +216,19 @@ func TestCgoCCodeSIGPROF(t *testing.T) {
 	}
 }
 
+func TestCgoPprofCallback(t *testing.T) {
+	t.Parallel()
+	switch runtime.GOOS {
+	case "windows", "plan9":
+		t.Skipf("skipping cgo pprof callback test on %s", runtime.GOOS)
+	}
+	got := runTestProg(t, "testprogcgo", "CgoPprofCallback")
+	want := "OK\n"
+	if got != want {
+		t.Errorf("expected %q got %v", want, got)
+	}
+}
+
 func TestCgoCrashTraceback(t *testing.T) {
 	t.Parallel()
 	switch platform := runtime.GOOS + "/" + runtime.GOARCH; platform {
diff --git a/src/runtime/testdata/testprogcgo/aprof.go b/src/runtime/testdata/testprogcgo/aprof.go
index 44a15b0865..c70d6333bb 100644
--- a/src/runtime/testdata/testprogcgo/aprof.go
+++ b/src/runtime/testdata/testprogcgo/aprof.go
@@ -7,8 +7,11 @@ package main
 // Test that SIGPROF received in C code does not crash the process
 // looking for the C code's func pointer.
 
-// The test fails when the function is the first C function.
-// The exported functions are the first C functions, so we use that.
+// This is a regression test for issue 14599, where profiling fails when the
+// function is the first C function. Exported functions are the first C
+// functions, so we use an exported function. Exported functions are created in
+// lexigraphical order of source files, so this file is named aprof.go to
+// ensure its function is first.
 
 // extern void CallGoNop();
 import "C"
diff --git a/src/runtime/testdata/testprogcgo/pprof_callback.go b/src/runtime/testdata/testprogcgo/pprof_callback.go
new file mode 100644
index 0000000000..e34564395e
--- /dev/null
+++ b/src/runtime/testdata/testprogcgo/pprof_callback.go
@@ -0,0 +1,89 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build !plan9 && !windows
+
+package main
+
+// Make many C-to-Go callback while collecting a CPU profile.
+//
+// This is a regression test for issue 50936.
+
+/*
+#include <unistd.h>
+
+void goCallbackPprof();
+
+static void callGo() {
+	// Spent >20us in C so this thread is eligible for sysmon to retake its
+	// P.
+	usleep(50);
+	goCallbackPprof();
+}
+*/
+import "C"
+
+import (
+	"fmt"
+	"os"
+	"runtime/pprof"
+	"runtime"
+	"time"
+)
+
+func init() {
+	register("CgoPprofCallback", CgoPprofCallback)
+}
+
+//export goCallbackPprof
+func goCallbackPprof() {
+	// No-op. We want to stress the cgocall and cgocallback internals,
+	// landing as many pprof signals there as possible.
+}
+
+func CgoPprofCallback() {
+	// Issue 50936 was a crash in the SIGPROF handler when the signal
+	// arrived during the exitsyscall following a cgocall(back) in dropg or
+	// execute, when updating mp.curg.
+	//
+	// These are reachable only when exitsyscall finds no P available. Thus
+	// we make C calls from significantly more Gs than there are available
+	// Ps. Lots of runnable work combined with >20us spent in callGo makes
+	// it possible for sysmon to retake Ps, forcing C calls to go down the
+	// desired exitsyscall path.
+	//
+	// High GOMAXPROCS is used to increase opportunities for failure on
+	// high CPU machines.
+	const (
+		P = 16
+		G = 64
+	)
+	runtime.GOMAXPROCS(P)
+
+	f, err := os.CreateTemp("", "prof")
+	if err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(2)
+	}
+	defer f.Close()
+
+	if err := pprof.StartCPUProfile(f); err != nil {
+		fmt.Fprintln(os.Stderr, err)
+		os.Exit(2)
+	}
+
+	for i := 0; i < G; i++ {
+		go func() {
+			for {
+				C.callGo()
+			}
+		}()
+	}
+
+	time.Sleep(time.Second)
+
+	pprof.StopCPUProfile()
+
+	fmt.Println("OK")
+}