cmd/gc: fix x=x crash

[Same as CL 102820043 except applied changes to 6g/gsubr.c
also to 5g/gsubr.c and 8g/gsubr.c. The problem I had last night
trying to do that was that 8g's copy of nodarg has different
(but equivalent) control flow and I was pasting the new code
into the wrong place.]

Description from CL 102820043:

The 'nodarg' function is used to obtain a Node*
representing a function argument or result.
It returned a brand new Node*, but that violates
the guarantee in most places in the compiler that
two Node*s refer to the same variable if and only if
they are the same Node* pointer. Reestablish that
invariant by making nodarg return a preexisting
named variable if present.

Having fixed that, avoid any copy during x=x in
componentgen, because the VARDEF we emit
before the copy marks the lhs x as dead incorrectly.

The change in walk.c avoids modifying the result
of nodarg. This was the only place in the compiler
that did so.

Fixes #8097.

LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, iant, khr, r
https://golang.org/cl/103750043

diff --git a/src/cmd/5g/cgen.c b/src/cmd/5g/cgen.c
index 9faf75461751acd6dd0244b9559acfcb8d3db038..9011b20228932d4fb0e27715933272b2710d0f32 100644 (file)
--- a/src/cmd/5g/cgen.c
+++ b/src/cmd/5g/cgen.c
@@ -1490,6 +1490,7 @@ sgen(Node *n, Node *res, int64 w)
        }
        if(osrc%align != 0 || odst%align != 0)
                fatal("sgen: unaligned offset src %d or dst %d (align %d)", osrc, odst, align);
+
        // if we are copying forward on the stack and
        // the src and dst overlap, then reverse direction
        dir = align;
@@ -1674,6 +1675,13 @@ componentgen(Node *nr, Node *nl)
                freer = 1;
        }
 
+       // nl and nr are 'cadable' which basically means they are names (variables) now.
+       // If they are the same variable, don't generate any code, because the
+       // VARDEF we generate will mark the old value as dead incorrectly.
+       // (And also the assignments are useless.)
+       if(nr != N && nl->op == ONAME && nr->op == ONAME && nl == nr)
+               goto yes;
+
        switch(nl->type->etype) {
        case TARRAY:
                if(nl->op == ONAME)

diff --git a/src/cmd/5g/gsubr.c b/src/cmd/5g/gsubr.c
index 528e8f8cc3133a4cc92b3c3a034a4a441e738094..f66c87b5a456db8f3c318483d78b100bc8b10f3e 100644 (file)
--- a/src/cmd/5g/gsubr.c
+++ b/src/cmd/5g/gsubr.c
@@ -470,6 +470,7 @@ Node*
 nodarg(Type *t, int fp)
 {
        Node *n;
+       NodeList *l;
        Type *first;
        Iter savet;
 
@@ -491,6 +492,14 @@ nodarg(Type *t, int fp)
        if(t->etype != TFIELD)
                fatal("nodarg: not field %T", t);
 
+       if(fp == 1) {
+               for(l=curfn->dcl; l; l=l->next) {
+                       n = l->n;
+                       if((n->class == PPARAM || n->class == PPARAMOUT) && !isblanksym(t->sym) && n->sym == t->sym)
+                               return n;
+               }
+       }
+
        n = nod(ONAME, N, N);
        n->type = t->type;
        n->sym = t->sym;

diff --git a/src/cmd/6g/cgen.c b/src/cmd/6g/cgen.c
index ae1309142c9f778844f1e1f0f87fb3c8c450a3d4..4dd505b086d76813d8a92b7951b035d7ce988af3 100644 (file)
--- a/src/cmd/6g/cgen.c
+++ b/src/cmd/6g/cgen.c
@@ -1585,6 +1585,13 @@ componentgen(Node *nr, Node *nl)
                        freer = 1;
                }
        }
