]> Cypherpunks repositories - gostls13.git/commitdiff
gc: stricter multiple assignment + test
authorRuss Cox <rsc@golang.org>
Thu, 13 Oct 2011 19:46:39 +0000 (15:46 -0400)
committerRuss Cox <rsc@golang.org>
Thu, 13 Oct 2011 19:46:39 +0000 (15:46 -0400)
Fixes #693.

R=ken2
CC=golang-dev
https://golang.org/cl/5265045

src/cmd/gc/dcl.c
src/cmd/gc/go.h
src/cmd/gc/subr.c
src/cmd/gc/typecheck.c
src/cmd/gc/walk.c
test/reorder.go [new file with mode: 0644]

index d8b89b4f383d0265789987e9724d95a8377a35a7..afbfd97bf67a3a152144526ffc1856fbc1f26f27 100644 (file)
@@ -415,6 +415,7 @@ oldname(Sym *s)
                        c->funcdepth = funcdepth;
                        c->outer = n->closure;
                        n->closure = c;
+                       n->addrtaken = 1;
                        c->closure = n;
                        c->xoffset = 0;
                        curfn->cvars = list(curfn->cvars, c);
index 2d4ba31c7e44616976d38aac9ca568968348fd34..9ce24eda8bc0f856bd937012ca4a996a592bd6c0 100644 (file)
@@ -266,6 +266,7 @@ struct      Node
        uchar   isddd;
        uchar   readonly;
        uchar   implicit;       // don't show in printout
+       uchar   addrtaken;      // address taken, even if not moved to heap
 
        // most nodes
        Type*   type;
index b584be8272e79cae7ef56174cf6448fb9ddc760a..bf83dd8fa646577ddbd9084964762ee2b5d1dc39 100644 (file)
@@ -1155,6 +1155,9 @@ Jconv(Fmt *fp)
        if(n->funcdepth != 0)
                fmtprint(fp, " f(%d)", n->funcdepth);
 
+       if(n->addrtaken != 0)
+               fmtprint(fp, " addrtaken(1)");
+
        switch(n->esc) {
        case EscUnknown:
                break;
index 9e9e9f9a8165bec391cf5fd6eae5d5434d79fe5e..052fc74dffc870e21c83ccca9cad4eaeb8c1089a 100644 (file)
@@ -532,6 +532,9 @@ reswitch:
                default:
                        checklvalue(n->left, "take the address of");
                }
+               for(l=n->left; l->op == ODOT; l=l->left)
+                       l->addrtaken = 1;
+               l->addrtaken = 1;
                defaultlit(&n->left, T);
                l = n->left;
                if((t = l->type) == T)
index 8dec4956bc777c9ae1c096f58f2a735a0d83a21f..de7004e3e9bd008b650bb8dca47da4af93f898bd 100644 (file)
@@ -124,8 +124,9 @@ paramoutheap(Node *fn)
 
        for(l=fn->dcl; l; l=l->next) {
                switch(l->n->class) {
+               case PPARAMOUT:
                case PPARAMOUT|PHEAP:
-                       return 1;
+                       return l->n->addrtaken;
                case PAUTO:
                case PAUTO|PHEAP:
                        // stop early - parameters are over
@@ -285,6 +286,9 @@ walkstmt(Node **np)
                                n->list = concat(list1(f), ascompatet(n->op, rl, &f->type, 0, &n->ninit));
                                break;
                        }
+
+                       // move function calls out, to make reorder3's job easier.
+                       walkexprlistsafe(n->list, &n->ninit);
                        ll = ascompatee(n->op, rl, n->list, &n->ninit);
                        n->list = reorder3(ll);
                        break;
@@ -1308,10 +1312,11 @@ ascompatet(int op, NodeList *nl, Type **nr, int fp, NodeList **init)
        }
 
        if(ll != nil || r != T)
-               yyerror("assignment count mismatch: %d = %d",
+               yyerror("ascompatet: assignment count mismatch: %d = %d",
                        count(nl), structcount(*nr));
+
        if(ucount)
-               fatal("reorder2: too many function calls evaluating parameters");
+               fatal("ascompatet: too many function calls evaluating parameters");
        return concat(nn, mm);
 }
 
@@ -1790,28 +1795,242 @@ reorder1(NodeList *all)
        return concat(g, r);
 }
 
