]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: fix panic/wrapper/recover math
authorRuss Cox <rsc@golang.org>
Sat, 6 Sep 2014 17:19:08 +0000 (13:19 -0400)
committerRuss Cox <rsc@golang.org>
Sat, 6 Sep 2014 17:19:08 +0000 (13:19 -0400)
The gp->panicwrap adjustment is just fatally flawed.
Now that there is a Panic.argp field, update that instead.
That can be done on entry only, so that unwinding doesn't
need to worry about undoing anything. The wrappers
emit a few more instructions in the prologue but everything
else in the system gets much simpler.

It also fixes (without trying) a broken test I never checked in.

Fixes #7491.

LGTM=khr
R=khr
CC=dvyukov, golang-codereviews, iant, r
https://golang.org/cl/135490044

17 files changed:
src/liblink/obj5.c
src/liblink/obj6.c
src/liblink/obj8.c
src/pkg/reflect/value.go
src/pkg/runtime/asm_386.s
src/pkg/runtime/asm_amd64.s
src/pkg/runtime/asm_amd64p32.s
src/pkg/runtime/asm_arm.s
src/pkg/runtime/cgocall.go
src/pkg/runtime/malloc.go
src/pkg/runtime/panic.go
src/pkg/runtime/panic1.go
src/pkg/runtime/proc.c
src/pkg/runtime/runtime.h
src/pkg/runtime/stack.c
src/pkg/runtime/stubs.go
test/recover.go

index de920b029e233e8b57e3fad32e184e07d1797e0b..d7008a48c0fe56f84171340a4d1a6bb2b97d40ea 100644 (file)
@@ -241,7 +241,7 @@ nocache(Prog *p)
 static void
 addstacksplit(Link *ctxt, LSym *cursym)
 {
-       Prog *p, *pl, *q, *q1, *q2;
+       Prog *p, *pl, *p1, *p2, *q, *q1, *q2;
        int o;
        int32 autosize, autoffset;
        
@@ -437,32 +437,93 @@ addstacksplit(Link *ctxt, LSym *cursym)
                        p->spadj = autosize;
                        
                        if(cursym->text->reg & WRAPPER) {
-                               // g->panicwrap += autosize;
-                               // MOVW panicwrap_offset(g), R3
-                               // ADD $autosize, R3
-                               // MOVW R3 panicwrap_offset(g)
+                               // if(g->panic != nil && g->panic->argp == FP) g->panic->argp = bottom-of-frame
+                               //
+                               //      MOVW g_panic(g), R1
+                               //      CMP $0, R1
+                               //      B.EQ end
+                               //      MOVW panic_argp(R1), R2
+                               //      ADD $(autosize+4), R13, R3
+                               //      CMP R2, R3
+                               //      B.NE end
+                               //      ADD $4, R13, R4
+                               //      MOVW R4, panic_argp(R1)
+                               // end:
+                               //      NOP
+                               //
+                               // The NOP is needed to give the jumps somewhere to land.
+                               // It is a liblink NOP, not an ARM NOP: it encodes to 0 instruction bytes.
+
                                p = appendp(ctxt, p);
                                p->as = AMOVW;
                                p->from.type = D_OREG;
                                p->from.reg = REGG;
-                               p->from.offset = 2*ctxt->arch->ptrsize;
+                               p->from.offset = 2*ctxt->arch->ptrsize; // G.panic
                                p->to.type = D_REG;
-                               p->to.reg = 3;
+                               p->to.reg = 1;
+                       
+                               p = appendp(ctxt, p);
+                               p->as = ACMP;
+                               p->from.type = D_CONST;
+                               p->from.offset = 0;
+                               p->to.type = D_REG;
+                               p->to.reg = 1;
+                       
+                               p = appendp(ctxt, p);
+                               p->as = AB;
+                               p->scond = C_SCOND_EQ;
+                               p->to.type = D_BRANCH;
+                               p1 = p;
+                               
+                               p = appendp(ctxt, p);
+                               p->as = AMOVW;
+                               p->from.type = D_OREG;
+                               p->from.reg = 1;
+                               p->from.offset = 0; // Panic.argp
+                               p->to.type = D_REG;
+                               p->to.reg = 2;
                        
                                p = appendp(ctxt, p);
                                p->as = AADD;
                                p->from.type = D_CONST;
-                               p->from.offset = autosize;
+                               p->from.offset = autosize+4;
+                               p->reg = 13;
                                p->to.type = D_REG;
                                p->to.reg = 3;
-                               
+
+                               p = appendp(ctxt, p);
+                               p->as = ACMP;
+                               p->from.type = D_REG;
+                               p->from.offset = 2;
+                               p->to.type = D_REG;
+                               p->to.reg = 3;
+
+                               p = appendp(ctxt, p);
+                               p->as = AB;
+                               p->scond = C_SCOND_NE;
+                               p->to.type = D_BRANCH;
+                               p2 = p;
+                       
+                               p = appendp(ctxt, p);
+                               p->as = AADD;
+                               p->from.type = D_CONST;
+                               p->from.offset = 4;
+                               p->reg = 13;
+                               p->to.type = D_REG;
+                               p->to.reg = 4;
+
                                p = appendp(ctxt, p);
                                p->as = AMOVW;
                                p->from.type = D_REG;
-                               p->from.reg = 3;
+                               p->from.reg = 4;
                                p->to.type = D_OREG;
-                               p->to.reg = REGG;
-                               p->to.offset = 2*ctxt->arch->ptrsize;
+                               p->to.reg = 1;
+                               p->to.offset = 0; // Panic.argp
+
+                               p = appendp(ctxt, p);
+                               p->as = ANOP;
+                               p1->pcond = p;
+                               p2->pcond = p;
                        }
                        break;
 
@@ -483,44 +544,6 @@ addstacksplit(Link *ctxt, LSym *cursym)
                                }
                        }
 
