Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cx_stack reallocation during sort (Bleadperl 133acf7b breaks JE) #10642

Closed
p5pRT opened this issue Sep 19, 2010 · 5 comments
Closed

cx_stack reallocation during sort (Bleadperl 133acf7b breaks JE) #10642

p5pRT opened this issue Sep 19, 2010 · 5 comments
Labels

Comments

@p5pRT
Copy link

p5pRT commented Sep 19, 2010

Migrated from rt.perl.org#77930 (status was 'resolved')

Searchable as RT77930$

@p5pRT
Copy link
Author

p5pRT commented Sep 19, 2010

From @cpansprout

This script crashes​:

perl5.13.4 -le' $sub = sub { local $count = $count+1; ()->$sub if $count < 1000; $a cmp $b }; () = sort $sub qw<a b c d e f g>'

This started as a JE test failure (<http​://www.cpantesters.org/cpan/report/bf35a6a8-b679-11df-8fbf-ba84984c9764>). Andreas König helped me track it down.

Change 133acf7​:

From​: Nicholas Clark <nick at, er, ccl4.org, I think>
Date​: Thu, 24 Jun 2010 19​:44​:07 +0000 (+0100)
Subject​: In pp_sort, ensure that @​_ is freed correctly.
X-Git-Tag​: v5.13.3~329
X-Git-Url​: http​://perl5.git.perl.org/perl.git/commitdiff_plain/133acf7b389614db651d1ed570d4a0ca0c747999

In pp_sort, ensure that @​_ is freed correctly.

Before this, if @​_ had become AvREAL(), it retains reference on its elements.


Inline Patch
diff --git a/pp_sort.c b/pp_sort.c
index 51cf216..48d4273 100644
--- a/pp_sort.c
+++ b/pp_sort.c
@@ -1678,9 +1678,9 @@ PP(pp_sort)
 		    sort_flags);
 
 	    if (!(flags & OPf_SPECIAL)) {
-		LEAVESUB(cv);
-		if (!is_xsub)
-		    CvDEPTH(cv)--;
+		SV *sv;
+		POPSUB(cx, sv);
+		LEAVESUB(sv);
 	    }
 	    POPBLOCK(cx,PL_curpm);
 	    PL_stack_sp = newsp;
... etc. ...

This change does not take into account that cx_stack may have been reallocated, and that cx may be pointing to freed memory.

(This bug bears a striking resemblance to 74170.)

@p5pRT
Copy link
Author

p5pRT commented Sep 19, 2010

From @cpansprout

On Sep 19, 2010, at 12​:15 PM, Father Chrysostomos wrote​:

This script crashes​:

perl5.13.4 -le' $sub = sub { local $count = $count+1; ()->$sub if $count < 1000; $a cmp $b }; () = sort $sub qw<a b c d e f g>'

...

This change does not take into account that cx_stack may have been reallocated, and that cx may be pointing to freed memory.

Here is a patch for it.

@p5pRT
Copy link
Author

p5pRT commented Sep 19, 2010

From @cpansprout

From​: Father Chrysostomos <sprout@​cpan.org>

[perl #77930] cx_stack reallocation during sort

Reset cx in pp_sort before POPSUB, as the pointer may no
longer be valid.

Inline Patch
diff -Nup blead/pp_sort.c blead-sort-crash/pp_sort.c
--- blead/pp_sort.c	2010-07-03 08:47:11.000000000 -0700
+++ blead-sort-crash/pp_sort.c	2010-09-14 14:40:55.000000000 -0700
@@ -1679,6 +1679,9 @@ PP(pp_sort)
 
 	    if (!(flags & OPf_SPECIAL)) {
 		SV *sv;
+		/* Reset cx, in case the context stack has been
+		   reallocated. */
+		cx = &cxstack[cxstack_ix];
 		POPSUB(cx, sv);
 		LEAVESUB(sv);
 	    }
diff -Nurp blead/t/op/sort.t blead-sort-crash/t/op/sort.t
--- blead/t/op/sort.t	2010-06-24 12:46:10.000000000 -0700
+++ blead-sort-crash/t/op/sort.t	2010-09-14 14:33:55.000000000 -0700
@@ -6,7 +6,7 @@ BEGIN {
     require 'test.pl';
 }
 use warnings;
-plan( tests => 159 );
+plan( tests => 160 );
 
 # these shouldn't hang
 {
@@ -890,3 +890,21 @@ fresh_perl_is('sub w ($$) {my ($l, my $r
 
     is($count, 0, 'all gone');
 }
+
+# [perl #77930] The context stack may be reallocated during a sort, as a
+#               result of deeply-nested (or not-so-deeply-nested) calls
+#               from a custom sort subroutine.
+fresh_perl_is
+ '
+   $sub = sub {
+    local $count = $count+1;
+    ()->$sub if $count < 1000;
+    $a cmp $b
+   };
+   () = sort $sub qw<a b c d e f g>;
+   print "ok"
+ ',
+ 'ok',
+  {},
+ '[perl #_____] cx_stack reallocation during sort'
+;

@p5pRT
Copy link
Author

p5pRT commented Sep 20, 2010

@rgs - Status changed from 'new' to 'resolved'

@p5pRT p5pRT closed this as completed Sep 20, 2010
@p5pRT
Copy link
Author

p5pRT commented Sep 20, 2010

From @rgarcia

On 19 September 2010 21​:18, Father Chrysostomos <sprout@​cpan.org> wrote​:

This script crashes​:

perl5.13.4 -le' $sub = sub { local $count = $count+1; ()->$sub if $count < 1000; $a cmp $b }; () = sort $sub qw<a b c d e f g>'

...

This change does not take into account that cx_stack may have been reallocated, and that cx may be pointing to freed memory.

Here is a patch for it.

Thanks, applied to bleadperl as 8e7d0c4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant