Advice on coredump
Rafael Garcia-Suarez
unless I uncomment the first line :
#!bleadperl
#use B qw/save_BEGINs/; BEGIN { save_BEGINs(); }
BEGIN {
$f = sub { eval $_[0] }
}
$bar = "ok\n";
print $f->( '$bar' ) ;
__END__
In some way the pad for the closure has been free'd
and perl tries to access unauthorized data via a pointer
that hasn't been zeroed. This is quite obscure to me for
the moment.
Anyway, what should perl do in this case ? Print "ok", or emit
an error, saying that the closure refers to an enclosing scope
that no longer exists ?
Andy Lester
> #use B qw/save_BEGINs/; BEGIN { save_BEGINs(); }
> BEGIN {
> $f = sub { eval $_[0] }
> }
> $bar = "ok\n";
> print $f->( '$bar' ) ;
If you change this to
print $f->( '$main::bar' ) ;
it runs fine. Not sure this is a help or not...
--
'Andy Lester an...@petdance.com
Programmer/author petdance.com
Daddy parsley.org/quinn Jk'=~/.+/s;print((split//,$&)
[unpack'C*',"n2]3%+>\"34.'%&.'^%4+!o.'"])
Rafael Garcia-Suarez
> > #!bleadperl
> > #use B qw/save_BEGINs/; BEGIN { save_BEGINs(); }
> > BEGIN {
> > $f = sub { eval $_[0] }
> > }
> > $bar = "ok\n";
> > print $f->( '$bar' ) ;
>
> If you change this to
> print $f->( '$main::bar' ) ;
> it runs fine. Not sure this is a help or not...
Yes, the coredump is caused by perl accessing a free'd pad (to
see if there's a lexical $bar in it).
$main::bar is not a lexical variable, it lives in a symbol table.
Hence you work around the core dump.
I don't know how to fix it, but I think this should print "ok".
On the other hand, I think that :
BEGIN {
my $bar;
$f = sub { eval $_[0] }
}
$bar = "ok\n";
print $f->( '$bar' ) ;
should fail with a fatal error "trying to access non-existent lexical
scope" or something.
Rafael Garcia-Suarez
> This code coredumps, due to unauthorized memory access,
Here's a patch. Basically this triggers cleaning up the pad
for free'd special CVs (BEGIN blocks, END blocks, et alii.)
All tests pass with and without threads, and valgrind reports
no errors. Looks good to me.
--- t/op/closure.t.orig Mon Nov 19 16:12:15 2001
+++ t/op/closure.t Wed Sep 18 22:08:14 2002
@@ -13,7 +13,7 @@
use Config;
-print "1..171\n";
+print "1..172\n";
my $test = 1;
sub test (&) {
@@ -510,3 +510,7 @@
}
+# The following dumps core with perl <= 5.8.0
+BEGIN { $vanishing_pad = sub { eval $_[0] } }
+$some_var = 123;
+test { $vanishing_pad->( '$some_var' ) == 123 };
--- op.c.orig Tue Sep 17 22:46:27 2002
+++ op.c Wed Sep 18 21:58:08 2002
@@ -4393,7 +4393,7 @@ Perl_cv_undef(pTHX_ CV *cv)
AV *padlist = CvPADLIST(cv);
I32 ix;
/* pads may be cleared out already during global destruction */
- if (is_eval && !PL_dirty) {
+ if ((is_eval && !PL_dirty) || CvSPECIAL(cv)) {
/* inner references to eval's cv must be fixed up */
AV *comppad_name = (AV*)AvARRAY(padlist)[0];
AV *comppad = (AV*)AvARRAY(padlist)[1];
End of Patch.
Jos Boumans
Rafael Garcia-Suarez wrote:
[kane@gremlin tmp]$ more t
BEGIN {
$f = sub { eval $_[0] }
}
$bar = "ok\n";
print $f->( '$bar' ) ;
[kane@gremlin tmp]$ perl5.6.0 t
ok
[kane@gremlin tmp]$ perl5.6.1 t
Segmentation fault
-- Jos
---
How do I prove I'm not crazy to people who are?
Rafael Garcia-Suarez
>
> [kane@gremlin tmp]$ more t
>
> BEGIN {
> $f = sub { eval $_[0] }
> }
> $bar = "ok\n";
> print $f->( '$bar' ) ;
>
>
>
> [kane@gremlin tmp]$ perl5.6.0 t
> ok
> [kane@gremlin tmp]$ perl5.6.1 t
> Segmentation fault
OK, so my patch restores that behaviour.
However, it doesn't revert completely to that behaviour :
("bleadperl" being bleadperl with my previously posted patch)
$ cat foo
#!perl -w
BEGIN {
my $bar = "xxx\n";
$f = sub { eval $_[0] }
}
$bar = "ok\n";
print $f->( '$bar' ) ;
$ perl5.8.0 foo
Name "main::bar" used only once: possible typo at foo line 6.
Segmentation fault (core dumped)
$ perl5.00503 foo
Name "main::bar" used only once: possible typo at foo line 6.
Use of uninitialized value at foo line 7.
$ bleadperl foo
Name "main::bar" used only once: possible typo at foo line 6.
ok
In this last case, perl outputs the value of $::bar because
the lexical $bar has gone away with the pad, and the outside
CV of the closure has been set to PL_main_cv.
Another fix is to increment the refcount of the BEGIN's CV
when a closure is declared within it.
So the closure's inner CV continues to refer to it.
This should restore
op.c has this comment :
/* If a potential closure prototype, don't keep a refcount on outer CV.
* This is okay as the lifetime of the prototype is tied to the
* lifetime of the outer CV. Avoids memory leak due to reference
* loop. --GSAR */
if (!name)
SvREFCNT_dec(CvOUTSIDE(cv));
and, in our case, the lifetime of the closure is longer than the lifetime
of the outer CV.
Well, I think restoring 5.6.0/5.00503 's behaviour is the
best thing to do. I'll have to write more regression tests.
If noone objects, I'll patch that way.
Rafael Garcia-Suarez
>
> Another fix is to increment the refcount of the BEGIN's CV
> when a closure is declared within it.
> So the closure's inner CV continues to refer to it.
I tried this, but with ithreads, ext/threads/shared/t/no_share.t
reports 'Scalars leaked: 1'. _Now_ I'm confused.
--- op.c.orig Sun Sep 8 21:30:45 2002
+++ op.c Thu Sep 19 23:09:46 2002
@@ -5075,7 +5075,9 @@ Perl_newATTRSUB(pTHX_ I32 floor, OP *o,
* This is okay as the lifetime of the prototype is tied to the
* lifetime of the outer CV. Avoids memory leak due to reference
* loop. --GSAR */
- if (!name)
+ /* When the outer CV is SPECIAL (as a BEGIN block), the closure
+ * may have a longer lifetime than its outer CV. */
+ if (!name && !CvSPECIAL(CvOUTSIDE(cv)))
SvREFCNT_dec(CvOUTSIDE(cv));
if (name || aname) {
Elizabeth Mattijsen
> > Another fix is to increment the refcount of the BEGIN's CV
> > when a closure is declared within it.
> > So the closure's inner CV continues to refer to it.
>I tried this, but with ithreads, ext/threads/shared/t/no_share.t
>reports 'Scalars leaked: 1'. _Now_ I'm confused.
There are a _lot_ of ways to get that message with threads (see at least on
of my bugreports from a few months ago on RT). So maybe you fixed one
problem, which now has an execution path that runs into another problem?
Liz
h...@crypt.org
:I wrote:
:> This code coredumps, due to unauthorized memory access,
:
:Here's a patch. Basically this triggers cleaning up the pad
:for free'd special CVs (BEGIN blocks, END blocks, et alii.)
:All tests pass with and without threads, and valgrind reports
:no errors. Looks good to me.
Thanks, applied as #17923.
Hugo
Rafael Garcia-Suarez
Well, that was one of the ways to fix it, but I'm not sure this
was the better one. See the rest of the thread. I did propose
another patch (that apparently caused a leak in ithreads),
because this one doesn't restore completely the behaviour
of perl 5.6.0. However it's difficult to define what should be
the appropriate behaviour for such an edge case.
h...@crypt.org
:h...@crypt.org wrote:
:> Thanks, applied as #17923.
:
:Well, that was one of the ways to fix it, but I'm not sure this
:was the better one. See the rest of the thread. I did propose
:another patch (that apparently caused a leak in ithreads),
:because this one doesn't restore completely the behaviour
:of perl 5.6.0. However it's difficult to define what should be
:the appropriate behaviour for such an edge case.
Is there a way to fix the later patch to avoid the leak?
For the given example:
BEGIN {
my $bar = "orig";
$foo = sub { eval $_[0] };
}
my $bar = "new";
$foo->('print $bar');
... I think "new" is probably a more correct output than undef.
While I can see the benefit of upping the refcnt of the block
when a closure is taken out of it, so that the above would
print "orig", I think we should not do so - I've wanted it
often enough, but I've worked around it just as often, and I
suspect we'd cause unreasonable breakage by changing that.
I'll leave the original patch in for now until we resolve what
should really go in.
Hugo