Bug #129975 for Syntax-Keyword-Try: t/11loop.t stalls on perl 5.31.2

Wed Jul 03 07:46:50 2019 leonerd-cpan [...] leonerd.org.uk - Ticket created

Subject:

t/11loop.t stalls on perl 5.31.2

(from https://rt.perl.org/Public/Bug/Display.html?id=134258) Show quoted text

> The problem is that this distro's t/11loop.t now hangs indefinitely and > has to be Ctrl-C-ed. All other files in the distro's test suite PASS.

... Show quoted text

> This commit removes the deferred mechanism, and instead > makes use of the newish OP_PARENT mechanism to iterate over > the optree, following each kid, then back up via the parent > pointer to the next sibling etc.

... Ah. I wonder then, whether this might be something that Syntax::Keyword::Try has to fix. I do some rather awkward optree fiddling in there, and I don't recall needing to take care of OP_PARENT so far. https://metacpan.org/source/PEVANS/Syntax-Keyword-Try-0.10/lib/Syntax/Keyword/Try.xs#L257 https://metacpan.org/source/PEVANS/Syntax-Keyword-Try-0.10/lib/Syntax/Keyword/Try.xs#L283 -- Paul Evans

Wed Jul 03 10:47:52 2019 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Confirmed reproducible: $ bleadperl -v This is perl 5, version 31, subversion 2 (v5.31.2 (v5.31.1-122-g82651abe60)) built for x86_64-linux ... $ bleadperl Build test t/00use.t .............. ok t/01trycatch.t ......... ok t/02tryfinally.t ....... ok t/03trycatchfinally.t .. ok t/10snail.t ............ ok t/11loop.t ............. ^C -- Paul Evans

Tue Jul 09 15:54:22 2019 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Actually, further debugging reveals more breakage on any -DDEBUGGING perl, because of extra assertion failures. perl: op.c:1888: Perl_scalar: Assertion `0' failed. Aborted on most .t files. bisected down to this core perl commit: db18005b26986d1d422fcb53989eece313134db5 is the first bad commit commit db18005b26986d1d422fcb53989eece313134db5 Author: David Mitchell <davem@iabyn.com> Date: Wed May 29 11:03:26 2019 +0100 Perl_scalar() tail-call optimise The part of this function that scans the children of e.g. $scalar = do { void; void; scalar } applying scalar context only to the last child: tail call optimise that call to Perl_scalar(). It also adds some extra 'warnings' tests. An earlier attempt at this patch caused some unrelated tests to start emitting spurious 'useless in void context' messages, which are covered by the new tests. This also showed up that the current method for updating PL_curcop while descending optrees in Perl_scalar/scalarvoid/S_scalarseq is a bit broken. It gets updated every time a newstate op is seen, but haphazardly (and sometimes wrongly) restored to &PL_compiling when going back up the tree. One of the tests is TODO based on PL_curcop being wrong and so the 'no warnings "void"' leaking into an outer scope. This commit maintains the status quo. -- Paul Evans

Tue Jul 23 12:50:03 2019 jkeenan [...] cpan.org - Correspondence added

On Tue Jul 09 15:54:22 2019, PEVANS wrote: Show quoted text

> Actually, further debugging reveals more breakage on any -DDEBUGGING > perl, because of extra assertion failures. > > perl: op.c:1888: Perl_scalar: Assertion `0' failed. > Aborted > > on most .t files. > > bisected down to this core perl commit: > > db18005b26986d1d422fcb53989eece313134db5 is the first bad commit > commit db18005b26986d1d422fcb53989eece313134db5 > Author: David Mitchell <davem@iabyn.com> > Date: Wed May 29 11:03:26 2019 +0100 > > Perl_scalar() tail-call optimise > > The part of this function that scans the children of e.g. > > $scalar = do { void; void; scalar } > > applying scalar context only to the last child: tail call optimise > that > call to Perl_scalar(). > > It also adds some extra 'warnings' tests. An earlier attempt at this > patch caused some unrelated tests to start emitting spurious 'useless > in > void context' messages, which are covered by the new tests. > > This also showed up that the current method for updating PL_curcop > while descending optrees in Perl_scalar/scalarvoid/S_scalarseq is a > bit > broken. It gets updated every time a newstate op is seen, but > haphazardly > (and sometimes wrongly) restored to &PL_compiling when going back up > the > tree. One of the tests is TODO based on PL_curcop being wrong and so > the > 'no warnings "void"' leaking into an outer scope. > > This commit maintains the status quo.

Additional failures found when testing this distribution against perl-5.31.2 on FreeBSD-12, using cpanm as the installer: see attached.

Subject:

rtc122975-test-failures.txt

### Syntax-Keyword-Try test_output => [ "Building and testing Syntax-Keyword-Try-0.10", "Building Syntax-Keyword-Try", "cc -I/home/jkeenan/var/tad/testing/perl-5.31.2/lib/5.31.2/amd64-freebsd-thread-multi/CORE -DVERSION=\"0.10\" -DXS_VERSION=\"0.10\" -DPIC -fPIC -c -DHAS_FPSETMASK -DHAS_FLOATINGPOINT_H -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_FORTIFY_SOURCE=2 -O2 -pipe -fstack-protector -fno-strict-aliasing -o lib/Syntax/Keyword/Try.o lib/Syntax/Keyword/Try.c", "ExtUtils::Mkbootstrap::Mkbootstrap('blib/arch/auto/Syntax/Keyword/Try/Try.bs')", "cc -shared -L/usr/local/lib -fstack-protector-strong -o blib/arch/auto/Syntax/Keyword/Try/Try.so lib/Syntax/Keyword/Try.o", "t/00use.t .............. ok", "t/01trycatch.t ......... ", "No subtests run ", "t/02tryfinally.t ....... ", "No subtests run ", "t/03trycatchfinally.t .. ok", "t/10snail.t ............ ok", "-> FAIL Timed out (> 1800s). Use --verbose to retry.", [Syntax-Keyword-Try-0.10] $ thisprove t/12return.t t/13die-in-finally.t t/14try-localises.t t/20try-do.t t/21try-do-finally.t t/80await+try.t t/90rt123547.t t/90rt125971.t t/99pod.t t/12return.t .......... No subtests run t/13die-in-finally.t .. ok 1 - die in both try{} and finally{} is still fatal ok 2 - die in finally{} does not corrupt $@ 1..2 ok t/14try-localises.t ... ok 1 - $@ before try/catch ok 2 - $@ after try/catch 1..2 ok t/20try-do.t .......... No subtests run t/21try-do-finally.t .. No subtests run t/80await+try.t ....... # Future::AsyncAwait 0.30, Syntax::Keyword::Try 0.10 ok 1 - $fdone for successful await in try/catch ok 2 - $ffail for failed await in try/catch ok 3 - $fdone for successful await in try/catch with return ok 4 - fallthrough after try{return} did not happen ok 5 - $fret for await in try/finally 1..5 ok t/90rt123547.t ........ ok 1 - try/catch/finally correct result ok 2 - try/catch/finally correct result ok 3 - Did not crash 1..3 ok t/90rt125971.t ........ ok 1 - No extra data in return 1..1 ok t/99pod.t ............. 1..1 ok 1 - POD test for blib/lib/Syntax/Keyword/Try.pm ok Test Summary Report ------------------- t/12return.t (Wstat: 139 Tests: 0 Failed: 0) Non-zero wait status: 139 Parse errors: No plan found in TAP output t/20try-do.t (Wstat: 139 Tests: 0 Failed: 0) Non-zero wait status: 139 Parse errors: No plan found in TAP output t/21try-do-finally.t (Wstat: 139 Tests: 0 Failed: 0) Non-zero wait status: 139 Parse errors: No plan found in TAP output Files=9, Tests=14, 2 wallclock secs ( 0.08 usr 0.03 sys + 0.90 cusr 0.83 csys = 1.84 CPU) Result: FAIL

Tue Jul 23 12:50:03 2019 The RT System itself - Status changed from 'new' to 'open'

Fri Sep 06 20:16:43 2019 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Turned out to be two different issues here; hopefully the attached patch fixes them both. Works fine on my bleadperl and perl-5.31.3 -- Paul Evans

Subject:

rt129975.patch

=== modified file 'lib/Syntax/Keyword/Try.xs' --- lib/Syntax/Keyword/Try.xs 2019-08-14 13:31:36 +0000 +++ lib/Syntax/Keyword/Try.xs 2019-09-07 00:13:54 +0000 @@ -36,6 +36,10 @@ #define PERL_VERSION_GE(r,v,s) \ (PERL_DECIMAL_VERSION >= PERL_VERSION_DECIMAL(r,v,s)) +#if PERL_VERSION_GE(5,26,0) +# define HAVE_OP_SIBPARENT +#endif + #if PERL_VERSION_GE(5,19,4) typedef SSize_t array_ix_t; #else /* <5.19.4 */ @@ -318,18 +322,28 @@ case OP_LAST: case OP_REDO: { - OP *stateop, *afterop = OpSIBLING(op); - - *op_ptr = newLISTOP(OP_SCOPE, 0, - stateop = newSTATEOP_nowarnings(), - op); - - (*op_ptr)->op_next = stateop; +#ifdef HAVE_OP_SIBPARENT + OP *parent = OpHAS_SIBLING(op) ? NULL : op->op_sibparent; +#endif + + OP *stateop = newSTATEOP_nowarnings(); + + OP *scope = newLISTOP(OP_SCOPE, 0, + stateop, op); +#ifdef HAVE_OP_SIBPARENT + if(parent) + OpLASTSIB_set(scope, parent); + else + OpLASTSIB_set(scope, NULL); +#else + op->op_sibling = NULL; +#endif + + /* Rethread */ + scope->op_next = stateop; stateop->op_next = op; - if(afterop) { - OpMORESIB_set(op, NULL); - OpMORESIB_set(*op_ptr, afterop); - } + + *op_ptr = scope; } break; @@ -355,7 +369,8 @@ else cUNOPx(op)->op_first = newkid; - OpMORESIB_set(newkid, next); + if(next) + OpMORESIB_set(newkid, next); } prev = kid; @@ -495,9 +510,10 @@ ret = op_prepend_elem(OP_LINESEQ, newPUSHFINALLYOP(finally), ret); } - ret = newLISTOP(OP_LEAVE, 0, - op_prepend_elem(OP_LINESEQ, newOP(OP_ENTER, 0), ret), - NULL); + ret = op_append_list(OP_LEAVE, + newOP(OP_ENTER, 0), + ret); + if(is_value) ret->op_ppaddr = &pp_leave_keeping_stack;

Fri Sep 06 20:16:44 2019 leonerd-cpan [...] leonerd.org.uk - Status changed from 'open' to 'patched'

Mon Sep 09 09:23:16 2019 leonerd-cpan [...] leonerd.org.uk - Correspondence added

Released in 0.11. -- Paul Evans

Mon Sep 09 09:23:16 2019 leonerd-cpan [...] leonerd.org.uk - Status changed from 'patched' to 'resolved'

Mon Sep 09 09:23:16 2019 leonerd-cpan [...] leonerd.org.uk - Fixed in 0.11 added