This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
[perl #30061] double DESTROY in for loop
authorDave Mitchell <davem@fdisolutions.com>
Tue, 8 Jun 2004 22:20:40 +0000 (22:20 +0000)
committerDave Mitchell <davem@fdisolutions.com>
Tue, 8 Jun 2004 22:20:40 +0000 (22:20 +0000)
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

pp_hot.c
t/cmd/for.t

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;
-    SVsv;
+    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;
 }
 
index 3a4bc9b..27fb5a2 100755 (executable)
@@ -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";
+}
+