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

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

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



Subject: Assertion Failure: *Perl_pp_subst *pp_hot.c:3203)
Subject: Assertion Failure: *Perl_pp_subst (pp_hot.c:3203)
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 180b
The attached script triggers an assertion failure in v5.25.5 (v5.25.4-5-g92d73bf). ./perl test610 perl: pp_hot.c:3203: OP *Perl_pp_subst(): Assertion `strend >= s' failed. Aborted
Subject: test610
Download test610
application/octet-stream 21b

Message body not shown because it is not plain text.

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 449b
On Sun Aug 21 15:20:17 2016, brian.carpenter@gmail.com wrote: Show quoted text
> The attached script triggers an assertion failure in v5.25.5 (v5.25.4- > 5-g92d73bf). > > ./perl test610 > perl: pp_hot.c:3203: OP *Perl_pp_subst(): Assertion `strend >= s' > failed. > Aborted
Some locale bug, apparently: $ ./miniperl -e '$_="\xff"; s/\w//l' Assertion failed: (strend >= s), function Perl_pp_subst, file pp_hot.c, line 3203. Abort trap: 6 -- Father Chrysostomos
Date: Wed, 24 Aug 2016 15:28:20 +0100
From: Dave Mitchell <davem [...] iabyn.com>
CC: perl5-porters [...] perl.org, public [...] khwilliamson.com
To: Father Chrysostomos via RT <perlbug-followup [...] perl.org>
Subject: Re: [perl #129038] Assertion Failure: *Perl_pp_subst *pp_hot.c:3203)
Download (untitled) / with headers
text/plain 925b
On Sun, Aug 21, 2016 at 05:27:55PM -0700, Father Chrysostomos via RT wrote: Show quoted text
> On Sun Aug 21 15:20:17 2016, brian.carpenter@gmail.com wrote:
> > The attached script triggers an assertion failure in v5.25.5 (v5.25.4- > > 5-g92d73bf). > > > > ./perl test610 > > perl: pp_hot.c:3203: OP *Perl_pp_subst(): Assertion `strend >= s' > > failed. > > Aborted
> > Some locale bug, apparently: > > $ ./miniperl -e '$_="\xff"; s/\w//l' > Assertion failed: (strend >= s), function Perl_pp_subst, file pp_hot.c, line 3203.
The problem is this in S_regmatch: case POSIXL: /* \w or [:punct:] etc. under /l */ ... /* Here, must be utf8 */ locinput += UTF8SKIP(locinput); break; AFAIKT, assuming at that point that the string is utf8, is incorrect. Karl, this bit of code was was last touched by you, fancy having a look? -- Standards (n). Battle insignia or tribal totems.
Subject: Re: [perl #129038] Assertion Failure: *Perl_pp_subst *pp_hot.c:3203)
To: Dave Mitchell <davem [...] iabyn.com>, Father Chrysostomos via RT <perlbug-followup [...] perl.org>
Date: Wed, 24 Aug 2016 09:09:19 -0600
CC: perl5-porters [...] perl.org
From: Karl Williamson <public [...] khwilliamson.com>
Download (untitled) / with headers
text/plain 948b
On 08/24/2016 08:28 AM, Dave Mitchell wrote: Show quoted text
> On Sun, Aug 21, 2016 at 05:27:55PM -0700, Father Chrysostomos via RT wrote:
>> On Sun Aug 21 15:20:17 2016, brian.carpenter@gmail.com wrote:
>>> The attached script triggers an assertion failure in v5.25.5 (v5.25.4- >>> 5-g92d73bf). >>> >>> ./perl test610 >>> perl: pp_hot.c:3203: OP *Perl_pp_subst(): Assertion `strend >= s' >>> failed. >>> Aborted
>> >> Some locale bug, apparently: >> >> $ ./miniperl -e '$_="\xff"; s/\w//l' >> Assertion failed: (strend >= s), function Perl_pp_subst, file pp_hot.c, line 3203.
> > The problem is this in S_regmatch: > > case POSIXL: /* \w or [:punct:] etc. under /l */ > > ... > > /* Here, must be utf8 */ > locinput += UTF8SKIP(locinput); > break; > > AFAIKT, assuming at that point that the string is utf8, is incorrect. > Karl, this bit of code was was last touched by you, fancy having a look? >
ok
To: Dave Mitchell <davem [...] iabyn.com>, Father Chrysostomos via RT <perlbug-followup [...] perl.org>
Subject: Re: [perl #129038] Assertion Failure: *Perl_pp_subst *pp_hot.c:3203)
CC: perl5-porters [...] perl.org
From: Karl Williamson <public [...] khwilliamson.com>
Date: Wed, 24 Aug 2016 10:08:17 -0600
Download (untitled) / with headers
text/plain 1.5k
On 08/24/2016 08:28 AM, Dave Mitchell wrote: Show quoted text
> On Sun, Aug 21, 2016 at 05:27:55PM -0700, Father Chrysostomos via RT wrote:
>> On Sun Aug 21 15:20:17 2016, brian.carpenter@gmail.com wrote:
>>> The attached script triggers an assertion failure in v5.25.5 (v5.25.4- >>> 5-g92d73bf). >>> >>> ./perl test610 >>> perl: pp_hot.c:3203: OP *Perl_pp_subst(): Assertion `strend >= s' >>> failed. >>> Aborted
>> >> Some locale bug, apparently: >> >> $ ./miniperl -e '$_="\xff"; s/\w//l' >> Assertion failed: (strend >= s), function Perl_pp_subst, file pp_hot.c, line 3203.
> > The problem is this in S_regmatch: > > case POSIXL: /* \w or [:punct:] etc. under /l */ > > ... > > /* Here, must be utf8 */ > locinput += UTF8SKIP(locinput); > break; > > AFAIKT, assuming at that point that the string is utf8, is incorrect. > Karl, this bit of code was was last touched by you, fancy having a look? >
Well, it's just plain bad logic, apparently on my part. But, I'm not going to bother to do archeology to see how it got that way. But in examining things, I do have a concern. Dave added a 'increment_locinput:' label in S_regmatch() in 28b98f76, but many of the cases bypass that. The code at that label allows the input to go one off the end. I'm wondering if more of the cases should goto that label instead of incrementing locinput themselves and doing a 'break'. I have some vague recollection around when one was supposed to do that, but I would have made it a comment so it would be plain, and I don't see any such comment. It might have been a different function().
Date: Thu, 25 Aug 2016 15:22:38 +0100
From: Dave Mitchell <davem [...] iabyn.com>
CC: Father Chrysostomos via RT <perlbug-followup [...] perl.org>, perl5-porters [...] perl.org
Subject: Re: [perl #129038] Assertion Failure: *Perl_pp_subst *pp_hot.c:3203)
To: Karl Williamson <public [...] khwilliamson.com>
Download (untitled) / with headers
text/plain 1.4k
On Wed, Aug 24, 2016 at 10:08:17AM -0600, Karl Williamson wrote: Show quoted text
> But in examining things, I do have a concern. Dave added a > 'increment_locinput:' label in S_regmatch() in 28b98f76, but many of the > cases bypass that. The code at that label allows the input to go one off > the end. I'm wondering if more of the cases should goto that label instead > of incrementing locinput themselves and doing a 'break'. I have some vague > recollection around when one was supposed to do that, but I would have made > it a comment so it would be plain, and I don't see any such comment. It > might have been a different function().
Its a while ago now; but IIRC this was part of my work to make the regex engine not read past the end of the string (including the not reading the implied \0). In particular, memory mapped stings were problematical: the regex engine could trigger a page fault. I suspect that 'goto increment_locinput' is probably generally the better bet, *but only when* *locinput == nextchr, since increment_locinput assumes that. I would speculate that some of the places that do their own increment do it because: *locinput is not equal to nextchr, or they already have a "if (utf8_target) { X } { else Y }" block of code so it's less efficient to test the condition again. I can't remember why its safe to be 1 byte over, but not 2+, and thus can't say off the top of my head whether other branches are bad for not doing that test. -- That he said that that that that is is is debatable, is debatable.
RT-Send-CC: perl5-porters [...] perl.org
Thanks for finding this. Fixed by 109ac342a6bc5a3a67c3b52341607100cedafdf7 -- Karl Williamson
Download (untitled) / with headers
text/plain 313b
Thank you for filing this report. You have helped make Perl better. With the release today of Perl 5.26.0, this and 210 other issues have been resolved. Perl 5.26.0 may be downloaded via: https://metacpan.org/release/XSAWYERX/perl-5.26.0 If you find that the problem persists, feel free to reopen this ticket.


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