From c3a33fa71bb22fc6e1948c97e6ae877adb9c0e29 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Thu, 29 May 2014 13:47:31 -0400 Subject: 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 issue 8097. LGTM=khr R=golang-codereviews, khr CC=golang-codereviews, iant, khr, r https://codereview.appspot.com/103750043 --- src/cmd/5g/cgen.c | 8 ++++++++ src/cmd/5g/gsubr.c | 9 +++++++++ src/cmd/6g/cgen.c | 7 +++++++ src/cmd/6g/gsubr.c | 9 +++++++++ src/cmd/8g/cgen.c | 7 +++++++ src/cmd/8g/gsubr.c | 9 +++++++++ src/cmd/gc/walk.c | 3 ++- test/live.go | 26 ++++++++++++++++++++++++++ 8 files changed, 77 insertions(+), 1 deletion(-) diff --git a/src/cmd/5g/cgen.c b/src/cmd/5g/cgen.c index 9faf75461..9011b2022 100644 --- 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 528e8f8cc..f66c87b5a 100644 --- 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 ae1309142..4dd505b08 100644 --- 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 bd2f2304b..e4d00bf41 100644 --- 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 1aae7771c..d626c2eb0 100644 --- 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 e83ae5d7a..2f3cb28c8 100644 --- 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 2d402d04f..1cb25512e 100644 --- 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 21d3e6a5f..286fcc306 100644 --- 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 +} -- cgit v1.2.1