Front page | perl.perl5.porters |
Postings from March 2000
Re: [ID 19990921.005] fix for SEGV in pp_leavesub
From: Nick Ing-Simmons
March 21, 2000 06:56
Re: [ID 19990921.005] fix for SEGV in pp_leavesub
Message ID: 200003211455.OAA20606@tiuk.ti.com
Root <firstname.lastname@example.org> writes:
>This is a bug report for perl from email@example.com,
>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 <firstname.lastname@example.org> reported what I
>believe is the same bug in comp.lang.perl.tk on 1998/10/06. ("my" and
>"destroy" in a Tk callback causing segfaults)
>Alan Lehotsky <email@example.com> 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:
> 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
> cc='cc', optimize='-O2', gccversion=184.108.40.206
> 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:
>Environment for perl 5.00404:
> LANG (unset)
> LOGDIR (unset)
> PERL_BADLANG (unset)
Nick Ing-Simmons <firstname.lastname@example.org>
Via, but not speaking for: Texas Instruments Ltd.