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

Owner: Nobody
Requestors: andreas.koenig.7os6VVqR [at] franz.ak.mind.de
Cc:
AdminCc:

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

Attachments
0002-PATCH-perl-130921-Don-t-use-Newx-in-decoding-UTF-8.patch



Date: Sun, 05 Mar 2017 06:48:36 +0100
To: perlbug [...] perl.org
From: Andreas Koenig <andreas.koenig.7os6VVqR [...] franz.ak.mind.de>
Subject: Bleadperl v5.25.5-100-g2b5e7bc2e6 breaks JDDPAUSE/re-engine-GNU-0.021.tar.gz
Download (untitled) / with headers
text/plain 3.7k
rt-cpan ------- https://rt.cpan.org/Ticket/Display.html?id=120498 bisect ------ commit 2b5e7bc2e60b4c4b5d87aa66e066363d9dce7930 Author: Karl Williamson <khw@cpan.org> Date: Wed Oct 5 19:09:02 2016 -0600 utf8n_to_uvchr(): Note multiple malformations perl -V ------- % /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.25.5-100-g2b5e7bc2e6/f11c/bin/perl -V Summary of my perl5 (revision 5 version 25 subversion 6) configuration: Derived from: 2b5e7bc2e60b4c4b5d87aa66e066363d9dce7930 Platform: osname=linux osvers=4.8.0-2-amd64 archname=x86_64-linux-thread-multi uname='linux k93msid 4.8.0-2-amd64 #1 smp debian 4.8.11-1 (2016-12-02) x86_64 gnulinux ' config_args='-Dprefix=/home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.25.5-100-g2b5e7bc2e6/f11c -Dmyhostname=k93msid -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Dlibswanted=cl pthread socket inet nsl gdbm dbm malloc dl ld sun m crypt sec util c cposix posix ucb BSD gdbm_compat -Duseithreads -Uuselongdouble -DDEBUGGING=-g' hint=recommended useposix=true d_sigaction=define useithreads=define usemultiplicity=define use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n bincompat5005=undef Compiler: cc='cc' ccflags ='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' optimize='-O2 -g' cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion='' gccversion='6.2.1 20161124' 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/6/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 -ldl -lm -lcrypt -lutil -lc perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc libc=libc-2.24.so so=so useshrplib=false libperl=libperl.a gnulibc_version='2.24' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags='-Wl,-E' cccdlflags='-fPIC' lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector-strong' Characteristics of this binary (from libperl): Compile-time options: HAS_TIMES MULTIPLICITY PERLIO_LAYERS PERL_COPY_ON_WRITE PERL_DONT_CREATE_GVSV PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_IMPLICIT_CONTEXT PERL_MALLOC_WRAP PERL_OP_PARENT PERL_PRESERVE_IVUV PERL_USE_DEVEL USE_64_BIT_ALL USE_64_BIT_INT USE_ITHREADS USE_LARGE_FILES USE_LOCALE USE_LOCALE_COLLATE USE_LOCALE_CTYPE USE_LOCALE_NUMERIC USE_LOCALE_TIME USE_PERLIO USE_PERL_ATOF USE_REENTRANT_API Locally applied patches: uncommitted-changes Built under linux Compiled at Mar 4 2017 17:39:36 @INC: /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.25.5-100-g2b5e7bc2e6/f11c/lib/site_perl/5.25.6/x86_64-linux-thread-multi /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.25.5-100-g2b5e7bc2e6/f11c/lib/site_perl/5.25.6 /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.25.5-100-g2b5e7bc2e6/f11c/lib/5.25.6/x86_64-linux-thread-multi /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.25.5-100-g2b5e7bc2e6/f11c/lib/5.25.6 . -- andreas
Subject: Re: [perl #130921] Bleadperl v5.25.5-100-g2b5e7bc2e6 breaks JDDPAUSE/re-engine-GNU-0.021.tar.gz
From: Dave Mitchell <davem [...] iabyn.com>
To: perl5-porters [...] perl.org
CC: public [...] khwilliamson.com
Date: Mon, 6 Mar 2017 12:39:39 +0000
Download (untitled) / with headers
text/plain 1.1k
On Sat, Mar 04, 2017 at 09:49:12PM -0800, Andreas J. Koenig via RT wrote: Show quoted text
> rt-cpan > ------- > https://rt.cpan.org/Ticket/Display.html?id=120498 > > bisect > ------ > commit 2b5e7bc2e60b4c4b5d87aa66e066363d9dce7930 > Author: Karl Williamson <khw@cpan.org> > Date: Wed Oct 5 19:09:02 2016 -0600 > > utf8n_to_uvchr(): Note multiple malformations
Hi Karl, that commit added this to Perl_utf8n_to_uvchr_error: Newx(adjusted_s0, OFFUNISKIP(min_uv) + 1, U8); SAVEFREEPV((U8 *) adjusted_s0); The problem is that re-engine-GNU is calling utf8n_to_uvchr() during regex cloning which is called during thread cloning. At this point in cloning a new thread, a savestack hasn't been allocated for the new thread, so PL_savestack is NULL< so the SAVEFREEPV() crashes. AFAIKT, the Newx is just just being used to create a temporary buffer big enough to hold one utf8 char; can't you instead just use a small buffer on the stack, e.g. Perl_utf8n_to_uvchr_error(...) { ... U8 adjusted_s0_buf[UTF8_MAXBYTES]; ... if (...) ... adjusted_s0 = adjusted_s0_buf; ... } -- Never do today what you can put off till tomorrow.
Date: Mon, 6 Mar 2017 12:31:09 -0700
From: Karl Williamson <public [...] khwilliamson.com>
Subject: Re: [perl #130921] Bleadperl v5.25.5-100-g2b5e7bc2e6 breaks JDDPAUSE/re-engine-GNU-0.021.tar.gz
To: Dave Mitchell <davem [...] iabyn.com>, perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.6k
On 03/06/2017 05:39 AM, Dave Mitchell wrote: Show quoted text
> On Sat, Mar 04, 2017 at 09:49:12PM -0800, Andreas J. Koenig via RT wrote:
>> rt-cpan >> ------- >> https://rt.cpan.org/Ticket/Display.html?id=120498 >> >> bisect >> ------ >> commit 2b5e7bc2e60b4c4b5d87aa66e066363d9dce7930 >> Author: Karl Williamson <khw@cpan.org> >> Date: Wed Oct 5 19:09:02 2016 -0600 >> >> utf8n_to_uvchr(): Note multiple malformations
> > > Hi Karl, > that commit added this to Perl_utf8n_to_uvchr_error: > > Newx(adjusted_s0, OFFUNISKIP(min_uv) + 1, U8); > SAVEFREEPV((U8 *) adjusted_s0); > > The problem is that re-engine-GNU is calling utf8n_to_uvchr() during > regex cloning which is called during thread cloning. At this point in > cloning a new thread, a savestack hasn't been allocated for the new > thread, so PL_savestack is NULL< so the SAVEFREEPV() crashes. > > AFAIKT, the Newx is just just being used to create a temporary buffer big > enough to hold one utf8 char; can't you instead just use a small buffer > on the stack, e.g. > > Perl_utf8n_to_uvchr_error(...) > { > ... > U8 adjusted_s0_buf[UTF8_MAXBYTES]; > ... > if (...) > ... > adjusted_s0 = adjusted_s0_buf; > ... > } > >
I agree, and I probably should have done it your way to begin with. Attached is a patch to this effect, though I changed the buffer name to be more general in case it might be of future use elsewhere. Since you have already got things set please verify that this fixes the problem. We will need to get approval before applying it for 5.26. Note that this is a regression in 5.25.

Message body is not shown because sender requested not to inline it.

Date: Mon, 6 Mar 2017 20:33:49 +0000
CC: perl5-porters [...] perl.org
Subject: Re: [perl #130921] Bleadperl v5.25.5-100-g2b5e7bc2e6 breaks JDDPAUSE/re-engine-GNU-0.021.tar.gz
From: Dave Mitchell <davem [...] iabyn.com>
To: Karl Williamson <public [...] khwilliamson.com>
Download (untitled) / with headers
text/plain 1.6k
On Mon, Mar 06, 2017 at 12:31:09PM -0700, Karl Williamson wrote: Show quoted text
> I agree, and I probably should have done it your way to begin with. Attached > is a patch to this effect, though I changed the buffer name to be more > general in case it might be of future use elsewhere. Since you have already > got things set please verify that this fixes the problem. We will need to > get approval before applying it for 5.26. Note that this is a regression in > 5.25.
It fails in a different way now - the test script no longer crashes while creating the thread, but instead assert fails later when running the thread, while trying to set a regex's capture string after a successful match. I strongly suspect its unrelated to to the utf8n_to_uvch() issue/regression. http://matrix.cpantesters.org/?dist=re-engine-GNU shows it failing on threaded builds since 5.25.5 - so it's probably a different regression we need to look at. re-engine-GNU appears to be using the non-documented, non-API function sv_setsv_cow() to set regex->saved_copy from an SV which isn't cowable: SV = PV(0x18b03b8) at 0x18acc20 REFCNT = 1 FLAGS = (PADSTALE,POK,READONLY,PROTECT,pPOK,UTF8) PV = 0x18b43e8 "\341\200\200\341\200\273\341\200\275\341\200\224\341\200\272\341\200\257\341\200\225\341\200\272xy"\0 [UTF8 "\x{1000}\x{103b}\x{103d}\x{1014}\x{103a}\x{102f}\x{1015}\x{103a}xy"] CUR = 26 LEN = 28 I don't know yet whether re-engine-GNU or sv_setsv_cow() has unrealistic expectations. I'll look further tomorrow. -- The crew of the Enterprise encounter an alien life form which is surprisingly neither humanoid nor made from pure energy. -- Things That Never Happen in "Star Trek" #22
CC: perl5-porters [...] perl.org
Date: Tue, 7 Mar 2017 11:15:48 +0000
From: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #130921] Bleadperl v5.25.5-100-g2b5e7bc2e6 breaks JDDPAUSE/re-engine-GNU-0.021.tar.gz
To: Karl Williamson <public [...] khwilliamson.com>
Download (untitled) / with headers
text/plain 2.3k
On Mon, Mar 06, 2017 at 08:33:49PM +0000, Dave Mitchell wrote: Show quoted text
> On Mon, Mar 06, 2017 at 12:31:09PM -0700, Karl Williamson wrote:
> > I agree, and I probably should have done it your way to begin with. Attached > > is a patch to this effect, though I changed the buffer name to be more > > general in case it might be of future use elsewhere. Since you have already > > got things set please verify that this fixes the problem. We will need to > > get approval before applying it for 5.26. Note that this is a regression in > > 5.25.
> > > It fails in a different way now - the test script no longer crashes while > creating the thread, but instead assert fails later when running the > thread, while trying to set a regex's capture string after a successful > match.
So I think your fix is good to apply to blead. Show quoted text
> re-engine-GNU appears to be using the non-documented, non-API function > sv_setsv_cow() to set regex->saved_copy from an SV which isn't cowable: > > SV = PV(0x18b03b8) at 0x18acc20 > REFCNT = 1 > FLAGS = (PADSTALE,POK,READONLY,PROTECT,pPOK,UTF8) > PV = 0x18b43e8 "\341\200\200\341\200\273\341\200\275\341\200\224\341\200\272\341\200\257\341\200\225\341\200\272xy"\0 [UTF8 "\x{1000}\x{103b}\x{103d}\x{1014}\x{103a}\x{102f}\x{1015}\x{103a}xy"] > CUR = 26 > LEN = 28 > > I don't know yet whether re-engine-GNU or sv_setsv_cow() has unrealistic > expectations. I'll look further tomorrow.
It fails as far back as at least 5.24.0 on threaded builds with debugging enabled. The function GNU_exec_set_capture_string() in GNU.xs is trying to make a copy of the string being matched, to be stored in the regex's saved_copy field. It seems to be cargo-culting code similar to that in perl core's S_reg_set_capture_string(), but the GNU.xs version ends up trying to call sv_setsv_cow() with a src SV which is *not* cowable. It can be reduced to use threads; #use re::engine::GNU; sub f { #use re::engine::GNU; "x" =~ /x/; } threads->new(\&f)->join; #f(); when the match is run within a child thread, the const SV holding "x" gets cloned and loses its COW flag and so ends up as shown above, i.e. FLAGS = (PADSTALE,POK,READONLY,PROTECT,pPOK,UTF8) This is not COWable, so sv_setsv_cow() triggers an assertion. -- Art is anything that has a label (especially if the label is "untitled 1")
From: Karl Williamson <public [...] khwilliamson.com>
Subject: Re: [perl #130921] Bleadperl v5.25.5-100-g2b5e7bc2e6 breaks JDDPAUSE/re-engine-GNU-0.021.tar.gz
To: Dave Mitchell <davem [...] iabyn.com>
CC: perl5-porters [...] perl.org
Date: Tue, 7 Mar 2017 10:58:32 -0700
Download (untitled) / with headers
text/plain 497b
On 03/07/2017 04:15 AM, Dave Mitchell wrote: Show quoted text
> So I think your fix is good to apply to blead.
Now done, as e9f2c446ed77eec13aad13748ac1b503b0cc3304 with the pumpking's approval on #irc. Aaron Crane had an interesting observation. Since this was a regression in 5.25 and if we didn't fix it now, it would require 3 committers to vote for fixing it in 5.26.1. He added his vote to yours and mine to come up with the requisite 3. We might as well not wait until 5.26.1, and put it into .0
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 777b
On Tue, 07 Mar 2017 09:59:16 -0800, public@khwilliamson.com wrote: Show quoted text
> On 03/07/2017 04:15 AM, Dave Mitchell wrote:
> > So I think your fix is good to apply to blead.
> > Now done, as e9f2c446ed77eec13aad13748ac1b503b0cc3304 with the > pumpking's approval on #irc. > > Aaron Crane had an interesting observation. Since this was a regression > in 5.25 and if we didn't fix it now, it would require 3 committers to > vote for fixing it in 5.26.1. He added his vote to yours and mine to > come up with the requisite 3. We might as well not wait until 5.26.1, > and put it into .0 >
I was careless with my commit message for e9f2c446ed77eec13aad13748ac1b503b0cc3304. The problem is not the Newx, but the SAVEFREEPV of the string allocated by the Newx -- Karl Williamson
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 979b
On Tue, 07 Mar 2017 23:08:35 GMT, khw wrote: Show quoted text
> On Tue, 07 Mar 2017 09:59:16 -0800, public@khwilliamson.com wrote:
> > On 03/07/2017 04:15 AM, Dave Mitchell wrote:
> > > So I think your fix is good to apply to blead.
> > > > Now done, as e9f2c446ed77eec13aad13748ac1b503b0cc3304 with the > > pumpking's approval on #irc. > > > > Aaron Crane had an interesting observation. Since this was a > > regression > > in 5.25 and if we didn't fix it now, it would require 3 committers to > > vote for fixing it in 5.26.1. He added his vote to yours and mine to > > come up with the requisite 3. We might as well not wait until > > 5.26.1, > > and put it into .0 > >
> I was careless with my commit message for > e9f2c446ed77eec13aad13748ac1b503b0cc3304. The problem is not the > Newx, but the SAVEFREEPV of the string allocated by the Newx
This ticket is marked as a blocker for 5.26.0. Can we get an update on its status? Thank you very much. -- James E Keenan (jkeenan@cpan.org)
From: Sawyer X <xsawyerx [...] gmail.com>
Date: Sat, 18 Mar 2017 18:40:35 +0100
Subject: Re: [perl #130921] Bleadperl v5.25.5-100-g2b5e7bc2e6 breaks JDDPAUSE/re-engine-GNU-0.021.tar.gz
To: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1021b
On 03/18/2017 01:09 AM, James E Keenan via RT wrote: Show quoted text
> On Tue, 07 Mar 2017 23:08:35 GMT, khw wrote:
>> On Tue, 07 Mar 2017 09:59:16 -0800, public@khwilliamson.com wrote:
>>> On 03/07/2017 04:15 AM, Dave Mitchell wrote:
>>>> So I think your fix is good to apply to blead.
>>> Now done, as e9f2c446ed77eec13aad13748ac1b503b0cc3304 with the >>> pumpking's approval on #irc. >>> >>> Aaron Crane had an interesting observation. Since this was a >>> regression >>> in 5.25 and if we didn't fix it now, it would require 3 committers to >>> vote for fixing it in 5.26.1. He added his vote to yours and mine to >>> come up with the requisite 3. We might as well not wait until >>> 5.26.1, >>> and put it into .0 >>>
>> I was careless with my commit message for >> e9f2c446ed77eec13aad13748ac1b503b0cc3304. The problem is not the >> Newx, but the SAVEFREEPV of the string allocated by the Newx
> This ticket is marked as a blocker for 5.26.0. Can we get an update on its status?
Karl, will it be possible to merge please?
Subject: Re: [perl #130921] Bleadperl v5.25.5-100-g2b5e7bc2e6 breaks JDDPAUSE/re-engine-GNU-0.021.tar.gz
From: Dave Mitchell <davem [...] iabyn.com>
To: Sawyer X <xsawyerx [...] gmail.com>
CC: perl5-porters [...] perl.org
Date: Sun, 19 Mar 2017 21:34:02 +0000
Download (untitled) / with headers
text/plain 1.4k
On Sat, Mar 18, 2017 at 06:40:35PM +0100, Sawyer X wrote: Show quoted text
> > > On 03/18/2017 01:09 AM, James E Keenan via RT wrote:
> > On Tue, 07 Mar 2017 23:08:35 GMT, khw wrote:
> >> On Tue, 07 Mar 2017 09:59:16 -0800, public@khwilliamson.com wrote:
> >>> On 03/07/2017 04:15 AM, Dave Mitchell wrote:
> >>>> So I think your fix is good to apply to blead.
> >>> Now done, as e9f2c446ed77eec13aad13748ac1b503b0cc3304 with the > >>> pumpking's approval on #irc. > >>> > >>> Aaron Crane had an interesting observation. Since this was a > >>> regression > >>> in 5.25 and if we didn't fix it now, it would require 3 committers to > >>> vote for fixing it in 5.26.1. He added his vote to yours and mine to > >>> come up with the requisite 3. We might as well not wait until > >>> 5.26.1, > >>> and put it into .0 > >>>
> >> I was careless with my commit message for > >> e9f2c446ed77eec13aad13748ac1b503b0cc3304. The problem is not the > >> Newx, but the SAVEFREEPV of the string allocated by the Newx
> > This ticket is marked as a blocker for 5.26.0. Can we get an update on its status?
> > Karl, will it be possible to merge please?
The fix for blead was merged by Karl back on 7 March. The secondary issue remaining was re-engine-GNU is a bug in that distribution, and I've updated its rt.cpan.org ticket with further info. So I'll close this ticket now. -- Justice is when you get what you deserve. Law is when you get what you pay for.


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