]> Cypherpunks repositories - gostls13.git/commitdiff
stack overflow debugging and fix.
authorRuss Cox <rsc@golang.org>
Mon, 12 Oct 2009 17:26:38 +0000 (10:26 -0700)
committerRuss Cox <rsc@golang.org>
Mon, 12 Oct 2009 17:26:38 +0000 (10:26 -0700)
  * in 6l, -K already meant check for stack underflow.
    add -KK to mean double-check stack overflows
    even in nosplit functions.

  * comment out print locks; they deadlock too easily
     but are still useful to put back for special occasions.

  * let runcgo assembly switch to scheduler stack
    without involving scheduler directly.  because runcgo
    gets called from matchmg, it is too hard to keep it
    from being called on other stacks.

R=r
DELTA=94  (65 added, 18 deleted, 11 changed)
OCL=35591
CL=35604

src/cmd/6l/pass.c
src/pkg/runtime/386/asm.s
src/pkg/runtime/amd64/asm.s
src/pkg/runtime/cgocall.c
src/pkg/runtime/print.c
src/pkg/runtime/proc.c
src/pkg/runtime/runtime.h

index e5b948a6edbaaa55d3015e60dc51a51ff85f1665..7a95f7427e3c622b5201406db3b806ea81855a15 100644 (file)
@@ -638,6 +638,9 @@ dostkoff(void)
 
                        q = P;
                        q1 = P;
+                       if((p->from.scale & NOSPLIT) && autoffset >= StackSmall)
+                               diag("nosplit func likely to overflow stack");
+
                        if(!(p->from.scale & NOSPLIT)) {
                                if(debug['K']) {
                                        // 6l -K means check not only for stack
@@ -792,8 +795,44 @@ dostkoff(void)
                                if(q != P)
                                        q->pcond = p;
                        }
-
                        deltasp = autoffset;
+
+                       if(debug['K'] > 1 && autoffset) {
+                               // 6l -KK means double-check for stack overflow
+                               // even after calling morestack and even if the
+                               // function is marked as nosplit.
+                               p = appendp(p);
+                               p->as = AMOVQ;
+                               p->from.type = D_INDIR+D_R15;
+                               p->from.offset = 0;
+                               p->to.type = D_BX;
+
+                               p = appendp(p);
+                               p->as = ASUBQ;
+                               p->from.type = D_CONST;
+                               p->from.offset = StackSmall+32;
+                               p->to.type = D_BX;
+
+                               p = appendp(p);
+                               p->as = ACMPQ;
+                               p->from.type = D_SP;
+                               p->to.type = D_BX;
+
+                               p = appendp(p);
+                               p->as = AJHI;
+                               p->to.type = D_BRANCH;
+                               q1 = p;
+
+                               p = appendp(p);
+                               p->as = AINT;
+                               p->from.type = D_CONST;
+                               p->from.offset = 3;
+
+                               p = appendp(p);
+                               p->as = ANOP;
+                               q1->pcond = p;
+                               q1 = P;
+                       }
                }
                pcsize = p->mode/8;
                a = p->from.type;
@@ -844,13 +883,12 @@ dostkoff(void)
                        goto become;
 
                if(autoffset) {
-                       q = p;
+                       p->as = AADJSP;
+                       p->from.type = D_CONST;
+                       p->from.offset = -autoffset;
+
                        p = appendp(p);
                        p->as = ARET;
-
-                       q->as = AADJSP;
-                       q->from.type = D_CONST;
-                       q->from.offset = -autoffset;
                }
                continue;
 
index 5aa73a6b8f73b228463068ca5c1d2d93fadb6093..9df7fb14660899f1fc297592dfcfdee26f9323d5 100644 (file)
@@ -300,12 +300,22 @@ TEXT      abort(SB),7,$0
        INT $0x3
 
 // runcgo(void(*fn)(void*), void *arg)
-// Just call fn(arg), but first align the stack
-// appropriately for the gcc ABI.
+// Call fn(arg) on the scheduler stack,
+// aligned appropriately for the gcc ABI.
 TEXT   runcgo(SB),7,$16
        MOVL    fn+0(FP), AX
        MOVL    arg+4(FP), BX
        MOVL    SP, CX
+
+       // Figure out if we need to switch to m->g0 stack.
+       MOVL    m, DX
+       MOVL    m_g0(DX), SI
+       CMPL    g, SI
+       JEQ     2(PC)
+       MOVL    (m_sched+gobuf_sp)(DX), SP
+
+       // Now on a scheduling stack (a pthread-created stack).
+       SUBL    $16, SP
        ANDL    $~15, SP        // alignment for gcc ABI
        MOVL    CX, 4(SP)
        MOVL    BX, 0(SP)
