readline assert on SvOK_off(glob)

[p5pRT]

Migrated from rt.perl.org#123790 (status was 'open')

Searchable as RT123790$

[p5pRT]

From @hvds

AFL (<http​://lcamtuf.coredump.cx/afl)>) finds this​:

% ./miniperl -e '*x=<y>'
miniperl​: pp_hot.c​:1637​: Perl_do_readline​: Assertion `!((((targ)->sv_flags & (0x00004000|0x00008000)) == 0x00008000) && (((svtype)((targ)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((targ)->sv_flags & 0xff)) == SVt_PVLV))' failed.
Aborted (core dumped)
%

Conversely, '*x=<>' avoids the problem going through the path a few lines further down after the have_fp label, generating a reasonable "Can't coerce GLOB to string" at the point of​:
  else if (isGV_with_GP(sv)) {
  SvPV_force_nomg_nolen(sv);
  }

I don't know near enough about this area to hazard a guess at a fix. Oddly, bisect points at this​:

commit 288b8c0
Author​: Nicholas Clark <nick@​ccl4.org>
Date​: Wed Jan 2 13​:47​:42 2008 +0000

  Make struct regexp the body of SVt_REGEXP SVs, REGEXPs become SVs,
  and regexp reference counting is via the regular SV reference counting.
  This was not as easy at it looks.

.. which doesn't seem a hugely likely culprit.

Hugo

[p5pRT]

From @cpansprout

On Tue Feb 10 18​:55​:00 2015, hv wrote​:

AFL (<http​://lcamtuf.coredump.cx/afl)>) finds this​:

% ./miniperl -e '*x=<y>'
miniperl​: pp_hot.c​:1637​: Perl_do_readline​: Assertion `!((((targ)-

sv_flags & (0x00004000|0x00008000)) == 0x00008000) &&
(((svtype)((targ)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((targ)-
sv_flags & 0xff)) == SVt_PVLV))' failed.
Aborted (core dumped)
%

Conversely, '*x=<>' avoids the problem going through the path a few
lines further down after the have_fp label, generating a reasonable
"Can't coerce GLOB to string" at the point of​:
else if (isGV_with_GP(sv)) {
SvPV_force_nomg_nolen(sv);
}

I don't know near enough about this area to hazard a guess at a fix.
Oddly, bisect points at this​:

commit 288b8c0
Author​: Nicholas Clark <nick@​ccl4.org>
Date​: Wed Jan 2 13​:47​:42 2008 +0000

Make struct regexp the body of SVt_REGEXP SVs, REGEXPs become SVs,
and regexp reference counting is via the regular SV reference
counting.
This was not as easy at it looks.

.. which doesn't seem a hugely likely culprit.

I probably won’t be able to fix this tonight, so here are my incoherent notes to myself, copied and pasted​:

perl-5.8.0-1046-gba92458 is related. It fixed #21628.
perl-5.8.0-620-g7962808 is also related. It fixed #19566.

The buggy code goes back to a0d0e21 (perl 5.000).

Instead of trying to inline the part of sv_setsv that we need, we should probably just do sv_setsv(TARG,NULL), since it knows how to handle special things like typeglobs (otherwise we would have to repeat the warning code, too).

--

Father Chrysostomos

[p5pRT]

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

[p5pRT]

From @cpansprout

On Tue Feb 10 18​:55​:00 2015, hv wrote​:

AFL (<http​://lcamtuf.coredump.cx/afl)>) finds this​:

% ./miniperl -e '*x=<y>'
miniperl​: pp_hot.c​:1637​: Perl_do_readline​: Assertion `!((((targ)-

sv_flags & (0x00004000|0x00008000)) == 0x00008000) &&
(((svtype)((targ)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((targ)-
sv_flags & 0xff)) == SVt_PVLV))' failed.
Aborted (core dumped)
%

Conversely, '*x=<>' avoids the problem going through the path a few
lines further down after the have_fp label, generating a reasonable
"Can't coerce GLOB to string" at the point of​:
else if (isGV_with_GP(sv)) {
SvPV_force_nomg_nolen(sv);
}

That’s not correct, either. If I assign a string to *x, it should do a typeglob aliasing.

This problem is actually quite pervasive. There are many cases that have been broken since 5.20, which I plan to fix soon. The rest of them, which go back to 5.6 or so, I will probably leave for now. Here is one example​:

$ perl5.20.2 -e 'for my $x (*_) { $x = $y x $z }'
$ ./miniperl -e 'for my $x (*_) { $x = $y x $z }'
Can't coerce GLOB to string in repeat (x) at -e line 1.

--

Father Chrysostomos

[p5pRT]

From @cpansprout

On Wed Mar 25 09​:20​:17 2015, sprout wrote​:

On Tue Feb 10 18​:55​:00 2015, hv wrote​:

AFL (<http​://lcamtuf.coredump.cx/afl)>) finds this​:

% ./miniperl -e '*x=<y>'
miniperl​: pp_hot.c​:1637​: Perl_do_readline​: Assertion `!((((targ)-

sv_flags & (0x00004000|0x00008000)) == 0x00008000) &&
(((svtype)((targ)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((targ)-
sv_flags & 0xff)) == SVt_PVLV))' failed.
Aborted (core dumped)
%

Conversely, '*x=<>' avoids the problem going through the path a few
lines further down after the have_fp label, generating a reasonable
"Can't coerce GLOB to string" at the point of​:
else if (isGV_with_GP(sv)) {
SvPV_force_nomg_nolen(sv);
}

That’s not correct, either. If I assign a string to *x, it should do
a typeglob aliasing.

This problem is actually quite pervasive. There are many cases that
have been broken since 5.20, which I plan to fix soon. The rest of
them, which go back to 5.6 or so, I will probably leave for now. Here
is one example​:

$ perl5.20.2 -e 'for my $x (*_) { $x = $y x $z }'
$ ./miniperl -e 'for my $x (*_) { $x = $y x $z }'
Can't coerce GLOB to string in repeat (x) at -e line 1.

These are the regressions from 5.20​:

Can't coerce​:

./miniperl -e 'for my $x (*_) { $x = $y x $z }'
./miniperl -Ilib -Minteger -e 'for my $x (*_) { $x = $y <=> $z }'
./miniperl -e 'for my $x (*_) { $x = $y cmp $z }'
./miniperl -e 'for my $x (*_) { $x = vec 1, 2, 4}'

Assertion failure​:

./miniperl -e 'for my $x (*_) { $x = ~${\""}}'

--

Father Chrysostomos

[p5pRT]

From @cpansprout

On Wed Mar 25 09​:27​:52 2015, sprout wrote​:

On Wed Mar 25 09​:20​:17 2015, sprout wrote​:

On Tue Feb 10 18​:55​:00 2015, hv wrote​:

AFL (<http​://lcamtuf.coredump.cx/afl)>) finds this​:

% ./miniperl -e '*x=<y>'
miniperl​: pp_hot.c​:1637​: Perl_do_readline​: Assertion `!((((targ)-

sv_flags & (0x00004000|0x00008000)) == 0x00008000) &&
(((svtype)((targ)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((targ)-
sv_flags & 0xff)) == SVt_PVLV))' failed.
Aborted (core dumped)
%

Conversely, '*x=<>' avoids the problem going through the path a few
lines further down after the have_fp label, generating a reasonable
"Can't coerce GLOB to string" at the point of​:
else if (isGV_with_GP(sv)) {
SvPV_force_nomg_nolen(sv);
}

That’s not correct, either. If I assign a string to *x, it should do
a typeglob aliasing.

This problem is actually quite pervasive. There are many cases that
have been broken since 5.20, which I plan to fix soon. The rest of
them, which go back to 5.6 or so, I will probably leave for now. Here
is one example​:

$ perl5.20.2 -e 'for my $x (*_) { $x = $y x $z }'
$ ./miniperl -e 'for my $x (*_) { $x = $y x $z }'
Can't coerce GLOB to string in repeat (x) at -e line 1.

These are the regressions from 5.20​:

Can't coerce​:

./miniperl -e 'for my $x (*_) { $x = $y x $z }'
./miniperl -Ilib -Minteger -e 'for my $x (*_) { $x = $y <=> $z }'
./miniperl -e 'for my $x (*_) { $x = $y cmp $z }'
./miniperl -e 'for my $x (*_) { $x = vec 1, 2, 4}'

Assertion failure​:

./miniperl -e 'for my $x (*_) { $x = ~${\""}}'

It also turns out that any bugs relating to lexical scalar assignment that result from this optimisation also started to occur with assignment to state variable declarations, because of a1b22ab. (See also #124160.) So I have disabled that for now in ada289e.

I have also disabled the targlex optimisation for those ops that did not have it in 5.20 (commit 21639bf), so, while things are still buggy, they are no buggier than 5.20.

As for fixing the bugs, I don’t yet know how I would do that.

This how the optimisations work​: Many ops have a target, which is a scalar that the op writes to and uses to return a value. If the op can be made to write directly to a named scalar, treating that as its target, then a subsequent assignment can be skipped. So​:

  $a = $b + $c;

can have the + operator write directly to $a. Without the optimisation, + writes to its target, and then = copies the value to $a.

None of these ops know how to handle a real typeglob on the lhs, and it is not known at compile time whether we will have a typeglob (due to foreach aliasing).

So the solution seems to be to arrange for the optimisation to be foregone if the target turns out to be a typeglob.

Modifying all the relevant pp_ functions individually to handle typeglobs would work, but I am hoping we can come up with a more robust approach, even if it just means making those pp_ functions use a different set of macros.

In any case, this ticket no longer needs to block 5.22.

--

Father Chrysostomos

[p5pRT]

From @cpansprout

I forgot to mention that I fixed the crash in the original report in commit aab1202.

--

Father Chrysostomos