[release-branch.go1.11] runtime: ignore races between close and len/cap

They aren't really races, or at least they don't have any
observable effect. The spec is silent on whether these are actually
races or not.

Fix this problem by not using the address of len (or of cap)
as the location where channel operations are recorded to occur.
Use a random other field of hchan for that.

I'm not 100% sure we should in fact fix this. Opinions welcome.

Fixes #27778

Change-Id: Ib4efd4b62e0d1ef32fa51e373035ef207a655084
Reviewed-on: https://go-review.googlesource.com/135698
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
(cherry picked from commit 83dfc3b001245f0b725afdc94c0b540fe1952d21)
Reviewed-on: https://go-review.googlesource.com/138179
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/runtime/chan.go b/src/runtime/chan.go
index ce71cee4c5b40d9bafaaf3f07b59e27911ac46a4..bf41dfddaf6d96dc127ec1c35f39567abe46bc74 100644 (file)
--- a/src/runtime/chan.go
+++ b/src/runtime/chan.go
@@ -92,7 +92,7 @@ func makechan(t *chantype, size int) *hchan {
                // Queue or element size is zero.
                c = (*hchan)(mallocgc(hchanSize, nil, true))
                // Race detector uses this location for synchronization.
-               c.buf = unsafe.Pointer(c)
+               c.buf = c.raceaddr()
        case elem.kind&kindNoPointers != 0:
                // Elements do not contain pointers.
                // Allocate hchan and buf in one call.
@@ -151,7 +151,7 @@ func chansend(c *hchan, ep unsafe.Pointer, block bool, callerpc uintptr) bool {
        }
 
        if raceenabled {
-               racereadpc(unsafe.Pointer(c), callerpc, funcPC(chansend))
+               racereadpc(c.raceaddr(), callerpc, funcPC(chansend))
        }
 
        // Fast path: check for failed non-blocking operation without acquiring the lock.
@@ -337,8 +337,8 @@ func closechan(c *hchan) {
 
        if raceenabled {
                callerpc := getcallerpc()
-               racewritepc(unsafe.Pointer(c), callerpc, funcPC(closechan))
-               racerelease(unsafe.Pointer(c))
+               racewritepc(c.raceaddr(), callerpc, funcPC(closechan))
+               racerelease(c.raceaddr())
        }
 
        c.closed = 1
@@ -361,7 +361,7 @@ func closechan(c *hchan) {
                gp := sg.g
                gp.param = nil
                if raceenabled {
-                       raceacquireg(gp, unsafe.Pointer(c))
+                       raceacquireg(gp, c.raceaddr())
                }
                gp.schedlink.set(glist)
                glist = gp
@@ -380,7 +380,7 @@ func closechan(c *hchan) {
                gp := sg.g
                gp.param = nil
                if raceenabled {
-                       raceacquireg(gp, unsafe.Pointer(c))
+                       raceacquireg(gp, c.raceaddr())
                }
                gp.schedlink.set(glist)
                glist = gp
@@ -457,7 +457,7 @@ func chanrecv(c *hchan, ep unsafe.Pointer, block bool) (selected, received bool)
 
        if c.closed != 0 && c.qcount == 0 {
                if raceenabled {
-                       raceacquire(unsafe.Pointer(c))
+                       raceacquire(c.raceaddr())
                }
                unlock(&c.lock)
                if ep != nil {
@@ -735,6 +735,15 @@ func (q *waitq) dequeue() *sudog {
        }
 }
 
+func (c *hchan) raceaddr() unsafe.Pointer {
+       // Treat read-like and write-like operations on the channel to
+       // happen at this address. Avoid using the address of qcount
+       // or dataqsiz, because the len() and cap() builtins read
+       // those addresses, and we don't want them racing with
+       // operations like close().
+       return unsafe.Pointer(&c.buf)
+}
+
 func racesync(c *hchan, sg *sudog) {
        racerelease(chanbuf(c, 0))
        raceacquireg(sg.g, chanbuf(c, 0))

diff --git a/src/runtime/race/testdata/chan_test.go b/src/runtime/race/testdata/chan_test.go
index 7f349c42ed783ee5e31ed0f842a1cde9746370c0..60e55ed66a4b0542864d0fc6df65447791336348 100644 (file)
--- a/src/runtime/race/testdata/chan_test.go
+++ b/src/runtime/race/testdata/chan_test.go
@@ -577,18 +577,32 @@ func TestRaceChanItselfCap(t *testing.T) {
        <-compl
 }
 
-func TestRaceChanCloseLen(t *testing.T) {
-       v := 0
-       _ = v
+func TestNoRaceChanCloseLen(t *testing.T) {
        c := make(chan int, 10)
-       c <- 0
+       r := make(chan int, 10)
+       go func() {
+               r <- len(c)
+       }()
        go func() {
-               v = 1
                close(c)
+               r <- 0
        }()
-       time.Sleep(1e7)
-       _ = len(c)
-       v = 2
+       <-r
+       <-r
+}
+
+func TestNoRaceChanCloseCap(t *testing.T) {
+       c := make(chan int, 10)
+       r := make(chan int, 10)
+       go func() {
+               r <- cap(c)
+       }()
+       go func() {
+               close(c)
+               r <- 0
+       }()
+       <-r
+       <-r
 }
 
 func TestRaceChanCloseSend(t *testing.T) {

diff --git a/src/runtime/select.go b/src/runtime/select.go
index 3a3ac6b7ac0bd5f08864621b0a3f8238f8e6613c..2729c2ecf98a225e86fbf5d5de010833d25fd569 100644 (file)
--- a/src/runtime/select.go
+++ b/src/runtime/select.go
@@ -245,7 +245,7 @@ loop:
 
                case caseSend:
                        if raceenabled {
-                               racereadpc(unsafe.Pointer(c), cas.pc, chansendpc)
+                               racereadpc(c.raceaddr(), cas.pc, chansendpc)
                        }
                        if c.closed != 0 {
                                goto sclose
@@ -462,7 +462,7 @@ rclose:
                typedmemclr(c.elemtype, cas.elem)
        }
        if raceenabled {
-               raceacquire(unsafe.Pointer(c))
+               raceacquire(c.raceaddr())
        }
        goto retc