develooper Front page | perl.perl5.porters | Postings from March 2000

Re: [ID 19990921.005] fix for SEGV in pp_leavesub

Thread Previous | Thread Next
Nick Ing-Simmons
March 21, 2000 06:56
Re: [ID 19990921.005] fix for SEGV in pp_leavesub
Message ID:
Root <> writes:
>This is a bug report for perl from,
>generated with the help of perlbug 1.20 running under perl 5.00404.
>The same thing happens with 5.004_04, 5.005_03, and perl.5005_61.

The fix is in perl5.6.0 - and the root cause of the worst cases is 
fixed in Tk800.019 (perl attempts to avoud the issue with an SvREFCNT_inc
on entry which LEAVE undoes as you note below, Tk though has had
"for a while" a bug where by bare "subs" as callbacks had REFCNT 
one too low - which meant CV went too early even with the fix.)

But as perl5.005_03 and earlier do NOT have the fix Tk800.020 will
restore my "stronger" fix which I (foolishly) backed out before releaseing 
Tk800.019 as "unnecessary" after testing against perl5.6.0.

The strong fix is to increment REFCNT of CV before passing to perl_call_sv
and decrement on return.

Many thanks for your helpful e-mail (and the fix of course) which showed
me when I re-read it that the problem was real even when REFCNT was

>I spent some time tracking this down, 

So have I :-( - I should pay more attention to the things I have 
flagged in my mail box!

>and I believe that the problem
>is as follows:
>When the callback executes the "my $t;", it does a SAVECLEARSV, which
>puts an index into curpad on the savestack.  Then at the end of the
>callback, pp_leavesub does a POPSUB2 which frees the cv including
>curpad, and then pp_leavesub does a LEAVE which tries to reference
>The obvious solution would seem to be not to decrement the refcnt on
>the cv until after LEAVE, and I've included a patch below to do that.
>I considered other possibilities.
>Perhaps a sub should not be destroying the only reference to itself.
>But it seems useful to do so, there seems to be no reason it should
>not work, and since pp_entersub increments the refcnt, it seems to
>allow for the possibilty as long as we don't decrement it too soon.
>Perhaps we should not be putting references to curpad on the
>savestack.  But the cv was there when it did the SAVECLEARSV, and it
>seems reasonable to keep it there until after the LEAVE.
>Perhaps SAVECLEARSV should increment a refcnt.  But the one
>incremented by pp_entersub seems like it should cover it.
>The PUSHBLOCK/PUSHSUB was done after the ENTER in pp_entersub, so to
>keep the nesting consistent, it would seem that one would want to do
>the POPSUB before the LEAVE.  But decrementing the refcnt doesn't
>really seem to be part of the POPSUB.  pp_entersub increments it after
>the PUSHSUB, but it can afford to do so because it knows that someone
>else has a reference to the sub, and won't take it away before the end
>of pp_entersub.  Conceptually, it would be incrementing the refcnt
>first thing, if it were convenient.  So waiting until after the LEAVE
>to decrement it is consistent with the nesting.
>Guillaume Curtil <> reported what I
>believe is the same bug in on 1998/10/06. ("my" and
>"destroy" in a Tk callback causing segfaults)
>Alan Lehotsky <> on 1999/07/06 in
>perl.porters-gw reported that "Perl seems to deallocate the array
>addressed via 'curpad' too soon", but didn't give a test case.
>In my patch, I didn't just move POPSUB2 after LEAVE, because it seemed
>to belong before LEAVE, and I didn't want to change anything that
>wasn't necessary to fix the problem.  Instead, I separated just the
>decrement of the refcnt from POPSUB2, and called it POPSUB2B, and put
>that after LEAVE.  I renamed the remainder of POPSUB2 to POPSUB2A, so
>anyone who was using POPSUB2 from the header file would have it break,
>instead of just silently stop working.  I put POPSUB2B after leave in
>pp_return and pp_last as well, because they seemed to have the same
>problem.  I didn't change the POPSUB in dounwind, because it doesn't
>seem to be followed by a LEAVE that depends on a curpad that might
>have been freed.
>The patch should be fairly safe, because in the normal case (when the
>cv is not being freed) it has no effect other than to decrement the
>refcnt after the LEAVE instead of before.
>The patch is a unified diff against 4.004_04 with only one line of
>context, so it will apply against 5.005_03, and 5.005_61 as well.
>begin 644 patch.gz
>[Please do not change anything below this line]
>Site configuration information for perl 5.00404:
>Configured by torin at Wed Feb  3 00:50:04 PST 1999.
>Summary of my perl5 (5.0 patchlevel 4 subversion 4) configuration:
>  Platform:
>    osname=linux, osvers=2.0.36, archname=i386-linux
>    uname='linux perv 2.0.36 #2 wed nov 18 03:00:48 pst 1998 i686 unknown '
>    hint=recommended, useposix=true, d_sigaction=define
>    bincompat3=n useperlio=undef d_sfio=undef
>  Compiler:
>    cc='cc', optimize='-O2', gccversion=
>    cppflags='-Dbool=char -DHAS_BOOL -D_REENTRANT'
>    ccflags ='-Dbool=char -DHAS_BOOL -D_REENTRANT'
>    stdchar='char', d_stdstdio=define, usevfork=false
>    voidflags=15, castflags=0, d_casti32=define, d_castneg=define
>    intsize=4, alignbytes=4, usemymalloc=n, prototype=define
>  Linker and Libraries:
>    ld='cc', ldflags =' -L/usr/local/lib'
>    libpth=/usr/local/lib /lib /usr/lib
>    libs=-lnsl -lndbm -lgdbm -ldbm -ldb -ldl -lm -lc -lposix -lcrypt
>    libc=, so=so
>    useshrplib=false, libperl=libperl.a
>  Dynamic Linking:
>    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic'
>    cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib'
>Locally applied patches:
>@INC for perl 5.00404:
>	/usr/lib/perl5/i386-linux/5.004
>	/usr/lib/perl5
>	/usr/local/lib/site_perl/i386-linux
>	/usr/local/lib/site_perl
>	.
>Environment for perl 5.00404:
>    HOME=/root
>    LANG (unset)
>    LD_LIBRARY_PATH=/usr/local/rvplayer5.0
>    LOGDIR (unset)
>    PATH=/usr/local/perl/bin:/usr/local/bin:/usr/local/scripts:/usr/bin:/usr/sbin:/bin:/sbin:/usr/bin/X11:/usr/games:/usr/andrew/bin:/usr/local/rvplayer5.0:.
>    PERL_BADLANG (unset)
>    SHELL=/bin/bash
Nick Ing-Simmons <>
Via, but not speaking for: Texas Instruments Ltd.

Thread Previous | Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About