Bug #89679 for PadWalker: peek_sub has bad error handling

Tue Oct 22 06:18:11 2013 zefram [...] fysh.org - Ticket created

Subject:	peek_sub has bad error handling
Date:	Tue, 22 Oct 2013 11:17:55 +0100
To:	bug-PadWalker [...] rt.cpan.org
From:	Zefram <zefram [...] fysh.org>

PadWalker::peek_sub() only works on defined Perl subs. That's inherent in its purpose. But if it's given an XS sub or undefined sub then it crashes, by dereferencing CvPADLIST on the assumption that it's always populated. It should check for these error cases and signal a catchable exception instead. -zefram

Thu Oct 24 15:50:02 2013 ribasushi [...] leporine.io - Cc RIBASUSHI added

Sun Oct 27 06:16:26 2013 robin [...] cpan.org - Correspondence added

Thanks! I think it’s okay with undefined subs, but crashes when given a sub with a null padlist. I’m uploading a fix now. Robin On Tue Oct 22 06:18:11 2013, zefram@fysh.org wrote: Show quoted text

> PadWalker::peek_sub() only works on defined Perl subs. That's inherent > in its purpose. But if it's given an XS sub or undefined sub then it > crashes, by dereferencing CvPADLIST on the assumption that it's always > populated. It should check for these error cases and signal a catchable > exception instead. > > -zefram

Sun Oct 27 06:16:27 2013 The RT System itself - Status changed from 'new' to 'open'

Sun Oct 27 08:03:19 2013 robin [...] cpan.org - Status changed from 'open' to 'resolved'

Sun Oct 27 08:03:47 2013 robin [...] cpan.org - Ticket deleted

Sun Oct 27 08:03:50 2013 robin [...] cpan.org - Status changed from 'deleted' to 'open'

Sun Oct 27 08:04:12 2013 robin [...] cpan.org - Correspondence added

Fixed in 1.97.

Sun Oct 27 08:04:52 2013 robin [...] cpan.org - Status changed from 'open' to 'resolved'

Sun Oct 27 08:04:52 2013 robin [...] cpan.org - Taken

Sun Oct 27 08:29:27 2013 zefram [...] fysh.org - Correspondence added

