Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/compile: race detector mishandles OBLOCK nodes #9137

Closed
dvyukov opened this issue Nov 20, 2014 · 11 comments
Closed

cmd/compile: race detector mishandles OBLOCK nodes #9137

dvyukov opened this issue Nov 20, 2014 · 11 comments
Labels
FrozenDueToAge RaceDetector
Milestone
Go1.5

Comments

@dvyukov
Copy link
Member

dvyukov commented Nov 20, 2014 

Factored from https://golang.org/issue/9136

I also have a simple program that fails with -race:

$ cat x.go
package main

import "fmt"

func main() {
    a := []string{"a"}
    i := 0
    a[i], a[len(a)-1], a = a[len(a)-1], "", a[:len(a)-1]
    fmt.Println(a)
}

$ go run x.go
panic: runtime error: index out of range
...
@dvyukov
Copy link
Member Author

dvyukov commented Nov 20, 2014

Comment 1:

Seems to be regression in 1.4. 1.3 works fine.

@gopherbot
Copy link

Comment 2 by szamcsi@google.com:

Sorry, I should have added that it was failing in 1.4rc1 and it was working with 1.3.3

@dvyukov
Copy link
Member Author

dvyukov commented Nov 21, 2014

Comment 3:

Most likely write barriers altered evaluation order in race mode; and now we call
racewrite(&a[i]) after assigning to a.
I somewhat afraid of touching that code before release.

@DanielMorsing
Copy link
Contributor

The racewalk gets this as its input

BLOCK
. CALLFUNC writebarrierstring
. a[len(a)-1] = ""
. a = a[:len(a)-1]

Because the first op in the block is a CALLFUNC, the racewalk code assumes that it's a multi-valued return and puts the instrumentation after the block in order to not clobber the stack.

@dvyukov
Copy link
Member Author

dvyukov commented Feb 10, 2015

What if we explicitly mark OBLOCKs that copy out return values of a call with a special flag. And then specially handle only these blocks in racewalk rather than try to guess OBLOCK type by looking at the first node.
The offending blocks seem to be created only by ascompatte during walk. Maybe there are few other functions, but they must be findable by calls to arch.nodarg.

@DanielMorsing
Copy link
Contributor

How about using rlist on the CALL* nodes? It's not used and it would remove the semantic conflation that the BLOCK node has right now.

@dvyukov
Copy link
Member Author

dvyukov commented Feb 10, 2015

This sounds equally good to me. The root cause of the issue is that OBLOCK guessing in racewalk. And your proposal also removes it.

@rsc
Copy link
Contributor

rsc commented Apr 10, 2015

See issue #8102. The handling of OBLOCK is just wrong in general. This is the second time. This needs to be fixed correctly. Again, see my comments on issue #8102.

@rsc rsc changed the title cmd/gc: spurious "index out of range" in race mode cmd/gc: race detector mishandles OBLOCK nodes Apr 10, 2015
@rsc rsc added this to the Go1.5 milestone Apr 10, 2015
@dvyukov
Copy link
Member Author

dvyukov commented Apr 10, 2015

Rewriting racewalk as SSA pass should fix this.

@rsc rsc removed accepted labels Apr 14, 2015
@rsc
Copy link
Contributor

rsc commented Apr 14, 2015

That's not a fix for Go 1.5. It needs to be fixed for Go 1.5.

@rsc rsc changed the title cmd/gc: race detector mishandles OBLOCK nodes cmd/compile: race detector mishandles OBLOCK nodes Jun 8, 2015
@gopherbot
Copy link

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

@rsc rsc closed this as completed in 3b6e86f Jun 30, 2015
@golang golang locked and limited conversation to collaborators Jun 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge RaceDetector
Projects
None yet
Milestone
Go1.5
Development

No branches or pull requests
4 participants
@rsc @DanielMorsing @dvyukov @gopherbot