]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: redo stack map entries to avoid false retention
authorKeith Randall <khr@golang.org>
Tue, 25 Mar 2014 21:11:34 +0000 (14:11 -0700)
committerKeith Randall <khr@golang.org>
Tue, 25 Mar 2014 21:11:34 +0000 (14:11 -0700)
Change two-bit stack map entries to encode:
0 = dead
1 = scalar
2 = pointer
3 = multiword

If multiword, the two-bit entry for the following word encodes:
0 = string
1 = slice
2 = iface
3 = eface

That way, during stack scanning we can check if a string
is zero length or a slice has zero capacity.  We can avoid
following the contained pointer in those cases.  It is safe
to do so because it can never be dereferenced, and it is
desirable to do so because it may cause false retention
of the following block in memory.

Slice feature turned off until issue 7564 is fixed.

Update #7549

LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/76380043

src/cmd/cc/pgen.c
src/cmd/gc/plive.c
src/pkg/runtime/extern.go
src/pkg/runtime/malloc.h
src/pkg/runtime/mfinal_test.go
src/pkg/runtime/mgc0.c
src/pkg/runtime/runtime.c
src/pkg/runtime/runtime.h
src/pkg/runtime/stack.c

index d63e90c24b01547d8f63785eb6511cbf0303bb7b..d3fc4193e017ed9f28f17602c1f3c1d6d943f39e 100644 (file)
@@ -661,7 +661,9 @@ walktype1(Type *t, int32 offset, Bvec *bv, int param)
 {
        Type *t1;
        int32 o;
+       int32 widthptr;
 
+       widthptr = ewidth[TIND];
        switch(t->etype) {
        case TCHAR:
        case TUCHAR:
@@ -676,14 +678,16 @@ walktype1(Type *t, int32 offset, Bvec *bv, int param)
        case TFLOAT:
        case TDOUBLE:
                // non-pointer types
+               for(o = 0; o < t->width; o++)
+                       bvset(bv, ((offset + t->offset + o) / widthptr) * BitsPerPointer); // 1 = live scalar
                break;
 
        case TIND:
        pointer:
                // pointer types
-               if((offset + t->offset) % ewidth[TIND] != 0)
+               if((offset + t->offset) % widthptr != 0)
                        yyerror("unaligned pointer");
-               bvset(bv, ((offset + t->offset) / ewidth[TIND])*BitsPerPointer);
+               bvset(bv, ((offset + t->offset) / widthptr)*BitsPerPointer + 1); // 2 = live ptr
                break;
 
        case TARRAY:
@@ -735,7 +739,7 @@ dumpgcargs(Type *fn, Sym *sym)
                // argument is a pointer.
                if(argoffset != ewidth[TIND])
                        yyerror("passbyptr arg not the right size");
-               bvset(bv, 0);
+               bvset(bv, 1); // 2 = live ptr
        }
        for(t = fn->down; t != T; t = t->down) {
                if(t->etype == TVOID)
index a6a1d48d7970fa124ec2476d7a35a224bad8737f..369b913f6dc970afd25c0969aac9cdd73e9f6d67 100644 (file)
@@ -1065,6 +1065,9 @@ twobitwalktype1(Type *t, vlong *xoffset, Bvec *bv)
        case TFLOAT64:
        case TCOMPLEX64:
        case TCOMPLEX128:
+               for(i = 0; i < t->width; i++) {
+                       bvset(bv, ((*xoffset + i) / widthptr) * BitsPerPointer); // 1 = live scalar
+               }
                *xoffset += t->width;
                break;
 
@@ -1076,7 +1079,7 @@ twobitwalktype1(Type *t, vlong *xoffset, Bvec *bv)
        case TMAP:
                if((*xoffset & (widthptr-1)) != 0)
                        fatal("twobitwalktype1: invalid alignment, %T", t);
-               bvset(bv, (*xoffset / widthptr) * BitsPerPointer);
+               bvset(bv, (*xoffset / widthptr) * BitsPerPointer + 1); // 2 = live ptr
                *xoffset += t->width;
                break;
 
@@ -1084,7 +1087,8 @@ twobitwalktype1(Type *t, vlong *xoffset, Bvec *bv)
                // struct { byte *str; intgo len; }
                if((*xoffset & (widthptr-1)) != 0)
                        fatal("twobitwalktype1: invalid alignment, %T", t);
-               bvset(bv, (*xoffset / widthptr) * BitsPerPointer);
+               bvset(bv, (*xoffset / widthptr) * BitsPerPointer + 0);
+               bvset(bv, (*xoffset / widthptr) * BitsPerPointer + 1); // 3:0 = multiword:string
                *xoffset += t->width;
                break;
 
@@ -1094,9 +1098,15 @@ twobitwalktype1(Type *t, vlong *xoffset, Bvec *bv)
                // struct { Type *type; union { void *ptr, uintptr val } data; }
                if((*xoffset & (widthptr-1)) != 0)
                        fatal("twobitwalktype1: invalid alignment, %T", t);
-               bvset(bv, ((*xoffset / widthptr) * BitsPerPointer) + 1);
-               if(isnilinter(t))
-                       bvset(bv, ((*xoffset / widthptr) * BitsPerPointer));
+               bvset(bv, ((*xoffset / widthptr) * BitsPerPointer) + 0);
+               bvset(bv, ((*xoffset / widthptr) * BitsPerPointer) + 1); // 3 = multiword
+               // next word contains 2 = Iface, 3 = Eface
+               if(isnilinter(t)) {
+                       bvset(bv, ((*xoffset / widthptr) * BitsPerPointer) + 2);
+                       bvset(bv, ((*xoffset / widthptr) * BitsPerPointer) + 3);
+               } else {
+                       bvset(bv, ((*xoffset / widthptr) * BitsPerPointer) + 3);
+               }
                *xoffset += t->width;
                break;
 
@@ -1109,11 +1119,20 @@ twobitwalktype1(Type *t, vlong *xoffset, Bvec *bv)
                        // struct { byte *array; uintgo len; uintgo cap; }
                        if((*xoffset & (widthptr-1)) != 0)
                                fatal("twobitwalktype1: invalid TARRAY alignment, %T", t);
-                       bvset(bv, (*xoffset / widthptr) * BitsPerPointer);
+                       if(0) {
+                               bvset(bv, (*xoffset / widthptr) * BitsPerPointer + 0);
+                               bvset(bv, (*xoffset / widthptr) * BitsPerPointer + 1);
+                               bvset(bv, (*xoffset / widthptr) * BitsPerPointer + 2); // 3:1 = multiword/slice
+                       } else {
+                               // Until bug 7564 is fixed, we consider a slice as
+                               // a separate pointer and integer.
+                               bvset(bv, (*xoffset / widthptr) * BitsPerPointer + 1);  // 2 = live ptr
+                               bvset(bv, (*xoffset / widthptr) * BitsPerPointer + 2);  // 1 = live scalar
+                       }
+                       // mark capacity as live
+                       bvset(bv, (*xoffset / widthptr) * BitsPerPointer + 4);  // 1 = live scalar
                        *xoffset += t->width;
-               } else if(!haspointers(t->type))
-                               *xoffset += t->width;
-               else
+               } else
                        for(i = 0; i < t->bound; i++)
                                twobitwalktype1(t->type, xoffset, bv);
                break;
