-
Notifications
You must be signed in to change notification settings - Fork 601
Description
This is the next ticket in a series of tickets originating in #23889 (which in turn originated in GH #23618). In preparation for the partial fatalization of goto LABEL, we've been examining the code in pp_ctl.c that governs the goto functionality. We've been trying to see that all exceptions defined in the relevant functions in pp_ctl.c are exercised ("covered") by the test suite.
In this ticket we turn our attention to this part of pp_ctl.c as it appears in blead as of v5.43.5-24-ge79b820fc6:
3364 if (!CvROOT(cv) && !CvXSUB(cv)) {
3365 const GV * const gv = CvGV(cv);
3366 if (gv) {
3367 SV * const tmpstr = sv_newmortal();
3368 gv_efullname3(tmpstr, gv, NULL);
3369 DIE(aTHX_ "Goto undefined subroutine &%" SVf, /* <-- case 'KKK' */
3370 SVfARG(tmpstr));
3371 }
3372 DIE(aTHX_ "Goto undefined subroutine"); /* <-- case 'LLL' */
3373 }
(The KKK: and LLL: notations are from #23889 (comment).)
My research indicates that the KKK: exception is covered by the test suite in t/op/goto-sub.t here:
68 # bug #99850, which is similar - freeing the subroutine we are about to
69 # go(in)to during a FREETMPS call should not crash perl.
70
71 package _99850 {
72 sub reftype{}
73 DESTROY { undef &reftype }
74 eval { sub { my $guard = bless []; goto &reftype }->() };
75 }
76 like $@, qr/^Goto undefined subroutine &_99850::reftype at /,
77 'goto &foo undefining &foo on sub cleanup';
This code was introduced, originally in t/op/goto.t, by Father C in 1d59c03 back in November 2011. The pp_ctl.c code, however, dates back much farther. It was introduced by @doughera in 4aa0a1f in June 1995:
4aa0a1f7324 (Andy Dougherty 1995-06-06 01:41:07 +0000 2103) DIE("Goto undefined subroutine &%s",SvPVX(tmpstr));
4aa0a1f7324 (Andy Dougherty 1995-06-06 01:41:07 +0000 2104) }
4aa0a1f7324 (Andy Dougherty 1995-06-06 01:41:07 +0000 2105) DIE("Goto undefined subroutine");
I don't believe the KKK: case was covered by the test suite until 2011. I can find no evidence that the LLL: case has ever been exercised by the test suite, and I have been pondering how it could be covered for the past week and have not come up with anything that works. Here are alternative ways we could deal with this situation:
-
Someone could come up with a unit test that exercises line 3372. That test would go into
t/op/goto-sub.t. Such a test would presumably cover the case whereif (gv)at line 3366 evaluated false. -
We could decide line 3372 is absolutely unreachable; I'm not sure how we would revise
pp_ctl.cin that case. -
In a somewhat similar case, @iabyn has suggested in pp_ctl.c: comment on unreachable code #23965 (review), we could hypothesize that though line 3372 is not exercisable by the test suite, someone could come up with some XS magic that exercises that line -- hence exception in that line should be retained, which conclusion we indicate with an inline comment.
Suggestions?