Don’t leak subs containing syntax errors
authorFather Chrysostomos <sprout@cpan.org>
Tue, 13 Nov 2012 07:04:16 +0000 (23:04 -0800)
committerFather Chrysostomos <sprout@cpan.org>
Tue, 13 Nov 2012 08:01:37 +0000 (00:01 -0800)
I fixed this for BEGIN blocks earlier, but missed the fact that
all subs are affected.

When called without an o argument (from newANONATTRSUB), newATTRSUB
is expected to return a CV with an unowned reference count of which
the caller will take ownership.  We cannot have newATTRSUB returning
a freed CV, so we have it return null instead.  But that means
ck_anoncode and pm_runtime have to account for that.

op.c
t/op/svleak.t

diff --git a/op.c b/op.c
index 3f71cff..b2801c7 100644 (file)
--- a/op.c
+++ b/op.c
@@ -4650,7 +4650,7 @@ Perl_pmruntime(pTHX_ OP *o, OP *expr, bool isreg, I32 floor)
 
                /* attach the anon CV to the pad so that
                 * pad_fixup_inner_anons() can find it */
-               (void)pad_add_anon(cv, o->op_type);
+               if (cv) (void)pad_add_anon(cv, o->op_type);
                SvREFCNT_inc_simple_void(cv);
            }
            else {
@@ -7370,15 +7370,14 @@ Perl_newATTRSUB_flags(pTHX_ I32 floor, OP *o, OP *proto, OP *attrs,
 
     if (ec) {
        op_free(block);
-       cv = PL_compcv;
+       SvREFCNT_dec(PL_compcv);
+       PL_compcv = 0;
        if (name && block) {
            const char *s = strrchr(name, ':');
            s = s ? s+1 : name;
            if (strEQ(s, "BEGIN")) {
                const char not_safe[] =
                    "BEGIN not safe after errors--compilation aborted";
-               PL_compcv = 0;
-               SvREFCNT_dec(cv);
                if (PL_in_eval & EVAL_KEEPERR)
                    Perl_croak(aTHX_ not_safe);
                else {
@@ -8176,6 +8175,9 @@ Perl_ck_anoncode(pTHX_ OP *o)
 {
     PERL_ARGS_ASSERT_CK_ANONCODE;
 
+    /* After errors, we won’t have any sub. */
+    if (!cSVOPo->op_sv) return o;
+
     cSVOPo->op_targ = pad_add_anon((CV*)cSVOPo->op_sv, o->op_type);
     if (!PL_madskills)
        cSVOPo->op_sv = NULL;
index 8134b56..8416656 100644 (file)
@@ -15,7 +15,7 @@ BEGIN {
 
 use Config;
 
-plan tests => 64;
+plan tests => 65;
 
 # run some code N times. If the number of SVs at the end of loop N is
 # greater than (N-1)*delta at the end of loop 1, we've got a leak
@@ -195,6 +195,7 @@ SKIP: {
 }
 
 eleak(2, 0, '+sub:a{}', 'anon subs with invalid attributes');
+eleak(2, 0, 'no warnings; sub a{1 1}', 'sub with syntax error');
 
 # Syntax errors
 eleak(2, 0, '"${<<END}"