Don’t let formats outlive their outer subs
authorFather Chrysostomos <sprout@cpan.org>
Thu, 28 Jun 2012 23:31:17 +0000 (16:31 -0700)
committerFather Chrysostomos <sprout@cpan.org>
Thu, 28 Jun 2012 23:40:31 +0000 (16:40 -0700)
commite09ac076a1dab8e2c5712775f478fcfb61cb7eb3
tree14236acc0223a1ffd21649471cdbe3e24c8f8e40
parentd142a0767037572181a72aa9b71be2ad3a6e1e98
Don’t let formats outlive their outer subs

This began crashing in 5.11.3:

sub foo {
  sub bar {
    my ($a,$b,$c,$d,$e,$f,$g,$h,$i,$j,$k,$l,$m,$n,$o,$p,$q,$r,$s,$x);
    format =
@||||||
$x
.
  }
}
undef *bar;
write;

(On some systems, you need more alphabet soup to make it crash.)

This commit (just the perly.y part shown) caused it to crash:

commit 421f30ed1e95009450bdc7905bf3433ee806ea4f
Author: Zefram <zefram@fysh.org>
Date:   Tue Dec 15 11:48:31 2009 +0100

    [perl #22977] Bug in format/write

diff --git a/perly.y b/perly.y
index 18e5875..a61a6b3 100644
--- a/perly.y
+++ b/perly.y
@@ -511,7 +511,9 @@ peg : PEG
  ;

 format : FORMAT startformsub formname block
- { SvREFCNT_inc_simple_void(PL_compcv);
+ {
+   CV *fmtcv = PL_compcv;
+   SvREFCNT_inc_simple_void(PL_compcv);
 #ifdef MAD
    $$ = newFORM($2, $3, $4);
    prepend_madprops($1->tk_mad, $$, 'F');
@@ -521,6 +523,10 @@ format : FORMAT startformsub formname block
    newFORM($2, $3, $4);
    $$ = (OP*)NULL;
 #endif
+   if (CvOUTSIDE(fmtcv) && !CvUNIQUE(CvOUTSIDE(fmtcv))) {
+     SvREFCNT_inc_simple_void(fmtcv);
+     pad_add_anon((SV*)fmtcv, OP_NULL);
+   }
  }
  ;

Unfortunately, adding the format to the pad like that (to allow
pad_fixup_inner_anons to fix up formats as well as subs) is proble-
matic.  It causes the format’s CvOUTSIDE to be weak.  Since the for-
mat does not hold a reference count on its outer sub, that sub can be
freed before the format.  When that happens, regular subs are fixed
up by having CvOUTSIDE change to point to the grandparent.  If you
do that for formats, you run into a problem: Formats can be cloned
even when the outer sub is not running.  Formats are cloned whenever
invoked *by name* via write.  If CvOUTSIDE points to a different sub,
then closing over the scalars in specific pad offsets in that sub can
result in reading past the end of the pad.  If you don’t read past the
end of the pad, you are still making variables close over unrelated variables, so the inner $x could close over an outer @y, etc.  Subrou-
tines don’t have that problem, as they can only be cloned when they
have an outer sub.  (Even though the outer sub’s prototype, if it is a
closure, might have been freed, the outer sub itself is still running
and referenced by the context stack.)

This commit changes the direction of the weak reference between an
outer sub’s pad and an inner format, fixing the crash.

To do so, it has to store, not the format itself, but a weak RV point-
ing to the format, in the outer sub’s pad.
pad.c
t/comp/form_scope.t