+       
+       // nl and nr are 'cadable' which basically means they are names (variables) now.
+       // If they are the same variable, don't generate any code, because the
+       // VARDEF we generate will mark the old value as dead incorrectly.
+       // (And also the assignments are useless.)
+       if(nr != N && nl->op == ONAME && nr->op == ONAME && nl == nr)
+               goto yes;
 
        switch(nl->type->etype) {
        case TARRAY:

diff --git a/src/cmd/6g/gsubr.c b/src/cmd/6g/gsubr.c
index bd2f2304b455fa21c720dc7555b9a1e9ef3371f7..e4d00bf4192ee64daf541cec2e8035447f30d424 100644 (file)
--- a/src/cmd/6g/gsubr.c
+++ b/src/cmd/6g/gsubr.c
@@ -462,6 +462,7 @@ Node*
 nodarg(Type *t, int fp)
 {
        Node *n;
+       NodeList *l;
        Type *first;
        Iter savet;
 
@@ -482,6 +483,14 @@ nodarg(Type *t, int fp)
 
        if(t->etype != TFIELD)
                fatal("nodarg: not field %T", t);
+       
+       if(fp == 1) {
+               for(l=curfn->dcl; l; l=l->next) {
+                       n = l->n;
+                       if((n->class == PPARAM || n->class == PPARAMOUT) && !isblanksym(t->sym) && n->sym == t->sym)
+                               return n;
+               }
+       }
 
        n = nod(ONAME, N, N);
        n->type = t->type;

diff --git a/src/cmd/8g/cgen.c b/src/cmd/8g/cgen.c
index 1aae7771c7110122fc07ca3698556fbf179272a7..d626c2eb02ef894c9d635c434ca9146c7b90fdc0 100644 (file)
--- a/src/cmd/8g/cgen.c
+++ b/src/cmd/8g/cgen.c
@@ -1397,6 +1397,13 @@ componentgen(Node *nr, Node *nl)
                }
        }
 
+       // nl and nr are 'cadable' which basically means they are names (variables) now.
+       // If they are the same variable, don't generate any code, because the
+       // VARDEF we generate will mark the old value as dead incorrectly.
+       // (And also the assignments are useless.)
+       if(nr != N && nl->op == ONAME && nr->op == ONAME && nl == nr)
+               goto yes;
+
        switch(nl->type->etype) {
        case TARRAY:
                if(nl->op == ONAME)

diff --git a/src/cmd/8g/gsubr.c b/src/cmd/8g/gsubr.c
index e83ae5d7a5202f268db7294c4e908d43348b5fc2..2f3cb28c839b7003ed4bc1fbec8c8fa2776dc6e6 100644 (file)
--- a/src/cmd/8g/gsubr.c
+++ b/src/cmd/8g/gsubr.c
@@ -1044,6 +1044,7 @@ Node*
 nodarg(Type *t, int fp)
 {
        Node *n;
+       NodeList *l;
        Type *first;
        Iter savet;
 
@@ -1068,6 +1069,14 @@ nodarg(Type *t, int fp)
                break;
 
        case TFIELD:
+               if(fp == 1 && t->sym != S && !isblanksym(t->sym)) {
+                       for(l=curfn->dcl; l; l=l->next) {
+                               n = l->n;
+                               if((n->class == PPARAM || n->class == PPARAMOUT) && n->sym == t->sym)
+                                       return n;
+                       }
+               }
+
                n = nod(ONAME, N, N);
                n->type = t->type;
                n->sym = t->sym;

diff --git a/src/cmd/gc/walk.c b/src/cmd/gc/walk.c
index 2d402d04f55f4f389049fed50433d07ad65589a8..1cb25512e5e2868d34711d9e5026aae5806a5b6b 100644 (file)
--- a/src/cmd/gc/walk.c
+++ b/src/cmd/gc/walk.c
@@ -1652,7 +1652,8 @@ ascompatte(int op, Node *call, int isddd, Type **nl, NodeList *lr, int fp, NodeL
                // optimization - can do block copy
                if(eqtypenoname(r->type, *nl)) {
                        a = nodarg(*nl, fp);
-                       a->type = r->type;
+                       r = nod(OCONVNOP, r, N);
+                       r->type = a->type;
                        nn = list1(convas(nod(OAS, a, r), init));
                        goto ret;
                }

diff --git a/test/live.go b/test/live.go
index 21d3e6a5fa79a1af7a16bdbc9b6c8d39c34ea171..286fcc30640ef87f63e799e70442d8e2cfff3c56 100644 (file)
--- a/test/live.go
+++ b/test/live.go
@@ -564,3 +564,29 @@ func f38(b bool) {
        }
        println()
 }
+
+// issue 8097: mishandling of x = x during return.
+
+func f39() (x []int) {
+       x = []int{1}
+       println() // ERROR "live at call to printnl: x"
+       return x
+}
+
+func f39a() (x []int) {
+       x = []int{1}
+       println() // ERROR "live at call to printnl: x"
+       return
+}
+
+func f39b() (x [10]*int) {
+       x = [10]*int{new(int)} // ERROR "live at call to new: x"
+       println() // ERROR "live at call to printnl: x"
+       return x
+}
+
+func f39c() (x [10]*int) {
+       x = [10]*int{new(int)} // ERROR "live at call to new: x"
+       println() // ERROR "live at call to printnl: x"
+       return
+}