pp_entersub: skip resetting @_

Whenever we leave a sub by whatever means (pp_leavesub, pp_return,
pp_goto, die etc) it is the responsibility of the code that pops
the SUB context to clean up the private @_ of that sub (i.e pad[0])
so that it's empty and not real.

There's some code in pp_entersub that duplicates this check. I believe
this check is unnecessary and so this commit removes it and replaces it
with an assert. It was added by 221373f04 in 1999 to fix the issue
described in

    Subject: [ID 19991015.010] [BUG 5.005_62 Assertation failed:
            "pp_ctl.c" line 2443]
    Message-Id: <19991016024500.A32541@athens.aocn.com>

There were two fixes applied for this issue; the other was
0253cb4. I think the second commit actually fixed the issue and the
first fix was a red herring.

If my newly-added assert fails, it implies that something needs fixing in
the context-popping code, rather than in pp_entersub.

In fact the new assert showed up one issue: the special-case handling
of @_ for &CORE::undef in pp_coreargs wasn't emptying the array,
so I added a CLEAR_ARGARRAY().

diff --git a/pp.c b/pp.c
index 30a2dd0..8900706 100644 (file)
--- a/pp.c
+++ b/pp.c
@@ -6440,15 +6440,19 @@ PP(pp_coreargs)
            if (opnum == OP_UNDEF && SvRV(*svp) == (SV *)PL_defgv
             && cxstack[cxstack_ix].cx_type & CXp_HASARGS) {
                /* Undo @_ localisation, so that sub exit does not undo
-                  part of our undeffing. */
+                  part of our undeffing.
+                   this corresponds to the part of POPSUB that
+                   does @_ cleanup */
                PERL_CONTEXT *cx = &cxstack[cxstack_ix];
+                AV *av = MUTABLE_AV(PAD_SVl(0));
                POP_SAVEARRAY();
                cx->cx_type &= ~ CXp_HASARGS;
                 assert(AvARRAY(MUTABLE_AV(
                     PadlistARRAY(CvPADLIST(cx->blk_sub.cv))[
                             CvDEPTH(cx->blk_sub.cv)])) == PL_curpad);
 
-               assert(!AvREAL(MUTABLE_AV(PAD_SVl(0))));
+               assert(!AvREAL(av));
+                CLEAR_ARGARRAY(av);
            }
          }
          break;

diff --git a/pp_hot.c b/pp_hot.c
index edd4cf7..b2922b1 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -3480,18 +3480,16 @@ PP(pp_entersub)
             SSize_t items;
             AV **defavp;
 
-           if (UNLIKELY(AvREAL(av))) {
-               /* @_ is normally not REAL--this should only ever
-                * happen when DB::sub() calls things that modify @_ */
-               av_clear(av);
-               AvREAL_off(av);
-               AvREIFY_on(av);
-           }
            defavp = &GvAV(PL_defgv);
            cx->blk_sub.savearray = *defavp;
            *defavp = MUTABLE_AV(SvREFCNT_inc_simple_NN(av));
-            items = SP - MARK;
 
+            /* it's the responsibility of whoever leaves a sub to ensure
+             * that a clean, empty AV is left in pad[0]. This is normally
+             * done by POPSUB() */
+            assert(!AvREAL(av) && AvFILLp(av) == -1);
+
+            items = SP - MARK;
            if (UNLIKELY(items - 1 > AvMAX(av))) {
                 SV **ary = AvALLOC(av);
                 AvMAX(av) = items - 1;