From: Russ Cox Date: Mon, 12 May 2014 21:19:02 +0000 (-0400) Subject: cmd/gc: fix liveness vs regopt mismatch for input variables X-Git-Tag: go1.3beta2~86 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=26ad5d4ff021e7784dca22e76c43494e76913911;p=gostls13.git cmd/gc: fix liveness vs regopt mismatch for input variables The inputs to a function are marked live at all times in the liveness bitmaps, so that the garbage collector will not free the things they point at and reuse the pointers, so that the pointers shown in stack traces are guaranteed not to have been recycled. Unfortunately, no one told the register optimizer that the inputs need to be preserved at all call sites. If a function is done with a particular input value, the optimizer will stop preserving it across calls. For single-word values this just means that the value recorded might be stale. For multi-word values like slices, the value recorded could be only partially stale: it can happen that, say, the cap was updated but not the len, or that the len was updated but not the base pointer. Either of these possibilities (and others) would make the garbage collector misinterpret memory, leading to memory corruption. This came up in a real program, in which the garbage collector's 'slice len ≤ slice cap' check caught the inconsistency. Fixes #7944. LGTM=iant R=golang-codereviews, iant CC=golang-codereviews, khr https://golang.org/cl/100370045 --- diff --git a/src/cmd/5g/opt.h b/src/cmd/5g/opt.h index 15b9d14582..e3e3f78ed2 100644 --- a/src/cmd/5g/opt.h +++ b/src/cmd/5g/opt.h @@ -96,6 +96,7 @@ EXTERN Bits externs; EXTERN Bits params; EXTERN Bits consts; EXTERN Bits addrs; +EXTERN Bits ivar; EXTERN Bits ovar; EXTERN int change; EXTERN int32 maxnr; diff --git a/src/cmd/5g/reg.c b/src/cmd/5g/reg.c index 80a14db3c4..3eadde4cf4 100644 --- a/src/cmd/5g/reg.c +++ b/src/cmd/5g/reg.c @@ -57,7 +57,7 @@ rcmp(const void *a1, const void *a2) } static void -setoutvar(void) +setvar(Bits *dst, Type **args) { Type *t; Node *n; @@ -66,18 +66,16 @@ setoutvar(void) Bits bit; int z; - t = structfirst(&save, getoutarg(curfn->type)); + t = structfirst(&save, args); while(t != T) { n = nodarg(t, 1); a = zprog.from; naddr(n, &a, 0); bit = mkvar(R, &a); for(z=0; zb[z] |= bit.b[z]; t = structnext(&save); } -//if(bany(&ovar)) -//print("ovar = %Q\n", ovar); } void @@ -190,11 +188,14 @@ regopt(Prog *firstp) params.b[z] = 0; consts.b[z] = 0; addrs.b[z] = 0; + ivar.b[z] = 0; ovar.b[z] = 0; } - // build list of return variables - setoutvar(); + // build lists of parameters and results + setvar(&ivar, getthis(curfn->type)); + setvar(&ivar, getinarg(curfn->type)); + setvar(&ovar, getoutarg(curfn->type)); /* * pass 1 @@ -895,8 +896,12 @@ prop(Reg *r, Bits ref, Bits cal) case ABL: if(noreturn(r1->f.prog)) break; + + // Mark all input variables (ivar) as used, because that's what the + // liveness bitmaps say. The liveness bitmaps say that so that a + // panic will not show stale values in the parameter dump. for(z=0; ztype)); + t = structfirst(&save, args); while(t != T) { n = nodarg(t, 1); a = zprog.from; naddr(n, &a, 0); bit = mkvar(R, &a); for(z=0; zb[z] |= bit.b[z]; t = structnext(&save); } -//if(bany(&ovar)) -//print("ovars = %Q\n", ovar); } static void @@ -176,11 +174,14 @@ regopt(Prog *firstp) params.b[z] = 0; consts.b[z] = 0; addrs.b[z] = 0; + ivar.b[z] = 0; ovar.b[z] = 0; } - // build list of return variables - setoutvar(); + // build lists of parameters and results + setvar(&ivar, getthis(curfn->type)); + setvar(&ivar, getinarg(curfn->type)); + setvar(&ovar, getoutarg(curfn->type)); /* * pass 1 @@ -750,8 +751,12 @@ prop(Reg *r, Bits ref, Bits cal) case ACALL: if(noreturn(r1->f.prog)) break; + + // Mark all input variables (ivar) as used, because that's what the + // liveness bitmaps say. The liveness bitmaps say that so that a + // panic will not show stale values in the parameter dump. for(z=0; ztype)); + t = structfirst(&save, args); while(t != T) { n = nodarg(t, 1); a = zprog.from; naddr(n, &a, 0); bit = mkvar(R, &a); for(z=0; zb[z] |= bit.b[z]; t = structnext(&save); } -//if(bany(ovar)) -//print("ovars = %Q\n", ovar); } static void @@ -146,11 +144,14 @@ regopt(Prog *firstp) params.b[z] = 0; consts.b[z] = 0; addrs.b[z] = 0; + ivar.b[z] = 0; ovar.b[z] = 0; } - // build list of return variables - setoutvar(); + // build lists of parameters and results + setvar(&ivar, getthis(curfn->type)); + setvar(&ivar, getinarg(curfn->type)); + setvar(&ovar, getoutarg(curfn->type)); /* * pass 1 @@ -715,8 +716,12 @@ prop(Reg *r, Bits ref, Bits cal) case ACALL: if(noreturn(r1->f.prog)) break; + + // Mark all input variables (ivar) as used, because that's what the + // liveness bitmaps say. The liveness bitmaps say that so that a + // panic will not show stale values in the parameter dump. for(z=0; z 0 { + n := len(b) + n = f1(n) + f2(b[n:]) + b = b[n:] + } + g() +} + +func f1(n int) int { + runtime.GC() + return n +} + +func f2(b []byte) { + runtime.GC() +} + +func g() { + runtime.GC() +} + +func main() { + f(make([]byte, 100)) +}