runtime: enable 'bad pointer' check during garbage collection of Go stack frames

This is the same check we use during stack copying.
The check cannot be applied to C stack frames, even
though we do emit pointer bitmaps for the arguments,
because (1) the pointer bitmaps assume all arguments
are always live, not true of outputs during the prologue,
and (2) the pointer bitmaps encode interface values as
pointer pairs, not true of interfaces holding integers.

For the rest of the frames, however, we should hold ourselves
to the rule that a pointer marked live really is initialized.
The interface scanning already implicitly checks this
because it interprets the type word  as a valid type pointer.

This may slow things down a little because of the extra loads.
Or it may speed things up because we don't bother enqueuing
nil pointers anymore. Enough of the rest of the system is slow
right now that we can't measure it meaningfully.
Enable for now, even if it is slow, to shake out bugs in the
liveness bitmaps, and then decide whether to turn it off
for the Go 1.3 release (issue 7650 reminds us to do this).

The new m->traceback field lets us force printing of fp=
values on all goroutine stack traces when we detect a
bad pointer. This makes it easier to understand exactly
where in the frame the bad pointer is, so that we can trace
it back to a specific variable and determine what is wrong.

Update #7650

LGTM=khr
R=khr
CC=golang-codereviews
https://golang.org/cl/80860044

diff --git a/src/pkg/runtime/mgc0.c b/src/pkg/runtime/mgc0.c
index c2519d32c367dcce893c4430b309c4e41621ec80..40106534c27c080d456929aec3cf20ed8d0d4b41 100644 (file)
--- a/src/pkg/runtime/mgc0.c
+++ b/src/pkg/runtime/mgc0.c
@@ -1447,7 +1447,7 @@ scaninterfacedata(uintptr bits, byte *scanp, bool afterprologue, void *wbufp)
 
 // Starting from scanp, scans words corresponding to set bits.
 static void
-scanbitvector(byte *scanp, BitVector *bv, bool afterprologue, void *wbufp)
+scanbitvector(Func *f, bool precise, byte *scanp, BitVector *bv, bool afterprologue, void *wbufp)
 {
        uintptr word, bits;
        uint32 *wordp;
@@ -1473,8 +1473,16 @@ scanbitvector(byte *scanp, BitVector *bv, bool afterprologue, void *wbufp)
                                break;
                        case BitsPointer:
                                p = *(byte**)scanp;
-                               if(p != nil)
+                               if(p != nil) {
+                                       if(precise && p < (byte*)PageSize) {
+                                               // Looks like a junk value in a pointer slot.
+                                               // Liveness analysis wrong?
+                                               m->traceback = 2;
+                                               runtime·printf("bad pointer in frame %s at %p: %p\n", runtime·funcname(f), scanp, p);
+                                               runtime·throw("bad pointer in scanbitvector");
+                                       }
                                        enqueue1(wbufp, (Obj){scanp, PtrSize, 0});
+                               }
                                break;
                        case BitsMultiWord:
                                p = *(byte**)scanp;
@@ -1498,8 +1506,11 @@ scanbitvector(byte *scanp, BitVector *bv, bool afterprologue, void *wbufp)
                                                        markonly(p);
                                                break;
                                        case BitsSlice:
-                                               if(((Slice*)(scanp - PtrSize))->cap < ((Slice*)(scanp - PtrSize))->len)
+                                               if(((Slice*)(scanp - PtrSize))->cap < ((Slice*)(scanp - PtrSize))->len) {
+                                                       m->traceback = 2;
+                                                       runtime·printf("bad slice in frame %s at %p: %p/%p/%p\n", runtime·funcname(f), scanp, ((byte**)scanp)[0], ((byte**)scanp)[1], ((byte**)scanp)[2]);
                                                        runtime·throw("slice capacity smaller than length");
+                                               }
                                                if(((Slice*)(scanp - PtrSize))->cap != 0)
                                                        enqueue1(wbufp, (Obj){scanp - PtrSize, PtrSize, 0});
                                                break;
@@ -1527,6 +1538,7 @@ scanframe(Stkframe *frame, void *wbufp)
        uintptr targetpc;
        int32 pcdata;
        bool afterprologue;
+       bool precise;
 
        f = frame->fn;
        targetpc = frame->pc;
@@ -1543,6 +1555,7 @@ scanframe(Stkframe *frame, void *wbufp)
        // Scan local variables if stack frame has been allocated.
        // Use pointer information if known.
        afterprologue = (frame->varp > (byte*)frame->sp);
+       precise = false;
        if(afterprologue) {
                stackmap = runtime·funcdata(f, FUNCDATA_LocalsPointerMaps);
                if(stackmap == nil) {
@@ -1564,7 +1577,8 @@ scanframe(Stkframe *frame, void *wbufp)
                        }
                        bv = runtime·stackmapdata(stackmap, pcdata);
                        size = (bv->n * PtrSize) / BitsPerPointer;
-                       scanbitvector(frame->varp - size, bv, afterprologue, wbufp);
+                       precise = true;
+                       scanbitvector(f, true, frame->varp - size, bv, afterprologue, wbufp);
                }
        }
 
