search

Perl / perl5

šŸŖ The Perl programming language
https://dev.perl.org/perl5/
Other
1.94k stars 554 forks source link

Segmentation fault when a destructor kills a sub about to be goto'd into #11664

Closed p5pRT closed 12 years ago

p5pRT commented 13 years ago

Migrated from rt.perl.org#99850 (status was 'resolved')

Searchable as RT99850$

p5pRT commented 13 years ago

From perl@profvince.com

Created by perl@profvince.com

The following code​:

perl -wle'sub reftype { "whatever" }; sub DESTROY { undef &reftype } print sub { my $guard = bless []; goto &reftype }->(\1)'

has been segfaulting since perl 5.8.0. It is not a very smart thing to do\, but it can be suprising if the undef is hidden below several abstraction layers. XSUBs suffer from the same issue\, as​:

perl -wle'use Scalar​::Util qw\; sub DESTROY { undef &reftype } print sub { my $guard = bless []; goto &reftype }->(\1)'

segfaults as well.

Perl Info ``` Flags: category=core severity=low Site configuration information for perl 5.12.4: Configured by Gentoo at Tue Aug 9 15:15:55 CEST 2011. Summary of my perl5 (revision 5 version 12 subversion 4) configuration: Platform: osname=linux, osvers=2.6.39.1-fuuka.profvince.com, archname=x86_64-linux uname='linux fuuka 2.6.39.1-fuuka.profvince.com #1 smp fri jun 10 20:18:17 cest 2011 x86_64 intel(r) core(tm)2 duo cpu e6750 @ 2.66ghz genuineintel gnulinux ' config_args='-des -Duseshrplib -Darchname=x86_64-linux -Dcc=x86_64-pc-linux-gnu-gcc -Doptimize=-march=core2 -O3 -fomit-frame-pointer -ftree-vectorize -ftree-vectorizer-verbose=1 -pipe -DNDEBUG -Dldflags=-Wl,-O1 -Wl,--as-needed -Dprefix=/usr -Dsiteprefix=/usr -Dvendorprefix=/usr -Dscriptdir=/usr/bin -Dprivlib=/usr/lib64/perl5/5.12.4 -Darchlib=/usr/lib64/perl5/5.12.4/x86_64-linux -Dsitelib=/usr/lib64/perl5/site_perl/5.12.4 -Dsitearch=/usr/lib64/perl5/site_perl/5.12.4/x86_64-linux -Dvendorlib=/usr/lib64/perl5/vendor_perl/5.12.4 -Dvendorarch=/usr/lib64/perl5/vendor_perl/5.12.4/x86_64-linux -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/share/man/man1 -Dsiteman3dir=/usr/share/man/man3 -Dvendorman1dir=/usr/share/man/man1 -Dvendorman3dir=/usr/share/man/man3 -Dman1ext=1 -Dman3ext=3pm -Dlibperl=libperl.so.5.12.4 -Dlocincpth= -Duselargefiles -Dd_semctl_semun -Dcf_by=Gentoo -Dmyhostname=localhost -Dperladmin=root@localhost -Dinstallusrbinperl=n -Ud_csh -Uusenm -Di_ndbm -Di_gdbm -Di_db -DDEBUGGING=none -Dinc_version_list=5.12.3/x86_64-linux 5.12.3 5.12.2/x86_64-linux 5.12.2 5.12.1/x86_64-linux 5.12.1 5.12.0/x86_64-linux 5.12.0 -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64' hint=recommended, useposix=true, d_sigaction=define useithreads=undef, usemultiplicity=undef useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=define, use64bitall=define, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='x86_64-pc-linux-gnu-gcc', ccflags ='-fno-strict-aliasing -pipe -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-march=core2 -O3 -fomit-frame-pointer -ftree-vectorize -ftree-vectorizer-verbose=1 -pipe -DNDEBUG', cppflags='-fno-strict-aliasing -pipe' ccversion='', gccversion='4.5.3', gccosandvers='' intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='x86_64-pc-linux-gnu-gcc', ldflags ='-Wl,-O1 -Wl,--as-needed' libpth=/usr/local/lib64 /lib64 /usr/lib64 libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc libc=/lib/libc-2.13.so, so=so, useshrplib=true, libperl=libperl.so.5.12.4 gnulibc_version='2.13' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E' cccdlflags='-fPIC', lddlflags='-shared -march=core2 -O3 -fomit-frame-pointer -ftree-vectorize -ftree-vectorizer-verbose=1 -pipe -DNDEBUG -Wl,-O1 -Wl,--as-needed' Locally applied patches: 0001-gentoo_MakeMaker-RUNPATH.diff 0002-gentoo_config_over.diff 0003-gentoo_cpan_definstalldirs.diff 0004-gentoo_cpanplus_definstalldirs.diff 0005-gentoo_create-libperl-soname.diff 0006-gentoo_MakeMaker-delete_packlist.diff 0007-fixes_8d66b3f9_h2hp_fix.diff 0008-fixes_f178b03b_h2ph_using_deprecated_goto.diff 0009-gentoo_mod-paths.diff 0010-gentoo_enc2xs.diff 0011-gentoo_IO-Compress_AutoLoader_dropped_from_Compress-Zlib.diff 0012-gentoo_drop-fstack-protector.diff @INC for perl 5.12.4: /etc/perl /usr/lib64/perl5/site_perl/5.12.4/x86_64-linux /usr/lib64/perl5/site_perl/5.12.4 /usr/lib64/perl5/vendor_perl/5.12.4/x86_64-linux /usr/lib64/perl5/vendor_perl/5.12.4 /usr/lib64/perl5/site_perl /usr/lib64/perl5/vendor_perl/5.12.3/x86_64-linux /usr/lib64/perl5/vendor_perl/5.12.3 /usr/lib64/perl5/vendor_perl/5.12.2/x86_64-linux /usr/lib64/perl5/vendor_perl/5.12.2 /usr/lib64/perl5/vendor_perl /usr/lib64/perl5/5.12.4/x86_64-linux /usr/lib64/perl5/5.12.4 /usr/local/lib/site_perl . Environment for perl 5.12.4: HOME=/home/vince LANG=fr_FR.UTF-8 LANGUAGE (unset) LC_ALL=fr_FR.UTF-8 LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/vince/bin:/home/vince/perl/builds/bin:/usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/4.5.3:/opt/intel/cce/10.1.018/bin:/usr/games/bin PERL_BADLANG (unset) SHELL=/bin/bash ```
p5pRT commented 13 years ago

