This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
pp_goto: skip restoring PL_comppad
authorDavid Mitchell <davem@iabyn.com>
Wed, 1 Jul 2015 06:56:39 +0000 (07:56 +0100)
committerDavid Mitchell <davem@iabyn.com>
Wed, 3 Feb 2016 08:59:34 +0000 (08:59 +0000)
Now that PL_comppad is saved in the context struct rather than on the save
stack, we can better control when and how it is restored. In the case of
pp_goto, there's no need to restore the caller's PL_comppad in the non-XS
case, since we will be immediately setting it to the new function's pad.

AS well as being slightly more efficient, this avoids restoring PL_comppad
too early, where if we subsequently croak we re-process the CXt_SUB block
in dounwind, but this time with the wrong pad.

In particular, by having always PL_curpad[0] == cx.argarray at sub exit
time, we can shortly eliminate the argarray field.

pp_ctl.c

index e05a99b..96b6e9f 100644 (file)
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -2745,8 +2745,12 @@ PP(pp_goto)
            assert(PL_scopestack_ix == cx->blk_oldscopesp);
            oldsave = PL_scopestack[cx->blk_oldscopesp - 1];
            LEAVE_SCOPE(oldsave);
-            PL_comppad = cx->blk_sub.prevcomppad;
-            PL_curpad = LIKELY(PL_comppad) ? AvARRAY(PL_comppad) : NULL;
+
+            /* don't restore PL_comppad here. It won't be needed if the
+             * sub we're going to is non-XS, but restoring it early then
+             * croaking (e.g. the "Goto undefined subroutine" below)
+             * means the CX block gets processed again in dounwind,
+             * but this time with the wrong PL_comppad */
 
            /* A destructor called during LEAVE_SCOPE could have undefined
             * our precious cv.  See bug #99850. */
@@ -2811,6 +2815,8 @@ PP(pp_goto)
                }
 
                retop = cx->blk_sub.retop;
+                PL_comppad = cx->blk_sub.prevcomppad;
+                PL_curpad = LIKELY(PL_comppad) ? AvARRAY(PL_comppad) : NULL;
                /* XS subs don't have a CxSUB, so pop it */
                POPBLOCK(cx, PL_curpm);
                /* Push a mark for the start of arglist */
@@ -2836,7 +2842,6 @@ PP(pp_goto)
                    pad_push(padlist, CvDEPTH(cv));
                }
                PL_curcop = cx->blk_oldcop;
-                cx->blk_sub.prevcomppad = PL_comppad;
                PAD_SET_CUR_NOSAVE(padlist, CvDEPTH(cv));
                if (CxHASARGS(cx))
                {