search

Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.99k stars 557 forks source link

Perl_yylex(): Assertion `!(((PL_parser->linestr))->sv_flags & 0x10000000)' failed (perl: toke.c:4821) #15741

Open p5pRT opened 8 years ago

p5pRT commented 8 years ago

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

Searchable as RT130229$

p5pRT commented 8 years ago

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

p5pRT commented 8 years ago

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

p5pRT commented 8 years ago

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

p5pRT commented 8 years ago

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

p5pRT commented 8 years ago

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

p5pRT commented 8 years ago

From @wolfsage

On Sat\, Dec 3\, 2016 at 3​:28 AM\, Hugo van der Sanden via RT \perlbug\-followup@&#8203;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)

p5pRT commented 8 years ago

From @demerphq

On 5 December 2016 at 04​:05\, Matthew Horsfall (alh) \wolfsage@&#8203;gmail\.com wrote​:

On Sat\, Dec 3\, 2016 at 3​:28 AM\, Hugo van der Sanden via RT \perlbug\-followup@&#8203;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 a14be3eb83c92f38596c3a429d631615e84e660b Author​: Yves Orton \demerphq@&#8203;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/"

p5pRT commented 8 years ago

From @demerphq

On 5 December 2016 at 11​:30\, demerphq \demerphq@&#8203;gmail\.com wrote​:

On 5 December 2016 at 04​:05\, Matthew Horsfall (alh) \wolfsage@&#8203;gmail\.com wrote​:

On Sat\, Dec 3\, 2016 at 3​:28 AM\, Hugo van der Sanden via RT \perlbug\-followup@&#8203;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 a14be3eb83c92f38596c3a429d631615e84e660b Author​: Yves Orton \demerphq@&#8203;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/"

p5pRT commented 8 years ago

From @demerphq

On 5 December 2016 at 11​:39\, demerphq \demerphq@&#8203;gmail\.com wrote​:

On 5 December 2016 at 11​:30\, demerphq \demerphq@&#8203;gmail\.com wrote​:

On 5 December 2016 at 04​:05\, Matthew Horsfall (alh) \wolfsage@&#8203;gmail\.com wrote​:

On Sat\, Dec 3\, 2016 at 3​:28 AM\, Hugo van der Sanden via RT \perlbug\-followup@&#8203;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 a14be3eb83c92f38596c3a429d631615e84e660b Author​: Yves Orton \demerphq@&#8203;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/"

p5pRT commented 8 years ago

From @wolfsage

On Mon\, Dec 5\, 2016 at 5​:42 AM\, demerphq \demerphq@&#8203;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)