Skip Menu |
 

This queue is for tickets about the Scalar-List-Utils CPAN distribution.

Report information
The Basics
Id: 80646
Status: resolved
Priority: 0/
Queue: Scalar-List-Utils

People
Owner: Nobody in particular
Requestors: dolmen [...] cpan.org
yyang [...] proofpoint.com
Cc: ribasushi [...] leporine.io
AdminCc:

Bug Information
Severity: Critical
Broken in: 1.25
Fixed in: 1.27



Subject: Recursive MULTICALL prematurely freed CV
Download (untitled) / with headers
text/plain 1.3k
Test script to make List::Util segfault: use List::Util 'first'; sub foo { if($comparison) { return 1; } else { local $comparison = 1; first \&foo, 1,2,3; } } for(1,2){ foo() } This is Perl Bug #78070, fixed in Perl 5.13.6: •The XS multicall API no longer causes subroutines to lose reference counts if called via the multicall interface from within those very subroutines. This affects modules like List::Util. Calling one of its functions with an active subroutine as the first argument could cause a crash So we can backport it here: --- multicall.h~ 2012-11-05 10:41:35.487993000 -0800 +++ multicall.h 2012-11-05 10:41:58.861218000 -0800 @@ -154,8 +154,8 @@ #define POP_MULTICALL \ STMT_START { \ - CvDEPTH(multicall_cv)--; \ - LEAVESUB(multicall_cv); \ + if (! --CvDEPTH(multicall_cv)) \ + LEAVESUB(multicall_cv); \ POPBLOCK(cx,PL_curpm); \ POPSTACK; \ CATCH_SET(multicall_oldcatch); \
Download (untitled) / with headers
text/plain 343b
Curious. Using that unit test I can confirm it breaks on e.g. 5.12.4, but the patch doesn't fix it. It breaks even with that patch in place. Moreover it doesn't appear that the included "multicall.h" file is actually used by default. I've tried just inserting silly syntax breakage into it, and that doesn't break the build. -- Paul Evans
Download (untitled) / with headers
text/plain 672b
On Wed Nov 07 15:40:22 2012, PEVANS wrote: Show quoted text
> Curious. > > Using that unit test I can confirm it breaks on e.g. 5.12.4, but the > patch doesn't fix it. It breaks even with that patch in place. > > Moreover it doesn't appear that the included "multicall.h" file is > actually used by default. I've tried just inserting silly syntax > breakage into it, and that doesn't break the build.
multicall.h is used only on 5.8, which didn’t provide the multicall API. The bug that this patch fixes for 5.8 is present in perl’s own multicall API in perl 5.10 and 5.12. Tweaking the reference count right before using the macro might be the best fix for those two versions.
Download (untitled) / with headers
text/plain 502b
Show quoted text
> multicall.h is used only on 5.8, which didn’t provide the multicall > API. > > The bug that this patch fixes for 5.8 is present in perl’s own > multicall API in perl 5.10 and > 5.12. > > Tweaking the reference count right before using the macro might be the > best fix for those > two versions.
Hi, Just reviving this again. I'm at a loss to understand exactly what is required here. Can you suggest what if any changes to code actually in Scalar-List-Utils needs to be done? -- Paul Evans
Download (untitled) / with headers
text/plain 155b
Don't have any suggestions, just chiming in with the severity of impact - the test fails and/or segfaults on all perls < 5.14, this includes even 5.12.5 :(
Download (untitled) / with headers
text/plain 809b
On Sun Dec 16 14:07:34 2012, PEVANS wrote: Show quoted text
> > multicall.h is used only on 5.8, which didn’t provide the multicall > > API. > > > > The bug that this patch fixes for 5.8 is present in perl’s own > > multicall API in perl 5.10 and > > 5.12. > > > > Tweaking the reference count right before using the macro might be the > > best fix for those > > two versions.
> > Hi, > > Just reviving this again. I'm at a loss to understand exactly what is > required here. Can you suggest what if any changes to code actually in > Scalar-List-Utils needs to be done?
Something like this (sorry, completely untested): #if PERL_VERSION == 10 || PERL_VERSION == 12 if (CvDEPTH(multicall_cv) > 1) SvREFCNT_inc_simple_void_NN(multicall_cv); #endif POP_MULTICALL; And the patch to multicall.h for 5.8.
This now looks to be patched at least for 5.8.9, 5.10 and 5.12. -- Paul Evans
Download (untitled) / with headers
text/plain 854b
On Thu Dec 27 10:16:11 2012, PEVANS wrote: Show quoted text
> This now looks to be patched at least for 5.8.9, 5.10 and 5.12.
Thank you very much. Confirmed to compile/pass tests on all of this, except for 5.6.2. The fix for that seems easy-ish, but I will leave patching to the more knowledgeable. rabbit@Thesaurus:~$ perlbrew list 5.10.0_2 5.10.0_dbg1 5.10.0_tr 5.10.1 5.10.1_2 5.10.1_dbg1 5.10.1_no_thr 5.12.0_dbg1 5.12.1 5.12.2 5.12.3 5.12.4 5.12.4_dbg1 5.12.5 5.12.5_nthr 5.14.1_thr 5.14.1_thr_mb_dbg 5.14.2_dbg_thr_mb 5.14.2_thr_cln 5.14.3 5.14.3_nthr 5.15.8 5.15.9 5.16.0 * 5.16.2.1 5.16.2_nthr 5.17.6 5.6.2 5.8.1 5.8.1_dbg_mb 5.8.2 5.8.3 5.8.3_cln 5.8.3_dbg_thr 5.8.3_tr 5.8.4 5.8.5 5.8.5_clean_cattest 5.8.6 5.8.7 5.8.8 5.8.8_cpanmeta 5.8.8_dbg 5.8.9 5.8.9_dbg1 5.8.9_f
Download (untitled) / with headers
text/plain 494b
On Sat Dec 29 01:01:48 2012, RIBASUSHI wrote: Show quoted text
> Thank you very much. Confirmed to compile/pass tests on all of this, > except for 5.6.2. The fix for that seems easy-ish, but I will leave > patching to the more knowledgeable.
Cool. That's nice to know. I don't think personally I care much as back as 5.6 - certainly, I won't put in any effort up-front now to fix it. If someone else wants to submit a patch I'll happily accept it, but for now I shall consider this one fixed. -- Paul Evans


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.