+++ This bug was initially created as a clone of Bug #625746 +++ --- Additional comment from tao on 2010-08-20 10:36:51 GMT --- Event posted on 2010-08-19 16:33 BST by spoyarek Problem Description: Perl gives the error "Attempt to free unreferenced scalar" with nested foreach loops using the same variable. How Reproducible: Always Steps to Reproduce: 1) Execute the following perl script: #! /usr/bin/perl use strict; my $element; foreach $element ("a", "b") { foreach $element ("c", "d") {1;} } Actual Result: Attempt to free unreferenced scalar: SV 0x986b90c, Perl interpreter: 0x986a008. Attempt to free unreferenced scalar: SV 0x986b918, Perl interpreter: 0x986a008. ---------- The same problem is in perl-5.10.0-91.fc12.x86_64. perl-5.10.1-118.fc13.x86_64 is not affected. So wee need to bisect 5.10.0..5.10.1.
Minimal test case as proposed in CPAN bug #70660: my $_; for (0) { for (0) {} }
This commit as first fixed tree has been found by bisecting git repository: commit b99874c7926b2f5919256cf4bcee8eb7bf3dff22 Author: Nicholas Clark <nick> Date: Sat Jan 26 15:14:25 2008 +0000 In POPLOOP, if CxITERVAR(cx) is non-NULL, then so is itersave, and itersave is a less complex expression for the C compiler. p4raw-id: //depot/perl@33074 diff --git a/cop.h b/cop.h index b9ca6c3..48f9d5b 100644 --- a/cop.h +++ b/cop.h @@ -514,15 +514,12 @@ struct block_loop { SvREFCNT_dec(cx->blk_loop.state_u.lazysv.cur); \ SvREFCNT_dec(cx->blk_loop.state_u.lazysv.end); \ } \ - if (CxITERVAR(cx)) { \ + if (cx->blk_loop.itersave) { \ if (SvPADMY(cx->blk_loop.itersave)) { \ SV ** const s_v_p = CxITERVAR(cx); \ sv_2mortal(*s_v_p); \ *s_v_p = cx->blk_loop.itersave; \ } \ - else { \ - SvREFCNT_dec(cx->blk_loop.itersave); \ - } \ } \ if (CxTYPE(cx) == CXt_LOOP_FOR) \ SvREFCNT_dec(cx->blk_loop.state_u.ary.ary);
The second change of the patch was just an accident. Correct patch is: commit c84a25c741881ec8b62eb5c2c67b2bd4fe4c977c Author: Nicholas Clark <nick> Date: Thu Oct 16 18:08:52 2008 +0000 Integrate: [ 33074] In POPLOOP, if CxITERVAR(cx) is non-NULL, then so is itersave, and itersave is a less complex expression for the C compiler. [ 33075] Restore the else block accidently eaten by change 33074. p4raw-link: @33075 on //depot/perl: 9a98d8a1ee5dd7f1caa95f2ec5ab0341e0b4f8d1 p4raw-link: @33074 on //depot/perl: b99874c7926b2f5919256cf4bcee8eb7bf3dff22 p4raw-id: //depot/maint-5.10/perl@34492 p4raw-integrated: from //depot/perl@33075 'edit in' cop.h (@33074..) diff --git a/cop.h b/cop.h index 10f8a04..572d9ec 100644 --- a/cop.h +++ b/cop.h @@ -501,7 +501,7 @@ struct block_loop { #define POPLOOP(cx) \ SvREFCNT_dec(cx->blk_loop.iterlval); \ - if (CxITERVAR(cx)) { \ + if (cx->blk_loop.itersave) { \ if (SvPADMY(cx->blk_loop.itersave)) { \ SV ** const s_v_p = CxITERVAR(cx); \ sv_2mortal(*s_v_p); \ However it does not fix it. (Bad bisect test?)
Git log: * Here it works. commit e846cb9248fdbc78eb6a883f17c20c3785e7f2de Author: Nicholas Clark <nick> Date: Sat Jan 26 21:55:51 2008 +0000 The layout for struct block_loop under ithreads can be simplified. Instead of wedging the pad offset into a void* iterdata, and always storing PL_comppad even when it isn't used, instead do this: PAD *oldcomppad; /* Also used for the GV, if targoffset is 0 */ /* This is also accesible via cx->blk_loop.my_op->op_targ */ PADOFFSET targoffset; and store the GV pointer in oldcompad. Pointers to pointers seems cleaner. This also allows us to eliminate the flag bit CXp_PADVAR. p4raw-id: //depot/perl@33081 commit 09edbca0f5c7caf9dd4acef80d8e6275e5a95ea1 Author: Nicholas Clark <nick> Date: Sat Jan 26 17:54:29 2008 +0000 Investigation reveals that the work of restoring the iterator to the pad is shared between POPLOOP, using itersave, and the end of scope restore action requested by Perl_save_padsv(). In fact, the only user of SAVEt_PADSV is pp_enteriter, and it already provides enough information to allow it to perform the sv_2mortal() in POPLOOP. So make it do so. Rather than creating a new routine, use the existing routine because nothing else (at least nothing else known to Google's codesearch) uses it. But rename it just in case something we can't see is being naughty and using our private functions - they will get link errors against 5.12. All this means that itersave is now redundant. So remove it. This makes struct context 48 bytes on ILP32 platforms with 32bit IVs, down from 64 bytes in 5.10. 33% more context stack in the same memory. p4raw-id: //depot/perl@33080 commit dceb5f6256f15f796b36d673cf34f57b7feb1127 Author: Nicholas Clark <nick> Date: Sat Jan 26 17:31:34 2008 +0000 Change 33072 missed embed.h. I wasn't aware that it was affected by opcode.pl. You live and learn (and should run p4 diff -se ...). p4raw-id: //depot/perl@33079 commit f32d39a24d025c50d0bd64a7aabe80c983be93ca Author: Nicholas Clark <nick> Date: Sat Jan 26 16:46:22 2008 +0000 POPLOOP is actually doing all the work of Perl_save_padsv() already! p4raw-id: //depot/perl@33078 commit 7fe44985589724dcec9673580abb16118ba49068 Author: Nicholas Clark <nick> Date: Sat Jan 26 16:44:43 2008 +0000 Standardise the conditional compilation protection of ({}) from #if defined(__GNUC__) && !defined(__STRICT_ANSI__) && !defined(PERL_GCC_PEDANTIC) to #if defined(__GNUC__) && !defined(PERL_GCC_BRACE_GROUPS_FORBIDDEN) because the ({}) construction can be used under __STRICT_ANSI__ (and should be, because it avoids temporary use of PL_Sv). p4raw-id: //depot/perl@33077 commit d28d780685cbd952928da9c691e2b5bae129457a Author: Nicholas Clark <nick> Date: Sat Jan 26 16:03:03 2008 +0000 As itersave points to the initial CxITERVAR(), and the state of SvPADMY() does not change over the duration of the scope, we can perform conditional actions at loop push time. For the non-pad case, a reference to the initial CxITERVAR() is already held on the scope stack thanks to SAVEGENERICSV(*svp); in pp_enteriter. So there is no need to save another reference to it in itersave - it's not going away. p4raw-id: //depot/perl@33076 commit 9a98d8a1ee5dd7f1caa95f2ec5ab0341e0b4f8d1 Author: Nicholas Clark <nick> Date: Sat Jan 26 15:17:09 2008 +0000 Restore the else block accidently eaten by change 33074. p4raw-id: //depot/perl@33075 * Here it does not work. commit b99874c7926b2f5919256cf4bcee8eb7bf3dff22 Author: Nicholas Clark <nick> Date: Sat Jan 26 15:14:25 2008 +0000 In POPLOOP, if CxITERVAR(cx) is non-NULL, then so is itersave, and itersave is a less complex expression for the C compiler. p4raw-id: //depot/perl@33074
09edbca0f5c7caf9dd4acef80d8e6275e5a95ea1 commit is first working one.
Created attachment 440658 [details] Fix This is 73072c95a77d4a9e353efb3fdb279b9650e31e8b commit backported from perl-git tree. It should fix the problem.
perl-5.10.0-96.fc12 has been submitted as an update for Fedora 12. https://admin.fedoraproject.org/updates/perl-5.10.0-96.fc12
perl-5.10.0-96.fc12 has been pushed to the Fedora 12 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update perl'. You can provide feedback for this update here: https://admin.fedoraproject.org/updates/perl-5.10.0-96.fc12
perl-5.10.0-96.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.