+static void reorder3save(Node**, NodeList*, NodeList*, NodeList**);
+static int aliased(Node*, NodeList*, NodeList*);
+
 /*
  * from ascompat[ee]
  *     a,b = c,d
  * simultaneous assignment. there cannot
  * be later use of an earlier lvalue.
+ *
+ * function calls have been removed.
+ */
+static NodeList*
+reorder3(NodeList *all)
+{
+       NodeList *list, *early;
+       Node *l;
+
+       // If a needed expression may be affected by an
+       // earlier assignment, make an early copy of that
+       // expression and use the copy instead.
+       early = nil;
+       for(list=all; list; list=list->next) {
+               l = list->n->left;
+
+               // Save subexpressions needed on left side.
+               // Drill through non-dereferences.
+               for(;;) {
+                       if(l->op == ODOT || l->op == OPAREN) {
+                               l = l->left;
+                               continue;
+                       }
+                       if(l->op == OINDEX && isfixedarray(l->left->type)) {
+                               reorder3save(&l->right, all, list, &early);
+                               l = l->left;
+                               continue;
+                       }
+                       break;
+               }
+               switch(l->op) {
+               default:
+                       fatal("reorder3 unexpected lvalue %#O", l->op);
+               case ONAME:
+                       break;
+               case OINDEX:
+                       reorder3save(&l->left, all, list, &early);
+                       reorder3save(&l->right, all, list, &early);
+                       break;
+               case OIND:
+               case ODOTPTR:
+                       reorder3save(&l->left, all, list, &early);
+               }
+
+               // Save expression on right side.
+               reorder3save(&list->n->right, all, list, &early);
+       }
+
+       return concat(early, all);
+}
+
+static int vmatch2(Node*, Node*);
+static int varexpr(Node*);
+
+/*
+ * if the evaluation of *np would be affected by the 
+ * assignments in all up to but not including stop,
+ * copy into a temporary during *early and
+ * replace *np with that temp.
+ */
+static void
+reorder3save(Node **np, NodeList *all, NodeList *stop, NodeList **early)
+{
+       Node *n, *q;
+
+       n = *np;
+       if(!aliased(n, all, stop))
+               return;
+       
+       q = temp(n->type);
+       q = nod(OAS, q, n);
+       q->typecheck = 1;
+       *early = list(*early, q);
+       *np = q->left;
+}
+
+/*
+ * what's the outer value that a write to n affects?
+ * outer value means containing struct or array.
  */
+static Node*
+outervalue(Node *n)
+{      
+       for(;;) {
+               if(n->op == ODOT || n->op == OPAREN) {
+                       n = n->left;
+                       continue;
+               }
+               if(n->op == OINDEX && isfixedarray(n->left->type)) {
+                       n = n->left;
+                       continue;
+               }
+               break;
+       }
+       return n;
+}
 
+/*
+ * Is it possible that the computation of n might be
+ * affected by writes in as up to but not including stop?
+ */
+static int
+aliased(Node *n, NodeList *all, NodeList *stop)
+{
+       int memwrite, varwrite;
+       Node *a;
+       NodeList *l;
+
+       if(n == N)
+               return 0;
+
+       // Look for obvious aliasing: a variable being assigned
+       // during the all list and appearing in n.
+       // Also record whether there are any writes to main memory.
+       // Also record whether there are any writes to variables
+       // whose addresses have been taken.
+       memwrite = 0;
+       varwrite = 0;
+       for(l=all; l!=stop; l=l->next) {
+               a = outervalue(l->n->left);
+               if(a->op != ONAME) {
+                       memwrite = 1;
+                       continue;
+               }
+               switch(n->class) {
+               default:
+                       varwrite = 1;
+                       continue;
+               case PAUTO:
+               case PPARAM:
+               case PPARAMOUT:
+                       if(n->addrtaken) {
+                               varwrite = 1;
+                               continue;
+                       }
+                       if(vmatch2(a, n)) {
+                               // Direct hit.
+                               return 1;
+                       }
+               }
+       }
+
+       // The variables being written do not appear in n.
+       // However, n might refer to computed addresses
+       // that are being written.
+       
+       // If no computed addresses are affected by the writes, no aliasing.
+       if(!memwrite && !varwrite)
+               return 0;
+
+       // If n does not refer to computed addresses
+       // (that is, if n only refers to variables whose addresses
+       // have not been taken), no aliasing.
+       if(varexpr(n))
+               return 0;
+
+       // Otherwise, both the writes and n refer to computed memory addresses.
+       // Assume that they might conflict.
+       return 1;
+}
+
+/*
+ * does the evaluation of n only refer to variables
+ * whose addresses have not been taken?
+ * (and no other memory)
+ */
+static int
+varexpr(Node *n)
+{
+       if(n == N)
+               return 1;
+
+       switch(n->op) {
+       case OLITERAL:  
+               return 1;
+       case ONAME:
+               switch(n->class) {
+               case PAUTO:
+               case PPARAM:
+               case PPARAMOUT:
+                       if(!n->addrtaken)
+                               return 1;
+               }
+               return 0;
+
+       case OADD:
+       case OSUB:
+       case OOR:
+       case OXOR:
+       case OMUL:
+       case ODIV:
+       case OMOD:
+       case OLSH:
+       case ORSH:
+       case OAND:
+       case OANDNOT:
+       case OPLUS:
+       case OMINUS:
+       case OCOM:
+       case OPAREN:
+       case OANDAND:
+       case OOROR:
+       case ODOT:  // but not ODOTPTR
+       case OCONV:
+       case OCONVNOP:
+       case OCONVIFACE:
+       case ODOTTYPE:
+               return varexpr(n->left) && varexpr(n->right);
+       }
+
+       // Be conservative.
+       return 0;
+}
+
+/*
+ * is the name l mentioned in r?
+ */
 static int
 vmatch2(Node *l, Node *r)
 {
        NodeList *ll;
 
-       /*
-        * isolate all right sides
-        */
        if(r == N)
                return 0;
        switch(r->op) {
        case ONAME:
                // match each right given left
-               if(l == r)
-                       return 1;
+               return l == r;
        case OLITERAL:
                return 0;
        }
@@ -1825,6 +2044,10 @@ vmatch2(Node *l, Node *r)
        return 0;
 }
 
