From @geeknik

Triggered with Perl v5.25.7-26-g7332835.

./perl -e '0<<<<~
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000@​0000000000000
'

Use of bare << to mean <<"" is deprecated at -e line 1.
perl​: toke.c​:4821​: int Perl_yylex()​: Assertion
`!(((PL_parser->linestr))->sv_flags & 0x10000000)' failed.
Aborted

From @hvds

This looks to have been introduced with indented here-docs​: I suspect Matthew needs to talk to Father C to understand why we need this​:
  /* We really do *not* want PL_linestr ever becoming a COW. */
  assert (!SvIsCOW(PL_linestr));
.. why we're hitting it, and what we can do about it.

I'm not sure how to debug it​: I can't reproduce with the program in a file, and I don't know how to get a newline into command-line arguments in gdb.

Shortest reproducer for me is (with 1247 zeroes)​:
% ./miniperl -e '<<~
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000$x'
Use of bare << to mean <<"" is deprecated at -e line 1.
miniperl​: toke.c​:4821​: Perl_yylex​: Assertion `!(((PL_parser->linestr))->sv_flags & 0x10000000)' failed.
Aborted (core dumped)
%

Hugo

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

From @cpansprout

On Fri, 02 Dec 2016 07​:26​:25 -0800, hv wrote​:

This looks to have been introduced with indented here-docs​: I suspect
Matthew needs to talk to Father C to understand why we need this​:
/* We really do *not* want PL_linestr ever becoming a COW. */
assert (!SvIsCOW(PL_linestr));
.. why we're hitting it, and what we can do about it.

Code all over toke.c expects to modify SvPVX(PL_linestr) in place, which cannot be done with COW scalars. It may be that we need to do sv_force_normal where we currently have an assert, though it would be good to know where it’s becoming a COW to begin with.

I'm not sure how to debug it​: I can't reproduce with the program in a
file, and I don't know how to get a newline into command-line
arguments in gdb.

Have you tried gdb --args ... ?

--

Father Chrysostomos

From @hvds

On Fri, 02 Dec 2016 09​:37​:01 -0800, sprout wrote​:

On Fri, 02 Dec 2016 07​:26​:25 -0800, hv wrote​:

I'm not sure how to debug it​: I can't reproduce with the program in a
file, and I don't know how to get a newline into command-line
arguments in gdb.

Have you tried gdb --args ... ?

Ah, that'll be the one, thanks!

Oh, COW is being set on both source and target at S_scan_heredoc​:9935​:
  sv_setsv(tmpstr,newstr);

It seems mildly odd in any case to be entering this section with indent_len==0, but I guess it isn't unreasonable if that's what the code asks for. We will need care though​: it's by no means obvious to me, for example, that memEQ() with length 0 is portable (and it might be worth a comment to that effect in handy.h, one way or another).

Hugo

From @wolfsage

On Sat, Dec 3, 2016 at 3​:28 AM, Hugo van der Sanden via RT
<perlbug-followup@​perl.org> wrote​:

On Fri, 02 Dec 2016 09​:37​:01 -0800, sprout wrote​:

On Fri, 02 Dec 2016 07​:26​:25 -0800, hv wrote​:

I'm not sure how to debug it​: I can't reproduce with the program in a
file, and I don't know how to get a newline into command-line
arguments in gdb.

Have you tried gdb --args ... ?

Ah, that'll be the one, thanks!

Oh, COW is being set on both source and target at S_scan_heredoc​:9935​:
sv_setsv(tmpstr,newstr);

It seems mildly odd in any case to be entering this section with indent_len==0, but I guess it isn't unreasonable if that's what the code asks for. We will need care though​: it's by no means obvious to me, for example, that memEQ() with length 0 is portable (and it might be worth a comment to that effect in handy.h, one way or another).

Thanks for digging into this. I'm not sure what the solution is here
though so pointers (or a fix :)) would be welcome. Thanks!

-- Matthew Horsfall (alh)

From @demerphq

On 5 December 2016 at 04​:05, Matthew Horsfall (alh) <wolfsage@​gmail.com> wrote​:

On Sat, Dec 3, 2016 at 3​:28 AM, Hugo van der Sanden via RT
<perlbug-followup@​perl.org> wrote​:

On Fri, 02 Dec 2016 09​:37​:01 -0800, sprout wrote​:

On Fri, 02 Dec 2016 07​:26​:25 -0800, hv wrote​:

I'm not sure how to debug it​: I can't reproduce with the program in a
file, and I don't know how to get a newline into command-line
arguments in gdb.

Have you tried gdb --args ... ?

Ah, that'll be the one, thanks!

Oh, COW is being set on both source and target at S_scan_heredoc​:9935​:
sv_setsv(tmpstr,newstr);

It seems mildly odd in any case to be entering this section with indent_len==0, but I guess it isn't unreasonable if that's what the code asks for. We will need care though​: it's by no means obvious to me, for example, that memEQ() with length 0 is portable (and it might be worth a comment to that effect in handy.h, one way or another).

Thanks for digging into this. I'm not sure what the solution is here
though so pointers (or a fix :)) would be welcome. Thanks!

Ah shoot. I already encountered this in my COW patch and have a fix
already which I just pushed.

The basic problem is that in your code you use sv_setsv(), which COW's.

There is also a problem with your code and the existing COW that it uses

SvPV_shrink_to_cur(tmpstr);

which I think breaks in-string COW.

Sorry I didnt push this upstream already.

commit a14be3e
Author​: Yves Orton <demerphq@​gmail.com>
Date​: Sat Nov 26 20​:12​:41 2016 +0100

  make sure that new heredoc parsing doesn't COW during prefix strip

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

From @demerphq

On 5 December 2016 at 11​:30, demerphq <demerphq@​gmail.com> wrote​:

On 5 December 2016 at 04​:05, Matthew Horsfall (alh) <wolfsage@​gmail.com> wrote​:

On Sat, Dec 3, 2016 at 3​:28 AM, Hugo van der Sanden via RT
<perlbug-followup@​perl.org> wrote​:

On Fri, 02 Dec 2016 09​:37​:01 -0800, sprout wrote​:

On Fri, 02 Dec 2016 07​:26​:25 -0800, hv wrote​:

I'm not sure how to debug it​: I can't reproduce with the program in a
file, and I don't know how to get a newline into command-line
arguments in gdb.

Have you tried gdb --args ... ?

Ah, that'll be the one, thanks!

Oh, COW is being set on both source and target at S_scan_heredoc​:9935​:
sv_setsv(tmpstr,newstr);

It seems mildly odd in any case to be entering this section with indent_len==0, but I guess it isn't unreasonable if that's what the code asks for. We will need care though​: it's by no means obvious to me, for example, that memEQ() with length 0 is portable (and it might be worth a comment to that effect in handy.h, one way or another).

Thanks for digging into this. I'm not sure what the solution is here
though so pointers (or a fix :)) would be welcome. Thanks!

Ah shoot. I already encountered this in my COW patch and have a fix
already which I just pushed.

The basic problem is that in your code you use sv_setsv(), which COW's.

There is also a problem with your code and the existing COW that it uses

SvPV_shrink_to_cur(tmpstr);

which I think breaks in-string COW.

Sorry I didnt push this upstream already.

commit a14be3e
Author​: Yves Orton <demerphq@​gmail.com>
Date​: Sat Nov 26 20​:12​:41 2016 +0100

make sure that new heredoc parsing doesn't COW during prefix strip

Yves

Can I leave it to you to write tests? I can confirm my patch fixes the
assertion.

Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"

From @demerphq

On 5 December 2016 at 11​:39, demerphq <demerphq@​gmail.com> wrote​:

On 5 December 2016 at 11​:30, demerphq <demerphq@​gmail.com> wrote​:

On 5 December 2016 at 04​:05, Matthew Horsfall (alh) <wolfsage@​gmail.com> wrote​:

On Sat, Dec 3, 2016 at 3​:28 AM, Hugo van der Sanden via RT
<perlbug-followup@​perl.org> wrote​:

On Fri, 02 Dec 2016 09​:37​:01 -0800, sprout wrote​:

