Assertion Failure: *Perl_pp_subst *pp_hot.c:3203)

[p5pRT]

Migrated from rt.perl.org#129038 (status was 'resolved')

Searchable as RT129038$

[p5pRT]

From @geeknik

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

[p5pRT]

From @geeknik

test610

[p5pRT]

From [Unknown Contact. See original ticket]

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

[p5pRT]

From @cpansprout

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.
Abort trap​: 6

--

Father Chrysostomos

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From @iabyn

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?

--
Standards (n). Battle insignia or tribal totems.

[p5pRT]

From @khwilliamson

On 08/24/2016 08​:28 AM, Dave Mitchell wrote​:

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

[p5pRT]

From @khwilliamson

On 08/24/2016 08​:28 AM, Dave Mitchell wrote​:

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 28b98f7, 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().

[p5pRT]

From @iabyn

On Wed, Aug 24, 2016 at 10​:08​:17AM -0600, Karl Williamson wrote​:

But in examining things, I do have a concern. Dave added a
'increment_locinput​:' label in S_regmatch() in 28b98f7, 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.

[p5pRT]

From @khwilliamson

Thanks for finding this. Fixed by 109ac34
--
Karl Williamson

[p5pRT]

@khwilliamson - Status changed from 'open' to 'pending release'

[p5pRT]

From @khwilliamson

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.

[p5pRT]

@khwilliamson - Status changed from 'pending release' to 'resolved'