From @nwc10

On Fri\, Sep 23\, 2011 at 03​:40​:25AM -0700\, perl@​profvince.com wrote​:

----------------------------------------------------------------- [Please describe your issue here]

The following code​:

perl -wle'sub reftype { "whatever" }; sub DESTROY { undef &reftype } print sub { my $guard = bless []; goto &reftype }->(\1)'

has been segfaulting since perl 5.8.0. It is not a very smart thing to do\, but it can be suprising if the undef is hidden below several abstraction layers. XSUBs suffer from the same issue\, as​:

perl -wle'use Scalar​::Util qw\; sub DESTROY { undef &reftype } print sub { my $guard = bless []; goto &reftype }->(\1)'

segfaults as well.

First example SEGV introduced by this commit​:

commit 840a7b70755d06740715e982aa756f9d77203c4e Author​: Ilya Zakharevich \ilya@​math\.berkeley\.edu Date​: Mon Dec 4 19​:40​:25 2000 -0500

  Re​: [PATCH] The largest hoax of all times?   Date​: Tue\, 5 Dec 2000 00​:40​:25 -0500   Message-ID​: \20001205004025\.A4050@​monk\.mps\.ohio\-state\.edu  
  Subject​: Re​: [PATCH] The largest hoax of all times?   From​: Ilya Zakharevich \ilya@​math\.ohio\-state\.edu   Date​: Mon\, 4 Dec 2000 23​:55​:53 -0500   Message-ID​: \20001204235553\.A1140@​monk\.mps\.ohio\-state\.edu  
  Subject​: Re​: [PATCH] The largest hoax of all times?   From​: Ilya Zakharevich \ilya@​math\.ohio\-state\.edu   Date​: Tue\, 5 Dec 2000 01​:28​:45 -0500   Message-ID​: \20001205012844\.A4227@​monk\.mps\.ohio\-state\.edu  
  Fix the unpredictable order of DESTROYs.  
  p4raw-id​: //depot/perl@​7991

Bisected with this rather quickly on the new dromedary​:

#!/bin/sh git clean -dxf # If you can use ccache\, add -Dcc=ccache\ gcc -Dld=gcc to the Configure line # if Encode is not needed for the test\, you can speed up the bisect by # excluding it from the runs with -Dnoextensions=Encode sh Configure -des -Dusedevel -Doptimize="-g" -Dcc=ccache\ gcc -Dld=gcc -Dlibpth='/usr/local/lib64 /lib64 /usr/lib64' test -f config.sh || exit 125 # Correct makefile for newer GNU gcc perl -ni -we 'print unless /\<(?​:built-in|command)/' makefile x2p/makefile # if you just need miniperl\, replace test_prep with miniperl make -j4 miniperl [ -x ./miniperl ] || exit 125 ./miniperl -wle 'sub reftype { "whatever" }; sub DESTROY { undef &reftype } print sub { my $guard = bless []; goto &reftype }->(\1)' ret=$? [ $ret -gt 127 ] && ret=127 git clean -dxf git reset --hard HEAD exit $ret

Couple of new gotchas there

1​: needed -Dlibpth='/usr/local/lib64 /lib64 /usr/lib64' to cope with the   x86_64 Linux layout being way to new for 10 year old Configure 2​: needed git reset --hard HEAD because one commit happened to modify   pod/perlapi.pod\, which caused git bisect good to abort 3​: Should have been run with \</dev/null as it seems at least one commit from   that era ends up with something in Configure hanging on input from stdin.

Nicholas Clark

p5pRT commented 13 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 13 years ago

From @nwc10

On Wed\, Sep 28\, 2011 at 05​:39​:43PM +0100\, Nicholas Clark wrote​:

On Fri\, Sep 23\, 2011 at 03​:40​:25AM -0700\, perl@​profvince.com wrote​:

----------------------------------------------------------------- [Please describe your issue here]

The following code​:

perl -wle'sub reftype { "whatever" }; sub DESTROY { undef &reftype } print sub { my $guard = bless []; goto &reftype }->(\1)'

has been segfaulting since perl 5.8.0. It is not a very smart thing to do\, but it can be suprising if the undef is hidden below several abstraction layers. XSUBs suffer from the same issue\, as​:

perl -wle'use Scalar​::Util qw\; sub DESTROY { undef &reftype } print sub { my $guard = bless []; goto &reftype }->(\1)'

segfaults as well.

First example SEGV introduced by this commit​:

commit 840a7b70755d06740715e982aa756f9d77203c4e Author​: Ilya Zakharevich \ilya@&#8203;math\.berkeley\.edu Date​: Mon Dec 4 19​:40​:25 2000 -0500

Re&#8203;: \[PATCH\] The largest hoax of all times?
Date&#8203;: Tue\, 5 Dec 2000 00&#8203;:40&#8203;:25 \-0500
Message\-ID&#8203;: \<20001205004025\.A4050@&#8203;monk\.mps\.ohio\-state\.edu>

Subject&#8203;: Re&#8203;: \[PATCH\] The largest hoax of all times?
From&#8203;: Ilya Zakharevich \<ilya@&#8203;math\.ohio\-state\.edu>
Date&#8203;: Mon\, 4 Dec 2000 23&#8203;:55&#8203;:53 \-0500
Message\-ID&#8203;: \<20001204235553\.A1140@&#8203;monk\.mps\.ohio\-state\.edu>

Subject&#8203;: Re&#8203;: \[PATCH\] The largest hoax of all times?
From&#8203;: Ilya Zakharevich \<ilya@&#8203;math\.ohio\-state\.edu>
Date&#8203;: Tue\, 5 Dec 2000 01&#8203;:28&#8203;:45 \-0500
Message\-ID&#8203;: \<20001205012844\.A4227@&#8203;monk\.mps\.ohio\-state\.edu>

Fix the unpredictable order of DESTROYs\.

p4raw\-id&#8203;: //depot/perl@&#8203;7991

XSUBs also broken at the same time​:

$ ./perl -Ilib -wle'use Socket qw\<sockaddr_in>; sub DESTROY { undef &sockaddr_in } print sub { my $guard = bless []; goto &sockaddr_in }->(\1)' Segmentation fault

vs

$ ./perl -Ilib -wle'use Socket qw\<sockaddr_in>; sub DESTROY { undef &sockaddr_in } print sub { my $guard = bless []; goto &sockaddr_in }->(\1)' Bad arg length for Socket​::unpack_sockaddr_in\, length is 18\, should be 16 at lib/Socket.pm line 311.