@@ -1573,7 +1587,7 @@ scanframe(Stkframe *frame, void *wbufp)
        stackmap = runtime·funcdata(f, FUNCDATA_ArgsPointerMaps);
        if(stackmap != nil) {
                bv = runtime·stackmapdata(stackmap, pcdata);
-               scanbitvector(frame->argp, bv, true, wbufp);
+               scanbitvector(f, precise, frame->argp, bv, true, wbufp);
        } else
                enqueue1(wbufp, (Obj){frame->argp, frame->arglen, 0});
        return true;

diff --git a/src/pkg/runtime/runtime.c b/src/pkg/runtime/runtime.c
index d77ff08af44ef28475faee54c6fb683a7dde9eab..d995bf97ae95159207ef4e1b60c1285ce17391c2 100644 (file)
--- a/src/pkg/runtime/runtime.c
+++ b/src/pkg/runtime/runtime.c
@@ -25,8 +25,11 @@ runtime·gotraceback(bool *crash)
        if(crash != nil)
                *crash = false;
        p = runtime·getenv("GOTRACEBACK");
-       if(p == nil || p[0] == '\0')
+       if(p == nil || p[0] == '\0') {
+               if(m->traceback != 0)
+                       return m->traceback;
                return 1;       // default is on
+       }
        if(runtime·strcmp(p, (byte*)"crash") == 0) {
                if(crash != nil)
                        *crash = true;

diff --git a/src/pkg/runtime/runtime.h b/src/pkg/runtime/runtime.h
index 1a06b8a1132502e118ddd74aa792feeb5783160e..28c831a0684cf3b338e19a07f9a5954af187c6d9 100644 (file)
--- a/src/pkg/runtime/runtime.h
+++ b/src/pkg/runtime/runtime.h
@@ -342,6 +342,7 @@ struct      M
        uint32  waitsemalock;
        GCStats gcstats;
        bool    needextram;
+       uint8   traceback;
        bool    (*waitunlockf)(G*, void*);
        void*   waitlock;
        uintptr forkstackguard;

diff --git a/src/pkg/runtime/stack.c b/src/pkg/runtime/stack.c
index 4d21c719b0222df8a3106442a9e14e30e5d7ff6e..27543a5778ec03527261a9c037b25aced267123b 100644 (file)
--- a/src/pkg/runtime/stack.c
+++ b/src/pkg/runtime/stack.c
@@ -365,6 +365,7 @@ adjustpointers(byte **scanp, BitVector *bv, AdjustInfo *adjinfo, Func *f)
                        if(f != nil && (byte*)0 < p && p < (byte*)PageSize) {
                                // Looks like a junk value in a pointer slot.
                                // Live analysis wrong?
+                               m->traceback = 2;
                                runtime·printf("%p: %p %s\n", &scanp[i], p, runtime·funcname(f));
                                runtime·throw("bad pointer!");
                        }

diff --git a/src/pkg/runtime/traceback_arm.c b/src/pkg/runtime/traceback_arm.c
index 171672a89d17e90393be66d3f89ea520538f5840..f5cd4133d46d5f235c24c54e88be4b003e913cbf 100644 (file)
--- a/src/pkg/runtime/traceback_arm.c
+++ b/src/pkg/runtime/traceback_arm.c
@@ -12,13 +12,15 @@ void runtime·sigpanic(void);
 int32
 runtime·gentraceback(uintptr pc0, uintptr sp0, uintptr lr0, G *gp, int32 skip, uintptr *pcbuf, int32 max, bool (*callback)(Stkframe*, void*), void *v, bool printall)
 {
-       int32 i, n, nprint, line;
+       int32 i, n, nprint, line, gotraceback;
        uintptr x, tracepc;
        bool waspanic, printing;
        Func *f, *flr;
        Stkframe frame;
        Stktop *stk;
        String file;
+       
+       gotraceback = runtime·gotraceback(nil);
 
        if(pc0 == ~(uintptr)0 && sp0 == ~(uintptr)0) { // Signal to fetch saved values from gp.
                if(gp->syscallstack != (uintptr)nil) {
@@ -167,7 +169,7 @@ runtime·gentraceback(uintptr pc0, uintptr sp0, uintptr lr0, G *gp, int32 skip,
                                runtime·printf("\t%S:%d", file, line);
                                if(frame.pc > f->entry)
                                        runtime·printf(" +%p", (uintptr)(frame.pc - f->entry));
-                               if(m->throwing > 0 && gp == m->curg)
+                               if(m->throwing > 0 && gp == m->curg || gotraceback >= 2)
                                        runtime·printf(" fp=%p", frame.fp);
                                runtime·printf("\n");
                                nprint++;

diff --git a/src/pkg/runtime/traceback_x86.c b/src/pkg/runtime/traceback_x86.c
index 20003350aed948941e1b636e02afb4113fc2693d..4c8074e9e40ae6173f869ba2e3f3623b354760f4 100644 (file)
--- a/src/pkg/runtime/traceback_x86.c
+++ b/src/pkg/runtime/traceback_x86.c
@@ -28,7 +28,7 @@ void runtime·sigtramp(void);
 int32
 runtime·gentraceback(uintptr pc0, uintptr sp0, uintptr lr0, G *gp, int32 skip, uintptr *pcbuf, int32 max, bool (*callback)(Stkframe*, void*), void *v, bool printall)
 {
-       int32 i, n, nprint, line;
+       int32 i, n, nprint, line, gotraceback;
        uintptr tracepc;
        bool waspanic, printing;
        Func *f, *flr;
@@ -38,6 +38,8 @@ runtime·gentraceback(uintptr pc0, uintptr sp0, uintptr lr0, G *gp, int32 skip,
 
        USED(lr0);
        
+       gotraceback = runtime·gotraceback(nil);
+       
        if(pc0 == ~(uintptr)0 && sp0 == ~(uintptr)0) { // Signal to fetch saved values from gp.
                if(gp->syscallstack != (uintptr)nil) {
                        pc0 = gp->syscallpc;
@@ -228,7 +230,7 @@ runtime·gentraceback(uintptr pc0, uintptr sp0, uintptr lr0, G *gp, int32 skip,
                                runtime·printf("\t%S:%d", file, line);
                                if(frame.pc > f->entry)
                                        runtime·printf(" +%p", (uintptr)(frame.pc - f->entry));
-                               if(m->throwing > 0 && gp == m->curg)
+                               if(m->throwing > 0 && gp == m->curg || gotraceback >= 2)
                                        runtime·printf(" fp=%p", frame.fp);
                                runtime·printf("\n");
                                nprint++;