cmd/gc: race detector rewrites for loop increment incorrectly

[dvyukov]

go version go1.3beta2 +1de165698f51 Thu May 22 11:45:03 2014 -0400 darwin/amd64

The program is

package a
type S struct {
    x interface{}
    i int
}
func f() {
    c := make(chan int)
    a := [2]*int{}
    for ; ; c <- *a[S{}.i] {
    }
}

$ go build -race src.go 
src.go:6: internal error: f autotmp_0001 (type S) recorded as live on entry

[ianlancetaylor]

Comment 1:

Labels changed: added release-go1.3maybe.

[rsc]

Comment 2:

If you compare the output of 6g -live=2 src.go and 6g -live=2 -race src.go you can see
that the -race mode is reordering the code in a weird way. The initialization of
autotmp_0001 looks like:
00020 (/tmp/x.go:11)    LEAQ    "".statictmp_0002+0(SB),BX
00021 (/tmp/x.go:11)    VARDEF  ,"".autotmp_0001+-24(SP)
    varkill=autotmp_0001
00022 (/tmp/x.go:11)    LEAQ    "".autotmp_0001+-24(SP),BP
    uevar=autotmp_0001
00023 (/tmp/x.go:11)    MOVQ    BP,DI
00024 (/tmp/x.go:11)    MOVQ    BX,SI
00025 (/tmp/x.go:11)    MOVSQ   ,
00026 (/tmp/x.go:11)    MOVSQ   ,
00027 (/tmp/x.go:11)    MOVSQ   ,
In the non-race version this is the beginning of basic block #1. In the race version
this is the middle (!) of basic block #3, and basic block #1 (reachable before #3) has
already referred to autotmp_0001. I don't know why the race detector is reordering code.
Leaving for Dmitriy. It won't block the release candidate. We'll see about the release.

Owner changed to @dvyukov.

Status changed to Accepted.

[rsc]

Comment 3:

I added dump calls for OFOR nodes before and after racewalknode. 
before:
for [100525368]
.   FOR l(9) tc(1)
.   FOR-incr
.   .   BLOCK l(9)
.   .   BLOCK-list
.   .   .   CALLFUNC-init
.   .   .   .   AS u(2) l(9) tc(1)
.   .   .   .   .   NAME-a.autotmp_0001 u(1) a(1) l(9) x(0+0) class(PAUTO) esc(N) tc(1)
used(1) a.S
.   .   .   .   .   NAME-a.statictmp_0002 u(1) a(1) l(9) x(0+0) class(PEXTERN) f(1)
tc(1) used(1) a.S
.   .   .   CALLFUNC u(100) l(9) tc(1)
.   .   .   .   NAME-runtime.chansend1 u(1) a(1) l(2) x(0+0) class(PFUNC) tc(1) used(1)
FUNC-funcSTRUCT-(FIELD-
.   .   .   .   .   NAME-runtime.chanType·1 u(1) a(1) l(2) x(0+0) class(PPARAM) f(1)
PTR64-*byte PTR64-*byte, FIELD-
.   .   .   .   .   NAME-runtime.hchan·2 u(1) a(1) l(2) x(8+0) class(PPARAM) f(1)
CHAN-chan<- any CHAN-chan<- int, FIELD-
.   .   .   .   .   NAME-runtime.elem·3 u(1) a(1) l(2) x(16+0) class(PPARAM) f(1)
PTR64-*any PTR64-*int)
...
after:
racefor [100525368]
.   FOR l(9) tc(1)
.   FOR-incr
.   .   BLOCK-init
.   .   .   CALLFUNC u(100) l(9) tc(1)
.   .   .   .   NAME-runtime.raceread u(1) a(1) l(2) x(0+0) class(PFUNC) tc(1) used(1)
FUNC-funcSTRUCT-(FIELD-uintptr)
.   .   .   CALLFUNC-list
.   .   .   .   AS u(2) l(9) tc(1)
.   .   .   .   .   INDREG-SP a(1) l(9) x(0+0) tc(1) uintptr
.   .   .   .   .   CONVNOP u(2) l(9) tc(1) uintptr
.   .   .   .   .   .   CONVNOP u(2) l(9) tc(1) E-30-unsafe.Pointer
.   .   .   .   .   .   .   ADDR u(2) l(9) tc(1) PTR64-*PTR64-*int
.   .   .   .   .   .   .   .   INDEX u(2) l(9) tc(1) PTR64-*int
.   .   .   .   .   .   .   .   .   NAME-a.a u(1) a(1) g(2) l(8) x(0+0) class(PAUTO)
f(1) ld(1) tc(1) used(1) ARRAY-[2]PTR64-*int
.   .   .   .   .   .   .   .   .   DOT u(2) l(9) x(16+0) tc(1) int
.   .   .   .   .   .   .   .   .   .   NAME-a.autotmp_0001 u(1) a(1) l(9) x(0+0)
class(PAUTO) esc(N) tc(1) used(1) a.S
.   .   .   .   .   .   .   .   .   .   NAME-a.i u(1) a(1) l(9) x(0+0)
.   .   BLOCK l(9)
.   .   BLOCK-list
.   .   .   CALLFUNC-init
.   .   .   .   AS u(2) l(9) tc(1)
.   .   .   .   .   NAME-a.autotmp_0001 u(1) a(1) l(9) x(0+0) class(PAUTO) esc(N) tc(1)
used(1) a.S
.   .   .   .   .   NAME-a.statictmp_0002 u(1) a(1) l(9) x(0+0) class(PEXTERN) f(1)
tc(1) used(1) a.S
...
notice that the raceread call has been lifted above the CALLFUNC-init. i believe that
the problem is the assumptions about OBLOCK that i outlined in my comments on CL
76520045 and that the fix is what i outlined there too.
that means this is probably a duplicate of the underlying cause for issue #7561, even
though 7561 itself has gone away.

[rsc]

Comment 4:

I'll fix the specific cause here but I still think OBLOCK is being handled wrong. That's
for someone other than me.

[gopherbot]

Comment 5:

CL https://golang.org/cl/100870046 mentions this issue.

[rsc]

Comment 6:

This issue was closed by revision e56dc99.

Status changed to Fixed.