Nicholas Clark

p5pRT commented 13 years ago

From @nwc10

My actual reaction is 2 works\, and unprintable.

On Wed\, Sep 28\, 2011 at 05​:39​:43PM +0100\, Nicholas Clark wrote​:

On Fri\, Sep 23\, 2011 at 03​:40​:25AM -0700\, perl@​profvince.com wrote​:

----------------------------------------------------------------- [Please describe your issue here]

The following code​:

perl -wle'sub reftype { "whatever" }; sub DESTROY { undef &reftype } print sub { my $guard = bless []; goto &reftype }->(\1)'

has been segfaulting since perl 5.8.0. It is not a very smart thing to do\, but it can be suprising if the undef is hidden below several abstraction layers. XSUBs suffer from the same issue\, as​:

perl -wle'use Scalar​::Util qw\; sub DESTROY { undef &reftype } print sub { my $guard = bless []; goto &reftype }->(\1)'

segfaults as well.

First example SEGV introduced by this commit​:

commit 840a7b70755d06740715e982aa756f9d77203c4e Author​: Ilya Zakharevich \ilya@&#8203;math\.berkeley\.edu Date​: Mon Dec 4 19​:40​:25 2000 -0500

Re&#8203;: \[PATCH\] The largest hoax of all times?
Date&#8203;: Tue\, 5 Dec 2000 00&#8203;:40&#8203;:25 \-0500
Message\-ID&#8203;: \<20001205004025\.A4050@&#8203;monk\.mps\.ohio\-state\.edu>

Subject&#8203;: Re&#8203;: \[PATCH\] The largest hoax of all times?
From&#8203;: Ilya Zakharevich \<ilya@&#8203;math\.ohio\-state\.edu>
Date&#8203;: Mon\, 4 Dec 2000 23&#8203;:55&#8203;:53 \-0500
Message\-ID&#8203;: \<20001204235553\.A1140@&#8203;monk\.mps\.ohio\-state\.edu>

Subject&#8203;: Re&#8203;: \[PATCH\] The largest hoax of all times?
From&#8203;: Ilya Zakharevich \<ilya@&#8203;math\.ohio\-state\.edu>
Date&#8203;: Tue\, 5 Dec 2000 01&#8203;:28&#8203;:45 \-0500
Message\-ID&#8203;: \<20001205012844\.A4227@&#8203;monk\.mps\.ohio\-state\.edu>

Fix the unpredictable order of DESTROYs\.

p4raw\-id&#8203;: //depot/perl@&#8203;7991

gdb) c Continuing. Hardware watchpoint 2​: *(AV **) 0x9afd28

