(perl #128263) handle PL_last_in_gv being &PL_sv_undef
authorTony Cook <tony@develop-help.com>
Thu, 31 Aug 2017 00:05:54 +0000 (10:05 +1000)
committerTony Cook <tony@develop-help.com>
Thu, 31 Aug 2017 00:21:05 +0000 (10:21 +1000)
rv2gv will return &PL_sv_undef when it can't get a GV, previously
this could cause an assertion failure in mg.c

My original fix for this changed each op that deals with GVs for I/O
to set PL_last_in_gv to NULL if there was no io object in the GV, but
this changes other behaviour as noted by FatherC.

Also partly reverts 84ee769f, which unnecessarily did the same for
readline(), so now we're consistent.

mg.c
pp_hot.c
t/op/magic.t

diff --git a/mg.c b/mg.c
index e0d1215..971fcee 100644 (file)
--- a/mg.c
+++ b/mg.c
@@ -1007,7 +1007,7 @@ Perl_magic_get(pTHX_ SV *sv, MAGIC *mg)
        break;
     case '\014':               /* ^LAST_FH */
        if (strEQ(remaining, "AST_FH")) {
-           if (PL_last_in_gv) {
+           if (PL_last_in_gv && (SV*)PL_last_in_gv != &PL_sv_undef) {
                assert(isGV_with_GP(PL_last_in_gv));
                SV_CHECK_THINKFIRST_COW_DROP(sv);
                prepare_SV_for_RV(sv);
index 528817f..ee6535c 100644 (file)
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -446,10 +446,7 @@ PP(pp_readline)
            PUTBACK;
            Perl_pp_rv2gv(aTHX);
            PL_last_in_gv = MUTABLE_GV(*PL_stack_sp--);
-           if (PL_last_in_gv == (GV *)&PL_sv_undef)
-               PL_last_in_gv = NULL;
-           else
-               assert(isGV_with_GP(PL_last_in_gv));
+            assert((SV*)PL_last_in_gv == &PL_sv_undef || isGV_with_GP(PL_last_in_gv));
        }
     }
     return do_readline();
index 3f71f8e..36abafb 100644 (file)
@@ -5,7 +5,7 @@ BEGIN {
     chdir 't' if -d 't';
     require './test.pl';
     set_up_inc( '../lib' );
-    plan (tests => 192); # some tests are run in BEGIN block
+    plan (tests => 196); # some tests are run in BEGIN block
 }
 
 # Test that defined() returns true for magic variables created on the fly,
@@ -643,6 +643,16 @@ is ${^LAST_FH}, \*STDIN, '${^LAST_FH} after another tell';
 # This also tests that ${^LAST_FH} is a weak reference:
 is ${^LAST_FH}, undef, '${^LAST_FH} is undef when PL_last_in_gv is NULL';
 
+# all of these would set PL_last_in_gv to a non-GV which would
+# assert when referenced by the magic for ${^LAST_FH}.
+# Instead it should act like <$0> which NULLs PL_last_in_gv and the magic
+# returns that as undef.
+# The approach to fixing this has changed (#128263), but it's still useful
+# to check each op.
+for my $code ('tell $0', 'sysseek $0, 0, 0', 'seek $0, 0, 0', 'eof $0') {
+    fresh_perl_is("$code; print defined \${^LAST_FH} ? qq(not ok\n) : qq(ok\n)", "ok\n",
+                  undef, "check $code doesn't define \${^LAST_FH}");
+}
 
 # $|
 fresh_perl_is 'print $| = ~$|', "1\n", {switches => ['-l']},