-                       if(cursym->text->reg & WRAPPER) {
-                               int scond;
-                               
-                               // Preserve original RET's cond, to allow RET.EQ
-                               // in the implementation of reflect.call.
-                               scond = p->scond;
-                               p->scond = C_SCOND_NONE;
-
-                               // g->panicwrap -= autosize;
-                               // MOVW panicwrap_offset(g), R3
-                               // SUB $autosize, R3
-                               // MOVW R3 panicwrap_offset(g)
-                               p->as = AMOVW;
-                               p->from.type = D_OREG;
-                               p->from.reg = REGG;
-                               p->from.offset = 2*ctxt->arch->ptrsize;
-                               p->to.type = D_REG;
-                               p->to.reg = 3;
-                               p = appendp(ctxt, p);
-                       
-                               p->as = ASUB;
-                               p->from.type = D_CONST;
-                               p->from.offset = autosize;
-                               p->to.type = D_REG;
-                               p->to.reg = 3;
-                               p = appendp(ctxt, p);
-
-                               p->as = AMOVW;
-                               p->from.type = D_REG;
-                               p->from.reg = 3;
-                               p->to.type = D_OREG;
-                               p->to.reg = REGG;
-                               p->to.offset = 2*ctxt->arch->ptrsize;
-                               p = appendp(ctxt, p);
-
-                               p->scond = scond;
-                       }
-
                        p->as = AMOVW;
                        p->scond |= C_PBIT;
                        p->from.type = D_OREG;
