]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/gc: fix liveness vs regopt mismatch for input variables
authorRuss Cox <rsc@golang.org>
Mon, 12 May 2014 21:19:02 +0000 (17:19 -0400)
committerRuss Cox <rsc@golang.org>
Mon, 12 May 2014 21:19:02 +0000 (17:19 -0400)
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

src/cmd/5g/opt.h
src/cmd/5g/reg.c
src/cmd/6g/opt.h
src/cmd/6g/reg.c
src/cmd/8g/opt.h
src/cmd/8g/reg.c
test/fixedbugs/issue7944.go [new file with mode: 0644]

index 15b9d14582aaf6cbf228dd67f13ec16dcc7c0ef4..e3e3f78ed28f48a78464a7594d78d5e935896f03 100644 (file)
@@ -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;
index 80a14db3c437b7b6f851bd9fee8f39b642883506..3eadde4cf4fa3031b6c9adbc2b52878c02dd31c1 100644 (file)
@@ -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; z<BITS; z++)
-                       ovar.b[z] |= bit.b[z];
+                       dst->b[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; 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;
                        }
                        
index 3dcc3d7476a6c2d4288aec99af9ae4ee75526f83..bf356af0c6891ab9d3e48158d5df34d8a78d19f8 100644 (file)
@@ -94,6 +94,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;
index 484c1c0cde582e1e8806fbc252cdf908d959087d..a1f0c756aa76c9b4e1e65adc76cd18c72e4b80f4 100644 (file)
@@ -55,7 +55,7 @@ rcmp(const void *a1, const void *a2)
 }
 
 static void
-setoutvar(void)
+setvar(Bits *dst, Type **args)
 {
        Type *t;
        Node *n;
@@ -64,18 +64,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; z<BITS; z++)
-                       ovar.b[z] |= bit.b[z];
+                       dst->b[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; 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;
                        }
                        
index b8f1875d80675d0f7a1ce127fddf7f94926105fb..77a69e13ab1423ae560e6195f3b030b7f9161379 100644 (file)
@@ -109,6 +109,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;
index d17e18b22709a099a1cbc3cdb92f7d55979123a2..046011c905a4bbbdf406d3cfbde20ae07cfc01ec 100644 (file)
@@ -55,7 +55,7 @@ rcmp(const void *a1, const void *a2)
 }
 
 static void
-setoutvar(void)
+setvar(Bits *dst, Type **args)
 {
        Type *t;
        Node *n;
@@ -64,18 +64,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; z<BITS; z++)
-                       ovar.b[z] |= bit.b[z];
+                       dst->b[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<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;
                        }
                        
diff --git a/test/fixedbugs/issue7944.go b/test/fixedbugs/issue7944.go
new file mode 100644 (file)
index 0000000..9e5bed1
--- /dev/null
@@ -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))
+}