Skip to content

Commit 2bcbe6a

Browse files
committed
runtime: add a test for getg with thread switch
With gccgo, if we generate getg inlined, the backend may cache the address of the TLS variable, which will become invalid after a thread switch. Currently there is no known bug for this. But if we didn't implement this carefully, we may get subtle bugs. This CL adds a test that will fail loudly if this is wrong. (See also https://go.googlesource.com/gofrontend/+/refs/heads/master/libgo/runtime/proc.c#333 and an incorrect attempt CL 185337.) Note: at least on Linux/AMD64, even with an incorrect implementation, this only fails if the test is compiled with -fPIC, which is not the default setting for gccgo test suite. So some manual work is needed. Maybe we could extend the test suite to run the runtime test with more settings (e.g. PIC and static). Change-Id: I459a3b4c31f09b9785c0eca19b7756f80e8ef54c Reviewed-on: https://go-review.googlesource.com/c/go/+/186357 Run-TryBot: Cherry Zhang <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Than McIntosh <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent 6bf2767 commit 2bcbe6a

File tree

2 files changed

+38
-0
lines changed

2 files changed

+38
-0
lines changed

src/runtime/export_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -681,3 +681,37 @@ func (t *Treap) CheckInvariants() {
681681
t.mTreap.treap.walkTreap(checkTreapNode)
682682
t.mTreap.treap.validateInvariants()
683683
}
684+
685+
func RunGetgThreadSwitchTest() {
686+
// Test that getg works correctly with thread switch.
687+
// With gccgo, if we generate getg inlined, the backend
688+
// may cache the address of the TLS variable, which
689+
// will become invalid after a thread switch. This test
690+
// checks that the bad caching doesn't happen.
691+
692+
ch := make(chan int)
693+
go func(ch chan int) {
694+
ch <- 5
695+
LockOSThread()
696+
}(ch)
697+
698+
g1 := getg()
699+
700+
// Block on a receive. This is likely to get us a thread
701+
// switch. If we yield to the sender goroutine, it will
702+
// lock the thread, forcing us to resume on a different
703+
// thread.
704+
<-ch
705+
706+
g2 := getg()
707+
if g1 != g2 {
708+
panic("g1 != g2")
709+
}
710+
711+
// Also test getg after some control flow, as the
712+
// backend is sensitive to control flow.
713+
g3 := getg()
714+
if g1 != g3 {
715+
panic("g1 != g3")
716+
}
717+
}

src/runtime/proc_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -977,3 +977,7 @@ func TestPreemptionAfterSyscall(t *testing.T) {
977977
})
978978
}
979979
}
980+
981+
func TestGetgThreadSwitch(t *testing.T) {
982+
runtime.RunGetgThreadSwitchTest()
983+
}

0 commit comments

Comments
 (0)