pp_ctl.c: PP(pp_goto): Two exceptions not exercised by test suite

[jkeenan]

As part of research into issue #23618 and pull request #23782, I had occasion to peer into the depths of pp_ctl.c -- specifically, the definition of PP(pp_goto) running from lines 3247 to 3705. (The deprecation warning which, in GH #23618 we will turn into an exception message occurs starting at line 3654 of this file.)

In the course of looking at this function I began to wonder whether our test suite exercised all the currently existing exceptions defined therewithin. I set up an outline of that function and then searched the test suite (under t/) for tests where the various exception messages were set as the "expected" value in unit tests.

3228 static void
3229 S_check_op_type(pTHX_ OP * const o) /* Used at blead lines 3646 and 3680 */
3230 {
3231     /* Eventually we may want to stack the needed arguments
3232      * for each op.  For now, we punt on the hard ones. */
3233     /* XXX This comment seems to me like wishful thinking.  --sprout */
3234     if (o == UNENTERABLE)
3235         croak(     /* t/lib/croak/pp_ctl */
3236                   "Can't \"goto\" into a binary or list expression");
3237     if (o->op_type == OP_ENTERITER)
3238         croak(     /* t/op/goto.t, t/op/eval.t */
3239                   "Can't \"goto\" into the middle of a foreach loop");
3240     if (o->op_type == OP_ENTERGIVEN)
3241         croak(     /* t/lib/croak/pp_ctl */
3242                   "Can't \"goto\" into a \"given\" block");
3243 }

3247 PP(pp_goto)      
3248 {                   
...                 /* t/op/sort.t */
3295:                DIE(aTHX_ "Can't goto subroutine outside a subroutine");
...                 /* t/op/goto-sub.t */
3302:                    DIE(aTHX_ "Can't goto subroutine from an eval-string");
...                 /* t/op/goto-sub.t */
3305:                    DIE(aTHX_ "Can't goto subroutine from an eval-block");
...                 /* t/op/sort.t */
3308:                DIE(aTHX_ "Can't goto subroutine from a sort sub (or similar callback)");
...                 /* UNCLEAR; see 3605 */
3314                     croak("Can't \"%s\" out of a \"%s\" block",
3315                             "goto", S_defer_blockname(&cxstack[ix]));
...
3554     if (label_len) {
...
3602:                DIE(aTHX_ "Can't \"goto\" out of a pseudo block");
                    /* t/op/sort.t line 813 */
...
3605:                DIE(aTHX_ "Can't \"%s\" out of a \"%s\" block", "goto", S_defer_blockname(cx));
...                 /* UNCLEAR; see 3314 */
3632:                DIE(aTHX_ "Can't \"goto\" out of a pseudo block");
...
                    /* t/lib/croak/pp_ctl, t/op/goto.t, t/op/tie.t, t/op/runlevel.t */
3636:            DIE(aTHX_ "Can't find label %" UTF8f,
...
3649         if (*enterops && enterops[1]) {
3650             I32 i = enterops[1] != UNENTERABLE
3651                  && enterops[1]->op_type == OP_ENTER && in_block
3652                     ? 2
3653                     : 1;
3654             if (enterops[i])
3655                 deprecate_fatal_in(WARN_DEPRECATED__GOTO_CONSTRUCT,
3656                         "5.42",
3657                         "Use of \"goto\" to jump into a construct");
3658         }
...
3687     }  /* END if (label_len */
...
3705 }   /* END PP(pp_goto) */               

What I noticed was that there were 3 very similar exception messages:

3314                     croak("Can't \"%s\" out of a \"%s\" block",
3315                             "goto", S_defer_blockname(&cxstack[ix]));
...
3601             case CXt_NULL:
3602                 DIE(aTHX_ "Can't \"goto\" out of a pseudo block");
...
3603             case CXt_DEFER: 
3604                 /* diag_listed_as: Can't "%s" out of a "defer" block */
3605                 DIE(aTHX_ "Can't \"%s\" out of a \"%s\" block", "goto", S_defer_blockname(cx)     );

I then created a branch in which I changed the word out in the 3 different exception messages to xout, yout and zout, respectively. I then ran make test_harness. Setting aside a porting test failure, the only code failure I found was in t/op/sort.t:

ok 164 - overload string called twice
AAA:
# Failed test 147 - goto out of a pseudo block 1 at op/sort.t line 813
#      got "Can\'t \"goto\" yout of a pseudo bloc"
# expected eq "Can\'t \"goto\" out of a pseudo block"
# Can't "goto" yout of a pseudo block at op/sort.t line 811.
BBB:
CCC:

So it appears that the exceptions where I used xout and zout are not being exercised by the test suite. In blead, these would be at pp_ctl.c lines 3314-3315 and line 3605.

Can someone confirm my hypothesis?

Are these lines in pp_ctl.c actually reachable?

If so, can we write tests that throw the exception which triggers those two messages?

[richardleach]

Are these lines in pp_ctl.c actually reachable?

I took a quick look and am not immediately sure. The comment above the 3314-3315 lines reads:

/* Check for  defer { goto &...; } */

But, trying to test that with the following code causes a croak within walk_ops_forbid in op.c, not anything in pp_ctl.c.

use feature "defer"; 
sub bloop{}
my $x;
if ($x) {} else { defer { goto &bloop } }}

