gc: fix some spurious leaks
Probably will spark some discussion. ☺
R=lvd
CC=golang-dev
https://golang.org/cl/4948041
diff --git a/src/cmd/gc/esc.c b/src/cmd/gc/esc.c
index 916a089..d83a7f7 100644
--- a/src/cmd/gc/esc.c
+++ b/src/cmd/gc/esc.c
@@ -148,7 +148,7 @@
lno = setlineno(n);
- if(n->op == OFOR)
+ if(n->op == OFOR || n->op == ORANGE)
loopdepth++;
esclist(n->ninit);
@@ -160,7 +160,7 @@
esc(n->left);
esc(n->right);
- if(n->op == OFOR)
+ if(n->op == OFOR || n->op == ORANGE)
loopdepth--;
if(debug['m'] > 1)
@@ -169,13 +169,7 @@
switch(n->op) {
case ODCL:
- case ODCLFIELD:
- // a declaration ties the node to the current
- // function, but we already have that edge in
- // curfn->dcl and will follow it explicitly in
- // escflood to avoid storing redundant information
- // What does have to happen here is note if the name
- // is declared inside a looping scope.
+ // Record loop depth at declaration.
if(n->left)
n->left->escloopdepth = loopdepth;
break;
@@ -184,28 +178,10 @@
loopdepth++;
break;
- case ORANGE: // for <list> = range <right> { <nbody> }
- switch(n->type->etype) {
- case TARRAY: // i, v = range sliceorarray
- if(n->list->next)
- escassign(n->list->next->n, n->right);
- break;
- case TMAP: // k [, v] = range map
- escassign(n->list->n, n->right);
- if(n->list->next)
- escassign(n->list->next->n, n->right);
- break;
- case TCHAN: // v = range chan
- escassign(n->list->n, n->right);
- break;
- }
- loopdepth++;
- esclist(n->nbody);
- loopdepth--;
- break;
-
- case OSELRECV: // v := <-ch left: v right->op = ORECV
- escassign(n->left, n->right);
+ case ORANGE:
+ // Everything but fixed array is a dereference.
+ if(isfixedarray(n->type))
+ escassign(n->list->next->n, n->right);
break;
case OSWITCH:
@@ -216,13 +192,6 @@
escassign(ll->n->nname, n->ntest->right);
esclist(ll->n->nbody);
}
- } else {
- escassign(N, n->ntest);
- for(ll=n->list; ll; ll=ll->next) { // cases
- for(lr=ll->n->list; lr; lr=lr->next)
- escassign(N, lr->n);
- esclist(ll->n->nbody);
- }
}
break;
@@ -245,7 +214,7 @@
break;
case OSEND: // ch <- x
- escassign(&theSink, n->right); // TODO: treat as *ch = x ?
+ escassign(&theSink, n->right);
break;
case ODEFER:
@@ -271,16 +240,10 @@
escassign(&theSink, n->left);
break;
- case OCOPY:
- // left leaks to right, but the return value is harmless
- // TODO: treat as *dst = *src, rather than as dst = src
- escassign(n->left, n->right);
- break;
-
case OAPPEND:
- // See TODO for OCOPY
- for(ll=n->list->next; ll; ll=ll->next)
- escassign(n->list->n, ll->n);
+ if(!n->isddd)
+ for(ll=n->list->next; ll; ll=ll->next)
+ escassign(&theSink, ll->n); // lose track of assign to dereference
break;
case OCALLMETH:
@@ -328,32 +291,47 @@
print("%L:[%d] %#S escassign: %hN = %hN\n", lineno, loopdepth,
(curfn && curfn->nname) ? curfn->nname->sym : S, dst, src);
- // the lhs of an assignment needs recursive analysis too
- // these are the only interesting cases
- // todo:check channel case
setlineno(dst);
-
+
+ // Analyze lhs of assignment.
+ // Replace dst with theSink if we can't track it.
switch(dst->op) {
- case OINDEX:
- case OSLICE:
- // slice: "dst[x] = src" is like *(underlying array)[x] = src
- // TODO maybe this never occurs b/c of OSLICEARR and it's inserted OADDR
- if(!isfixedarray(dst->left->type))
- goto doref;
- // fallthrough; treat "dst[x] = src" as "dst = src"
+ default:
+ dump("dst", dst);
+ fatal("escassign: unexpected dst");
+
+ case OARRAYLIT:
+ case OCLOSURE:
+ case OCONV:
+ case OCONVIFACE:
+ case OCONVNOP:
+ case OMAPLIT:
+ case OSTRUCTLIT:
+ break;
+
+ case ONAME:
+ if(dst->class == PEXTERN)
+ dst = &theSink;
+ break;
case ODOT: // treat "dst.x = src" as "dst = src"
escassign(dst->left, src);
return;
- case OINDEXMAP:
- escassign(&theSink, dst->right); // map key is put in map
- // fallthrough
+ case OINDEX:
+ if(isfixedarray(dst->left->type)) {
+ escassign(dst->left, src);
+ return;
+ }
+ dst = &theSink; // lose track of dereference
+ break;
case OIND:
case ODOTPTR:
- case OSLICEARR: // ->left is the OADDR of the array
- doref:
- // assignment to dereferences: for now we lose track
- escassign(&theSink, src);
- return;
+ dst = &theSink; // lose track of dereference
+ break;
+ case OINDEXMAP:
+ // lose track of key and value
+ escassign(&theSink, dst->right);
+ dst = &theSink;
+ break;
}
if(src->typecheck == 0 && src->op != OKEY) {
@@ -380,10 +358,23 @@
case ODOT:
case ODOTTYPE:
case ODOTTYPE2:
+ case OSLICE:
+ case OSLICEARR:
// Conversions, field access, slice all preserve the input value.
escassign(dst, src->left);
break;
+ case OAPPEND:
+ // Append returns first argument.
+ escassign(dst, src->list->n);
+ break;
+
+ case OINDEX:
+ // Index of array preserves input value.
+ if(isfixedarray(src->left->type))
+ escassign(dst, src->left);
+ break;
+
case OARRAYLIT:
case OSTRUCTLIT:
case OMAPLIT:
@@ -410,20 +401,6 @@
escflows(dst, src);
escfunc(src);
break;
-
- // end of the leaf cases. no calls to escflows() in the cases below.
- case OAPPEND:
- escassign(dst, src->list->n);
- break;
-
- case OSLICEARR: // like an implicit OIND to the underlying buffer, but typecheck has inserted an OADDR
- case OSLICESTR:
- case OSLICE:
- case OINDEX:
- case OINDEXMAP:
- // the big thing flows, the keys just need checking
- escassign(dst, src->left);
- break;
}
pdepth--;
@@ -525,10 +502,6 @@
if(debug['m']>2)
print("%L::flows:: %hN <- %hN\n", lineno, dst, src);
- // Assignments to global variables get lumped into theSink.
- if(dst->op == ONAME && dst->class == PEXTERN)
- dst = &theSink;
-
if(dst->escflowsrc == nil) {
dsts = list(dsts, dst);
dstcount++;
diff --git a/test/escape2.go b/test/escape2.go
index abbb574..f9d377a 100644
--- a/test/escape2.go
+++ b/test/escape2.go
@@ -613,3 +613,89 @@
return [2]*int{ x, nil }
}
+// does not leak c
+func foo93(c chan *int) *int {
+ for v := range c {
+ return v
+ }
+ return nil
+}
+
+// does not leak m
+func foo94(m map[*int]*int, b bool) *int {
+ for k, v := range m {
+ if b {
+ return k
+ }
+ return v
+ }
+ return nil
+}
+
+// does leak x
+func foo95(m map[*int]*int, x *int) { // ERROR "leaking param: NAME-x"
+ m[x] = x
+}
+
+// does not leak m
+func foo96(m []*int) *int {
+ return m[0]
+}
+
+// does leak m
+func foo97(m [1]*int) *int { // ERROR "leaking param: NAME-m"
+ return m[0]
+}
+
+// does not leak m
+func foo98(m map[int]*int) *int {
+ return m[0]
+}
+
+// does leak m
+func foo99(m *[1]*int) []*int { // ERROR "leaking param: NAME-m"
+ return m[:]
+}
+
+// does not leak m
+func foo100(m []*int) *int {
+ for _, v := range m {
+ return v
+ }
+ return nil
+}
+
+// does leak m
+func foo101(m [1]*int) *int { // ERROR "leaking param: NAME-m"
+ for _, v := range m {
+ return v
+ }
+ return nil
+}
+
+// does leak x
+func foo102(m []*int, x *int) { // ERROR "leaking param: NAME-x"
+ m[0] = x
+}
+
+// does not leak x
+func foo103(m [1]*int, x *int) {
+ m[0] = x
+}
+
+var y []*int
+
+// does not leak x
+func foo104(x []*int) {
+ copy(y, x)
+}
+
+// does not leak x
+func foo105(x []*int) {
+ _ = append(y, x...)
+}
+
+// does leak x
+func foo106(x *int) { // ERROR "leaking param: NAME-x"
+ _ = append(y, x)
+}
