1
0
mirror of https://github.com/golang/go synced 2024-11-23 05:00:07 -07:00

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
This commit is contained in:
Russ Cox 2014-05-12 17:19:02 -04:00
parent 03c0f3fea9
commit 26ad5d4ff0
7 changed files with 82 additions and 24 deletions

View File

@ -96,6 +96,7 @@ EXTERN Bits externs;
EXTERN Bits params; EXTERN Bits params;
EXTERN Bits consts; EXTERN Bits consts;
EXTERN Bits addrs; EXTERN Bits addrs;
EXTERN Bits ivar;
EXTERN Bits ovar; EXTERN Bits ovar;
EXTERN int change; EXTERN int change;
EXTERN int32 maxnr; EXTERN int32 maxnr;

View File

@ -57,7 +57,7 @@ rcmp(const void *a1, const void *a2)
} }
static void static void
setoutvar(void) setvar(Bits *dst, Type **args)
{ {
Type *t; Type *t;
Node *n; Node *n;
@ -66,18 +66,16 @@ setoutvar(void)
Bits bit; Bits bit;
int z; int z;
t = structfirst(&save, getoutarg(curfn->type)); t = structfirst(&save, args);
while(t != T) { while(t != T) {
n = nodarg(t, 1); n = nodarg(t, 1);
a = zprog.from; a = zprog.from;
naddr(n, &a, 0); naddr(n, &a, 0);
bit = mkvar(R, &a); bit = mkvar(R, &a);
for(z=0; z<BITS; z++) for(z=0; z<BITS; z++)
ovar.b[z] |= bit.b[z]; dst->b[z] |= bit.b[z];
t = structnext(&save); t = structnext(&save);
} }
//if(bany(&ovar))
//print("ovar = %Q\n", ovar);
} }
void void
@ -190,11 +188,14 @@ regopt(Prog *firstp)
params.b[z] = 0; params.b[z] = 0;
consts.b[z] = 0; consts.b[z] = 0;
addrs.b[z] = 0; addrs.b[z] = 0;
ivar.b[z] = 0;
ovar.b[z] = 0; ovar.b[z] = 0;
} }
// build list of return variables // build lists of parameters and results
setoutvar(); setvar(&ivar, getthis(curfn->type));
setvar(&ivar, getinarg(curfn->type));
setvar(&ovar, getoutarg(curfn->type));
/* /*
* pass 1 * pass 1
@ -895,8 +896,12 @@ prop(Reg *r, Bits ref, Bits cal)
case ABL: case ABL:
if(noreturn(r1->f.prog)) if(noreturn(r1->f.prog))
break; 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<BITS; z++) { for(z=0; z<BITS; z++) {
cal.b[z] |= ref.b[z] | externs.b[z]; cal.b[z] |= ref.b[z] | externs.b[z] | ivar.b[z];
ref.b[z] = 0; ref.b[z] = 0;
} }

View File

@ -94,6 +94,7 @@ EXTERN Bits externs;
EXTERN Bits params; EXTERN Bits params;
EXTERN Bits consts; EXTERN Bits consts;
EXTERN Bits addrs; EXTERN Bits addrs;
EXTERN Bits ivar;
EXTERN Bits ovar; EXTERN Bits ovar;
EXTERN int change; EXTERN int change;
EXTERN int32 maxnr; EXTERN int32 maxnr;

View File

@ -55,7 +55,7 @@ rcmp(const void *a1, const void *a2)
} }
static void static void
setoutvar(void) setvar(Bits *dst, Type **args)
{ {
Type *t; Type *t;
Node *n; Node *n;
@ -64,18 +64,16 @@ setoutvar(void)
Bits bit; Bits bit;
int z; int z;
t = structfirst(&save, getoutarg(curfn->type)); t = structfirst(&save, args);
while(t != T) { while(t != T) {
n = nodarg(t, 1); n = nodarg(t, 1);
a = zprog.from; a = zprog.from;
naddr(n, &a, 0); naddr(n, &a, 0);
bit = mkvar(R, &a); bit = mkvar(R, &a);
for(z=0; z<BITS; z++) for(z=0; z<BITS; z++)
ovar.b[z] |= bit.b[z]; dst->b[z] |= bit.b[z];
t = structnext(&save); t = structnext(&save);
} }
//if(bany(&ovar))
//print("ovars = %Q\n", ovar);
} }
static void static void
@ -176,11 +174,14 @@ regopt(Prog *firstp)
params.b[z] = 0; params.b[z] = 0;
consts.b[z] = 0; consts.b[z] = 0;
addrs.b[z] = 0; addrs.b[z] = 0;
ivar.b[z] = 0;
ovar.b[z] = 0; ovar.b[z] = 0;
} }
// build list of return variables // build lists of parameters and results
setoutvar(); setvar(&ivar, getthis(curfn->type));
setvar(&ivar, getinarg(curfn->type));
setvar(&ovar, getoutarg(curfn->type));
/* /*
* pass 1 * pass 1
@ -750,8 +751,12 @@ prop(Reg *r, Bits ref, Bits cal)
case ACALL: case ACALL:
if(noreturn(r1->f.prog)) if(noreturn(r1->f.prog))
break; 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<BITS; z++) { for(z=0; z<BITS; z++) {
cal.b[z] |= ref.b[z] | externs.b[z]; cal.b[z] |= ref.b[z] | externs.b[z] | ivar.b[z];
ref.b[z] = 0; ref.b[z] = 0;
} }

View File

@ -109,6 +109,7 @@ EXTERN Bits externs;
EXTERN Bits params; EXTERN Bits params;
EXTERN Bits consts; EXTERN Bits consts;
EXTERN Bits addrs; EXTERN Bits addrs;
EXTERN Bits ivar;
EXTERN Bits ovar; EXTERN Bits ovar;
EXTERN int change; EXTERN int change;
EXTERN int32 maxnr; EXTERN int32 maxnr;

View File

@ -55,7 +55,7 @@ rcmp(const void *a1, const void *a2)
} }
static void static void
setoutvar(void) setvar(Bits *dst, Type **args)
{ {
Type *t; Type *t;
Node *n; Node *n;
@ -64,18 +64,16 @@ setoutvar(void)
Bits bit; Bits bit;
int z; int z;
t = structfirst(&save, getoutarg(curfn->type)); t = structfirst(&save, args);
while(t != T) { while(t != T) {
n = nodarg(t, 1); n = nodarg(t, 1);
a = zprog.from; a = zprog.from;
naddr(n, &a, 0); naddr(n, &a, 0);
bit = mkvar(R, &a); bit = mkvar(R, &a);
for(z=0; z<BITS; z++) for(z=0; z<BITS; z++)
ovar.b[z] |= bit.b[z]; dst->b[z] |= bit.b[z];
t = structnext(&save); t = structnext(&save);
} }
//if(bany(ovar))
//print("ovars = %Q\n", ovar);
} }
static void static void
@ -146,11 +144,14 @@ regopt(Prog *firstp)
params.b[z] = 0; params.b[z] = 0;
consts.b[z] = 0; consts.b[z] = 0;
addrs.b[z] = 0; addrs.b[z] = 0;
ivar.b[z] = 0;
ovar.b[z] = 0; ovar.b[z] = 0;
} }
// build list of return variables // build lists of parameters and results
setoutvar(); setvar(&ivar, getthis(curfn->type));
setvar(&ivar, getinarg(curfn->type));
setvar(&ovar, getoutarg(curfn->type));
/* /*
* pass 1 * pass 1
@ -715,8 +716,12 @@ prop(Reg *r, Bits ref, Bits cal)
case ACALL: case ACALL:
if(noreturn(r1->f.prog)) if(noreturn(r1->f.prog))
break; 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<BITS; z++) { for(z=0; z<BITS; z++) {
cal.b[z] |= ref.b[z] | externs.b[z]; cal.b[z] |= ref.b[z] | externs.b[z] | ivar.b[z];
ref.b[z] = 0; ref.b[z] = 0;
} }

View File

@ -0,0 +1,40 @@
// run
// Copyright 2014 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.
// Issue 7944:
// Liveness bitmaps said b was live at call to g,
// but no one told the register optimizer.
package main
import "runtime"
func f(b []byte) {
for len(b) > 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))
}