]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: add a test for getg with thread switch
authorCherry Zhang <cherryyz@google.com>
Tue, 16 Jul 2019 15:33:10 +0000 (11:33 -0400)
committerCherry Zhang <cherryyz@google.com>
Tue, 16 Jul 2019 20:53:01 +0000 (20:53 +0000)
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 <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
src/runtime/export_test.go
src/runtime/proc_test.go

index 62b7730c44c6a9c28452c2e152692eeac6cbe9fd..6009932056349b3d9f2e699f32419465ca2dc0d3 100644 (file)
@@ -681,3 +681,37 @@ func (t *Treap) CheckInvariants() {
        t.mTreap.treap.walkTreap(checkTreapNode)
        t.mTreap.treap.validateInvariants()
 }
+
+func RunGetgThreadSwitchTest() {
+       // Test that getg works correctly 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. This test
+       // checks that the bad caching doesn't happen.
+
+       ch := make(chan int)
+       go func(ch chan int) {
+               ch <- 5
+               LockOSThread()
+       }(ch)
+
+       g1 := getg()
+
+       // Block on a receive. This is likely to get us a thread
+       // switch. If we yield to the sender goroutine, it will
+       // lock the thread, forcing us to resume on a different
+       // thread.
+       <-ch
+
+       g2 := getg()
+       if g1 != g2 {
+               panic("g1 != g2")
+       }
+
+       // Also test getg after some control flow, as the
+       // backend is sensitive to control flow.
+       g3 := getg()
+       if g1 != g3 {
+               panic("g1 != g3")
+       }
+}
index 09b0652bee74d9f26f691ece2077a47f41f59f90..6e6272e80a2a57248e7b02453aadae01f1429f17 100644 (file)
@@ -977,3 +977,7 @@ func TestPreemptionAfterSyscall(t *testing.T) {
                })
        }
 }
+
+func TestGetgThreadSwitch(t *testing.T) {
+       runtime.RunGetgThreadSwitchTest()
+}