From e150ca9c9aba9b8d8e61d0953ea4b90deef620bc Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 28 Mar 2014 11:30:02 -0400 Subject: [PATCH] cmd/gc: never pass ptr to uninit temp to runtime chanrecv now expects a pointer to the data to be filled in. mapiterinit expects a pointer to the hash iterator to be filled in. In both cases, the temporary being pointed at changes from dead to alive during the call. In order to make sure it is preserved if a garbage collection happens after that transition but before the call returns, the temp must be marked as live during the entire call. But if it is live during the entire call, it needs to be safe for the garbage collector to scan at the beginning of the call, before the new data has been filled in. Therefore, it must be zeroed by the caller, before the call. Do that. My previous attempt waited to mark it live until after the call returned, but that's unsafe (see first paragraph); undo that change in plive.c. This makes powser2 pass again reliably. I looked at every call to temp in the compiler. The vast majority are followed immediately by an initialization of temp, so those are fine. The only ones that needed changing were the ones where the next operation is to pass the address of the temp to a function call, and there aren't too many. Maps are exempted from this because mapaccess returns a pointer to the data and lets the caller make the copy. Fixes many builds. TBR=khr CC=golang-codereviews https://golang.org/cl/80700046 --- src/cmd/gc/plive.c | 3 +-- src/cmd/gc/range.c | 3 +++ src/cmd/gc/select.c | 12 ++++++++++++ src/cmd/gc/walk.c | 13 +++++++++++++ 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/cmd/gc/plive.c b/src/cmd/gc/plive.c index 55bdee2418d..fb2b2fdaf6d 100644 --- a/src/cmd/gc/plive.c +++ b/src/cmd/gc/plive.c @@ -720,8 +720,7 @@ progeffects(Prog *prog, Array *vars, Bvec *uevar, Bvec *varkill, Bvec *avarinit) if(pos == -1) goto Next; if(from->node->addrtaken) { - if(info.flags & (LeftRead|LeftWrite)) - bvset(avarinit, pos); + bvset(avarinit, pos); } else { if(info.flags & (LeftRead | LeftAddr)) bvset(uevar, pos); diff --git a/src/cmd/gc/range.c b/src/cmd/gc/range.c index cfe713c1307..33a45fe5cfe 100644 --- a/src/cmd/gc/range.c +++ b/src/cmd/gc/range.c @@ -197,6 +197,7 @@ walkrange(Node *n) // allocate an iterator state structure on the stack th = hiter(t); hit = temp(th); + init = list(init, nod(OAS, hit, N)); keyname = newname(th->type->sym); // depends on layout of iterator struct. See reflect.c:hiter valname = newname(th->type->down->sym); // ditto @@ -227,6 +228,8 @@ walkrange(Node *n) case TCHAN: hv1 = temp(t->type); + if(haspointers(t->type)) + init = list(init, nod(OAS, hv1, N)); hb = temp(types[TBOOL]); n->ntest = nod(ONE, hb, nodbool(0)); diff --git a/src/cmd/gc/select.c b/src/cmd/gc/select.c index d3c04c659e4..48066c2e7fe 100644 --- a/src/cmd/gc/select.c +++ b/src/cmd/gc/select.c @@ -211,6 +211,11 @@ walkselect(Node *sel) else if(n->left->op == ONAME && (!n->colas || (n->left->class&PHEAP) == 0) && convertop(ch->type->type, n->left->type, nil) == OCONVNOP) { + if(n->colas && haspointers(ch->type->type)) { + r = nod(OAS, n->left, N); + typecheck(&r, Etop); + sel->ninit = concat(sel->ninit, list1(r)); + } n->left = nod(OADDR, n->left, N); n->left->etype = 1; // pointer does not escape typecheck(&n->left, Erv); @@ -222,6 +227,13 @@ walkselect(Node *sel) } else { tmp = temp(ch->type->type); a = nod(OADDR, tmp, N); + if(haspointers(ch->type->type)) { + // clear tmp for garbage collector, because the recv + // must execute with tmp appearing to be live. + r = nod(OAS, tmp, N); + typecheck(&r, Etop); + sel->ninit = concat(sel->ninit, list1(r)); + } a->etype = 1; // pointer does not escape typecheck(&a, Erv); r = nod(OAS, n->left, tmp); diff --git a/src/cmd/gc/walk.c b/src/cmd/gc/walk.c index 91e87192a44..4416c87b083 100644 --- a/src/cmd/gc/walk.c +++ b/src/cmd/gc/walk.c @@ -671,6 +671,12 @@ walkexpr(Node **np, NodeList **init) walkexprlistsafe(n->list, init); walkexpr(&r->left, init); var = temp(r->left->type->type); + if(haspointers(var->type)) { + // clear for garbage collector - var is live during chanrecv2 call. + a = nod(OAS, var, N); + typecheck(&a, Etop); + *init = concat(*init, list1(a)); + } n1 = nod(OADDR, var, N); fn = chanfn("chanrecv2", 2, r->left->type); r = mkcall1(fn, types[TBOOL], init, typename(r->left->type), r->left, n1); @@ -1177,6 +1183,12 @@ walkexpr(Node **np, NodeList **init) case ORECV: walkexpr(&n->left, init); var = temp(n->left->type->type); + if(haspointers(var->type)) { + // clear for garbage collector - var is live during chanrecv1 call. + a = nod(OAS, var, N); + typecheck(&a, Etop); + *init = concat(*init, list1(a)); + } n1 = nod(OADDR, var, N); n = mkcall1(chanfn("chanrecv1", 2, n->left->type), T, init, typename(n->left->type), n->left, n1); walkexpr(&n, init); @@ -1440,6 +1452,7 @@ walkexpr(Node **np, NodeList **init) case OMAPLIT: case OSTRUCTLIT: case OPTRLIT: + // XXX TODO do we need to clear var? var = temp(n->type); anylit(0, n, var, init); n = var;