Bug #92501 for Scalar-List-Utils: pairmap segfaults

Mon Jan 27 09:08:13 2014 YKAR [...] cpan.org - Ticket created

Subject:

pairmap segfaults

Hello! I had data strucutre on which Perl terminated abnormally when perofrming pairmap. I have narrowed down it to this test case (I'm sorry I didn't make it shorter, but it's the shortest version which breaks perl). Backtrace: /lib64/libc.so.6(+0x7e9d6)[0x7ff8056c69d6] /lib64/libc.so.6(+0x7f783)[0x7ff8056c7783] /usr/lib64/libperl.so.5.16(Perl_av_extend+0x1db)[0x7ff805aa1f5b] /usr/lib64/libperl.so.5.16(Perl_stack_grow+0x2f)[0x7ff805ad140f] /usr/lib64/libperl.so.5.16(Perl_pp_mapwhile+0x37c)[0x7ff805adb7ac] /usr/lib64/libperl.so.5.16(Perl_runops_standard+0x13)[0x7ff805aa3c83] /usr/lib64/perl5/vendor_perl/5.16.3/x86_64-linux/auto/List/Util/Util.so(+0x4943)[0x7ff8048a8943] /usr/lib64/libperl.so.5.16(Perl_pp_entersub+0x645)[0x7ff805aab6e5] /usr/lib64/libperl.so.5.16(Perl_runops_standard+0x13)[0x7ff805aa3c83] /usr/lib64/libperl.so.5.16(perl_run+0x3ca)[0x7ff805a49bea] /usr/bin/perl(main+0x11b)[0x400f2b] I hope my test case would work for you. Thank you in advance!

Subject:

pairmap-bug.pl

#! /usr/bin/perl use strict; use warnings; use List::Util qw(pairmap); my @x = ( 'a' => [ '1', [ 'b' => [ '2', '3', '4' ], 'c' => [ '5', '6', '7', '8', '9', '10', '11', '12' ], 'd' => [ '13', '14', '15', '16', '17', '18', '19', '20', '21', '22', '23', '24', '25', '26', '27', '28', '29', '30', '31', '32' ], ] ], ); sub _extract { map { ref $_ ? pairmap(\&_extract, @$_) : $_ } @$b; } pairmap(\&_extract, @x);

Mon Jan 27 14:07:49 2014 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Mon Jan 27 09:08:13 2014, YKAR wrote: Show quoted text

> I had data strucutre on which Perl terminated abnormally when > perofrming pairmap. I have narrowed down it to this test case (I'm > sorry I didn't make it shorter, but it's the shortest version which > breaks perl).

Indeed. Seems to be the combination of a nested pairmap inside pairmap, combined with the inner one needing to copy the arg stack out of the way due to count > 2. -- Paul Evans

Mon Jan 27 14:07:49 2014 The RT System itself - Status changed from 'new' to 'open'

Sat Apr 05 11:24:28 2014 YKAR [...] cpan.org - Correspondence added

Show quoted text

> Seems to be the combination of a nested pairmap inside pairmap, > combined with the inner one needing to copy the arg stack out of the > way due to count > 2.

I prepared terse version of test case, and spent some time with valgrind. I found that there is a write to unallocated memory at line 658 (when reti == 29): stack[reti++] = newSVsv(PL_stack_sp[i - count + 1]); Maybe it's not safe to write to stack just by incrementing pointer, sooner or later it may exceed bounds. Probably exists safer alternative in Perl API which checks bounds before writing and grows stack if needed.

Subject:

pairmap-bug-terse.t

#! /usr/bin/perl use strict; use warnings; use List::Util qw(pairmap); sub _extract { map { ref $_ ? pairmap(\&_extract, @$_) : $_ } @$b; } pairmap(\&_extract, a => [ b => [ c => [ 1 .. 31 ]]]);

Sun Apr 06 13:24:03 2014 YKAR [...] cpan.org - Correspondence added

I spent some time inserting debugging output and now I'm sure that error was caused due to insufficient capacity of outer stack. I made a patch which reallocates original stack (which was before PUSH_MULTICALL) when needed. But I'm not in the slightest degree an expert in Perl guts, so please consider this patch just as an illustration to have an idea what's broken and how it could be fixed.

Subject:

0001-pairmap-extend-stack-outside-MULTICALL-when-needed.patch

From 914c56bcad5a0bacd3da36de2459e150e0ed25c2 Mon Sep 17 00:00:00 2001 From: YKAR <ykar@cpan.org> Date: Sun, 6 Apr 2014 17:11:40 +0000 Subject: [PATCH] pairmap: extend stack outside MULTICALL when needed --- ListUtil.xs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ListUtil.xs b/ListUtil.xs index bba138a..0190bbf 100644 --- a/ListUtil.xs +++ b/ListUtil.xs @@ -618,6 +618,9 @@ PPCODE: if(!CvISXSUB(cv)) { /* Since MULTICALL is about to move it */ SV **stack = PL_stack_base + ax; + AV* prev_stack = PL_curstack; + I32 prev_stack_used = stack - PL_stack_base; + I32 prev_stack_free = PL_stack_max - stack; I32 ret_gimme = GIMME_V; int i; @@ -654,8 +657,16 @@ PPCODE: items = n_args; } + if (count > prev_stack_free) { + prev_stack_free = count + 128; + av_extend(prev_stack, prev_stack_used + prev_stack_free); + stack = AvARRAY(prev_stack) + ax; + } + for(i = 0; i < count; i++) stack[reti++] = newSVsv(PL_stack_sp[i - count + 1]); + prev_stack_free -= count; + prev_stack_used += count; } POP_MULTICALL; -- 1.9.1