index eef3b4294a0101b02dd5a58c2176665e3bb263a6..c6b8e0964ed5c801e23530462817d0ff2d6244b2 100644 (file)
@@ -388,7 +388,7 @@ parsetextconst(vlong arg, vlong *textstksiz, vlong *textarg)
 static void
 addstacksplit(Link *ctxt, LSym *cursym)
 {
-       Prog *p, *q, *q1;
+       Prog *p, *q, *q1, *p1, *p2;
        int32 autoffset, deltasp;
        int a, pcsize;
        uint32 i;
@@ -463,13 +463,64 @@ addstacksplit(Link *ctxt, LSym *cursym)
        deltasp = autoffset;
        
        if(cursym->text->from.scale & WRAPPER) {
-               // g->panicwrap += autoffset + ctxt->arch->regsize;
+               // if(g->panic != nil && g->panic->argp == FP) g->panic->argp = bottom-of-frame
+               //
+               //      MOVQ g_panic(CX), BX
+               //      TESTQ BX, BX
+               //      JEQ end
+               //      LEAQ (autoffset+8)(SP), DI
+               //      CMPQ panic_argp(BX), DI
+               //      JNE end
+               //      MOVQ SP, panic_argp(BX)
+               // end:
+               //      NOP
+               //
+               // The NOP is needed to give the jumps somewhere to land.
+               // It is a liblink NOP, not an x86 NOP: it encodes to 0 instruction bytes.
+
                p = appendp(ctxt, p);
-               p->as = AADDL;
-               p->from.type = D_CONST;
-               p->from.offset = autoffset + ctxt->arch->regsize;
-               indir_cx(ctxt, &p->to);
-               p->to.offset = 2*ctxt->arch->ptrsize;
+               p->as = AMOVQ;
+               p->from.type = D_INDIR+D_CX;
+               p->from.offset = 2*ctxt->arch->ptrsize; // G.panic
+               p->to.type = D_BX;
+
+               p = appendp(ctxt, p);
+               p->as = ATESTQ;
+               p->from.type = D_BX;
+               p->to.type = D_BX;
+
+               p = appendp(ctxt, p);
+               p->as = AJEQ;
+               p->to.type = D_BRANCH;
+               p1 = p;
+
+               p = appendp(ctxt, p);
+               p->as = ALEAQ;
+               p->from.type = D_INDIR+D_SP;
+               p->from.offset = autoffset+8;
+               p->to.type = D_DI;
+
+               p = appendp(ctxt, p);
+               p->as = ACMPQ;
+               p->from.type = D_INDIR+D_BX;
+               p->from.offset = 0; // Panic.argp
+               p->to.type = D_DI;
+
+               p = appendp(ctxt, p);
+               p->as = AJNE;
+               p->to.type = D_BRANCH;
+               p2 = p;
+
+               p = appendp(ctxt, p);
+               p->as = AMOVQ;
+               p->from.type = D_SP;
+               p->to.type = D_INDIR+D_BX;
+               p->to.offset = 0; // Panic.argp
+
+               p = appendp(ctxt, p);
+               p->as = ANOP;
+               p1->pcond = p;
+               p2->pcond = p;
        }
 
        if(ctxt->debugstack > 1 && autoffset) {
@@ -589,19 +640,6 @@ addstacksplit(Link *ctxt, LSym *cursym)
                if(autoffset != deltasp)
                        ctxt->diag("unbalanced PUSH/POP");
 
-               if(cursym->text->from.scale & WRAPPER) {
-                       p = load_g_cx(ctxt, p);
-                       p = appendp(ctxt, p);
-                       // g->panicwrap -= autoffset + ctxt->arch->regsize;
-                       p->as = ASUBL;
-                       p->from.type = D_CONST;
-                       p->from.offset = autoffset + ctxt->arch->regsize;
-                       indir_cx(ctxt, &p->to);
-                       p->to.offset = 2*ctxt->arch->ptrsize;
-                       p = appendp(ctxt, p);
-                       p->as = ARET;
-               }
-
                if(autoffset) {
                        p->as = AADJSP;
                        p->from.type = D_CONST;
index 50e6d8236d6e40efde5e0ee2f2dabbe2f352acbf..fa1e1ca243984be316dba8a58cc98be93c07e7d9 100644 (file)
@@ -261,7 +261,7 @@ static Prog*        stacksplit(Link*, Prog*, int32, int, Prog**);
 static void
 addstacksplit(Link *ctxt, LSym *cursym)
 {
-       Prog *p, *q;
+       Prog *p, *q, *p1, *p2;
        int32 autoffset, deltasp;
        int a;
 
@@ -317,13 +317,64 @@ addstacksplit(Link *ctxt, LSym *cursym)
        deltasp = autoffset;
        
        if(cursym->text->from.scale & WRAPPER) {
-               // g->panicwrap += autoffset + ctxt->arch->ptrsize;
+               // if(g->panic != nil && g->panic->argp == FP) g->panic->argp = bottom-of-frame
+               //
+               //      MOVL g_panic(CX), BX
+               //      TESTL BX, BX
+               //      JEQ end
+               //      LEAL (autoffset+4)(SP), DI
+               //      CMPL panic_argp(BX), DI
+               //      JNE end
+               //      MOVL SP, panic_argp(BX)
+               // end:
+               //      NOP
+               //
+               // The NOP is needed to give the jumps somewhere to land.
+               // It is a liblink NOP, not an x86 NOP: it encodes to 0 instruction bytes.
+
                p = appendp(ctxt, p);
-               p->as = AADDL;
-               p->from.type = D_CONST;
-               p->from.offset = autoffset + ctxt->arch->ptrsize;
-               p->to.type = D_INDIR+D_CX;
-               p->to.offset = 2*ctxt->arch->ptrsize;
+               p->as = AMOVL;
+               p->from.type = D_INDIR+D_CX;
+               p->from.offset = 2*ctxt->arch->ptrsize; // G.panic
+               p->to.type = D_BX;
+
+               p = appendp(ctxt, p);
+               p->as = ATESTL;
+               p->from.type = D_BX;
+               p->to.type = D_BX;
+
+               p = appendp(ctxt, p);
+               p->as = AJEQ;
+               p->to.type = D_BRANCH;
+               p1 = p;
+
+               p = appendp(ctxt, p);
+               p->as = ALEAL;
+               p->from.type = D_INDIR+D_SP;
+               p->from.offset = autoffset+4;
+               p->to.type = D_DI;
+
+               p = appendp(ctxt, p);
+               p->as = ACMPL;
+               p->from.type = D_INDIR+D_BX;
+               p->from.offset = 0; // Panic.argp
+               p->to.type = D_DI;
+
+               p = appendp(ctxt, p);
+               p->as = AJNE;
+               p->to.type = D_BRANCH;
+               p2 = p;
+
+               p = appendp(ctxt, p);
+               p->as = AMOVL;
+               p->from.type = D_SP;
+               p->to.type = D_INDIR+D_BX;
+               p->to.offset = 0; // Panic.argp
+
+               p = appendp(ctxt, p);
+               p->as = ANOP;
+               p1->pcond = p;
+               p2->pcond = p;
        }
        
        if(ctxt->debugzerostack && autoffset && !(cursym->text->from.scale&NOSPLIT)) {
@@ -396,19 +447,6 @@ addstacksplit(Link *ctxt, LSym *cursym)
                if(autoffset != deltasp)
                        ctxt->diag("unbalanced PUSH/POP");
 
-               if(cursym->text->from.scale & WRAPPER) {
-                       p = load_g_cx(ctxt, p);
-                       p = appendp(ctxt, p);
-                       // g->panicwrap -= autoffset + ctxt->arch->ptrsize;
-                       p->as = ASUBL;
-                       p->from.type = D_CONST;
-                       p->from.offset = autoffset + ctxt->arch->ptrsize;
-                       p->to.type = D_INDIR+D_CX;
-                       p->to.offset = 2*ctxt->arch->ptrsize;
-                       p = appendp(ctxt, p);
-                       p->as = ARET;
-               }
-
                if(autoffset) {
                        p->as = AADJSP;
                        p->from.type = D_CONST;
index b02b8ea0c26886cfd1cc4a27a0f84df22e7e91ab..adaafab9c9d8b37371757720593af89acaca99e9 100644 (file)
@@ -563,7 +563,7 @@ func (v Value) call(op string, in []Value) []Value {
        }
 
        // Call.
-       call(fn, args, uint32(frametype.size), uint32(retOffset), nil)
+       call(fn, args, uint32(frametype.size), uint32(retOffset))
 
        // For testing; see TestCallMethodJump.
        if callGC {
@@ -761,7 +761,7 @@ func callMethod(ctxt *methodValue, frame unsafe.Pointer) {
        memmove(unsafe.Pointer(uintptr(args)+ptrSize), frame, argSize-ptrSize)
 
        // Call.
-       call(fn, args, uint32(frametype.size), uint32(retOffset), nil)
+       call(fn, args, uint32(frametype.size), uint32(retOffset))
 
        // Copy return values. On amd64p32, the beginning of return values
        // is 64-bit aligned, so the caller's frame layout (which doesn't have
@@ -2699,9 +2699,7 @@ func mapiterinit(t *rtype, m unsafe.Pointer) unsafe.Pointer
 func mapiterkey(it unsafe.Pointer) (key unsafe.Pointer)
 func mapiternext(it unsafe.Pointer)
 func maplen(m unsafe.Pointer) int
-
-// panicpos is for use by runtime and should be nil in all calls in this package
-func call(fn, arg unsafe.Pointer, n uint32, retoffset uint32, panicpos unsafe.Pointer)
+func call(fn, arg unsafe.Pointer, n uint32, retoffset uint32)
 
 func ifaceE2I(t *rtype, src interface{}, dst unsafe.Pointer)
 
index 0b5ded6836c1fdf8ce4ad91a5a196c1f6ed245c8..25026417bf5954b6d10a47c20ed667b08cba7475 100644 (file)
@@ -329,7 +329,7 @@ TEXT runtime·morestack_noctxt(SB),NOSPLIT,$0-0
        JMP runtime·morestack(SB)
 
 // reflect·call: call a function with the given argument list
-// func call(f *FuncVal, arg *byte, argsize, retoffset uint32, p *Panic).
+// func call(f *FuncVal, arg *byte, argsize, retoffset uint32).
 // we don't have variable-sized frames, so we use a small number
 // of constant-sized-frame functions to encode a few bits of size in the pc.
 // Caution: ugly multiline assembly macros in your future!
@@ -341,7 +341,7 @@ TEXT runtime·morestack_noctxt(SB),NOSPLIT,$0-0
        JMP     AX
 // Note: can't just "JMP NAME(SB)" - bad inlining results.
 
-TEXT reflect·call(SB), NOSPLIT, $0-20
+TEXT reflect·call(SB), NOSPLIT, $0-16
        MOVL    argsize+8(FP), CX
        DISPATCH(runtime·call16, 16)
        DISPATCH(runtime·call32, 32)
@@ -375,9 +375,8 @@ TEXT reflect·call(SB), NOSPLIT, $0-20
 
 // Argument map for the callXX frames.  Each has one stack map.
 DATA gcargs_reflectcall<>+0x00(SB)/4, $1  // 1 stackmap
-DATA gcargs_reflectcall<>+0x04(SB)/4, $10  // 5 words
+DATA gcargs_reflectcall<>+0x04(SB)/4, $8  // 4 words
 DATA gcargs_reflectcall<>+0x08(SB)/1, $(const_BitsPointer+(const_BitsPointer<<2)+(const_BitsScalar<<4)+(const_BitsScalar<<6))
-DATA gcargs_reflectcall<>+0x09(SB)/1, $(const_BitsPointer)
 GLOBL gcargs_reflectcall<>(SB),RODATA,$12
 
 // callXX frames have no locals
@@ -386,7 +385,7 @@ DATA gclocals_reflectcall<>+0x04(SB)/4, $0  // 0 locals
 GLOBL gclocals_reflectcall<>(SB),RODATA,$8
 
 #define CALLFN(NAME,MAXSIZE)                   \
-TEXT NAME(SB), WRAPPER, $MAXSIZE-20;   \
+TEXT NAME(SB), WRAPPER, $MAXSIZE-16;   \
        FUNCDATA $FUNCDATA_ArgsPointerMaps,gcargs_reflectcall<>(SB);    \
        FUNCDATA $FUNCDATA_LocalsPointerMaps,gclocals_reflectcall<>(SB);\
        /* copy arguments to stack */           \
@@ -394,22 +393,11 @@ TEXT NAME(SB), WRAPPER, $MAXSIZE-20;      \
        MOVL    argsize+8(FP), CX;              \
        MOVL    SP, DI;                         \
        REP;MOVSB;                              \
-       /* initialize panic argp */             \
-       MOVL    panic+16(FP), CX;               \
-       CMPL    CX, $0;                         \
-       JEQ     3(PC);                          \
-       LEAL    (MAXSIZE+4)(SP), BX;            \
-       MOVL    BX, panic_argp(CX);             \
        /* call function */                     \
        MOVL    f+0(FP), DX;                    \
        MOVL    (DX), AX;                       \
        PCDATA  $PCDATA_StackMapIndex, $0;      \
        CALL    AX;                             \
-       /* clear panic argp */                  \
-       MOVL    panic+16(FP), CX;               \
-       CMPL    CX, $0;                         \
-       JEQ     2(PC);                          \
-       MOVL    $0, panic_argp(CX);             \
        /* copy return values back */           \
        MOVL    argptr+4(FP), DI;               \
        MOVL    argsize+8(FP), CX;              \
index 587fcf4806938d758410dcf883993726d538d366..cc32ad8a18dcc7e811606138a695d07eee54563e 100644 (file)
@@ -308,7 +308,7 @@ TEXT runtime·morestack(SB),NOSPLIT,$0-0
        RET
 
 // reflect·call: call a function with the given argument list
-// func call(f *FuncVal, arg *byte, argsize, retoffset uint32, p *Panic).
+// func call(f *FuncVal, arg *byte, argsize, retoffset uint32).
 // we don't have variable-sized frames, so we use a small number
 // of constant-sized-frame functions to encode a few bits of size in the pc.
 // Caution: ugly multiline assembly macros in your future!
@@ -320,7 +320,7 @@ TEXT runtime·morestack(SB),NOSPLIT,$0-0
        JMP     AX
 // Note: can't just "JMP NAME(SB)" - bad inlining results.
 
-TEXT reflect·call(SB), NOSPLIT, $0-32
+TEXT reflect·call(SB), NOSPLIT, $0-24
        MOVLQZX argsize+16(FP), CX
        DISPATCH(runtime·call16, 16)
        DISPATCH(runtime·call32, 32)
@@ -354,8 +354,8 @@ TEXT reflect·call(SB), NOSPLIT, $0-32
 
 // Argument map for the callXX frames.  Each has one stack map.
 DATA gcargs_reflectcall<>+0x00(SB)/4, $1  // 1 stackmap
-DATA gcargs_reflectcall<>+0x04(SB)/4, $8  // 4 words
-DATA gcargs_reflectcall<>+0x08(SB)/1, $(const_BitsPointer+(const_BitsPointer<<2)+(const_BitsScalar<<4)+(const_BitsPointer<<6))
+DATA gcargs_reflectcall<>+0x04(SB)/4, $6  // 3 words
+DATA gcargs_reflectcall<>+0x08(SB)/1, $(const_BitsPointer+(const_BitsPointer<<2)+(const_BitsScalar<<4))
 GLOBL gcargs_reflectcall<>(SB),RODATA,$12
 
 // callXX frames have no locals
@@ -363,16 +363,8 @@ DATA gclocals_reflectcall<>+0x00(SB)/4, $1  // 1 stackmap
 DATA gclocals_reflectcall<>+0x04(SB)/4, $0  // 0 locals
 GLOBL gclocals_reflectcall<>(SB),RODATA,$8
 
-// CALLFN is marked as a WRAPPER so that a deferred reflect.call func will
-// see the right answer for recover. However, CALLFN is also how we start
-// the panic in the first place. We record the panic argp if this is the start of
-// a panic. Since the wrapper adjustment has already happened, though
-// (in the implicit prologue), we have to write not SP but MAXSIZE+8+SP into
-// p.argp. The MAXSIZE+8 will counter the MAXSIZE+8 the wrapper prologue
-// added to g->panicwrap.
-
 #define CALLFN(NAME,MAXSIZE)                   \
-TEXT NAME(SB), WRAPPER, $MAXSIZE-32;           \
+TEXT NAME(SB), WRAPPER, $MAXSIZE-24;           \
        FUNCDATA $FUNCDATA_ArgsPointerMaps,gcargs_reflectcall<>(SB);    \
        FUNCDATA $FUNCDATA_LocalsPointerMaps,gclocals_reflectcall<>(SB);\
        /* copy arguments to stack */           \
@@ -380,21 +372,10 @@ TEXT NAME(SB), WRAPPER, $MAXSIZE-32;              \
        MOVLQZX argsize+16(FP), CX;             \
        MOVQ    SP, DI;                         \
        REP;MOVSB;                              \
-       /* initialize panic argp */             \
-       MOVQ    panic+24(FP), CX;               \
-       CMPQ    CX, $0;                         \
-       JEQ     3(PC);                          \
-       LEAQ    (MAXSIZE+8)(SP), BX;            \
-       MOVQ    BX, panic_argp(CX);             \
        /* call function */                     \
        MOVQ    f+0(FP), DX;                    \
        PCDATA  $PCDATA_StackMapIndex, $0;      \
        CALL    (DX);                           \
-       /* clear panic argp */                  \
-       MOVQ    panic+24(FP), CX;               \
-       CMPQ    CX, $0;                         \
-       JEQ     2(PC);                          \
-       MOVQ    $0, panic_argp(CX);             \
        /* copy return values back */           \
        MOVQ    argptr+8(FP), DI;               \
        MOVLQZX argsize+16(FP), CX;             \
index 5647d77627e644de8287c8a489f7fbb0c654869b..2597b8250f0e473b9fa3a27614daf39b3a197024 100644 (file)
@@ -349,9 +349,8 @@ TEXT NAME(SB), WRAPPER, $MAXSIZE-20;                \
        /* initialize panic argp */             \
        MOVL    panic+16(FP), CX;               \
        CMPL    CX, $0;                         \
-       JEQ     3(PC);                          \
-       LEAL    (MAXSIZE+8)(SP), BX;            \
-       MOVL    BX, panic_argp(CX);             \
+       JEQ     2(PC);                          \
+       MOVL    SP, panic_argp(CX);             \
        /* call function */                     \
        MOVL    f+0(FP), DX;                    \
        MOVL    (DX), AX;                               \
index 96a21ceac14f3e09d4700fd100e943a8a41209fd..752ea08e5759513a6a95bafb195fd81de49dea7d 100644 (file)
@@ -306,7 +306,7 @@ TEXT runtime·morestack_noctxt(SB),NOSPLIT,$-4-0
        B runtime·morestack(SB)
 
 // reflect·call: call a function with the given argument list
-// func call(f *FuncVal, arg *byte, argsize, retoffset uint32, p *Panic).
+// func call(f *FuncVal, arg *byte, argsize, retoffset uint32).
 // we don't have variable-sized frames, so we use a small number
 // of constant-sized-frame functions to encode a few bits of size in the pc.
 // Caution: ugly multiline assembly macros in your future!
@@ -317,7 +317,7 @@ TEXT runtime·morestack_noctxt(SB),NOSPLIT,$-4-0
        MOVW    $NAME(SB), R1;          \
        B       (R1)
 
-TEXT reflect·call(SB),NOSPLIT,$-4-20
+TEXT reflect·call(SB),NOSPLIT,$-4-16
        MOVW    argsize+8(FP), R0
        DISPATCH(runtime·call16, 16)
        DISPATCH(runtime·call32, 32)
@@ -351,9 +351,8 @@ TEXT reflect·call(SB),NOSPLIT,$-4-20
 
 // Argument map for the callXX frames.  Each has one stack map.
 DATA gcargs_reflectcall<>+0x00(SB)/4, $1  // 1 stackmap
-DATA gcargs_reflectcall<>+0x04(SB)/4, $10  // 5 words
+DATA gcargs_reflectcall<>+0x04(SB)/4, $8  // 4 words
 DATA gcargs_reflectcall<>+0x08(SB)/1, $(const_BitsPointer+(const_BitsPointer<<2)+(const_BitsScalar<<4)+(const_BitsScalar<<6))
-DATA gcargs_reflectcall<>+0x09(SB)/1, $(const_BitsPointer)
 GLOBL gcargs_reflectcall<>(SB),RODATA,$12
 
 // callXX frames have no locals
@@ -362,7 +361,7 @@ DATA gclocals_reflectcall<>+0x04(SB)/4, $0  // 0 locals
 GLOBL gclocals_reflectcall<>(SB),RODATA,$8
 
 #define CALLFN(NAME,MAXSIZE)                   \
-TEXT NAME(SB), WRAPPER, $MAXSIZE-20;           \
+TEXT NAME(SB), WRAPPER, $MAXSIZE-16;           \
        FUNCDATA $FUNCDATA_ArgsPointerMaps,gcargs_reflectcall<>(SB);    \
        FUNCDATA $FUNCDATA_LocalsPointerMaps,gclocals_reflectcall<>(SB);\
        /* copy arguments to stack */           \
@@ -375,23 +374,11 @@ TEXT NAME(SB), WRAPPER, $MAXSIZE-20;              \
        MOVBU.P R5, 1(R1);                      \
        SUB     $1, R2, R2;                     \
        B       -5(PC);                         \
-       /* initialize panic argp */             \
-       MOVW    panic+16(FP), R4;               \
-       CMP     $0, R4;                         \
-       B.EQ    3(PC);                          \
-       ADD     $(4+MAXSIZE+4), R13, R5;        \
-       MOVW    R5, panic_argp(R4);             \
        /* call function */                     \
        MOVW    f+0(FP), R7;                    \
        MOVW    (R7), R0;                       \
        PCDATA  $PCDATA_StackMapIndex, $0;      \
        BL      (R0);                           \
-       /* clear panic argp */                  \
-       MOVW    panic+16(FP), R4;               \
-       CMP     $0, R4;                         \
-       B.EQ    3(PC);                          \
-       MOVW    $0, R5;                         \
-       MOVW    R5, panic_argp(R4);             \
        /* copy return values back */           \
        MOVW    argptr+4(FP), R0;               \
        MOVW    argsize+8(FP), R2;              \
index 4040fee8e632df2f596350f36aca29a4c6c533fd..c00694e6695386cfc2ba05df367e69d438238229 100644 (file)
@@ -225,7 +225,7 @@ func cgocallbackg1() {
        }
 
        // Invoke callback.
-       reflectcall(unsafe.Pointer(cb.fn), unsafe.Pointer(cb.arg), uint32(cb.argsize), 0, nil)
+       reflectcall(unsafe.Pointer(cb.fn), unsafe.Pointer(cb.arg), uint32(cb.argsize), 0)
 
        if raceenabled {
                racereleasemerge(unsafe.Pointer(&racecgosync))
index 664b03b151e9451f05adfbae1f1289b7f4557fc4..883ca0cef79dbeba2e30dd089bc2850bfedaa484 100644 (file)
@@ -743,7 +743,7 @@ func runfinq() {
                                default:
                                        gothrow("bad kind in runfinq")
                                }
-                               reflectcall(unsafe.Pointer(f.fn), frame, uint32(framesz), uint32(framesz), nil)
+                               reflectcall(unsafe.Pointer(f.fn), frame, uint32(framesz), uint32(framesz))
 
                                // drop finalizer queue references to finalized object
                                f.fn = nil
index b8fa213f6690f5110944fd135eba7548b8e8b686..1e35561d1541af81e92c88958b699db17e3c0a55 100644 (file)
@@ -208,7 +208,7 @@ func Goexit() {
        for gp._defer != nil {
                d := gp._defer
                gp._defer = d.link
-               reflectcall(unsafe.Pointer(d.fn), unsafe.Pointer(&d.args), uint32(d.siz), uint32(d.siz), nil)
+               reflectcall(unsafe.Pointer(d.fn), unsafe.Pointer(&d.args), uint32(d.siz), uint32(d.siz))
                freedefer(d)
                // Note: we ignore recovers here because Goexit isn't a panic
        }
index 7bdfb4b2c04117e1d24c7e7f5b221d91b14e89cf..1f2f54ec20ce19b35cd154e77f9fee5b466dee9c 100644 (file)
@@ -46,25 +46,19 @@ func gopanic(e interface{}) {
                }
                // take defer off list in case of recursive panic
                gp._defer = d.link
-               gp.ispanic = true              // rock for runtime·newstack, where runtime·newstackcall ends up
                argp := unsafe.Pointer(d.argp) // must be pointer so it gets adjusted during stack copy
                pc := d.pc
 
                // The deferred function may cause another panic,
-               // so newstackcall may not return. Set up a defer
+               // so reflectcall may not return. Set up a defer
                // to mark this panic aborted if that happens.
                dabort.link = gp._defer
                gp._defer = (*_defer)(noescape(unsafe.Pointer(&dabort)))
                p._defer = d
-               p.outerwrap = gp.panicwrap
 
-               // TODO(rsc): I am pretty sure the panicwrap manipulation here is not correct.
-               // It is close enough to pass all the tests we have, but I think it needs to be
-               // restored during recovery too. I will write better tests and fix it in a separate CL.
-
-               gp.panicwrap = 0
-               reflectcall(unsafe.Pointer(d.fn), unsafe.Pointer(&d.args), uint32(d.siz), uint32(d.siz), (*_panic)(noescape(unsafe.Pointer(&p))))
-               gp.panicwrap = p.outerwrap
+               p.argp = getargp(0)
+               reflectcall(unsafe.Pointer(d.fn), unsafe.Pointer(&d.args), uint32(d.siz), uint32(d.siz))
+               p.argp = 0
 
                // reflectcall did not panic. Remove dabort.
                if gp._defer != &dabort {
@@ -102,6 +96,21 @@ func gopanic(e interface{}) {
        *(*int)(nil) = 0 // not reached
 }
 
+// getargp returns the location where the caller
+// writes outgoing function call arguments.
+//go:nosplit
+func getargp(x int) uintptr {
+       // x is an argument mainly so that we can return its address.
+       // However, we need to make the function complex enough
+       // that it won't be inlined. We always pass x = 0, so this code
+       // does nothing other than keep the compiler from thinking
+       // the function is simple enough to inline.
+       if x > 0 {
+               return getcallersp(unsafe.Pointer(&x)) * 0
+       }
+       return uintptr(noescape(unsafe.Pointer(&x)))
+}
+
 func abortpanic(p *_panic) {
        p.aborted = true
 }
@@ -109,23 +118,20 @@ func abortpanic(p *_panic) {
 // The implementation of the predeclared function recover.
 // Cannot split the stack because it needs to reliably
 // find the stack segment of its caller.
+//
+// TODO(rsc): Once we commit to CopyStackAlways,
+// this doesn't need to be nosplit.
 //go:nosplit
 func gorecover(argp uintptr) interface{} {
-       // Must be an unrecovered panic in progress.
-       // Must be on a stack segment created for a deferred call during a panic.
-       // Must be at the top of that segment, meaning the deferred call itself
-       // and not something it called. The top frame in the segment will have
-       // argument pointer argp == top - top.argsize.
-       // The subtraction of g.panicwrap allows wrapper functions that
-       // do not count as official calls to adjust what we consider the top frame
-       // while they are active on the stack. The linker emits adjustments of
-       // g.panicwrap in the prologue and epilogue of functions marked as wrappers.
+       // Must be in a function running as part of a deferred call during the panic.
+       // Must be called from the topmost function of the call
+       // (the function used in the defer statement).
+       // p.argp is the argument pointer of that topmost deferred function call.
+       // Compare against argp reported by caller.
+       // If they match, the caller is the one who can recover.
        gp := getg()
        p := gp._panic
-       //      if p != nil {
-       //              println("recover?", p, p.recovered, hex(argp), hex(p.argp), uintptr(gp.panicwrap), p != nil && !p.recovered && argp == p.argp-uintptr(gp.panicwrap))
-       //      }
-       if p != nil && !p.recovered && argp == p.argp-uintptr(gp.panicwrap) {
+       if p != nil && !p.recovered && argp == p.argp {
                p.recovered = true
                return p.arg
        }
index 414196ceb0d07777dfb4f3119c7159dbf1502010..698be9ffaeb6355d020efb7770b98d97a108da61 100644 (file)
@@ -2327,7 +2327,6 @@ runtime·newproc1(FuncVal *fn, byte *argp, int32 narg, int32 nret, void *callerp
                p->goidcacheend = p->goidcache + GoidCacheBatch;
        }
        newg->goid = p->goidcache++;
-       newg->panicwrap = 0;
        if(raceenabled)
                newg->racectx = runtime·racegostart((void*)callerpc);
        runqput(p, newg);
index 52796f6fe4a8b3bf2bb400cb112e950a44f0ca71..02563fd36cceb0e49af163fbd9a39e261106916f 100644 (file)
@@ -268,11 +268,10 @@ struct    WinCallbackContext
 struct G
 {
        // stackguard0 can be set to StackPreempt as opposed to stackguard
-       uintptr stackguard0;    // cannot move - also known to linker, libmach, runtime/cgo
+       uintptr stackguard0;    // cannot move - also known to liblink, libmach, runtime/cgo
        uintptr stackbase;      // cannot move - also known to libmach, runtime/cgo
-       uint32  panicwrap;      // cannot move - also known to linker
+       Panic*  panic;  // cannot move - also known to liblink
        Defer*  defer;
-       Panic*  panic;
        Gobuf   sched;
        uintptr syscallstack;   // if status==Gsyscall, syscallstack = stackbase to use during gc
        uintptr syscallsp;      // if status==Gsyscall, syscallsp = sched.sp to use during gc
@@ -287,7 +286,6 @@ struct      G
        int64   waitsince;      // approx time when the G become blocked
        String  waitreason;     // if status==Gwaiting
        G*      schedlink;
-       bool    ispanic;
        bool    issystem;       // do not output in stack dump, ignore in deadlock detector
        bool    preempt;        // preemption signal, duplicates stackguard0 = StackPreempt
        bool    paniconfault;   // panic (instead of crash) on unexpected fault address
@@ -449,7 +447,6 @@ struct      Stktop
        uintptr stackbase;
        Gobuf   gobuf;
        uint32  argsize;
-       uint32  panicwrap;
 
        uint8*  argp;   // pointer to arguments in old frame
 };
@@ -654,11 +651,10 @@ struct Defer
  */
 struct Panic
 {
+       uintptr argp;   // pointer to arguments of deferred call run during panic; cannot move - known to liblink
        Eface   arg;            // argument to panic
        Panic*  link;           // link to earlier panic
        Defer*  defer;          // current executing defer
-       uintptr argp;           // pointer to arguments of deferred call, for recover
-       uint32  outerwrap;      // outer gp->panicwrap
        bool    recovered;      // whether this panic is over
        bool    aborted;        // the panic was aborted
 };
index 20a37046f932ce9a26614e27dc5d042b3f48e719..18b3f40648806ed34b63c90235a2443859b89fa2 100644 (file)
@@ -371,7 +371,6 @@ runtime·oldstack(void)
        gp->stackbase = top->stackbase;
        gp->stackguard = top->stackguard;
        gp->stackguard0 = gp->stackguard;
-       gp->panicwrap = top->panicwrap;
        runtime·stackfree(gp, old, top);
        runtime·casgstatus(gp, Gcopystack, oldstatus); // oldstatus is Grunning or Gsyscall
        runtime·gogo(&gp->sched);
@@ -1033,9 +1032,6 @@ runtime·newstack(void)
        top->argp = moreargp;
        top->argsize = argsize;
 
-       top->panicwrap = gp->panicwrap;
-       gp->panicwrap = 0;
-
        gp->stackbase = (uintptr)top;
        gp->stackguard = (uintptr)stk + StackGuard;
        gp->stackguard0 = gp->stackguard;
index c97831a7c4c6c633691854a049c1b895e4527ae2..03f618e155b78e12349a85aeb66aa8ec5a26ee9b 100644 (file)
@@ -171,7 +171,7 @@ func cputicks() int64
 func mmap(addr unsafe.Pointer, n uintptr, prot, flags, fd int32, off uint32) unsafe.Pointer
 func munmap(addr unsafe.Pointer, n uintptr)
 func madvise(addr unsafe.Pointer, n uintptr, flags int32)
-func reflectcall(fn, arg unsafe.Pointer, n uint32, retoffset uint32, p *_panic)
+func reflectcall(fn, arg unsafe.Pointer, n uint32, retoffset uint32)
 func osyield()
 func procyield(cycles uint32)
 func cgocallback_gofunc(fv *funcval, frame unsafe.Pointer, framesize uintptr)
index 071be6667acff1c47931d2c8dcc6c489576e1d7c..6287d65076d333eb7157d5c148040986e75a38f4 100644 (file)
@@ -47,6 +47,7 @@ func main() {
                test11reflect1()
                test11reflect2()
        }
+       test111()
        test12()
        if !interp {
                test12reflect1()
@@ -77,7 +78,7 @@ func mustRecoverBody(v1, v2, v3, x interface{}) {
        }
        v = v2
        if v == nil {
-               println("missing recover")
+               println("missing recover", x.(int))
                die() // panic is useless here
        }
        if v != x {
@@ -137,7 +138,7 @@ func test1WithClosures() {
                mustNotRecover()
                v := recover()
                if v == nil {
-                       println("missing recover")
+                       println("missing recover", x.(int))
                        die()
                }
                if v != x {
@@ -406,6 +407,49 @@ func test11reflect2() {
        panic(11)
 }
 
+// tiny receiver, so basic wrapper in i.M()
+type T3deeper struct{}
+
+func (T3deeper) M() {
+       badstate() // difference from T3
+       mustRecoverBody(doubleRecover(), recover(), recover(), 111)
+}
+
+func test111() {
+       var i I = T3deeper{}
+       defer i.M()
+       panic(111)
+}
+
+type Tiny struct{}
+
+func (Tiny) M() {
+       panic(112)
+}
+
+// i.M is a wrapper, and i.M panics.
+//
+// This is a torture test for an old implementation of recover that
+// tried to deal with wrapper functions by doing some argument
+// positioning math on both entry and exit. Doing anything on exit
+// is a problem because sometimes functions exit via panic instead
+// of an ordinary return, so panic would have to know to do the
+// same math when unwinding the stack. It gets complicated fast.
+// This particular test never worked with the old scheme, because
+// panic never did the right unwinding math.
+//
+// The new scheme adjusts Panic.argp on entry to a wrapper.
+// It has no exit work, so if a wrapper is interrupted by a panic,
+// there's no cleanup that panic itself must do.
+// This test just works now.
+func badstate() {
+       defer func() {
+               recover()
+       }()
+       var i I = Tiny{}
+       i.M()
+}
+
 // large receiver, so basic wrapper in i.M()
 type T4 [2]string