[perl #30061] double DESTROY in for loop

pp_iter decremented the ref count of the previous iterant before
unaliasing it. This could lead to DESTROY being called with the
loop variable still aliased to the freed value. If the DESTROY
also contained a for loop with the same iterator variable, the
freed value would get resurrected then freed for a second time.

p4raw-id: //depot/perl@22913

diff --git a/pp_hot.c b/pp_hot.c
index 3a89b6f..b7aad81 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -1824,7 +1824,7 @@ PP(pp_iter)
 {
     dSP;
     register PERL_CONTEXT *cx;
-    SV* sv;
+    SV *sv, *oldsv;
     AV* av;
     SV **itersvp;
 
@@ -1852,8 +1852,9 @@ PP(pp_iter)
                    /* we need a fresh SV every time so that loop body sees a
                     * completely new SV for closures/references to work as
                     * they used to */
-                   SvREFCNT_dec(*itersvp);
+                   oldsv = *itersvp;
                    *itersvp = newSVsv(cur);
+                   SvREFCNT_dec(oldsv);
                }
                if (strEQ(SvPVX(cur), max))
                    sv_setiv(cur, 0); /* terminate next time */
@@ -1877,8 +1878,9 @@ PP(pp_iter)
            /* we need a fresh SV every time so that loop body sees a
             * completely new SV for closures/references to work as they
             * used to */
-           SvREFCNT_dec(*itersvp);
+           oldsv = *itersvp;
            *itersvp = newSViv(cx->blk_loop.iterix++);
+           SvREFCNT_dec(oldsv);
        }
        RETPUSHYES;
     }
@@ -1887,8 +1889,6 @@ PP(pp_iter)
     if (cx->blk_loop.iterix >= (av == PL_curstack ? cx->blk_oldsp : AvFILL(av)))
        RETPUSHNO;
 
-    SvREFCNT_dec(*itersvp);
-
     if (SvMAGICAL(av) || AvREIFY(av)) {
        SV **svp = av_fetch(av, ++cx->blk_loop.iterix, FALSE);
        if (svp)
@@ -1928,7 +1928,10 @@ PP(pp_iter)
        sv = (SV*)lv;
     }
 
+    oldsv = *itersvp;
     *itersvp = SvREFCNT_inc(sv);
+    SvREFCNT_dec(oldsv);
+
     RETPUSHYES;
 }
 

diff --git a/t/cmd/for.t b/t/cmd/for.t
index 3a4bc9b..27fb5a2 100755 (executable)
--- a/t/cmd/for.t
+++ b/t/cmd/for.t
@@ -1,6 +1,6 @@
 #!./perl
 
-print "1..13\n";
+print "1..14\n";
 
 for ($i = 0; $i <= 10; $i++) {
     $x[$i] = $i;
@@ -76,3 +76,22 @@ print $loop_count == 4 ? "ok" : "not ok", " 12\n";
 @a = (3,4);
 eval { @a = () for (1,2,@a) };
 print $@ =~ /Use of freed value in iteration/ ? "ok" : "not ok", " 13\n";
+
+# [perl #30061] double destory when same iterator variable (eg $_) used in
+# DESTROY as used in for loop that triggered the destroy
+
+{
+
+    my $x = 0;
+    sub X::DESTROY {
+       my $o = shift;
+       $x++;
+       1 for (1);
+    }
+
+    my %h;
+    $h{foo} = bless [], 'X';
+    delete $h{foo} for $h{foo}, 1;
+    print $x == 1 ? "ok" : "not ok", " 14 - double destroy, x=$x\n";
+}
+