Closed p5pRT closed 14 years ago
Hi!
Thank you for your work on perl. Unfortunately\, perl 5.10.0 segfaults on the following program:
The problem is in the write statement. It only shows up in functions with a pre-declared prototype that use lexical variables in formats. Former perl versions (tested through 5.8.8) had a different problem: They will will output "1" for f(1) as well as for f(2).
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 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"
The RT System itself - Status changed from 'new' to 'open'
Dave notes:
a closure/format segfault in 5.10
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'
-- c
----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
On Thu Jun 25 14:50:51 2009\, chromatic@wgz.org wrote:
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.
I wrote:
Attached patch fixes bug #22977. make regen_perly after applying.
Should also fix bug #50528\, apparently a duplicate.
-zefram
@rgs - Status changed from 'open' to 'resolved'
On Tue Feb 05 16:33:41 2008\, davem wrote:
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 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. :-)
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.
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
On Thu Jun 28 13:17:40 2012\, sprout wrote:
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.
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
Migrated from rt.perl.org#50528 (status was 'resolved')
Searchable as RT50528$