On Fri, 02 Dec 2016 07​:26​:25 -0800, hv wrote​:

I'm not sure how to debug it​: I can't reproduce with the program in a
file, and I don't know how to get a newline into command-line
arguments in gdb.

Have you tried gdb --args ... ?

Ah, that'll be the one, thanks!

Oh, COW is being set on both source and target at S_scan_heredoc​:9935​:
sv_setsv(tmpstr,newstr);

It seems mildly odd in any case to be entering this section with indent_len==0, but I guess it isn't unreasonable if that's what the code asks for. We will need care though​: it's by no means obvious to me, for example, that memEQ() with length 0 is portable (and it might be worth a comment to that effect in handy.h, one way or another).

Thanks for digging into this. I'm not sure what the solution is here
though so pointers (or a fix :)) would be welcome. Thanks!

Ah shoot. I already encountered this in my COW patch and have a fix
already which I just pushed.

The basic problem is that in your code you use sv_setsv(), which COW's.

There is also a problem with your code and the existing COW that it uses

SvPV_shrink_to_cur(tmpstr);

which I think breaks in-string COW.

Sorry I didnt push this upstream already.

commit a14be3e
Author​: Yves Orton <demerphq@​gmail.com>
Date​: Sat Nov 26 20​:12​:41 2016 +0100

make sure that new heredoc parsing doesn't COW during prefix strip

Yves

Can I leave it to you to write tests? I can confirm my patch fixes the
assertion.

BTW, to explain​:

--- a/toke.c
+++ b/toke.c
@​@​ -9897,8 +9897,8 @​@​ S_scan_heredoc(pTHX_ char *s)
  STRLEN herelen = SvCUR(tmpstr);
  char *ss = SvPVX(tmpstr);
  char *se = ss + herelen;
- SV *newstr = newSVpvs("");
- SvGROW(newstr, herelen);
+ SV *newstr = newSV(herelen+1);
+ SvPOK_on(newstr);

The original code is bad because it creates a newSV() from "", and
then grows it to be a bigger string. This is what the newSV()
interface is for, but you have to manually turn on the POK flag.

  /* Trim leading whitespace */
  while (ss < se) {
@​@​ -9931,9 +9931,8 @​@​ S_scan_heredoc(pTHX_ char *s)
  );
  }
  }
-
- sv_setsv(tmpstr,newstr);
-
+ /* avoid sv_setsv() as we dont wan't to COW here */
+ sv_setpvn(tmpstr,SvPVX(newstr),SvCUR(newstr));
  Safefree(indent);

We use sv_setpvn() from the *contents* of the newstr sv, not
sv_setsv() which does a COW copy of the buffer of newstr.

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

From @wolfsage

On Mon, Dec 5, 2016 at 5​:42 AM, demerphq <demerphq@​gmail.com> wrote​:

BTW, to explain​:

--- a/toke.c
+++ b/toke.c
@​@​ -9897,8 +9897,8 @​@​ S_scan_heredoc(pTHX_ char *s)
STRLEN herelen = SvCUR(tmpstr);
char *ss = SvPVX(tmpstr);
char *se = ss + herelen;
- SV *newstr = newSVpvs("");
- SvGROW(newstr, herelen);
+ SV *newstr = newSV(herelen+1);
+ SvPOK_on(newstr);

The original code is bad because it creates a newSV() from "", and
then grows it to be a bigger string. This is what the newSV()
interface is for, but you have to manually turn on the POK flag.

    /\* Trim leading whitespace \*/
    while \(ss \< se\) \{

@​@​ -9931,9 +9931,8 @​@​ S_scan_heredoc(pTHX_ char *s)
);
}
}
-
- sv_setsv(tmpstr,newstr);
-
+ /* avoid sv_setsv() as we dont wan't to COW here */
+ sv_setpvn(tmpstr,SvPVX(newstr),SvCUR(newstr));
Safefree(indent);

We use sv_setpvn() from the *contents* of the newstr sv, not
sv_setsv() which does a COW copy of the buffer of newstr.

Yves

Thank you, that's helpful. I'll make a note to try to come up with
tests for this later.

Cheers,

-- Matthew Horsfall (alh)