Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
p5pRT opened this issue Aug 21, 2016 · 13 comments
Closed

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

p5pRT opened this issue Aug 21, 2016 · 13 comments

Comments

@p5pRT
Copy link

p5pRT commented Aug 21, 2016

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

Searchable as RT129038$

@p5pRT
Copy link
Author

p5pRT commented Aug 21, 2016

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
Copy link
Author

p5pRT commented Aug 21, 2016

From @geeknik

test610

@p5pRT
Copy link
Author

p5pRT commented Aug 21, 2016

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
Copy link
Author

p5pRT commented Aug 22, 2016

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
Copy link
Author

p5pRT commented Aug 22, 2016

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

@p5pRT
Copy link
Author

p5pRT commented Aug 24, 2016

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
Copy link
Author

p5pRT commented Aug 24, 2016

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
Copy link
Author

p5pRT commented Aug 24, 2016

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
Copy link
Author

p5pRT commented Aug 25, 2016

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
Copy link
Author

p5pRT commented Aug 29, 2016

From @khwilliamson

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

@p5pRT
Copy link
Author

p5pRT commented Aug 29, 2016

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

@p5pRT
Copy link
Author

p5pRT commented May 30, 2017

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
Copy link
Author

p5pRT commented May 30, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant