FAQ
Reviewers: ken2,

Message:
Hello ken2 (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/


Description:
cmd/gc: fix eval order in select

Ordinary variable load was assumed to be not worth saving,
but not if one of the function calls later might change
its value.

Fixes issue 4313.

Please review this at https://codereview.appspot.com/6997047/

Affected files:
M src/cmd/gc/order.c
M src/cmd/gc/select.c
M src/cmd/gc/subr.c
A test/fixedbugs/issue4313.go


Index: src/cmd/gc/order.c
===================================================================
--- a/src/cmd/gc/order.c
+++ b/src/cmd/gc/order.c
@@ -276,11 +276,11 @@
case OSELRECV2:
orderexprinplace(&r->left);
orderexprinplace(&r->ntest);
- orderexpr(&r->right->left, out);
+ orderexpr(&r->right->left, &l->n->ninit);
break;
case OSEND:
- orderexpr(&r->left, out);
- orderexpr(&r->right, out);
+ orderexpr(&r->left, &l->n->ninit);
+ orderexpr(&r->right, &l->n->ninit);
break;
}
}
Index: src/cmd/gc/select.c
===================================================================
--- a/src/cmd/gc/select.c
+++ b/src/cmd/gc/select.c
@@ -297,15 +297,15 @@
setlineno(cas);
n = cas->left;
r = nod(OIF, N, N);
- r->nbody = cas->ninit;
+ r->ninit = cas->ninit;
cas->ninit = nil;
if(n != nil) {
- r->nbody = concat(r->nbody, n->ninit);
+ r->ninit = concat(r->ninit, n->ninit);
n->ninit = nil;
}
if(n == nil) {
// selectdefault(sel *byte);
- r->ntest = mkcall("selectdefault", types[TBOOL], &init, var);
+ r->ntest = mkcall("selectdefault", types[TBOOL], &r->ninit, var);
} else {
switch(n->op) {
default:
@@ -313,25 +313,25 @@

case OSEND:
// selectsend(sel *byte, hchan *chan any, elem *any) (selected bool);
- n->left = safeexpr(n->left, &r->ninit);
+ n->left = localexpr(safeexpr(n->left, &r->ninit), n->left->type,
&r->ninit);
n->right = localexpr(n->right, n->left->type->type, &r->ninit);
n->right = nod(OADDR, n->right, N);
n->right->etype = 1; // pointer does not escape
typecheck(&n->right, Erv);
r->ntest = mkcall1(chanfn("selectsend", 2, n->left->type),
types[TBOOL],
- &init, var, n->left, n->right);
+ &r->ninit, var, n->left, n->right);
break;

case OSELRECV:
// selectrecv(sel *byte, hchan *chan any, elem *any) (selected bool);
r->ntest = mkcall1(chanfn("selectrecv", 2, n->right->left->type),
types[TBOOL],
- &init, var, n->right->left, n->left);
+ &r->ninit, var, n->right->left, n->left);
break;

case OSELRECV2:
// selectrecv2(sel *byte, hchan *chan any, elem *any, received *bool)
(selected bool);
r->ntest = mkcall1(chanfn("selectrecv2", 2, n->right->left->type),
types[TBOOL],
- &init, var, n->right->left, n->left, n->ntest);
+ &r->ninit, var, n->right->left, n->left, n->ntest);
break;
}
}
Index: src/cmd/gc/subr.c
===================================================================
--- a/src/cmd/gc/subr.c
+++ b/src/cmd/gc/subr.c
@@ -2040,11 +2040,13 @@

/*
* return n in a local variable of type t if it is not already.
+ * the value is guaranteed not to change except by direct
+ * assignment to it.
*/
Node*
localexpr(Node *n, Type *t, NodeList **init)
{
- if(n->op == ONAME &&
+ if(n->op == ONAME && !n->addrtaken &&
(n->class == PAUTO || n->class == PPARAM || n->class == PPARAMOUT) &&
convertop(n->type, t, nil) == OCONVNOP)
return n;
Index: test/fixedbugs/issue4313.go
===================================================================
new file mode 100644
--- /dev/null
+++ b/test/fixedbugs/issue4313.go
@@ -0,0 +1,28 @@
+// run
+
+// Copyright 2012 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Order of operations in select.
+
+package main
+
+func main() {
+ c := make(chan int, 1)
+ x := 0
+ select {
+ case c <- x: // should see x = 0, not x = 42 (after makec)
+ case <-makec(&x): // should be evaluated only after c and x on previous
line
+ }
+ y := <-c
+ if y != 0 {
+ panic(y)
+ }
+}
+
+func makec(px *int) chan bool {
+ if false { for {} }
+ *px = 42
+ return make(chan bool, 0)
+}

Search Discussions

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedDec 22, '12 at 9:45p
activeDec 22, '12 at 10:48p
posts3
users2
websitegolang.org

2 users in discussion

Rsc: 2 posts Ken: 1 post

People

Translate

site design / logo © 2022 Grokbase