Revert "runtime/cgo: store M for C-created thread in pthread key"

This reverts CL 481061.

Reason for revert: When built with C TSAN, x_cgo_getstackbound triggers
race detection on `g->stacklo` because the synchronization is in Go,
which isn't instrumented.

For #51676.
For #59294.
For #59678.

Change-Id: I38afcda9fcffd6537582a39a5214bc23dc147d47
Reviewed-on: https://go-review.googlesource.com/c/go/+/485275
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Michael Pratt <[email protected]>
Run-TryBot: Michael Pratt <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>

Author: prattmic

misc/cgo/test/cgo_test.go

@@ -104,7 +104,6 @@ func TestThreadLock(t *testing.T)            { testThreadLockFunc(t) }
 func TestUnsignedInt(t *testing.T)           { testUnsignedInt(t) }
 func TestZeroArgCallback(t *testing.T)       { testZeroArgCallback(t) }
 
-func BenchmarkCgoCall(b *testing.B)      { benchCgoCall(b) }
-func BenchmarkGoString(b *testing.B)     { benchGoString(b) }
-func BenchmarkCGoCallback(b *testing.B)  { benchCallback(b) }
-func BenchmarkCGoInCThread(b *testing.B) { benchCGoInCthread(b) }
+func BenchmarkCgoCall(b *testing.B)     { benchCgoCall(b) }
+func BenchmarkGoString(b *testing.B)    { benchGoString(b) }
+func BenchmarkCGoCallback(b *testing.B) { benchCallback(b) }

misc/cgo/test/cthread_unix.c

@@ -32,27 +32,3 @@ doAdd(int max, int nthread)
 	for(i=0; i<nthread; i++)
 		pthread_join(thread_id[i], 0);		
 }
-
-static void*
-goDummyCallbackThread(void* p)
-{
-	int i, max;
-
-	max = *(int*)p;
-	for(i=0; i<max; i++)
-		goDummy();
-	return NULL;
-}
-
-int
-callGoInCThread(int max)
-{
-	pthread_t thread;
-
-	if (pthread_create(&thread, NULL, goDummyCallbackThread, (void*)(&max)) != 0)
-		return -1;
-	if (pthread_join(thread, NULL) != 0)
-		return -1;
-
-	return max;
-}

misc/cgo/test/cthread_windows.c

@@ -35,25 +35,3 @@ doAdd(int max, int nthread)
 		CloseHandle((HANDLE)thread_id[i]);
 	}
 }
-
-__stdcall
-static unsigned int
-goDummyCallbackThread(void* p)
-{
-	int i, max;
-
-	max = *(int*)p;
-	for(i=0; i<max; i++)
-		goDummy();
-	return 0;
-}
-
-int
-callGoInCThread(int max)
-{
-	uintptr_t thread_id;
-	thread_id = _beginthreadex(0, 0, goDummyCallbackThread, &max, 0, 0);
-	WaitForSingleObject((HANDLE)thread_id, INFINITE);
-	CloseHandle((HANDLE)thread_id);
-	return max;
-}

misc/cgo/test/testx.go