[jkeenan]

Are these lines in pp_ctl.c actually reachable?

I took a quick look and am not immediately sure. The comment above the 3314-3315 lines reads:

/* Check for  defer { goto &...; } */

But, trying to test that with the following code causes a croak within walk_ops_forbid in op.c, not anything in pp_ctl.c.

use feature "defer"; 
sub bloop{}
my $x;
if ($x) {} else { defer { goto &bloop } }}

How do you determine that the exception is being thrown from this place in op.c ...

 5364     /* diag_listed_as: Can't "%s" out of a "defer" block */
 5366     croak("Can't \"%s\" out of %s", PL_op_name[o->op_type], blockname);

... rather than one of these four locations in pp_ctl.c?

2867 PP(pp_return)
2868 {
...
2879:                /* diag_listed_as: Can't "%s" out of a "defer" block */
2881:                Perl_croak(aTHX_ "Can't \"%s\" out of a \"%s\" block",
...

3000 static PERL_CONTEXT *
3001 S_unwind_loop(pTHX)
3002 {
...
3049:                /* diag_listed_as: Can't "%s" out of a "defer" block */
3051:                Perl_croak(aTHX_ "Can't \"%s\" out of a \"%s\" block",
...

3239 PP(pp_goto)
3240 {

3305:                    /* diag_listed_as: Can't "%s" out of a "defer" block */
3306:                    Perl_croak(aTHX_ "Can't \"%s\" out of a \"%s\" block",

3596:                /* diag_listed_as: Can't "%s" out of a "defer" block */
3597:                DIE(aTHX_ "Can't \"%s\" out of a \"%s\" block", "goto", S_defer_blockname(cx));

[richardleach]

How do you determine that the exception is being thrown from this place in op.c

I followed your route of changing out to xout, yout and zout in the same places you had, saw that none of them fired with the if ($x) {} else { defer { goto &bloop } }} test, grepped for other instanced of out of and reworded the out in there too. The one in walk_ops_forbid turned out to be the one throwing.

(Setting breakpoints on all of them in gdb would probably be the "proper" method of going about it. 😁 )

[jkeenan]

How do you determine that the exception is being thrown from this place in op.c

I followed your route of changing out to xout, yout and zout in the same places you had, saw that none of them fired with the if ($x) {} else { defer { goto &bloop } }} test, grepped for other instanced of out of and reworded the out in there too. The one in walk_ops_forbid turned out to be the one throwing.

Confirmed. With:

diff --git a/op.c b/op.c
index 8897612a8c..e3f15961c5 100644
--- a/op.c
+++ b/op.c
@@ -5407,7 +5407,7 @@ static void walk_ops_forbid(pTHX_ OP *o, U32 flags, HV *permittedloops, HV *perm
 forbid:
             /* diag_listed_as: Can't "%s" out of a "defer" block */
             /* diag_listed_as: Can't "%s" out of a "finally" block */
-            croak("Can't \"%s\" out of %s", PL_op_name[o->op_type], blockname);
+            croak("Can't \"%s\" wout of %s", PL_op_name[o->op_type], blockname);
 
         default:
             break;

... in my goto-out-of-error-messages-20251031 branch, I now get this:

$ cd t; TEST_JOBS=1 ./perl harness lib/croak.t op/sort.t; cd -
lib/croak.t .. 1/312 FILE: lib/croak/class ; line 100
PROG: 
# This test is known to leak: see GH #20812.
no warnings 'experimental::class';
use feature 'class';
class XXX {
  field $x = last;
}
EXPECTED:
Can't "last" out of field initialiser expression at - line 5.
EXIT STATUS: != 0
GOT:
Can't "last" wout of field initialiser expression at - line 5.
EXIT STATUS: 255
# Failed test 14 - test from lib/croak/class at line 100 at lib/croak/class line 100
FILE: lib/croak/op ; line 230
PROG: 
use feature 'defer';
no warnings 'experimental::defer';
defer { return "retval" }
EXPECTED:
Can't "return" out of a "defer" block at - line 3.
EXIT STATUS: != 0
GOT:
Can't "return" wout of a "defer" block at - line 3.
EXIT STATUS: 255
# Failed test 77 - test from lib/croak/op at line 230 at lib/croak/op line 230
FILE: lib/croak/op ; line 236
PROG: 
use feature 'defer';
no warnings 'experimental::defer';
defer { goto HERE }
HERE:
EXPECTED:
Can't "goto" out of a "defer" block at - line 3.
EXIT STATUS: != 0
GOT:
Can't "goto" wout of a "defer" block at - line 3.
EXIT STATUS: 255
# Failed test 78 - test from lib/croak/op at line 236 at lib/croak/op line 236
FILE: lib/croak/op ; line 243
PROG: 
use feature 'defer';
no warnings 'experimental::defer';
my $subB = sub {};
defer { goto &$subB }
EXPECTED:
Can't "goto" out of a "defer" block at - line 4.
EXIT STATUS: != 0
GOT:
Can't "goto" wout of a "defer" block at - line 4.
EXIT STATUS: 255
# Failed test 79 - test from lib/croak/op at line 243 at lib/croak/op line 243
FILE: lib/croak/op ; line 250
PROG: 
use feature 'defer';
no warnings 'experimental::defer';
LOOP: while(1) {
  defer { last LOOP }
}
EXPECTED:
Can't "last" out of a "defer" block at - line 4.
EXIT STATUS: != 0
GOT:
Can't "last" wout of a "defer" block at - line 4.
EXIT STATUS: 255
# Failed test 80 - test from lib/croak/op at line 250 at lib/croak/op line 250
lib/croak.t .. 81/312 FILE: lib/croak/op ; line 258
PROG: 
use feature 'try';
no warnings 'experimental::try';
try {} catch ($e) {} finally { return "123" }
EXPECTED:
Can't "return" out of a "finally" block at - line 3.
EXIT STATUS: != 0
GOT:
Can't "return" wout of a "finally" block at - line 3.
EXIT STATUS: 255
# Failed test 81 - test from lib/croak/op at line 258 at lib/croak/op line 258
FILE: lib/croak/op ; line 264
PROG: 
use feature 'try';
no warnings 'experimental::try';
try {} catch ($e) {} finally { goto HERE; }
HERE:
EXPECTED:
Can't "goto" out of a "finally" block at - line 3.
EXIT STATUS: != 0
GOT:
Can't "goto" wout of a "finally" block at - line 3.
EXIT STATUS: 255
# Failed test 82 - test from lib/croak/op at line 264 at lib/croak/op line 264
FILE: lib/croak/op ; line 271
PROG: 
use feature 'try';
no warnings 'experimental::try';
LOOP: {
  try {} catch ($e) {} finally { last LOOP; }
}
EXPECTED:
Can't "last" out of a "finally" block at - line 4.
EXIT STATUS: != 0
GOT:
Can't "last" wout of a "finally" block at - line 4.
EXIT STATUS: 255
# Failed test 83 - test from lib/croak/op at line 271 at lib/croak/op line 271
lib/croak.t .. 285/312 #
# Note: 'run_multiple_progs' run has one or more failures
#        you can consider setting the environment variable
#        PERL_TEST_ABORT_FIRST_FAILURE=1 before running the test
#        to stop on the first error.
#
lib/croak.t .. Failed 8/312 subtests 
	(less 3 skipped subtests: 301 okay)
op/sort.t .... 1/205 AAA:
# Failed test 147 - goto out of a pseudo block 1 at op/sort.t line 813
#      got "Can\'t \"goto\" yout of a pseudo bloc"
# expected eq "Can\'t \"goto\" out of a pseudo block"
# Can't "goto" yout of a pseudo block at op/sort.t line 811.
BBB:
CCC:
op/sort.t .... Failed 1/205 subtests 
	(less 1 skipped subtest: 203 okay)

Test Summary Report
-------------------
lib/croak.t (Wstat: 0 Tests: 312 Failed: 8)
  Failed tests:  14, 77-83
op/sort.t  (Wstat: 0 Tests: 205 Failed: 1)
  Failed test:  147
Files=2, Tests=517,  2 wallclock secs ( 0.06 usr  0.01 sys +  0.71 cusr  0.93 csys =  1.71 CPU)
Result: FAIL

So that exception you cited in op.c actually gets triggered quite a bit -- and for more than just goto.