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
golang · May 29, 2014 · 89d46fe · 89d46fe
1 parent 9dd062b
commit 89d46fe
Show file tree

Hide file tree

Showing 8 changed files with 77 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
+}