@@ -24,7 +24,6 @@ import (
 /*
 // threads
 extern void doAdd(int, int);
-extern int callGoInCThread(int);
 
 // issue 1328
 void IntoC(void);
@@ -147,10 +146,6 @@ func Add(x int) {
 	*p = 2
 }
 
-//export goDummy
-func goDummy() {
-}
-
 func testCthread(t *testing.T) {
 	if (runtime.GOOS == "darwin" || runtime.GOOS == "ios") && runtime.GOARCH == "arm64" {
 		t.Skip("the iOS exec wrapper is unable to properly handle the panic from Add")
@@ -164,15 +159,6 @@ func testCthread(t *testing.T) {
 	}
 }
 
-// Benchmark measuring overhead from C to Go in a C thread.
-// Create a new C thread and invoke Go function repeatedly in the new C thread.
-func benchCGoInCthread(b *testing.B) {
-	n := C.callGoInCThread(C.int(b.N))
-	if int(n) != b.N {
-		b.Fatal("unmatch loop times")
-	}
-}
-
 // issue 1328
 
 //export BackIntoGo

misc/cgo/testcarchive/carchive_test.go

@@ -1247,57 +1247,3 @@ func TestPreemption(t *testing.T) {
 		t.Error(err)
 	}
 }
-
-// Issue 59294. Test calling Go function from C after using some
-// stack space.
-func TestDeepStack(t *testing.T) {
-	t.Parallel()
-
-	if !testWork {
-		defer func() {
-			os.Remove("testp9" + exeSuffix)
-			os.Remove("libgo9.a")
-			os.Remove("libgo9.h")
-		}()
-	}
-
-	cmd := exec.Command("go", "build", "-buildmode=c-archive", "-o", "libgo9.a", "./libgo9")
-	out, err := cmd.CombinedOutput()
-	t.Logf("%v\n%s", cmd.Args, out)
-	if err != nil {
-		t.Fatal(err)
-	}
-	checkLineComments(t, "libgo9.h")
-	checkArchive(t, "libgo9.a")
-
-	// build with -O0 so the C compiler won't optimize out the large stack frame
-	ccArgs := append(cc, "-O0", "-o", "testp9"+exeSuffix, "main9.c", "libgo9.a")
-	out, err = exec.Command(ccArgs[0], ccArgs[1:]...).CombinedOutput()
-	t.Logf("%v\n%s", ccArgs, out)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	argv := cmdToRun("./testp9")
-	cmd = exec.Command(argv[0], argv[1:]...)
-	sb := new(strings.Builder)
-	cmd.Stdout = sb
-	cmd.Stderr = sb
-	if err := cmd.Start(); err != nil {
-		t.Fatal(err)
-	}
-
-	timer := time.AfterFunc(time.Minute,
-		func() {
-			t.Error("test program timed out")
-			cmd.Process.Kill()
-		},
-	)
-	defer timer.Stop()
-
-	err = cmd.Wait()
-	t.Logf("%v\n%s", cmd.Args, sb)
-	if err != nil {
-		t.Error(err)
-	}
-}

misc/cgo/testcarchive/testdata/libgo9/a.go

@@ -689,20 +689,7 @@ nosave:
 TEXT ·cgocallback(SB),NOSPLIT,$12-12  // Frame size must match commented places below
 	NO_LOCAL_POINTERS
 
-	// Skip cgocallbackg, just dropm when fn is nil, and frame is the saved g.
-	// It is used to dropm while thread is exiting.
-	MOVL	fn+0(FP), AX
-	CMPL	AX, $0
-	JNE	loadg
-	// Restore the g from frame.
-	get_tls(CX)
-	MOVL	frame+4(FP), BX
-	MOVL	BX, g(CX)
-	JMP	dropm
-
-loadg:
-	// If g is nil, Go did not create the current thread,
-	// or if this thread never called into Go on pthread platforms.
+	// If g is nil, Go did not create the current thread.
 	// Call needm to obtain one for temporary use.
 	// In this case, we're running on the thread stack, so there's
 	// lots of space, but the linker doesn't know. Hide the call from
@@ -720,9 +707,9 @@ loadg:
 	MOVL	BP, savedm-4(SP) // saved copy of oldm
 	JMP	havem
 needm:
-	MOVL	$runtime·needAndBindM(SB), AX
+	MOVL	$runtime·needm(SB), AX
 	CALL	AX
-	MOVL	$0, savedm-4(SP)
+	MOVL	$0, savedm-4(SP) // dropm on return
 	get_tls(CX)
 	MOVL	g(CX), BP
 	MOVL	g_m(BP), BP
@@ -797,29 +784,13 @@ havem:
 	MOVL	0(SP), AX
 	MOVL	AX, (g_sched+gobuf_sp)(SI)
 
-	// If the m on entry was nil, we called needm above to borrow an m,
-	// 1. for the duration of the call on non-pthread platforms,
-	// 2. or the duration of the C thread alive on pthread platforms.
-	// If the m on entry wasn't nil,
-	// 1. the thread might be a Go thread,
-	// 2. or it's wasn't the first call from a C thread on pthread platforms,
-	//    since the we skip dropm to resue the m in the first call.
+	// If the m on entry was nil, we called needm above to borrow an m
+	// for the duration of the call. Since the call is over, return it with dropm.
 	MOVL	savedm-4(SP), DX
 	CMPL	DX, $0
-	JNE	droppedm
-
-	// Skip dropm to reuse it in the next call, when a pthread key has been created.
-	MOVL	_cgo_pthread_key_created(SB), DX
-	// It means cgo is disabled when _cgo_pthread_key_created is a nil pointer, need dropm.
-	CMPL	DX, $0
-	JEQ	dropm
-	CMPL	(DX), $0
-	JNE	droppedm
-
-dropm:
+	JNE 3(PC)
 	MOVL	$runtime·dropm(SB), AX
 	CALL	AX
-droppedm:
 
 	// Done!
 	RET

misc/cgo/testcarchive/testdata/main9.c

@@ -918,20 +918,7 @@ GLOBL zeroTLS<>(SB),RODATA,$const_tlsSize
 TEXT ·cgocallback(SB),NOSPLIT,$24-24
 	NO_LOCAL_POINTERS
 
-	// Skip cgocallbackg, just dropm when fn is nil, and frame is the saved g.
-	// It is used to dropm while thread is exiting.
-	MOVQ	fn+0(FP), AX
-	CMPQ	AX, $0
-	JNE	loadg
-	// Restore the g from frame.
-	get_tls(CX)
-	MOVQ	frame+8(FP), BX
-	MOVQ	BX, g(CX)
-	JMP	dropm
-
-loadg:
-	// If g is nil, Go did not create the current thread,
-	// or if this thread never called into Go on pthread platforms.
+	// If g is nil, Go did not create the current thread.
 	// Call needm to obtain one m for temporary use.
 	// In this case, we're running on the thread stack, so there's
 	// lots of space, but the linker doesn't know. Hide the call from
@@ -969,9 +956,9 @@ needm:
 	// a bad value in there, in case needm tries to use it.
 	XORPS	X15, X15
 	XORQ    R14, R14
-	MOVQ	$runtime·needAndBindM<ABIInternal>(SB), AX
+	MOVQ	$runtime·needm<ABIInternal>(SB), AX
 	CALL	AX
-	MOVQ	$0, savedm-8(SP)
+	MOVQ	$0, savedm-8(SP) // dropm on return
 	get_tls(CX)
 	MOVQ	g(CX), BX
 	MOVQ	g_m(BX), BX
@@ -1060,26 +1047,11 @@ havem:
 	MOVQ	0(SP), AX
 	MOVQ	AX, (g_sched+gobuf_sp)(SI)
 
-	// If the m on entry was nil, we called needm above to borrow an m,
-	// 1. for the duration of the call on non-pthread platforms,
-	// 2. or the duration of the C thread alive on pthread platforms.
-	// If the m on entry wasn't nil,
-	// 1. the thread might be a Go thread,
-	// 2. or it's wasn't the first call from a C thread on pthread platforms,
-	//    since the we skip dropm to resue the m in the first call.
+	// If the m on entry was nil, we called needm above to borrow an m
+	// for the duration of the call. Since the call is over, return it with dropm.
 	MOVQ	savedm-8(SP), BX
 	CMPQ	BX, $0
 	JNE	done
-
-	// Skip dropm to reuse it in the next call, when a pthread key has been created.
-	MOVQ	_cgo_pthread_key_created(SB), AX
-	// It means cgo is disabled when _cgo_pthread_key_created is a nil pointer, need dropm.
-	CMPQ	AX, $0
-	JEQ	dropm
-	CMPQ	(AX), $0
-	JNE	done
-
-dropm:
 	MOVQ	$runtime·dropm(SB), AX
 	CALL	AX
 #ifdef GOOS_windows

src/runtime/asm_386.s

@@ -630,16 +630,6 @@ nosave:
 TEXT	·cgocallback(SB),NOSPLIT,$12-12
 	NO_LOCAL_POINTERS
 
-	// Skip cgocallbackg, just dropm when fn is nil, and frame is the saved g.
-	// It is used to dropm while thread is exiting.
-	MOVW	fn+0(FP), R1
-	CMP	$0, R1
-	B.NE	loadg
-	// Restore the g from frame.
-	MOVW	frame+4(FP), g
-	B	dropm
-
-loadg:
 	// Load m and g from thread-local storage.
 #ifdef GOOS_openbsd
 	BL	runtime·load_g(SB)
@@ -649,8 +639,7 @@ loadg:
 	BL.NE	runtime·load_g(SB)
 #endif
 
-	// If g is nil, Go did not create the current thread,
-	// or if this thread never called into Go on pthread platforms.
+	// If g is nil, Go did not create the current thread.
 	// Call needm to obtain one for temporary use.
 	// In this case, we're running on the thread stack, so there's
 	// lots of space, but the linker doesn't know. Hide the call from
@@ -664,7 +653,7 @@ loadg:
 
 needm:
 	MOVW	g, savedm-4(SP) // g is zero, so is m.
-	MOVW	$runtime·needAndBindM(SB), R0
+	MOVW	$runtime·needm(SB), R0
 	BL	(R0)
 
 	// Set m->g0->sched.sp = SP, so that if a panic happens
@@ -735,31 +724,14 @@ havem:
 	MOVW	savedsp-12(SP), R4	// must match frame size
 	MOVW	R4, (g_sched+gobuf_sp)(g)
 
-	// If the m on entry was nil, we called needm above to borrow an m,
-	// 1. for the duration of the call on non-pthread platforms,
-	// 2. or the duration of the C thread alive on pthread platforms.
-	// If the m on entry wasn't nil,
-	// 1. the thread might be a Go thread,
-	// 2. or it's wasn't the first call from a C thread on pthread platforms,
-	//    since the we skip dropm to resue the m in the first call.
+	// If the m on entry was nil, we called needm above to borrow an m
+	// for the duration of the call. Since the call is over, return it with dropm.
 	MOVW	savedm-4(SP), R6
 	CMP	$0, R6
-	B.NE	done
-
-	// Skip dropm to reuse it in the next call, when a pthread key has been created.
-	MOVW	_cgo_pthread_key_created(SB), R6
-	// It means cgo is disabled when _cgo_pthread_key_created is a nil pointer, need dropm.
-	CMP	$0, R6
-	B.EQ	dropm
-	MOVW	(R6), R6
-	CMP	$0, R6
-	B.NE	done
-
-dropm:
+	B.NE	3(PC)
 	MOVW	$runtime·dropm(SB), R0
 	BL	(R0)
 
-done:
 	// Done!
 	RET