From: Russ Cox Date: Thu, 25 Aug 2011 13:26:13 +0000 (-0400) Subject: gc: fix some spurious leaks X-Git-Tag: weekly.2011-09-01~87 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=0227c45edec55aa05c8546f06edf24530241aee5;p=gostls13.git 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 916a08976d..d83a7f75b9 100644 --- a/src/cmd/gc/esc.c +++ b/src/cmd/gc/esc.c @@ -148,7 +148,7 @@ esc(Node *n) lno = setlineno(n); - if(n->op == OFOR) + if(n->op == OFOR || n->op == ORANGE) loopdepth++; esclist(n->ninit); @@ -160,7 +160,7 @@ esc(Node *n) 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 @@ esc(Node *n) 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 @@ esc(Node *n) loopdepth++; break; - case ORANGE: // for = range { } - 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 @@ esc(Node *n) 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 @@ esc(Node *n) 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 @@ esc(Node *n) 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 @@ escassign(Node *dst, Node *src) 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 @@ escassign(Node *dst, Node *src) 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 @@ escassign(Node *dst, Node *src) 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 @@ escflows(Node *dst, Node *src) 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 abbb574940..f9d377acf0 100644 --- a/test/escape2.go +++ b/test/escape2.go @@ -613,3 +613,89 @@ func foo92(x *int) [2]*int { // ERROR "leaking param: NAME-x" 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) +}