Skip Menu |
 
Report information
Id: 50528
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: springl-perlbug [at] bfw-online.de
Cc:
AdminCc:

Operating System: Linux
PatchStatus: (no value)
Severity: High
Type: core
Perl Version: 5.10.0
Fixed In: (no value)



Subject: Perl 5.10.0 segfaults on Format with my variables
Date: Mon, 4 Feb 2008 16:21:12 +0100 (MEZ)
To: perlbug [...] perl.org
From: Stephan Springl <springl-perlbug [...] bfw-online.de>

Message body is not shown because it is too large.

Subject: Re: [perl #50528] Perl 5.10.0 segfaults on Format with my variables
Date: Wed, 6 Feb 2008 00:29:12 +0000
To: perl5-porters [...] perl.org
From: Dave Mitchell <davem [...] iabyn.com>
Download (untitled) / with headers
text/plain 1.3k
On Mon, Feb 04, 2008 at 07:22:29AM -0800, Stephan Springl wrote: Show quoted text
> Thank you for your work on perl. Unfortunately, perl 5.10.0 segfaults on > the following program: > > ============================================== > sub f ($); # Comment out to get right result! > sub f ($) { > my $test = $_[0]; > write; > format STDOUT = > @<<<<<<< > $test > . > } > f(1); > f(2); > ==============================================
Thanks for the concise test case. P5Pers: The bug is due to the fact that in the presence of prototypes, after the real sub is compiled, its body is physically copied across to a new CV, which messes the CvOUTSIDE field of any child anon or format subs. The anon subs are fixed up by looking through the parent's pad for any '&' entries which point to the children that need fixing. There isn't a similar way to find formats, so they can't be fixed up. The two solutions seem to be: a) have pointers in parent pads to formats, or b) rejig newATTRSUB in such a way that it doesn't have to do the abomination of copying the CV body. Don't really have time to look at this further yet. -- "Strange women lying in ponds distributing swords is no basis for a system of government. Supreme executive power derives from a mandate from the masses, not from some farcical aquatic ceremony." -- Dennis, "Monty Python and the Holy Grail"
Dave notes: a closure/format segfault in 5.10
CC: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #50528] Perl 5.10.0 segfaults on Format with my variables
Date: Thu, 25 Jun 2009 14:50:08 -0700
To: perl5-porters [...] perl.org
From: chromatic <chromatic [...] wgz.org>
Download (untitled) / with headers
text/plain 1.6k
On Tuesday 05 February 2008 16:29:12 Dave Mitchell wrote: Show quoted text
> The bug is due to the fact that in the presence of prototypes, after the > real sub is compiled, its body is physically copied across to a new CV, > which messes the CvOUTSIDE field of any child anon or format subs. > The anon subs are fixed up by looking through the parent's pad for any > '&' entries which point to the children that need fixing. There isn't a > similar way to find formats, so they can't be fixed up. > > The two solutions seem to be: > > a) have pointers in parent pads to formats, or > b) rejig newATTRSUB in such a way that it doesn't have to do the > abomination of copying the CV body. > > Don't really have time to look at this further yet.
I don't think b) is tractable. If anything's taken a reference to a sub prototype, it points to the first CV. If you stuff a new CV (a properly mangled PL_compcv, for example) in the GV (or wherever), you break at least an AutoLoader test in the core, and some expectations elsewhere. My failed attempt produced failures in: ../ext/Math-BigInt-FastCalc/t/bigintfc.t ../lib/AutoLoader/t/01AutoLoader.t ../lib/Exporter.t ../lib/Math/BigInt/t/sub_mbf.t ../lib/bignum/t/bninfnan.t ../lib/bignum/t/brinfnan.t op/closure.t op/method.t ... along these lines: not ok 41 # Failed at t/op/method.t line 136 # got 'B: In B::e, 2' # expected 'C: In C::e, 1' not ok 42 # Failed at t/op/method.t line 138 # got 'B: In A::ee, 3' # expected 'B: In A::ee, 2' not ok 43 # Failed at t/op/method.t line 139 # got 'B: In A::ee, 3' # expected 'B: In A::ee, 2' -- c
Download (untitled) / with headers
text/plain 957b
Show quoted text
----Program---- #!/usr/bin/perl -l sub f ($); # Comment out to get right result! sub f ($) { my $test = $_[0]; write; format STDOUT = @<<<<<<< $test . } f(1); f(2); ----Output of .../pd2oePD/perl-5.8.0@19648/bin/perl---- 1 1 ----EOF ($?='0')---- ----Output of .../pKd6Hks/perl-5.8.0@19649/bin/perl---- ----EOF ($?='11')---- http://perl5.git.perl.org/perl.git/commit/ 71f882da828ecd892a162839f27e4625d69023fb uthor Dave Mitchell <davem@fdisolutions.com> Sat, 31 May 2003 19:54:48 +0000 (20:54 +0100) committer Rafael Garcia-Suarez <rgarciasuarez@gmail.com> Sat, 31 May 2003 18:33:07 +0000 (18:33 +0000) commit 71f882da828ecd892a162839f27e4625d69023fb tree aec07a1ff1cac538c9b9c2426fbb4b427a03e341 tree | snapshot parent 0aeb64d0d71d7345dba69c9999c49f4fbb55b24b commit | diff jumbo closure patch broke formats Message-ID: <20030531185448.GA6055@fdgroup.com> Plus restore the original test script for bug #22372 p4raw-id: //depot/perl@19649
Download (untitled) / with headers
text/plain 2.1k
On Thu Jun 25 14:50:51 2009, chromatic@wgz.org wrote: Show quoted text
> On Tuesday 05 February 2008 16:29:12 Dave Mitchell wrote: >
> > The bug is due to the fact that in the presence of prototypes, after
> the
> > real sub is compiled, its body is physically copied across to a new
> CV,
> > which messes the CvOUTSIDE field of any child anon or format subs. > > The anon subs are fixed up by looking through the parent's pad for
> any
> > '&' entries which point to the children that need fixing. There
> isn't a
> > similar way to find formats, so they can't be fixed up. > > > > The two solutions seem to be: > > > > a) have pointers in parent pads to formats, or > > b) rejig newATTRSUB in such a way that it doesn't have to do the > > abomination of copying the CV body. > > > > Don't really have time to look at this further yet.
> > I don't think b) is tractable. If anything's taken a reference to a > sub > prototype, it points to the first CV. If you stuff a new CV (a > properly mangled > PL_compcv, for example) in the GV (or wherever), you break at least an > AutoLoader test in the core, and some expectations elsewhere. My > failed > attempt produced failures in: > > ../ext/Math-BigInt-FastCalc/t/bigintfc.t > ../lib/AutoLoader/t/01AutoLoader.t > ../lib/Exporter.t > ../lib/Math/BigInt/t/sub_mbf.t > ../lib/bignum/t/bninfnan.t > ../lib/bignum/t/brinfnan.t > op/closure.t > op/method.t > > ... along these lines: > > not ok 41 > # Failed at t/op/method.t line 136 > # got 'B: In B::e, 2' > # expected 'C: In C::e, 1' > not ok 42 > # Failed at t/op/method.t line 138 > # got 'B: In A::ee, 3' > # expected 'B: In A::ee, 2' > not ok 43 > # Failed at t/op/method.t line 139 > # got 'B: In A::ee, 3' > # expected 'B: In A::ee, 2'
What happens if we don't throw away the newly compiled sub after copying it into the prototype? That seems to solve the dangling reference problem but I have no idea what else it might break. The naive test I just did does indeed make the test case behave - but causes other breakage. But that may just be because I'm not quite sure which refcounts I need to bump.
Subject: [perl #50528] Perl 5.10.0 segfaults on Format with my variables
Date: Mon, 14 Dec 2009 23:41:15 +0000
To: perl5-porters [...] perl.org
From: Zefram <zefram [...] fysh.org>
Download (untitled) / with headers
text/plain 140b
I wrote: Show quoted text
>Attached patch fixes bug #22977. make regen_perly after applying.
Should also fix bug #50528, apparently a duplicate. -zefram
RT-Send-CC: perl5-porters [...] perl.org, davem [...] iabyn.com, zefram [...] fysh.org
Download (untitled) / with headers
text/plain 1.7k
On Tue Feb 05 16:33:41 2008, davem wrote: Show quoted text
> On Mon, Feb 04, 2008 at 07:22:29AM -0800, Stephan Springl wrote:
> > Thank you for your work on perl. Unfortunately, perl 5.10.0
segfaults on Show quoted text
> > the following program: > > > > ============================================== > > sub f ($); # Comment out to get right result! > > sub f ($) { > > my $test = $_[0]; > > write; > > format STDOUT = > > @<<<<<<< > > $test > > . > > } > > f(1); > > f(2); > > ==============================================
> > Thanks for the concise test case.
That was actually your own test case, from #22977. :-) Show quoted text
> > P5Pers: > > The bug is due to the fact that in the presence of prototypes, after the > real sub is compiled, its body is physically copied across to a new CV, > which messes the CvOUTSIDE field of any child anon or format subs. > The anon subs are fixed up by looking through the parent's pad for any > '&' entries which point to the children that need fixing. There isn't a > similar way to find formats, so they can't be fixed up. > > The two solutions seem to be: > > a) have pointers in parent pads to formats, or
Zefram did that in commit 421f30ed1e, but it can cause a crash. A format’s CvOUTSIDE must never be touched, because a format can be cloned at any time, so its CvOUTSIDE must always be available for it to close over. If CvOUTSIDE is weakened, the outer sub could be freed, causing the format to point to its grandfather. Then cloning will read random pad items from the wrong pad. Show quoted text
> b) rejig newATTRSUB in such a way that it doesn't have to do the > abomination of copying the CV body.
Like copying it in perly.y:subname instead? An alternative fix would be to have the outer pad hold a weak reference on the format. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org, davem [...] iabyn.com, zefram [...] fysh.org
Download (untitled) / with headers
text/plain 1.1k
On Thu Jun 28 13:17:40 2012, sprout wrote: Show quoted text
> On Tue Feb 05 16:33:41 2008, davem wrote:
> > The two solutions seem to be: > > > > a) have pointers in parent pads to formats, or
> > Zefram did that in commit 421f30ed1e, but it can cause a crash. A > format’s CvOUTSIDE must never be touched, because a format can be cloned > at any time, so its CvOUTSIDE must always be available for it to close > over. If CvOUTSIDE is weakened, the outer sub could be freed, causing > the format to point to its grandfather. Then cloning will read random > pad items from the wrong pad.
I changed the direction of the weak reference, in commit e09ac07, since that seemed the easiest fix. Show quoted text
> > b) rejig newATTRSUB in such a way that it doesn't have to do the > > abomination of copying the CV body.
> > Like copying it in perly.y:subname instead?
That is problematic, in that newATTRSUB is an API function, so we would have to do it in both places, solving nothing. If we could change that, with the new op slab allocator (not merged yet), after errors we would end up with stubs floating around with slabs attached to them--not something I find comforting. -- Father Chrysostomos


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

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org