+/*
+ * is any name mentioned in l also mentioned in r?
+ * called by sinit.c
+ */
 int
 vmatch1(Node *l, Node *r)
 {
@@ -1863,33 +2086,6 @@ vmatch1(Node *l, Node *r)
        return 0;
 }
 
-static NodeList*
-reorder3(NodeList *all)
-{
-       Node *n1, *n2, *q;
-       int c1, c2;
-       NodeList *l1, *l2, *r;
-
-       r = nil;
-       for(l1=all, c1=0; l1; l1=l1->next, c1++) {
-               n1 = l1->n;
-               for(l2=all, c2=0; l2; l2=l2->next, c2++) {
-                       n2 = l2->n;
-                       if(c2 > c1) {
-                               if(vmatch1(n1->left, n2->right)) {
-                                       // delay assignment to n1->left
-                                       q = temp(n1->right->type);
-                                       q = nod(OAS, n1->left, q);
-                                       n1->left = q->right;
-                                       r = list(r, q);
-                                       break;
-                               }
-                       }
-               }
-       }
-       return concat(all, r);
-}
-
 /*
  * walk through argin parameters.
  * generate and return code to allocate
diff --git a/test/reorder.go b/test/reorder.go
new file mode 100644 (file)
index 0000000..67d0752
--- /dev/null
@@ -0,0 +1,121 @@
+// $G $D/$F.go && $L $F.$A && ./$A.out
+
+// Copyright 2011 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.
+
+// Check reordering of assignments.
+
+package main
+
+import "fmt"
+
+func main() {
+       p1()
+       p2()
+       p3()
+       p4()
+       p5()
+       p6()
+       p7()
+       p8()
+}
+
+var gx []int
+
+func f(i int) int {
+       return gx[i]
+}
+
+func check(x []int, x0, x1, x2 int) {
+       if x[0] != x0 || x[1] != x1 || x[2] != x2 {
+               fmt.Printf("%v, want %d,%d,%d\n", x, x0, x1, x2)
+               panic("failed")
+       }
+}
+
+func check3(x, y, z, xx, yy, zz int) {
+       if x != xx || y != yy || z != zz {
+               fmt.Printf("%d,%d,%d, want %d,%d,%d\n", x, y, z, xx, yy, zz)
+               panic("failed")
+       }
+}
+
+func p1() {
+       x := []int{1,2,3}
+       i := 0
+       i, x[i] = 1, 100
+       _ = i
+       check(x, 100, 2, 3)
+}
+
+func p2() {
+       x := []int{1,2,3}
+       i := 0
+       x[i], i = 100, 1
+       _ = i
+       check(x, 100, 2, 3)
+}
+
+func p3() {
+       x := []int{1,2,3}
+       y := x
+       gx = x
+       x[1], y[0] = f(0), f(1)
+       check(x, 2, 1, 3)
+}
+
+func p4() {
+       x := []int{1,2,3}
+       y := x
+       gx = x
+       x[1], y[0] = gx[0], gx[1]
+       check(x, 2, 1, 3)
+}
+
+func p5() {
+       x := []int{1,2,3}
+       y := x
+       p := &x[0]
+       q := &x[1]
+       *p, *q = x[1], y[0]
+       check(x, 2, 1, 3)
+}
+
+func p6() {
+       x := 1
+       y := 2
+       z := 3
+       px := &x
+       py := &y
+       *px, *py = y, x
+       check3(x, y, z, 2, 1, 3)        
+}
+
+func f1(x, y, z int) (xx, yy, zz int) {
+       return x, y, z
+}
+
+func f2() (x, y, z int) {
+       return f1(2, 1, 3)
+}
+
+func p7() {
+       x, y, z := f2()
+       check3(x, y, z, 2, 1, 3)
+}
+
+func p8() {
+       x := []int{1,2,3}
+
+       defer func() {
+               err := recover()
+               if err == nil {
+                       panic("not panicking")
+               }
+               check(x, 100, 2, 3)
+       }()
+
+       i := 0
+       i, x[i], x[5] = 1, 100, 500
+}