Skip Menu |
Report information
Id: 132788
Status: open
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: slaven [at] rezic.de
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: (no value)

Attachments
0001-Ensure-we-don-t-unnecessarily-vivify-UNIVERSAL-isa.patch



To: perlbug [...] perl.org
From: slaven [...] rezic.de
Date: Tue, 30 Jan 2018 07:43:39 +0100
Subject: Blead Breaks CPAN: LEMBARK/Object-Trampoline-1.42.tar.gz
CC: srezic [...] cpan.org
Download (untitled) / with headers
text/plain 3.3k
This is a bug report for perl from slaven@rezic.de, generated with the help of perlbug 1.41 running under perl 5.27.8. ----------------------------------------------------------------- The test t/03-Universal-Override.t in Object-Trampoline-1.42 fails with 5.27.7 and 5.27.8, and was OK until 5.27.6: # Failed test 'Object is 'Object::Trampoline::Bounce' (Foo::Bar)' # at t/03-Universal-Override.t line 21. # Looks like you failed 1 test of 5. t/03-Universal-Override.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/5 subtests ----------------------------------------------------------------- --- Flags: category=core severity=low --- Site configuration information for perl 5.27.8: Configured by eserte at Sat Jan 20 09:22:10 CET 2018. Summary of my perl5 (revision 5 version 27 subversion 8) configuration: Platform: osname=linux osvers=3.16.0-4-amd64 archname=x86_64-linux uname='linux cabulja 3.16.0-4-amd64 #1 smp debian 3.16.51-3 (2017-12-13) x86_64 gnulinux ' config_args='-ds -e -Dprefix=/opt/perl-5.27.8 -Dusedevel -Dusemallocwrap=no -Dcf_email=srezic@cpan.org' hint=recommended useposix=true d_sigaction=define useithreads=undef usemultiplicity=undef use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n default_inc_excludes_dot=define bincompat5005=undef Compiler: cc='cc' ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2' optimize='-O2' cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion='' gccversion='4.9.2' gccosandvers='' intsize=4 longsize=8 ptrsize=8 doublesize=8 byteorder=12345678 doublekind=3 d_longlong=define longlongsize=8 d_longdbl=define longdblsize=16 longdblkind=3 ivtype='long' ivsize=8 nvtype='double' nvsize=8 Off_t='off_t' lseeksize=8 alignbytes=8 prototype=define Linker and Libraries: ld='cc' ldflags =' -fstack-protector-strong -L/usr/local/lib' libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/4.9/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc libc=libc-2.19.so so=so useshrplib=false libperl=libperl.a gnulibc_version='2.19' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags='-Wl,-E' cccdlflags='-fPIC' lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong' --- @INC for perl 5.27.8: /opt/perl-5.27.8/lib/site_perl/5.27.8/x86_64-linux /opt/perl-5.27.8/lib/site_perl/5.27.8 /opt/perl-5.27.8/lib/5.27.8/x86_64-linux /opt/perl-5.27.8/lib/5.27.8 --- Environment for perl 5.27.8: HOME=/home/eserte LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/home/eserte/bin/linux-gnu:/home/eserte/bin/sh:/home/eserte/bin:/home/eserte/bin/pistachio-perl/bin:/usr/games:/home/eserte/devel PERLDOC=-MPod::Perldoc::ToTextOverstrike PERL_BADLANG (unset) SHELL=/bin/zsh
Date: Tue, 30 Jan 2018 07:13:25 +0000
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #132788] Blead Breaks CPAN: LEMBARK/Object-Trampoline-1.42.tar.gz
To: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 919b
slaven@rezic.de wrote: Show quoted text
>The test t/03-Universal-Override.t in Object-Trampoline-1.42 >fails with 5.27.7 and 5.27.8, and was OK until 5.27.6:
Bisects to commit 915a6810d3e3198d759f025f85d1fd6f3171dd27 "Carp: optimize format_arg when arguments contain many references". The issue is that the new Carp code in that commit refers to $UNIVERSAL::isa::VERSION, and so vivifies $UNIVERSAL::{"isa::"}. The test iterates over %UNIVERSAL::, using each key found as a method name, and isn't prepared for there to be a key that doesn't syntactically behave as a method name. The test is erroneous in using all keys it finds as method names. It should skip ones that aren't syntactically OK. The edit to Carp is also arguably faulty. In other areas Carp goes to significant effort to avoid vivifying stash entries that it doesn't intend to. It should probably apply similar logic in this search for &UNIVERSAL::isa. -zefram
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.1k
On Tue, 30 Jan 2018 07:13:47 GMT, zefram@fysh.org wrote: Show quoted text
> slaven@rezic.de wrote:
> >The test t/03-Universal-Override.t in Object-Trampoline-1.42 > >fails with 5.27.7 and 5.27.8, and was OK until 5.27.6:
> > Bisects to commit 915a6810d3e3198d759f025f85d1fd6f3171dd27 "Carp: optimize > format_arg when arguments contain many references". The issue is that > the new Carp code in that commit refers to $UNIVERSAL::isa::VERSION, and > so vivifies $UNIVERSAL::{"isa::"}. The test iterates over %UNIVERSAL::, > using each key found as a method name, and isn't prepared for there to > be a key that doesn't syntactically behave as a method name. > > The test is erroneous in using all keys it finds as method names. > It should skip ones that aren't syntactically OK. > > The edit to Carp is also arguably faulty. In other areas Carp goes > to significant effort to avoid vivifying stash entries that it doesn't > intend to. It should probably apply similar logic in this search for > &UNIVERSAL::isa. > > -zefram
Are you saying we should apply something like the patch attached? Thank you very much. -- James E Keenan (jkeenan@cpan.org)
Subject: 0001-Ensure-we-don-t-unnecessarily-vivify-UNIVERSAL-isa.patch
From 7d30fb46959b73e8ed43fc7ee0eec5775bf8c816 Mon Sep 17 00:00:00 2001 From: James E Keenan <jkeenan@cpan.org> Date: Wed, 31 Jan 2018 09:52:57 -0500 Subject: [PATCH] Ensure we don't unnecessarily vivify $UNIVERSAL::{"isa::"}. In response to issue raised in RT #132788 --- dist/Carp/lib/Carp.pm | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dist/Carp/lib/Carp.pm b/dist/Carp/lib/Carp.pm index 65d22b1..f31b5be 100644 --- a/dist/Carp/lib/Carp.pm +++ b/dist/Carp/lib/Carp.pm @@ -116,7 +116,7 @@ BEGIN { ; } -our $VERSION = '1.45'; +our $VERSION = '1.46'; $VERSION =~ tr/_//d; our $MaxEvalLen = 0; @@ -286,7 +286,9 @@ sub format_arg { # lazy check if the CPAN module UNIVERSAL::isa is used or not # if we use a rogue version of UNIVERSAL this would lead to infinite loop - my $isa = $UNIVERSAL::isa::VERSION ? sub { 1 } : \&UNIVERSAL::isa; + my $isa = (exists $UNIVERSAL::{"isa::"} and $UNIVERSAL::isa::VERSION) + ? sub { 1 } + : \&UNIVERSAL::isa; # legitimate, let's not leak it. if (!$in_recurse && $isa->( $arg, 'UNIVERSAL' ) && -- 2.7.4
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.2k
On Wed, 31 Jan 2018 14:57:32 GMT, jkeenan wrote: Show quoted text
> On Tue, 30 Jan 2018 07:13:47 GMT, zefram@fysh.org wrote:
> > slaven@rezic.de wrote:
> > >The test t/03-Universal-Override.t in Object-Trampoline-1.42 > > >fails with 5.27.7 and 5.27.8, and was OK until 5.27.6:
> > > > Bisects to commit 915a6810d3e3198d759f025f85d1fd6f3171dd27 "Carp: optimize > > format_arg when arguments contain many references". The issue is that > > the new Carp code in that commit refers to $UNIVERSAL::isa::VERSION, and > > so vivifies $UNIVERSAL::{"isa::"}. The test iterates over %UNIVERSAL::, > > using each key found as a method name, and isn't prepared for there to > > be a key that doesn't syntactically behave as a method name. > > > > The test is erroneous in using all keys it finds as method names. > > It should skip ones that aren't syntactically OK. > > > > The edit to Carp is also arguably faulty. In other areas Carp goes > > to significant effort to avoid vivifying stash entries that it doesn't > > intend to. It should probably apply similar logic in this search for > > &UNIVERSAL::isa. > > > > -zefram
> > Are you saying we should apply something like the patch attached? > > Thank you very much.
In the branch, to avert a test failure I have also incremented $Carp::Heavy::VERSION. -- James E Keenan (jkeenan@cpan.org)
From: Zefram <zefram [...] fysh.org>
Date: Thu, 1 Feb 2018 15:17:42 +0000
Subject: Re: [perl #132788] Blead Breaks CPAN: LEMBARK/Object-Trampoline-1.42.tar.gz
To: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 413b
James E Keenan via RT wrote: Show quoted text
>Are you saying we should apply something like the patch attached?
... Show quoted text
>+ my $isa = (exists $UNIVERSAL::{"isa::"} and $UNIVERSAL::isa::VERSION)
That doesn't help: the reference to $UNIVERSAL::isa::VERSION still vivifies $UNIVERSAL::{"isa::"} at compile time. It takes more to avoid that. Look at the other vivification-avoiding code in Carp, and the tests thereof. -zefram
To: perl5-porters [...] perl.org
Subject: Re: [perl #132788] Blead Breaks CPAN: LEMBARK/Object-Trampoline-1.42.tar.gz
From: Zefram <zefram [...] fysh.org>
Date: Thu, 15 Feb 2018 18:32:14 +0000
Download (untitled) / with headers
text/plain 275b
I wrote: Show quoted text
>The test is erroneous in using all keys it finds as method names. >It should skip ones that aren't syntactically OK.
Reported as [rt.cpan.org #124441]. Show quoted text
>The edit to Carp is also arguably faulty.
Fixed in commit 682f3ac7d3c98fe1aa251a2e30684eeb7b859a0e. -zefram
From: demerphq <demerphq [...] gmail.com>
To: Perl5 Porteros <perl5-porters [...] perl.org>
Date: Sat, 17 Feb 2018 03:53:38 +0100
Subject: Fwd: Re: [perl #132788] Blead Breaks CPAN: LEMBARK/Object-Trampoline-1.42.tar.gz
Download (untitled) / with headers
text/plain 1.7k
I accidentally sent this response only to Zefram.... Resend
---------- Forwarded message ----------
From: "demerphq" <demerphq@gmail.com>
Date: 16 Feb 2018 15:48
Subject: Re: [perl #132788] Blead Breaks CPAN: LEMBARK/Object-Trampoline-1.42.tar.gz
To: "Zefram" <zefram@fysh.org>
Cc:

Show quoted text


On 30 Jan 2018 15:13, "Zefram" <zefram@fysh.org> wrote:
slaven@rezic.de wrote:
>The test t/03-Universal-Override.t in Object-Trampoline-1.42
>fails with 5.27.7 and 5.27.8, and was OK until 5.27.6:

Bisects to commit 915a6810d3e3198d759f025f85d1fd6f3171dd27 "Carp: optimize
format_arg when arguments contain many references".  The issue is that
the new Carp code in that commit refers to $UNIVERSAL::isa::VERSION, and
so vivifies $UNIVERSAL::{"isa::"}.  The test iterates over %UNIVERSAL::,
using each key found as a method name, and isn't prepared for there to
be a key that doesn't syntactically behave as a method name.

The test is erroneous in using all keys it finds as method names.
It should skip ones that aren't syntactically OK.

The edit to Carp is also arguably faulty.  In other areas Carp goes
to significant effort to avoid vivifying stash entries that it doesn't
intend to.  It should probably apply similar logic in this search for
&UNIVERSAL::isa.

I don't think such restrictions are tenable. They mean that in some cases you can use Carp to trigger c stack overflow and thus a segv. I will soon be pushing patches that fix this and they will almost certainly result in loading modules and modifying stash entries.

It is unfortunate that so many critical functions and checks in Perl require namespace mutations, but that's how things work, and imo not touching other namespaces has a *much* lower priority than Carp working correctly and not segfaulting.

Yves

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 383b
On Fri, 16 Feb 2018 18:53:47 -0800, demerphq wrote: Show quoted text
> > It is unfortunate that so many critical functions and checks in Perl > > require namespace mutations, but that's how things work, and imo not > > touching other namespaces has a *much* lower priority than Carp working > > correctly and not segfaulting.
Why cannot we have both? You are being vague. -- Father Chrysostomos
From: demerphq <demerphq [...] gmail.com>
To: Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Subject: Re: [perl #132788] Blead Breaks CPAN: LEMBARK/Object-Trampoline-1.42.tar.gz
Date: Sat, 17 Feb 2018 15:55:54 +0100
CC: Perl5 Porteros <perl5-porters [...] perl.org>
Download (untitled) / with headers
text/plain 3.4k
Download (untitled) / with headers
text/html 15.9k

Message body is not shown because it is too large.

To: Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Subject: Re: [perl #132788] Blead Breaks CPAN: LEMBARK/Object-Trampoline-1.42.tar.gz
Date: Sun, 18 Feb 2018 11:44:41 +0100
CC: Perl5 Porteros <perl5-porters [...] perl.org>
From: demerphq <demerphq [...] gmail.com>
Download (untitled) / with headers
text/plain 1.6k
On 17 February 2018 at 15:55, demerphq <demerphq@gmail.com> wrote: Show quoted text
> On 17 Feb 2018 21:36, "Father Chrysostomos via RT" > <perlbug-followup@perl.org> wrote: > > On Fri, 16 Feb 2018 18:53:47 -0800, demerphq wrote:
>> > It is unfortunate that so many critical functions and checks in Perl >> > require namespace mutations, but that's how things work, and imo not >> > touching other namespaces has a *much* lower priority than Carp working >> > correctly and not segfaulting.
> > Why cannot we have both? You are being vague > > > Carp incorrectly assumes that the only way you can have an overloaded object > is if you have loaded overload.pm. > > Which means that this segfaults due to stack overflow: > > perl -MCarp -E 'my $p = "OverloadedInXS"; *{$p."::(("} = sub{}; > *{$p.q!::(""!} = sub { Carp::cluck "<My Stringify>" }; sub { > Carp::cluck("") }->(bless {}, $p);' > > See #132828. > > To fix that without loading overload.pm I would have to more or less extract > a nice chunk of the internals of overload.pm into Carp.
After writing I revisited solving it without loading overload.pm, which I was able to do more or less, with only two test result changes: ../cpan/Encode/t/truncated_utf8.t (Wstat: 0 Tests: 9 Failed: 0) TODO passed: 9 ../cpan/Math-BigInt/t/mbimbf.t (Wstat: 256 Tests: 738 Failed: 1) Failed test: 718 Non-zero exit status: 1 It is interesting that a change to Carp fixes cpan/Encode/t/truncated_utf8.t On the other hand, I havent figured out what to do about Math-BigInt and I am still digging into why it fails witth the "tricky" logic I am doing, but not when using overload::StrVal. Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
To: demerphq <demerphq [...] gmail.com>, Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Subject: Re: [perl #132788] Blead Breaks CPAN: LEMBARK/Object-Trampoline-1.42.tar.gz
Date: Sun, 18 Feb 2018 12:58:00 -0700
CC: Perl5 Porteros <perl5-porters [...] perl.org>
From: Karl Williamson <public [...] khwilliamson.com>
Download (untitled) / with headers
text/plain 196b
On 02/18/2018 03:44 AM, demerphq wrote: Show quoted text
> It is interesting that a change to Carp fixes cpan/Encode/t/truncated_utf8.t
Carp did not fix this; commit c31ca2013f287840fcddf498ead9602666569966 did.
Date: Sun, 18 Feb 2018 22:39:55 +0000
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #132788] Blead Breaks CPAN: LEMBARK/Object-Trampoline-1.42.tar.gz
To: Perl5 Porteros <perl5-porters [...] perl.org>
Download (untitled) / with headers
text/plain 2.2k
demerphq wrote: Show quoted text
>To fix that without loading overload.pm I would have to more or less >extract a nice chunk of the internals of overload.pm into Carp.
It would be reasonable to load overload.pm when a reference arg is seen, in order to deal with that situation. However, it doesn't need to load overload.pm until that situation arises, and should not. It shouldn't impose the loading of overload.pm on programs that don't make stack traces. There's a general principle in Carp of being minimal in what it loads, because of its unique role and its ubiquity. Show quoted text
>Some of them appear to be related to back-compat, but some of them are >just egregious reimplementation of another modules functionality for the >purpose of avoiding that module:
No, they're not. The downgrade example isn't about whether the utf8.pm module is loaded, because that's not how utf8::downgrade() gets defined. utf8::downgrade() is defined by the core, regardless of the utf8.pm module, on any Perl from 5.7.1 onwards. The alternate implementation exists for compatibility to Perl 5.6, which is still a consideration for CPAN releases (though we're now on the tail end of that era). The circumspectness about looking for utf8::downgrade() is to avoid vivifying the unwanted utf8:: stash on 5.6. The optionality of utf8::is_utf8() is likewise for 5.6 compat. The optionality of B::svref_2object() *is* to avoid a dependency, but it's not reimplemented if not available: the extra information that it would have supplied is just omitted from a rare diagnostic message. The optionality of overload::StrVal() is, as you noted, predicated on the incorrect assumption that overloading only happens with overload.pm loaded: if that had been correct then it would mean that overload.pm wasn't required unless it was already loaded, so a dependency was avoidable. Show quoted text
> I plan to just use >overload.pm and break this expectation.
Don't just do that. Be cautious; follow the precedent of minimising module loading and avoiding stash vivification. Carp is quite a subtle module, in response to a unique set of constraints arising from its role as the standard error-handling module. Heavy-handed editing is likely to do damage. -zefram


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org