Bug #131487 for Future-AsyncAwait: Segmentation fault when there are compilation failures in anon async sub{} construction

Thu Jan 16 01:22:26 2020 TEAM [...] cpan.org - Ticket created

Subject:

Segmentation fault when there are multiple compilation failures

With 0.35, a single undeclared variable under `strict` is fine - the error is displayed, things fail to compile and everything exits cleanly. So far so good. With *two* undeclared variables, we're back to segfaults: use strict; use warnings; use Future::AsyncAwait; (async sub { for my $i (1..$x) { await $api->method; } })->()->get;

Thu Jan 16 08:42:51 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Thu Jan 16 01:22:26 2020, TEAM wrote: Show quoted text

> With 0.35, a single undeclared variable under `strict` is fine - the > error is displayed, things fail to compile and everything exits > cleanly. So far so good. > > With *two* undeclared variables, we're back to segfaults:

So; good news, bad news. Good news: I can reproduce it: Program received signal SIGSEGV, Segmentation fault. Bad news: the stack trace in gdb doesn't go through AsyncAwait.xs: #0 0x00005555555a8140 in Perl_newCVREF (my_perl=my_perl@entry=0x5555558a9260, flags=flags@entry=0, o=0x0) at op.c:11081 #1 0x00005555555f6043 in Perl_yyparse (my_perl=my_perl@entry=0x5555558a9260, gramtype=gramtype@entry=258) at perly.y:957 #2 0x0000555555697927 in S_doeval_compile (my_perl=my_perl@entry=0x5555558a9260, gimme=gimme@entry=2 '\002', outside=outside@entry=0x5555558ac688, seq=<optimized out>, hh=hh@entry=0x55555608c028) at pp_ctl.c:3502 #3 0x00005555556a294a in Perl_pp_entereval (my_perl=0x5555558a9260) at pp_ctl.c:4478 #4 0x0000555555652016 in Perl_runops_standard (my_perl=0x5555558a9260) at run.c:42 #5 0x00005555555c662c in S_run_body (oldscope=<optimized out>, my_perl=<optimized out>) at perl.c:2716 #6 perl_run (my_perl=0x5555558a9260) at perl.c:2639 #7 0x000055555559c462 in main (argc=<optimized out>, argv=<optimized out>, env=<optimized out>) at perlmain.c:127 This is going to require some more subtle trickery... -- Paul Evans

Thu Jan 16 08:42:51 2020 The RT System itself - Status changed from 'new' to 'open'

Thu Jan 16 09:05:03 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Subject:

Segmentation fault when there are compilation failures in anon async sub{} construction

Ahhah. Adjusting the example in various ways suggests it isn't to do with there being multiple failures, but it being an anon (async sub {}) construction. It also SEGVs if I add a `my $x` in scope so there's only one failure; but it doesn't SEGV and just makes eval{} throw in the expected way if I change it to async sub example { ... } -- Paul Evans

Thu Jan 16 09:05:29 2020 leonerd-cpan [...] leonerd.org.uk - Subject changed from 'Segmentation fault when there are multiple compilation failures' to 'Segmentation fault when there are compilation failures in anon async sub{} construction'

Thu Jan 16 09:52:17 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

I think I have it. The solution is that you have to a) yield a valid UNOP(OP_NULL), rather than just a NULL pointer b) wrap the sub {} parse logic in an ENTER/LEAVE pair This ensures the correct PL_compcv/et.al. is seen by the core parser when it tries to tidy up after the failure. Now: $ perl -Mblib use strict; use warnings; use Future::AsyncAwait; (async sub { for my $i (1..$x) { await $api->method; } })->()->get; __END__ Global symbol "$x" requires explicit package name (did you forget to declare "my $x"?) at - line 5. Global symbol "$api" requires explicit package name (did you forget to declare "my $api"?) at - line 6. Execution of - aborted due to compilation errors. Patch attached. -- Paul Evans

Subject:

rt131487.patch

=== modified file 'lib/Future/AsyncAwait.xs' --- old/lib/Future/AsyncAwait.xs 2020-01-16 14:33:16 +0000 +++ new/lib/Future/AsyncAwait.xs 2020-01-16 14:36:41 +0000 @@ -2142,6 +2142,7 @@ #endif *op_ptr = newOP(OP_NULL, 0); if(name) { + SvREFCNT_dec(name); LEAVE_with_name("parse_block"); return KEYWORD_PLUGIN_STMT; }

Thu Jan 16 09:52:58 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

On Thu Jan 16 09:52:17 2020, PEVANS wrote: Show quoted text

> Patch attached.

Er, wrong patch. Now attached. -- Paul Evans

Subject:

rt131487.patch

=== modified file 'lib/Future/AsyncAwait.xs' --- old/lib/Future/AsyncAwait.xs 2019-12-18 15:47:31 +0000 +++ new/lib/Future/AsyncAwait.xs 2020-01-16 14:33:16 +0000 @@ -2069,6 +2069,9 @@ SV *name = lex_scan_ident(); lex_read_space(0); + ENTER_with_name("parse_block"); + /* From here onwards any `return` must be prefixed by LEAVE_with_name() */ + I32 floor_ix = start_subparse(FALSE, name ? 0 : CVf_ANON); SAVEFREESV(PL_compcv); @@ -2087,8 +2090,10 @@ sigop = parse_subsignature(0); lex_read_space(0); - if(PL_parser->error_count) + if(PL_parser->error_count) { + LEAVE_with_name("parse_block"); return 0; + } if(lex_peek_unichar(0) != ')') croak("Expected ')'"); @@ -2135,8 +2140,15 @@ if(sigop) op_free(sigop); #endif - *op_ptr = NULL; - return name ? KEYWORD_PLUGIN_STMT : KEYWORD_PLUGIN_EXPR; + *op_ptr = newOP(OP_NULL, 0); + if(name) { + LEAVE_with_name("parse_block"); + return KEYWORD_PLUGIN_STMT; + } + else { + LEAVE_with_name("parse_block"); + return KEYWORD_PLUGIN_EXPR; + } } #ifdef HAVE_PARSE_SUBSIGNATURE @@ -2166,6 +2178,8 @@ if(CvLVALUE(cv)) warn("Pointless use of :lvalue on async sub"); + LEAVE_with_name("parse_block"); + if(name) { *op_ptr = newOP(OP_NULL, 0); === modified file 't/32compile-errors.t' --- old/t/32compile-errors.t 2020-01-16 13:29:34 +0000 +++ new/t/32compile-errors.t 2020-01-16 14:33:16 +0000 @@ -42,4 +42,25 @@ 'Failure message complains about undeclared $x' ); } +# RT131487 +{ + local $@; + + ok( !defined eval q' + package segfault; + use strict; + use warnings; + + use Future::AsyncAwait; + (async sub { + for my $i (1..5) { + await $api->method; + } + })->()->get; + ', + 'RT131487 strict-failing code fails to compile' ); + like( "$@", qr/^Global symbol "\$api" requires explicit package name/, + 'Failure message complains about undeclared $api' ); +} + done_testing;

Mon Jan 27 21:01:34 2020 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Released in 0.36 -- Paul Evans

Mon Jan 27 21:01:35 2020 leonerd-cpan [...] leonerd.org.uk - Status changed from 'open' to 'resolved'

Mon Jan 27 21:01:35 2020 leonerd-cpan [...] leonerd.org.uk - Fixed in 0.36 added