index 6cb6d5c77a5dc61c1404f3201de5cb635f6adb38..87bc222e14cf795ba24d762bd53b07df2d1ea2c0 100644 (file)
@@ -272,20 +272,32 @@ TEXT jmpdefer(SB), 7, $0
        JMP     AX      // but first run the deferred function
 
 // runcgo(void(*fn)(void*), void *arg)
-// Call fn(arg), but align the stack
-// appropriately for the gcc ABI
-// and also save g and m across the call,
+// Call fn(arg) on the scheduler stack,
+// aligned appropriately for the gcc ABI.
+// Save g and m across the call,
 // since the foreign code might reuse them.
 TEXT runcgo(SB),7,$32
+       // Save old registers.
        MOVQ    fn+0(FP),AX
        MOVQ    arg+8(FP),DI    // DI = first argument in AMD64 ABI
        MOVQ    SP, CX
+
+       // Figure out if we need to switch to m->g0 stack.
+       MOVQ    m_g0(m), R8
+       CMPQ    R8, g
+       JEQ     2(PC)
+       MOVQ    (m_sched+gobuf_sp)(m), SP
+
+       // Now on a scheduling stack (a pthread-created stack).
+       SUBQ    $32, SP
        ANDQ    $~15, SP        // alignment for gcc ABI
        MOVQ    g, 24(SP)       // save old g, m, SP
        MOVQ    m, 16(SP)
        MOVQ    CX, 8(SP)
        CALL    AX
-       MOVQ    16(SP), m       // restore
+
+       // Restore registers, stack pointer.
+       MOVQ    16(SP), m
        MOVQ    24(SP), g
        MOVQ    8(SP), SP
        RET
index a475603957257e1d5c1c0baae2e0d96eb6ca59fa..70382ceee12769e805073382b9eeb710234b9aaf 100644 (file)
@@ -25,10 +25,7 @@ cgocall(void (*fn)(void*), void *arg)
         * foreign code.
         */
        sys·entersyscall();
-       g->cgofn = fn;
-       g->cgoarg = arg;
-       g->status = Gcgocall;
-       gosched();
+       runcgo(fn, arg);
        sys·exitsyscall();
        return;
 }
index fb2881be55a49a3f3e02530a7333fbdafc102dee..4a358a8116f555d28e6d4975543648ce2b576722 100644 (file)
@@ -4,7 +4,7 @@
 
 #include "runtime.h"
 
-static Lock debuglock;
+//static Lock debuglock;
 
 void
 dump(byte *p, int32 n)
@@ -37,7 +37,7 @@ printf(int8 *s, ...)
        int8 *p, *lp;
        byte *arg, *narg;
 
-       lock(&debuglock);
+//     lock(&debuglock);
 
        lp = p = s;
        arg = (byte*)(&s+1);
@@ -100,7 +100,7 @@ printf(int8 *s, ...)
        if(p > lp)
                write(1, lp, p-lp);
 
-       unlock(&debuglock);
+//     unlock(&debuglock);
 }
 
 
index f6f2bb2b3654170b2ba011f2660f16245393e3f0..4113002ada56194515485ac0722d53b9e1da8dfd 100644 (file)
@@ -95,7 +95,7 @@ schedinit(void)
 {
        int32 n;
        byte *p;
-       
+
        allm = m;
 
        mallocinit();
@@ -452,15 +452,6 @@ scheduler(void)
        lock(&sched);
        if(gosave(&m->sched) != 0){
                gp = m->curg;
-               if(gp->status == Gcgocall){
-                       // Runtime call into external code (FFI).
-                       // When running with FFI, the scheduler stack is a
-                       // native pthread stack, so it suffices to switch to the
-                       // scheduler stack and make the call.
-                       runcgo(gp->cgofn, gp->cgoarg);
-                       gp->status = Grunning;
-                       gogo(&gp->sched, 1);
-               }
 
                // Jumped here via gosave/gogo, so didn't
                // execute lock(&sched) above.
index b44eb929ccc153cfb23fcbcb8a233310453d3e3c..b560d68f98c23128c38912eb63fea932d305f31c 100644 (file)
@@ -94,7 +94,6 @@ enum
        Gwaiting,
        Gmoribund,
        Gdead,
-       Gcgocall,
 };
 enum
 {