CC:	ribasushi [...] cpan.org
Subject:	Re: [rt.cpan.org #89679] peek_sub has bad error handling
Date:	Sun, 27 Oct 2013 12:29:05 +0000
To:	Robin Houston via RT <bug-PadWalker [...] rt.cpan.org>
From:	Zefram <zefram [...] fysh.org>

Robin Houston via RT wrote: Show quoted text

>Thanks! I think it's okay with undefined subs,

Interestingly, it turns out to depend on perl version. When making the bug report I tested with perl 5.18.1, on which peek_sub does crash on undefined subs. With PadWalker 1.96: $ perl5.16.3 -MPadWalker=peek_sub -lwe 'peek_sub(\&foo); print "OK"' OK $ perl5.18.1 -MPadWalker=peek_sub -lwe 'peek_sub(\&foo); print "OK"' zsh: segmentation fault perl5.18.1 -MPadWalker=peek_sub -lwe Don't know if you've fixed that in PW 1.97. -zefram

Sun Oct 27 08:36:24 2013 robin.houston [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #89679] peek_sub has bad error handling
Date:	Sun, 27 Oct 2013 12:35:50 +0000
To:	bug-PadWalker [...] rt.cpan.org
From:	Robin Houston <robin.houston [...] gmail.com>

Ah! I probably haven’t. I didn’t test with 5.18. I’ll try now. Thanks. Robin On 27 October 2013 12:29, Zefram via RT <bug-PadWalker@rt.cpan.org> wrote: Show quoted text

> Queue: PadWalker > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=89679 > > > Robin Houston via RT wrote:

> >Thanks! I think it's okay with undefined subs,

> > Interestingly, it turns out to depend on perl version. When making the > bug report I tested with perl 5.18.1, on which peek_sub does crash on > undefined subs. With PadWalker 1.96: > > $ perl5.16.3 -MPadWalker=peek_sub -lwe 'peek_sub(\&foo); print "OK"' > OK > $ perl5.18.1 -MPadWalker=peek_sub -lwe 'peek_sub(\&foo); print "OK"' > zsh: segmentation fault perl5.18.1 -MPadWalker=peek_sub -lwe > > Don't know if you've fixed that in PW 1.97. > > -zefram > >

Sun Oct 27 11:15:00 2013 robin [...] cpan.org - Correspondence added

I can’t reproduce it here with 5.18.1 (see below). What is your perl -V? Thanks again for reporting this. Robin PadWalker  perl5.18.1 -Mblib -MPadWalker=peek_sub -lwe 'peek_sub(\&foo); print "OK"' PadWalker: cv has no padlist at -e line 1. PadWalker  perl5.18.1 -V Summary of my perl5 (revision 5 version 18 subversion 1) configuration: Commit id: f397717ad61dbc2f0de8d8bd25438708e86ab7ad Platform: osname=darwin, osvers=13.0.0, archname=darwin-2level uname='darwin robins-mbp.home 13.0.0 darwin kernel version 13.0.0: thu sep 19 22:22:27 pdt 2013; root:xnu-2422.1.72~6release_x86_64 x86_64 ' config_args='-ders' 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='cc', ccflags ='-fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include', optimize='-O3', cppflags='-fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='', gccversion='4.2.1 Compatible Apple LLVM 5.0 (clang-500.2.79)', 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='env MACOSX_DEPLOYMENT_TARGET=10.3 cc', ldflags =' -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib /usr/lib libs=-lgdbm -ldbm -ldl -lm -lutil -lc perllibs=-ldl -lm -lutil -lc libc=, so=dylib, useshrplib=false, libperl=libperl.a gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib -fstack-protector' Characteristics of this binary (from libperl): Compile-time options: HAS_TIMES PERLIO_LAYERS PERL_DONT_CREATE_GVSV PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_MALLOC_WRAP PERL_PRESERVE_IVUV PERL_SAWAMPERSAND USE_64_BIT_ALL USE_64_BIT_INT USE_LARGE_FILES USE_LOCALE USE_LOCALE_COLLATE USE_LOCALE_CTYPE USE_LOCALE_NUMERIC USE_PERLIO USE_PERL_ATOF Built under darwin Compiled at Oct 27 2013 14:23:28 @INC: /usr/local/lib/perl5/site_perl/5.18.1/darwin-2level /usr/local/lib/perl5/site_perl/5.18.1 /usr/local/lib/perl5/5.18.1/darwin-2level /usr/local/lib/perl5/5.18.1 /usr/local/lib/perl5/site_perl/5.16.1 /usr/local/lib/perl5/site_perl/5.15.1 /usr/local/lib/perl5/site_perl/5.14.2 /usr/local/lib/perl5/site_perl/5.14.0 /usr/local/lib/perl5/site_perl/5.8.2 /usr/local/lib/perl5/site_perl/5.8.1 /usr/local/lib/perl5/site_perl .

Sun Oct 27 11:15:10 2013 paul [...] city-fan.org - Correspondence added

From:

paul [...] city-fan.org

Test suite fails for me (one of the newly-added tests) with 5.12.x or anything older: $ make test PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0, 'blib/lib', 'blib/arch')" t/*.t t/closure.....ok t/dm..........ok t/foo.........ok t/our.........ok t/recurse.....ok t/sub.........FAILED test 5 Failed 1/6 tests, 83.33% okay t/test........ok t/tt..........ok t/var_name....ok Failed 1/9 test scripts, 88.89% okay. 1/81 subtests failed, 98.77% okay. Failed Test Stat Wstat Total Fail Failed List of Failed ------------------------------------------------------------------------------- t/sub.t 6 1 16.67% 5

Sun Oct 27 11:16:30 2013 robin [...] cpan.org - Correspondence added

Oh, never mind! This is the new error message I just added. So yes, this case should also be fixed in PadWalker 1.97. Robin

Sun Oct 27 11:16:30 2013 robin [...] cpan.org - Fixed in 1.97 added

Sun Oct 27 11:19:29 2013 robin [...] cpan.org - Correspondence added

Thanks Paul! Interesting. All tests are passing here with 5.12.5 and 5.8.2. Which version(s) exactly do you see that problem with? Any non-default compilation options? Robin

Sun Oct 27 11:38:28 2013 paul [...] city-fan.org - Correspondence added

From:

paul [...] city-fan.org

On Sun Oct 27 11:19:29 2013, ROBIN wrote: Show quoted text

> Thanks Paul! Interesting. All tests are passing here with 5.12.5 and > 5.8.2. Which version(s) exactly do you see that problem with? Any non- > default compilation options?

Various versions of Red Hat / Fedora perl from 5.8.5 through to 5.18.1. They're all threaded, and the 5.8.x, 5.10.x and 5.12.x builds all fail the same way, and the 5.16.x and 5.18.x builds all pass. Anything I can patch in to provide more debug info?

Sun Oct 27 11:47:46 2013 robin.houston [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #89679] peek_sub has bad error handling
Date:	Sun, 27 Oct 2013 15:47:11 +0000
To:	bug-PadWalker [...] rt.cpan.org
From:	Robin Houston <robin.houston [...] gmail.com>

If you can get a backtrace from the crash, that would be really helpful – but I guess that depends how easily you can get your hands on a debug build of perl that has this problem. Robin On 27 October 2013 15:38, paul@city-fan.org via RT < bug-PadWalker@rt.cpan.org> wrote: Show quoted text

> Queue: PadWalker > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=89679 > > > On Sun Oct 27 11:19:29 2013, ROBIN wrote:

> > Thanks Paul! Interesting. All tests are passing here with 5.12.5 and > > 5.8.2. Which version(s) exactly do you see that problem with? Any non- > > default compilation options?

> > Various versions of Red Hat / Fedora perl from 5.8.5 through to 5.18.1. > They're all threaded, and the 5.8.x, 5.10.x and 5.12.x builds all fail the > same way, and the 5.16.x and 5.18.x builds all pass. > > Anything I can patch in to provide more debug info? > >

Sun Oct 27 12:20:50 2013 robin.houston [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #89679] peek_sub has bad error handling
Date:	Sun, 27 Oct 2013 16:20:16 +0000
To:	bug-PadWalker [...] rt.cpan.org
From:	Robin Houston <robin.houston [...] gmail.com>

Okay, I can reproduce this with -Dusethreads. Looking into it now. Thanks! Robin On 27 October 2013 15:38, paul@city-fan.org via RT < bug-PadWalker@rt.cpan.org> wrote: Show quoted text

> Queue: PadWalker > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=89679 > > > On Sun Oct 27 11:19:29 2013, ROBIN wrote:

> > Thanks Paul! Interesting. All tests are passing here with 5.12.5 and > > 5.8.2. Which version(s) exactly do you see that problem with? Any non- > > default compilation options?

> > Various versions of Red Hat / Fedora perl from 5.8.5 through to 5.18.1. > They're all threaded, and the 5.8.x, 5.10.x and 5.12.x builds all fail the > same way, and the 5.16.x and 5.18.x builds all pass. > > Anything I can patch in to provide more debug info? > >

Sun Oct 27 12:34:42 2013 paul [...] city-fan.org - Correspondence added

From:

paul [...] city-fan.org

On Sun Oct 27 12:20:50 2013, robin.houston@gmail.com wrote: Show quoted text

> Okay, I can reproduce this with -Dusethreads. Looking into it now. Thanks! > > Robin > > > On 27 October 2013 15:38, paul@city-fan.org via RT < > bug-PadWalker@rt.cpan.org> wrote: >

> > Queue: PadWalker > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=89679 > > > > > On Sun Oct 27 11:19:29 2013, ROBIN wrote:

> > > Thanks Paul! Interesting. All tests are passing here with 5.12.5 and > > > 5.8.2. Which version(s) exactly do you see that problem with? Any non- > > > default compilation options?

> > > > Various versions of Red Hat / Fedora perl from 5.8.5 through to 5.18.1. > > They're all threaded, and the 5.8.x, 5.10.x and 5.12.x builds all fail the > > same way, and the 5.16.x and 5.18.x builds all pass. > > > > Anything I can patch in to provide more debug info?

Turns out the older perls have "code" rather than "CODE" in the error message, so a simple patch to the test has fixed this for me.

Subject:

PadWalker-1.97-compat.patch

--- t/sub.t +++ t/sub.t @@ -51,7 +51,7 @@ # Make sure it correctly signals an exception if the sub is not a Perl sub eval { peek_sub(undef); }; -print (($@ =~ /cv is not a CODE reference/) ? "ok 5\n" : "not ok 5\n"); +print (($@ =~ /cv is not a CODE reference/i) ? "ok 5\n" : "not ok 5\t# $@\n"); eval { peek_sub(\&peek_sub); }; print (($@ =~ /cv has no padlist/) ? "ok 6\n" : "not ok 6\n");

Sun Oct 27 12:44:40 2013 robin.houston [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #89679] peek_sub has bad error handling
Date:	Sun, 27 Oct 2013 16:44:08 +0000
To:	bug-PadWalker [...] rt.cpan.org
From:	Robin Houston <robin.houston [...] gmail.com>

Thanks Paul. I just noticed the same thing. I was getting very confused, because it turns out this error message is inserted by the xs-to-c compiler. This has the very confusing effect that if you try to switch versions by running perl5.xx.y Makefile.PL && make test for example, the xs compilation is *not* redone, only the C is recompiled, which is why the tests continued to pass even when I tried other versions. Robin On 27 October 2013 16:34, paul@city-fan.org via RT < bug-PadWalker@rt.cpan.org> wrote: Show quoted text

> Queue: PadWalker > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=89679 > > > On Sun Oct 27 12:20:50 2013, robin.houston@gmail.com wrote:

> > Okay, I can reproduce this with -Dusethreads. Looking into it now.

> Thanks!

> > > > Robin > > > > > > On 27 October 2013 15:38, paul@city-fan.org via RT < > > bug-PadWalker@rt.cpan.org> wrote: > >

> > > Queue: PadWalker > > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=89679 > > > > > > > On Sun Oct 27 11:19:29 2013, ROBIN wrote:

> > > > Thanks Paul! Interesting. All tests are passing here with 5.12.5 and > > > > 5.8.2. Which version(s) exactly do you see that problem with? Any

> non-

> > > > default compilation options?

> > > > > > Various versions of Red Hat / Fedora perl from 5.8.5 through to 5.18.1. > > > They're all threaded, and the 5.8.x, 5.10.x and 5.12.x builds all fail

> the

> > > same way, and the 5.16.x and 5.18.x builds all pass. > > > > > > Anything I can patch in to provide more debug info?

> > Turns out the older perls have "code" rather than "CODE" in the error > message, so a simple patch to the test has fixed this for me. > > >

Sun Oct 27 12:51:50 2013 robin [...] cpan.org - Correspondence added

Version 1.98 is now on CPAN. Fingers crossed this one is okay!

Sun Oct 27 13:28:01 2013 paul [...] city-fan.org - Correspondence added

From:

paul [...] city-fan.org

On Sun Oct 27 12:51:50 2013, ROBIN wrote: Show quoted text

> Version 1.98 is now on CPAN. Fingers crossed this one is okay!

Works for me.