develooper Front page | perl.perl5.porters | Postings from May 2015

Re: [perl #124368] Perl_sv_2pv_flags: Assertion`((svtype)((sv)->sv_flags & 0xff)) != SVt_PVAV && ((svtype)((sv)->sv_flags &0xff)) != SVt_PVHV && ((svtype)((sv)->sv_flags & 0xff)) != SVt_PVFM' failed

Thread Previous | Thread Next
Dave Mitchell
May 8, 2015 20:57
Re: [perl #124368] Perl_sv_2pv_flags: Assertion`((svtype)((sv)->sv_flags & 0xff)) != SVt_PVAV && ((svtype)((sv)->sv_flags &0xff)) != SVt_PVHV && ((svtype)((sv)->sv_flags & 0xff)) != SVt_PVFM' failed
Message ID:
On Wed, Apr 29, 2015 at 10:22:00PM -0700, Tony Cook via RT wrote:
> On Wed Apr 29 12:07:25 2015, hv wrote:
> > On Wed Apr 29 04:14:52 2015, davem wrote:
> > > I can't reproduce the failure with the original code (no assertion
> > > failure; valgrind and ASan ok).
> > 
> > I can, with a plain build of blead. Most of the original code is red
> > herrings, I was able to reduce it to this:
> > 
> > % ./miniperl -e '@a = qw{x y}; for (@a) { s{.}{}go =~ @b }'
> > miniperl: sv.c:2964: Perl_sv_2pv_flags: Assertion `((svtype)((sv)-
> > >sv_flags & 0xff)) != SVt_PVAV && ((svtype)((sv)->sv_flags & 0xff)) !=
> > SVt_PVHV && ((svtype)((sv)->sv_flags & 0xff)) != SVt_PVFM' failed.
> > Aborted (core dumped)
> >  %
> > 
> > Are you able to reproduce it that way? If not I may be able to make
> > some time to dig more over the weekend.
> The problem seems to be this code in pp_regcomp:
>     /* can't change the optree at runtime either */
>     /* PMf_KEEP is handled differently under threads to avoid these problems */
>     if (!RX_PRELEN(PM_GETRE(pm)) && PL_curpm)
> 	pm = PL_curpm;
>     if (pm->op_pmflags & PMf_KEEP) {
> 	pm->op_private &= ~OPpRUNTIME;	/* no point compiling again */
> 	cLOGOP->op_first->op_next = PL_op->op_next;
>     }
> before this starts pm point at the match op and PL_curpm points at the subst op.
> RX_PRELEN() is zero for rx, so pm is updated, and since the subst has
> the /o flag (PMf_KEEP) the op tree is rewritten to skip the regcomp for
> the second time around the loop, leaving the raw AV on the stack as the
> regexp for pp_match.

This bug has in fact been present since 5.0 or before; it's just that
a) a change in 5.18.0 made "$_ =~ @a", where @a is empty, match "@a" (i.e.
"") rather than the string "0"; and b) an assertion added in 5.19.x made
getting the string value of an AV an error. So I don't think that it needs
to be a 5.22 blocker any more.

The main issue is the mis-implementation and interaction between two
regexp "features"; first that the 'o' flag in /$expr/o means that the
pattern is only compiled once, and second that in /$expr/, if $expr
evaluates to the empty string, then the last successful regex is used
instead. It is documented in perlop that:

    In this case, only the C<g> and C<c> flags on the empty pattern are
    honored; the other flags are taken from the original pattern

So in something like the following:

    $_ =~ @b;

if @b is empty, then (since 5.18.0), it evaluates to the empty string,
so the null pattern rule kicks in, and  the /./ pattern is used instead.
Then the "other flags are taken from the original pattern" rule kicks
in, so the //o flag from the earlier successful match is honoured,
so the optree of the current match ($_ =~ @b), i.e. the one *without*
the /o flag, is manipulated to make it skip future compiles. Since that
optree wasn't compiled with //o present, it doesn't have the extra
regcmaybe op in the tree, which the code blindly assumes is the first
child of the regcomp op,  and so happily corrupts that tree.

This is fixable, but first we need to agree on what the semantics should
be, in particular, in the following:

    for (...) {

on executing /$expr/ if $expr evaluates to the null string, should it
honour the //o of the previous successful match, and so skip evaluating
$expr on subsequent iterations? This would be hard to implement.

Conversely, should /$expr/o honour the //o flag even if $expr is null?

My opinion is that, like c and g, the o flag should be honoured on the
match op it appears on, not on the last successful match op, and we fix
the docs (as well as the code).

So I propose that:

    /abc/o; $foo =~ /$null/

matches $foo against "abc" each time round the loop (it currently crashes
on the second iteration); and that:

    /abc/; $foo =~ /$null/o

always only evaluates $null once; if that happens to be the empty string,
then subsequent iterations will see the empty string each time and so each
time use the last successful match, which may be /abc/ (it currently
ignores the //o on the first iteration if $null is empty).

Note that nothing in the test suite currently tests these cases as far as
I can tell.

"But Sidley Park is already a picture, and a most amiable picture too.
The slopes are green and gentle. The trees are companionably grouped at
intervals that show them to advantage. The rill is a serpentine ribbon
unwound from the lake peaceably contained by meadows on which the right
amount of sheep are tastefully arranged." -- Lady Croom, "Arcadia"

Thread Previous | Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About