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
cmd/gc: internal error: foo recorded as live on entry #7561
Labels
Comments
The following change fixes *both* compiler error on tip and runtime crash in 1.2 diff --git a/proto/iris/broadcast_test.go b/proto/iris/broadcast_test.go index d6685a9..2d2b522 100644 --- a/proto/iris/broadcast_test.go +++ b/proto/iris/broadcast_test.go @@ -104,12 +104,12 @@ func testBroadcast(t *testing.T, nodes, conns, msgs int) { liveHands[i] = make([]*broadcaster, conns) liveConns[i] = make([]*Connection, conns) for j := 0; j < conns; j++ { - var err error liveHands[i][j] = &broadcaster{make(chan []byte, nodes*conns*msgs)} - liveConns[i][j], err = node.Connect(cluster, liveHands[i][j]) + conn, err := node.Connect(cluster, liveHands[i][j]) if err != nil { t.Fatalf("failed to connect to the iris overlay: %v.", err) } + liveConns[i][j] = conn defer func(conn *Connection) { if err := conn.Close(); err != nil { t.Fatalf("failed to close iris connection: %v.", err) diff --git a/proto/iris/pubsub_test.go b/proto/iris/pubsub_test.go index 5e29610..d1c25eb 100644 --- a/proto/iris/pubsub_test.go +++ b/proto/iris/pubsub_test.go @@ -97,12 +97,12 @@ func testPubSub(t *testing.T, nodes, conns, msgs int) { liveHands[i] = make([]*subscriber, conns) liveConns[i] = make([]*Connection, conns) for j := 0; j < conns; j++ { - var err error // Connect to the iris network - liveConns[i][j], err = node.Connect(cluster, nil) + conn, err := node.Connect(cluster, nil) if err != nil { t.Fatalf("failed to connect to the iris overlay: %v.", err) } + liveConns[i][j] = conn defer func(conn *Connection) { if err := conn.Close(); err != nil { t.Fatalf("failed to close iris connection: %v.", err) diff --git a/proto/iris/reqrep_test.go b/proto/iris/reqrep_test.go index d589c41..d04c833 100644 --- a/proto/iris/reqrep_test.go +++ b/proto/iris/reqrep_test.go @@ -109,12 +109,12 @@ func testReqRep(t *testing.T, nodes, conns, reqs int) { liveHands[i] = make([]*requester, conns) liveConns[i] = make([]*Connection, conns) for j := 0; j < conns; j++ { - var err error liveHands[i][j] = &requester{i, 0} - liveConns[i][j], err = node.Connect(cluster, liveHands[i][j]) + conn, err := node.Connect(cluster, liveHands[i][j]) if err != nil { t.Fatalf("failed to connect to the iris overlay: %v.", err) } + liveConns[i][j] = conn defer func(conn *Connection) { if err := conn.Close(); err != nil { t.Fatalf("failed to close iris connection: %v.", err) diff --git a/proto/iris/tunnel_test.go b/proto/iris/tunnel_test.go index 053548d..8e27d6e 100644 --- a/proto/iris/tunnel_test.go +++ b/proto/iris/tunnel_test.go @@ -118,12 +118,12 @@ func testTunnel(t *testing.T, nodes, conns, tuns, msgs int) { liveHands[i] = make([]*tunneler, conns) liveConns[i] = make([]*Connection, conns) for j := 0; j < conns; j++ { - var err error liveHands[i][j] = &tunneler{i, 0} - liveConns[i][j], err = node.Connect(cluster, liveHands[i][j]) + conn, err := node.Connect(cluster, liveHands[i][j]) if err != nil { t.Fatalf("failed to connect to the iris overlay: %v.", err) } + liveConns[i][j] = conn defer func(conn *Connection) { if err := conn.Close(); err != nil { t.Fatalf("failed to close iris connection: %v.", err) |
I have investigated this: here is a minimal example: package p var connect func() (int, error) var conns = make(map[int][]int) func TestMultipleReturnToMap() { var i, j int var err error conns[i][j], err = connect() _ = err } The issues arises because multiple returns "a, b = f()" is walked to: { CALL f a = SP[n0] b = SP[n1] } Then because racewalk cannot insert instrumentation between CALL and y = SP[n], it inserts them at the end. However, map indexing is not an actual assignement so if you have "m[x], y = f()" it becomes: { CALL f tmp = SP[n0] y = SP[n1] m[x] = tmp } and racewalk again puts all instrumentation at the end. Thus is becomes: { CALL f tmp = SP[n0] y = SP[n1] *p = tmp // init of previous statement p = mapassign1(m, x) raceread(...); readwrite(...) } Instead it should be: { CALL f tmp = SP[n0] y = SP[n1] <instrumentation of y if it is on the heap> // instrumentation of m[x] = tmp p = mapassign1(m, x) raceread(m); readwrite(p) *p = tmp } I'm not sure yet how to do this properly, I need a closer look later in the week. |
Wall, you may want to try https://golang.org/cl/76520045 but it feels a bit hackish. Status changed to Started. |
> However, map indexing is not an actual assignement so if you have "m[x], y = f()" it becomes: > { > CALL f > tmp = SP[n0] > y = SP[n1] > m[x] = tmp > } Is "m[x] = tmp" a separate statement here? Or a part of a more complex statement? Gc should have been lowered it to a separate statement. Then things would be much simpler. But we probably don't wan't to do such changes before release... Owner changed to @remyoudompheng. |
https://golang.org/cl/76520045 makes sense to me. We also need a test. |
Seems to be working now, because order.c now moves mapassigns to separate statements. g% cat >x.go package p var connect func() (int, error) var conns = make(map[int][]int) func TestMultipleReturnToMap() { var i, j int var err error conns[i][j], err = connect() _ = err } g% go tool 6g -race x.go g% I still believe 76520045 should be rewritten to do what I suggested, but it doesn't seem to be release-critical. Labels changed: removed release-go1.3. |
CL https://golang.org/cl/76520045 references this issue. |
CL https://golang.org/cl/11713 mentions this issue. |
rsc
added a commit
that referenced
this issue
Jun 30, 2015
Fixes #7561 correctly. Fixes #9137. Change-Id: I7f27e199d7101b785a7645f789e8fe41a405a86f Reviewed-on: https://go-review.googlesource.com/11713 Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
The text was updated successfully, but these errors were encountered: