Skip Menu |
Report information
Id: 132545
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: pipcet [at] gmail.com
Cc:
AdminCc:

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



To: perlbug [...] perl.org
Date: Fri, 8 Dec 2017 08:43:58 +0000
Subject: Double macro argument evaluation in S_init_main_stash
From: Pip Cet <pipcet [...] gmail.com>
Download (untitled) / with headers
text/plain 4.2k
This is a bug report for perl from pipcet@gmail.com, generated with the help of perlbug 1.41 running under perl 5.27.7. ----------------------------------------------------------------- [Please describe your issue here] I just came across the following problem: S_init_main_stash contains the code: PL_curstash = PL_defstash = (HV *)SvREFCNT_inc_simple_NN(newHV()); SvREFCNT_inc_simple_NN is defined as: #define SvREFCNT_inc_simple_NN(sv) (++(SvREFCNT(sv)),MUTABLE_SV(sv)) That evaluates its argument twice, which breaks with side effects. So newHV() is called twice, one of the new HVs gets refcount 2 and gets leaked, the other gets refcount 1 and gets stored in two places. I suspect this is low-priority, but there might be a way to actually destroy the stash while it is still stored in PL_defstash, in which case it's a user-visible bug. Sorry if this message gets duplicated, I tried sending it a few days ago and it appears to have vanished. [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=core severity=low --- Site configuration information for perl 5.27.7: Configured by pip at Mon Dec 4 13:27:33 UTC 2017. Summary of my perl5 (revision 5 version 27 subversion 7) configuration: Platform: osname=linux osvers=4.13.0-1-amd64 archname=x86_64-linux uname='linux amygdala 4.13.0-1-amd64 #1 smp debian 4.13.10-1 (2017-10-30) x86_64 gnulinux ' config_args='' 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='g++' ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/home/pip/git/emacs-c++/js/dist/include -pthread -g3 -ggdb' optimize='-O2' cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -pthread' ccversion='' gccversion='8.0.0 20171124 (experimental)' 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='ld' ldflags =' -fstack-protector-strong -g3 -ggdb -L/usr/local/lib' libpth=/usr/local/lib/gcc/x86_64-pc-linux-gnu/8.0.0/../../../../include/c++/8.0.0 /usr/local/lib/gcc/x86_64-pc-linux-gnu/8.0.0/../../../../include/c++/8.0.0/x86_64-pc-linux-gnu /usr/local/lib/gcc/x86_64-pc-linux-gnu/8.0.0/../../../../include/c++/8.0.0/backward /usr/local/lib /usr/local/lib/gcc/x86_64-pc-linux-gnu/8.0.0/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 /lib64 /usr/lib64 /usr/local/lib64 libs=-lpthread -pthread -lnsl -ldl -lm -lcrypt -lutil -lc -lmozjs-58a1 -Wl,--whole-archive mozglue.a -Wl,--no-whole-archive perllibs=-lpthread -pthread -lnsl -ldl -lm -lcrypt -lutil -lc -L /home/pip/git/emacs-c++/js/dist/bin -lmozjs-58a1 -Wl,--whole-archive /home/pip/git/emacs-c++/js/mozglue/build/libmozglue.a -Wl,--no-whole-archive libc=libc-2.25.so so=so useshrplib=false libperl=libperl.a gnulibc_version='2.25' Dynamic Linking: dlsrc=dl_none.xs dlext=none d_dlsymun=undef ccdlflags='' cccdlflags='' lddlflags='' --- @INC for perl 5.27.7: . lib /usr/local/lib/perl5/site_perl/5.27.7/x86_64-linux /usr/local/lib/perl5/site_perl/5.27.7 /usr/local/lib/perl5/5.27.7/x86_64-linux /usr/local/lib/perl5/5.27.7 --- Environment for perl 5.27.7: HOME=/home/pip LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH=/home/pip/git/emacs-c++/js/dist/bin:/home/pip/amd/AMDAPPSDK-3.0/lib/x86_64/ LOGDIR (unset) PATH=/home/pip/.cargo/bin:/home/pip/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/games:/sbin:/usr/local/sbin:/usr/sbin:/sbin:/usr/local/sbin:/usr/sbin PERL_BADLANG (unset) SHELL=/bin/bash
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 130b
Thanks very much for your report. I believe this is fixed as of commit 9842f1a008ea4490ae903e34ed554be5e0ffc139. -- Aaron Crane
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 968b
On Fri, 08 Dec 2017 02:28:38 -0800, arc wrote: Show quoted text
> Thanks very much for your report. I believe this is fixed as of commit > 9842f1a008ea4490ae903e34ed554be5e0ffc139.
Thank you for the quick fix. I was guilty of making that mistake, in this commit: commit 03d9f026ae253e9e69212a3cf6f1944437e9f070 Author: Father Chrysostomos <sprout@cpan.org> Date: Sat Oct 22 11:06:35 2011 -0700 [perl #101486] Make PL_curstash refcounted This stops PL_curstash from pointing to a freed-and-reused scalar in cases like ‘package Foo; BEGIN {*Foo:: = *Bar::}’. In such cases, another BEGIN block, or any subroutine definition, would cause a crash. Now it just happily proceeds. newATTRSUB and newXS have been modified not to call mro_method_changed_in in such cases, as it doesn’t make sense. This was when I added refcounting to PL_curstash to begin with. So I wonder whether it is possible to test the fix. -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 440b
On Fri, 08 Dec 2017 02:28:38 -0800, arc wrote: Show quoted text
> Thanks very much for your report. I believe this is fixed as of commit > 9842f1a008ea4490ae903e34ed554be5e0ffc139.
Is there also some portable way to fix the double-evaluation of the macro, or can we use some naming convention to call out the macros that will do multiple evaluation? Are we at the point where our target set of compilers can all handle inline functions, for example? Hugo
Subject: Re: [perl #132545] Double macro argument evaluation in S_init_main_stash
Date: Sun, 10 Dec 2017 19:02:16 +0000
To: perlbug-followup [...] perl.org
CC: perl5-porters [...] perl.org
From: Pip Cet <pipcet [...] gmail.com>
Download (untitled) / with headers
text/plain 442b
On Fri, Dec 8, 2017 at 2:36 PM, Father Chrysostomos via RT <perlbug-followup@perl.org> wrote: Show quoted text
> This was when I added refcounting to PL_curstash to begin with. So I wonder whether it is possible to test the fix.
I don't think it is. The code a few lines down from where the bug was increments PL_defstash's refcount again, for *INC, which is unfreeable, so PL_curstash never noticed that its refcount was one less than it should have been.
Subject: Re: [perl #132545] Double macro argument evaluation in S_init_main_stash
From: Zefram <zefram [...] fysh.org>
Date: Sun, 10 Dec 2017 19:09:28 +0000
To: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 855b
Hugo van der Sanden via RT wrote: Show quoted text
>Is there also some portable way to fix the double-evaluation of the >macro, or can we use some naming convention to call out the macros that >will do multiple evaluation?
We do the latter, to some extent. The SvREFCNT_inc() macros with "_simple" in their name are permitted to double evaluate: the "_simple" indicates that the caller guarantees that the argument is simple enough. So another way to fix this particular bug would have been to drop the "_simple". With some other macros we have the convention of an "x" suffix to indicate that the macro will not double evaluate: for example, SvIVx() as the single-evaluation version of SvIV(). Show quoted text
>Are we at the point where our target set of compilers can all handle >inline functions, for example?
They can all handle static functions, which is close enough. -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