Old value = (AV *) 0x9b1cd0 New value = (AV *) 0x0 Perl_cv_undef (cv=0x9b1cb8) at pad.c​:454 454 if (!SvREFCNT(cv) && CvOUTSIDE(cv)) { (gdb) where #0 Perl_cv_undef (cv=0x9b1cb8) at pad.c​:454 #1 0x00000000005b31fb in Perl_pp_undef () at pp.c​:993 #2 0x00000000004f2699 in Perl_runops_debug () at dump.c​:2199 #3 0x000000000040bf43 in Perl_call_sv (sv=0x9b1d60\, flags=45) at perl.c​:2685 #4 0x000000000058a4e3 in S_curse (sv=0x99ef68\, check_refcnt=true) at sv.c​:6260 #5 0x0000000000588279 in Perl_sv_clear (orig_sv=0x99ef68) at sv.c​:5938 #6 0x000000000058af32 in Perl_sv_free2 (sv=0x99ef68) at sv.c​:6392 #7 0x000000000059f3df in Perl_sv_unref_flags (ref=0x9b1e68\, flags=1)   at sv.c​:9460 #8 0x000000000057eaed in Perl_sv_force_normal_flags (sv=0x9b1e68\, flags=1)   at sv.c​:4786 #9 0x00000000005dec93 in Perl_leave_scope (base=2) at scope.c​:903 #10 0x00000000005fb32f in Perl_pp_goto () at pp_ctl.c​:2878 #11 0x00000000004f2699 in Perl_runops_debug () at dump.c​:2199 #12 0x000000000040ab8d in S_run_body (oldscope=1) at perl.c​:2382 #13 0x0000000000409fa9 in perl_run (my_perl=0x99c010) at perl.c​:2300 #14 0x000000000043e901 in main (argc=2\, argv=0x7fffffffe958\,   env=0x7fffffffe970) at miniperlmain.c​:119

because that's the NULL pointer that now SEGVs​:

(gdb) c Continuing.

Program received signal SIGSEGV\, Segmentation fault. 0x00000000005fb8bf in Perl_pp_goto () at pp_ctl.c​:2922 2922 PAD_SET_CUR_NOSAVE(padlist\, CvDEPTH(cv));

and the precise point of Ilya's patch was to avoid deferring the deletion of the referent (in the general case) to give order to destruction (I think at scope exit).

[Note that the way hardware watchpoints seem to end up working\, the line reported is the next executed line after the one that was interesting. So in this case it's actually line 449 of pad.c​:

  AvREAL_off(CvPADLIST(cv));   SvREFCNT_dec(MUTABLE_SV(CvPADLIST(cv)));   CvPADLIST(cv) = NULL;   }

Yet it was the deferred destruction of the CV that was protecting the ability to call it.

I'm not even sure what the right behaviour should be. [and once there are some candidates for that\, how to implement them]

Nicholas Clark

p5pRT commented 13 years ago

From perl@profvince.com

I'm not even sure what the right behaviour should be. [and once there are some candidates for that\, how to implement them] Nicholas Clark

Thanks for searching the history.

I believe the right behaviour is goto'ing into whatever is alive before destructors are processed. That's how it used to work on 5.6 (maybe at the price of numerous oddities and bugs\, I don't know enough to say)\, and is in line with what happens when the code is freed at goto() time :

  sub swish {   say 'original'   }

  sub rakkk {   local *swish = sub { say 'local' };   goto &swish;   }

  rakkk() # says "local"

To implement this\, I suspect it is enough to save CvROOT()/CvXSUB() at the beginning of pp_goto()\, then proceed to LEAVE_SCOPE()\, and then call the saved value. After all\, pp_goto() is already temporarily bumping the refcount of the CV to make the local example work\, and that strategy is quite similar.

Vincent.

p5pRT commented 13 years ago

From @nwc10

On Thu\, Sep 29\, 2011 at 09​:55​:53PM +0200\, Vincent Pit wrote​:

I'm not even sure what the right behaviour should be. [and once there are some candidates for that\, how to implement them] Nicholas Clark

Thanks for searching the history.

I believe the right behaviour is goto'ing into whatever is alive before destructors are processed. That's how it used to work on 5.6 (maybe at the price of numerous oddities and bugs\, I don't know enough to say)\, and is in line with what happens when the code is freed at goto() time :

 sub swish \{
  say 'original'
 \}

 sub rakkk \{
  local \*swish = sub \{ say 'local' \};
  goto &swish;
 \}

 rakkk\(\) \# says "local"

To implement this\, I suspect it is enough to save CvROOT()/CvXSUB() at the beginning of pp_goto()\, then proceed to LEAVE_SCOPE()\, and then call the saved value. After all\, pp_goto() is already temporarily bumping the refcount of the CV to make the local example work\, and that strategy is quite similar.

That approach is not going to restore the pre 5.8 behaviour for the example with a DESTROY that explicity uses undef on the subroutien. You'd get a "Can't undef active subroutine" error.

[Forgot to show in the gdb output that the reference count for the CV is decidedly non-zero for the example from your bug report]

Nicholas Clark

p5pRT commented 13 years ago

From @nwc10

On Thu\, Sep 29\, 2011 at 09​:55​:53PM +0200\, Vincent Pit wrote​:

I'm not even sure what the right behaviour should be. [and once there are some candidates for that\, how to implement them] Nicholas Clark

Thanks for searching the history.

But everyone should be able to search history! :-)

[So I've written Porting/bisect.pl to make it easier]

Nicholas Clark

p5pRT commented 12 years ago

From @cpansprout

On Thu Sep 29 13​:29​:31 2011\, nicholas wrote​:

On Thu\, Sep 29\, 2011 at 09​:55​:53PM +0200\, Vincent Pit wrote​:

I'm not even sure what the right behaviour should be. [and once there are some candidates for that\, how to implement them] Nicholas Clark

Thanks for searching the history.

I believe the right behaviour is goto'ing into whatever is alive before destructors are processed. That's how it used to work on 5.6 (maybe at the price of numerous oddities and bugs\, I don't know enough to say)\, and is in line with what happens when the code is freed at goto() time :

 sub swish \{
  say 'original'
 \}

 sub rakkk \{
  local \*swish = sub \{ say 'local' \};
  goto &swish;
 \}

 rakkk\(\) \# says "local"

To implement this\, I suspect it is enough to save CvROOT()/CvXSUB() at the beginning of pp_goto()\, then proceed to LEAVE_SCOPE()\, and then call the saved value. After all\, pp_goto() is already temporarily bumping the refcount of the CV to make the local example work\, and that strategy is quite similar.

That approach is not going to restore the pre 5.8 behaviour for the example with a DESTROY that explicity uses undef on the subroutien. You'd get a "Can't undef active subroutine" error.

The most logical fix\, IMO\, is to check to make sure that CvXSUB or CvROOT is still present and die with ā€˜Undefined subroutine calledā€™ instead.

--

Father Chrysostomos

p5pRT commented 12 years ago

From @cpansprout

On Sat Nov 26 22​:27​:35 2011\, sprout wrote​:

On Thu Sep 29 13​:29​:31 2011\, nicholas wrote​:

On Thu\, Sep 29\, 2011 at 09​:55​:53PM +0200\, Vincent Pit wrote​:

I'm not even sure what the right behaviour should be. [and once there are some candidates for that\, how to implement them] Nicholas Clark

Thanks for searching the history.

I believe the right behaviour is goto'ing into whatever is alive before destructors are processed. That's how it used to work on 5.6 (maybe at the price of numerous oddities and bugs\, I don't know enough to say)\, and is in line with what happens when the code is freed at goto() time :

 sub swish \{
  say 'original'
 \}

 sub rakkk \{
  local \*swish = sub \{ say 'local' \};
  goto &swish;
 \}

 rakkk\(\) \# says "local"

To implement this\, I suspect it is enough to save CvROOT()/CvXSUB() at the beginning of pp_goto()\, then proceed to LEAVE_SCOPE()\, and then call the saved value. After all\, pp_goto() is already temporarily bumping the refcount of the CV to make the local example work\, and that strategy is quite similar.

That approach is not going to restore the pre 5.8 behaviour for the example with a DESTROY that explicity uses undef on the subroutien. You'd get a "Can't undef active subroutine" error.

The most logical fix\, IMO\, is to check to make sure that CvXSUB or CvROOT is still present and die with ā€˜Undefined subroutine calledā€™ instead.

But then should it trigger autoloading? Should I duck?

--

Father Chrysostomos

p5pRT commented 12 years ago

From @cpansprout

On Sat Nov 26 22​:27​:35 2011\, sprout wrote​:

On Thu Sep 29 13​:29​:31 2011\, nicholas wrote​:

On Thu\, Sep 29\, 2011 at 09​:55​:53PM +0200\, Vincent Pit wrote​:

I'm not even sure what the right behaviour should be. [and once there are some candidates for that\, how to implement them] Nicholas Clark

Thanks for searching the history.

I believe the right behaviour is goto'ing into whatever is alive before destructors are processed. That's how it used to work on 5.6 (maybe at the price of numerous oddities and bugs\, I don't know enough to say)\, and is in line with what happens when the code is freed at goto() time :

 sub swish \{
  say 'original'
 \}

 sub rakkk \{
  local \*swish = sub \{ say 'local' \};
  goto &swish;
 \}

 rakkk\(\) \# says "local"

To implement this\, I suspect it is enough to save CvROOT()/CvXSUB() at the beginning of pp_goto()\, then proceed to LEAVE_SCOPE()\, and then call the saved value. After all\, pp_goto() is already temporarily bumping the refcount of the CV to make the local example work\, and that strategy is quite similar.

That approach is not going to restore the pre 5.8 behaviour for the example with a DESTROY that explicity uses undef on the subroutien. You'd get a "Can't undef active subroutine" error.

The most logical fix\, IMO\, is to check to make sure that CvXSUB or CvROOT is still present and die with ā€˜Undefined subroutine calledā€™ instead.

I fixed it this way\, but with the correct error message\, with commit 1d59c03.

--

Father Chrysostomos

p5pRT commented 12 years ago

@cpansprout - Status changed from 'open' to 'resolved'