@@ -1164,14 +1183,14 @@ twobitlivepointermap(Liveness *lv, Bvec *liveout, Array *vars, Bvec *args, Bvec
                node = *(Node**)arrayget(vars, i);
                switch(node->class) {
                case PAUTO:
-                       if(bvget(liveout, i) && haspointers(node->type)) {
+                       if(bvget(liveout, i)) {
                                xoffset = node->xoffset + stkptrsize;
                                twobitwalktype1(node->type, &xoffset, locals);
                        }
                        break;
                case PPARAM:
                case PPARAMOUT:
-                       if(bvget(liveout, i) && haspointers(node->type)) {
+                       if(bvget(liveout, i)) {
                                xoffset = node->xoffset;
                                twobitwalktype1(node->type, &xoffset, args);
                        }
index 30fc34c0ba9c872d73f02e76ea3e010368a4bb4f..0c5041d38b169ffd33813823ec4a93bd1f1a6bde 100644 (file)
@@ -36,6 +36,9 @@ a comma-separated list of name=val pairs. Supported names are:
        length of the pause. Setting gctrace=2 emits the same summary but also
        repeats each collection.
 
+       gcdead: setting gcdead=1 causes the garbage collector to clobber all stack slots
+       that it thinks are dead.
+
        scheddetail: setting schedtrace=X and scheddetail=1 causes the scheduler to emit
        detailed multiline info every X milliseconds, describing state of the scheduler,
        processors, threads and goroutines.
index ca6289174e2eb100822b0593ea8c1d841d0bb6a0..97b5a924fe48a412920abbb4b9820cde4def86e6 100644 (file)
@@ -606,8 +606,14 @@ struct StackMap
 enum {
        // Pointer map
        BitsPerPointer = 2,
-       BitsNoPointer = 0,
-       BitsPointer = 1,
+       BitsDead = 0,
+       BitsScalar = 1,
+       BitsPointer = 2,
+       BitsMultiWord = 3,
+       // BitsMultiWord will be set for the first word of a multi-word item.
+       // When it is set, one of the following will be set for the second word.
+       BitsString = 0,
+       BitsSlice = 1,
        BitsIface = 2,
        BitsEface = 3,
 };
index 32f26a6b29ab4d9f42dccfbd5837983d07a0dd2b..41213138d2a9f0ae2fcd3dffe410f4b8073746dc 100644 (file)
@@ -8,6 +8,7 @@ import (
        "runtime"
        "testing"
        "time"
+       "unsafe"
 )
 
 type Tintptr *int // assignable to *int
@@ -135,3 +136,83 @@ func BenchmarkFinalizerRun(b *testing.B) {
                }
        })
 }
+
+// One chunk must be exactly one sizeclass in size.
+// It should be a sizeclass not used much by others, so we
+// have a greater chance of finding adjacent ones.
+// size class 19: 320 byte objects, 25 per page, 1 page alloc at a time
+const objsize = 320
+
+type objtype [objsize]byte
+
+func adjChunks() (*objtype, *objtype) {
+       var s []*objtype
+
+       for {
+               c := new(objtype)
+               for _, d := range s {
+                       if uintptr(unsafe.Pointer(c))+unsafe.Sizeof(*c) == uintptr(unsafe.Pointer(d)) {
+                               return c, d
+                       }
+                       if uintptr(unsafe.Pointer(d))+unsafe.Sizeof(*c) == uintptr(unsafe.Pointer(c)) {
+                               return d, c
+                       }
+               }
+               s = append(s, c)
+       }
+}
+
+// Make sure an empty slice on the stack doesn't pin the next object in memory.
+func TestEmptySlice(t *testing.T) {
+       if true { // disable until bug 7564 is fixed.
+               return
+       }
+       x, y := adjChunks()
+
+       // the pointer inside xs points to y.
+       xs := x[objsize:] // change objsize to objsize-1 and the test passes
+
+       fin := make(chan bool, 1)
+       runtime.SetFinalizer(y, func(z *objtype) { fin <- true })
+       runtime.GC()
+       select {
+       case <-fin:
+       case <-time.After(4 * time.Second):
+               t.Errorf("finalizer of next object in memory didn't run")
+       }
+       xsglobal = xs // keep empty slice alive until here
+}
+
+var xsglobal []byte
+
+func adjStringChunk() (string, *objtype) {
+       b := make([]byte, objsize)
+       for {
+               s := string(b)
+               t := new(objtype)
+               p := *(*uintptr)(unsafe.Pointer(&s))
+               q := uintptr(unsafe.Pointer(t))
+               if p+objsize == q {
+                       return s, t
+               }
+       }
+}
+
+// Make sure an empty string on the stack doesn't pin the next object in memory.
+func TestEmptyString(t *testing.T) {
+       x, y := adjStringChunk()
+
+       ss := x[objsize:] // change objsize to objsize-1 and the test passes
+       fin := make(chan bool, 1)
+       // set finalizer on string contents of y
+       runtime.SetFinalizer(y, func(z *objtype) { fin <- true })
+       runtime.GC()
+       select {
+       case <-fin:
+       case <-time.After(4 * time.Second):
+               t.Errorf("finalizer of next string in memory didn't run")
+       }
+       ssglobal = ss // keep 0-length string live until here
+}
+
+var ssglobal string
index 166c52b2ad7da229ec4a98174e82475297a12933..04dd93608a5950f457d16ca001724516122e5c57 100644 (file)
@@ -1489,6 +1489,7 @@ scanbitvector(byte *scanp, BitVector *bv, bool afterprologue, void *wbufp)
        uintptr word, bits;
        uint32 *wordp;
        int32 i, remptrs;
+       byte *p;
 
        wordp = bv->data;
        for(remptrs = bv->n; remptrs > 0; remptrs -= 32) {
@@ -1500,11 +1501,52 @@ scanbitvector(byte *scanp, BitVector *bv, bool afterprologue, void *wbufp)
                i /= BitsPerPointer;
                for(; i > 0; i--) {
                        bits = word & 3;
-                       if(bits != BitsNoPointer && *(void**)scanp != nil)
-                               if(bits == BitsPointer)
+                       switch(bits) {
+                       case BitsDead:
+                               if(runtime·debug.gcdead)
+                                       *(uintptr*)scanp = (uintptr)0x6969696969696969LL;
+                               break;
+                       case BitsScalar:
+                               break;
+                       case BitsPointer:
+                               p = *(byte**)scanp;
+                               if(p != nil)
                                        enqueue1(wbufp, (Obj){scanp, PtrSize, 0});
-                               else
-                                       scaninterfacedata(bits, scanp, afterprologue, wbufp);
+                               break;
+                       case BitsMultiWord:
+                               p = *(byte**)scanp;
+                               if(p != nil) {
+                                       word >>= BitsPerPointer;
+                                       scanp += PtrSize;
+                                       i--;
+                                       if(i == 0) {
+                                               // Get next chunk of bits
+                                               remptrs -= 32;
+                                               word = *wordp++;
+                                               if(remptrs < 32)
+                                                       i = remptrs;
+                                               else
+                                                       i = 32;
+                                               i /= BitsPerPointer;
+                                       }
+                                       switch(word & 3) {
+                                       case BitsString:
+                                               if(((String*)(scanp - PtrSize))->len != 0)
+                                                       markonly(p);
+                                               break;
+                                       case BitsSlice:
+                                               if(((Slice*)(scanp - PtrSize))->cap < ((Slice*)(scanp - PtrSize))->len)
+                                                       runtime·throw("slice capacity smaller than length");
+                                               if(((Slice*)(scanp - PtrSize))->cap != 0)
+                                                       enqueue1(wbufp, (Obj){scanp - PtrSize, PtrSize, 0});
+                                               break;
+                                       case BitsIface:
+                                       case BitsEface:
+                                               scaninterfacedata(word & 3, scanp - PtrSize, afterprologue, wbufp);
+                                               break;
+                                       }
+                               }
+                       }
                        word >>= BitsPerPointer;
                        scanp += PtrSize;
                }
index 2198bc68507faa44cc364022fb077743de325c98..d77ff08af44ef28475faee54c6fb683a7dde9eab 100644 (file)
@@ -314,6 +314,7 @@ static struct {
        {"allocfreetrace", &runtime·debug.allocfreetrace},
        {"efence", &runtime·debug.efence},
        {"gctrace", &runtime·debug.gctrace},
+       {"gcdead", &runtime·debug.gcdead},
        {"scheddetail", &runtime·debug.scheddetail},
        {"schedtrace", &runtime·debug.schedtrace},
 };
index 9cb6960c624def66b89bdaca36b02c81c1bbdf24..7bd45d1a2473002654c7de853b507d6686c76fe3 100644 (file)
@@ -578,6 +578,7 @@ struct DebugVars
        int32   allocfreetrace;
        int32   efence;
        int32   gctrace;
+       int32   gcdead;
        int32   scheddetail;
        int32   schedtrace;
 };
index c73991470ea3c256535416cf0034e82da0e5bd57..6e5d9f1f5877e8225768883ac7c925941b56be4b 100644 (file)
@@ -354,7 +354,11 @@ adjustpointers(byte **scanp, BitVector *bv, AdjustInfo *adjinfo, Func *f)
                if(StackDebug >= 4)
                        runtime·printf("        %p:%s:%p\n", &scanp[i], mapnames[bv->data[i / (32 / BitsPerPointer)] >> (i * BitsPerPointer & 31) & 3], scanp[i]);
                switch(bv->data[i / (32 / BitsPerPointer)] >> (i * BitsPerPointer & 31) & 3) {
-               case BitsNoPointer:
+               case BitsDead:
+                       if(runtime·debug.gcdead)
+                               scanp[i] = (byte*)0x6868686868686868LL;
+                       break;
+               case BitsScalar:
                        break;
                case BitsPointer:
                        p = scanp[i];
@@ -370,33 +374,53 @@ adjustpointers(byte **scanp, BitVector *bv, AdjustInfo *adjinfo, Func *f)
                                scanp[i] = p + delta;
                        }
                        break;
-               case BitsEface:
-                       t = (Type*)scanp[i];
-                       if(t != nil && (t->size > PtrSize || (t->kind & KindNoPointers) == 0)) {
-                               p = scanp[i+1];
+               case BitsMultiWord:
+                       switch(bv->data[(i+1) / (32 / BitsPerPointer)] >> ((i+1) * BitsPerPointer & 31) & 3) {
+                       case BitsString:
+                               // string referents are never on the stack, never need to be adjusted
+                               i++; // skip len
+                               break;
+                       case BitsSlice:
+                               p = scanp[i];
                                if(minp <= p && p < maxp) {
                                        if(StackDebug >= 3)
-                                               runtime·printf("adjust eface %p\n", p);
-                                       if(t->size > PtrSize) // currently we always allocate such objects on the heap
-                                               runtime·throw("large interface value found on stack");
-                                       scanp[i+1] = p + delta;
+                                               runtime·printf("adjust slice %p\n", p);
+                                       scanp[i] = p + delta;
                                }
-                       }
-                       break;
-               case BitsIface:
-                       tab = (Itab*)scanp[i];
-                       if(tab != nil) {
-                               t = tab->type;
-                               if(t->size > PtrSize || (t->kind & KindNoPointers) == 0) {
+                               i += 2; // skip len, cap
+                               break;
+                       case BitsEface:
+                               t = (Type*)scanp[i];
+                               if(t != nil && (t->size > PtrSize || (t->kind & KindNoPointers) == 0)) {
                                        p = scanp[i+1];
                                        if(minp <= p && p < maxp) {
                                                if(StackDebug >= 3)
-                                                       runtime·printf("adjust iface %p\n", p);
+                                                       runtime·printf("adjust eface %p\n", p);
                                                if(t->size > PtrSize) // currently we always allocate such objects on the heap
                                                        runtime·throw("large interface value found on stack");
                                                scanp[i+1] = p + delta;
                                        }
                                }
+                               i++;
+                               break;
+                       case BitsIface:
+                               tab = (Itab*)scanp[i];
+                               if(tab != nil) {
+                                       t = tab->type;
+                                       //runtime·printf("          type=%p\n", t);
+                                       if(t->size > PtrSize || (t->kind & KindNoPointers) == 0) {
+                                               p = scanp[i+1];
+                                               if(minp <= p && p < maxp) {
+                                                       if(StackDebug >= 3)
+                                                               runtime·printf("adjust iface %p\n", p);
+                                                       if(t->size > PtrSize) // currently we always allocate such objects on the heap
+                                                               runtime·throw("large interface value found on stack");
+                                                       scanp[i+1] = p + delta;
+                                               }
+                                       }
+                               }
+                               i++;
+                               break;
                        }
                        break;
                }