retain the last file handle even if it's an IO reference

It's always been possible to extract a reference to the IO SV from
a glob, and use that for I/O, but such use would set PL_last_in_gv
to a temporary GV which would almost immediately be released,
clearing PL_last_in_gv.

This would result in reads from $. returning its most recently read
value (even from another handle) and modification being ignored, while
${^LAST_FH} would return undef, even though the file handle is still
open.

To fix this, store the underlying IO object as well as the GV.
Retaining the GV lets errors continue to report the GV name where
possible, while the IO object means $. and ${^LAST_FH} can still work
if the IO object still exists.

This does not try to fix the problem where localization can leave
PL_last_in_gv (and now PL_last_in_io) pointing at a SV that has been
destroyed, the original changes that introduced PL_last_in_gv didn't
keep a reference count for the GV, since this would prevent the file
being close automatically when the reference to to the glob went out
of scope.

Fixes Perl#1420

embed.fnc

@@ -2542,6 +2542,9 @@ CTp	|Signal_t|perly_sighandler					\
 				|NULLOK void *uap			\
 				|bool safe
 
+Adm	|const char * const|phase_name					\
+				|enum perl_phase
+
 Adm	|const char * const|phase_name					\
 				|enum perl_phase
 Adp	|void	|pmop_dump	|NULLOK PMOP *pm
@@ -4001,6 +4004,7 @@ EXpx	|SV *	|sv_setsv_cow	|NULLOK SV *dsv 			\
 				|NN SV *ssv
 #endif
 #if defined(PERL_CORE)
+dip	|GV *	|last_in_gv
 p	|void	|opslab_force_free					\
 				|NN OPSLAB *slab
 p	|void	|opslab_free	|NN OPSLAB *slab

embed.h

@@ -1137,6 +1137,7 @@
 #   define yyparse(a)                           Perl_yyparse(aTHX_ a)
 #   define yyquit()                             Perl_yyquit(aTHX)
 #   define yyunlex()                            Perl_yyunlex(aTHX)
+#   define last_in_gv()                         Perl_last_in_gv(aTHX)
 #   define opslab_force_free(a)                 Perl_opslab_force_free(aTHX_ a)
 #   define opslab_free(a)                       Perl_opslab_free(aTHX_ a)
 #   define opslab_free_nopad(a)                 Perl_opslab_free_nopad(aTHX_ a)

inline.h

@@ -4442,6 +4442,40 @@ Perl_my_strlcpy(char *dst, const char *src, Size_t size)
 }
 #endif
 
+#ifdef PERL_CORE
+
+/*
+=for apidoc last_in_gv
+
+Accessor for L<perlintern/PL_last_in_gv>/L<perlintern/PL_last_in_io>.
+
+Thie returns C<PL_last_in_gv> if valid, otherwise if C<PL_last_in_io>
+is non-NULL, returns a mortal GV referring to that.
+
+Returns NULL if neither is usable.
+
+=cut
+*/
+
+PERL_STATIC_INLINE GV *
+Perl_last_in_gv(pTHX)
+{
+   if (PL_last_in_gv) {
+        return PL_last_in_gv;
+   }
+   else if (PL_last_in_io && SvTYPE(PL_last_in_io) == SVt_PVIO) {
+        GV *gv = (GV*)sv_newmortal();
+        gv_init(gv, 0, "__ANONIO__", 10, 0);
+        SvREFCNT_inc_simple_void(PL_last_in_io);
+        GvIOp(gv) = MUTABLE_IO(PL_last_in_io);
+        return gv;
+    }
+
+    return NULL;
+}
+
+#endif
+
 /*
  * ex: set ts=8 sts=4 sw=4 et:
  */

intrpvar.h

@@ -297,7 +297,14 @@ On threaded perls, each thread has an independent copy of this variable;
 each initialized at creation time with the current value of the creating
 thread's copy.
 
-=for apidoc_section $io
+=for apidoc mn|IO*|PL_last_in_io
+
+The IO which was last used for a filehandle input operation.  (C<< <FH> >>)
+
+On threaded perls, each thread has an independent copy of this variable;
+each initialized at creation time with the current value of the creating
+thread's copy.
+
 =for apidoc mn|GV*|PL_ofsgv
 
 The glob containing the output field separator - C<*,> in Perl space.
@@ -311,6 +318,7 @@ thread's copy.
 
 PERLVAR(I, rs,		SV *)		/* input record separator $/ */
 PERLVAR(I, last_in_gv,	GV *)		/* GV used in last <FH> */
+PERLVAR(I, last_in_io,  IO *)           /* IO used in last <FH> */
 PERLVAR(I, ofsgv,	GV *)		/* GV of output field separator *, */
 PERLVAR(I, defoutgv,	GV *)		/* default FH for output */
 PERLVARI(I, chopset,	const char *, " \n-")	/* $: */

mg.c

@@ -887,6 +887,7 @@ Perl_magic_get(pTHX_ SV *sv, MAGIC *mg)
     const char *s = NULL;
     REGEXP *rx;
     char nextchar;
+    GV *gv;
 
     PERL_ARGS_ASSERT_MAGIC_GET;
 
@@ -1060,6 +1061,24 @@ Perl_magic_get(pTHX_ SV *sv, MAGIC *mg)
                 sv_setrv_inc(sv, MUTABLE_SV(PL_last_in_gv));
                 sv_rvweaken(sv);
             }
+            else if (PL_last_in_io && SvTYPE(PL_last_in_io) == SVt_PVIO) {
+                /* sv_rvweaken() will remove one reference, and we
+                   want the GV referenced to live long enough for the
+                   accessing code to see it, so make the other
+                   reference mortalized.
+                */
+                GV *gv = (GV*)sv_newmortal();
+                SvREFCNT_inc_simple_NN(gv); /* one reference for sv_rvweaken() to eat */
+                gv_init(gv, 0, "__ANONIO__", 10, 0);
+                SvREFCNT_inc_simple_NN(PL_last_in_io);
+                GvIOp(gv) = MUTABLE_IO(PL_last_in_io);
+                SV_CHECK_THINKFIRST_COW_DROP(sv);
+                prepare_SV_for_RV(sv);
+                SvOK_off(sv);
+                SvRV_set(sv, (SV*)gv);
+                SvROK_on(sv);
+                sv_rvweaken(sv);
+            }
             else
                 sv_set_undef(sv);
         }
@@ -1169,8 +1188,8 @@ Perl_magic_get(pTHX_ SV *sv, MAGIC *mg)
         }
         goto set_undef;
     case '.':
-        if (GvIO(PL_last_in_gv)) {
-            sv_setiv(sv, (IV)IoLINES(GvIOp(PL_last_in_gv)));
+        if ((gv = last_in_gv()) && GvIO(gv)) {
+            sv_setiv(sv, (IV)IoLINES(GvIOp(gv)));
         }
         break;
     case '?':
@@ -3008,6 +3027,7 @@ Perl_magic_set(pTHX_ SV *sv, MAGIC *mg)
     I32 i;
     STRLEN len;
     MAGIC *tmg;
+    GV *gv;
 
     PERL_ARGS_ASSERT_MAGIC_SET;
 
@@ -3203,11 +3223,13 @@ Perl_magic_set(pTHX_ SV *sv, MAGIC *mg)
         break;
     case '.':
         if (PL_localizing) {
-            if (PL_localizing == 1)
+            if (PL_localizing == 1) {
                 SAVESPTR(PL_last_in_gv);
+                SAVESPTR(PL_last_in_io);
+            }
         }
-        else if (SvOK(sv) && GvIO(PL_last_in_gv))
-            IoLINES(GvIOp(PL_last_in_gv)) = SvIV(sv);
+        else if (SvOK(sv) && (gv = last_in_gv()) && GvIO(gv))
+            IoLINES(GvIOp(gv)) = SvIV(sv);
         break;
     case '^':
         {

perl.c

@@ -1072,6 +1072,7 @@ perl_destruct(pTHXx)
     PL_stdingv = NULL;
     PL_stderrgv = NULL;
     PL_last_in_gv = NULL;
+    PL_last_in_io = NULL;
     PL_DBsingle = NULL;
     PL_DBtrace = NULL;
     PL_DBsignal = NULL;

pp_ctl.c

@@ -1334,8 +1334,9 @@ PP_wrapped(pp_flip,((GIMME_V == G_LIST) ? 0 : 1), 0)
         int flip = 0;
 
         if (PL_op->op_private & OPpFLIP_LINENUM) {
-            if (GvIO(PL_last_in_gv)) {
-                flip = SvIV(sv) == (IV)IoLINES(GvIOp(PL_last_in_gv));
+            GV *gv = last_in_gv();
+            if (GvIO(gv)) {
+                flip = SvIV(sv) == (IV)IoLINES(GvIOp(gv));
             }
             else {
                 GV * const gv = gv_fetchpvs(".", GV_ADD|GV_NOTQUAL, SVt_PV);
@@ -1453,8 +1454,9 @@ PP_wrapped(pp_flop, (GIMME_V == G_LIST) ? 2 : 1, 0)
         sv_inc(targ);
 
         if (PL_op->op_private & OPpFLIP_LINENUM) {
-            if (GvIO(PL_last_in_gv)) {
-                flop = SvIV(sv) == (IV)IoLINES(GvIOp(PL_last_in_gv));
+            GV *gv = last_in_gv();
+            if (GvIO(gv)) {
+                flop = SvIV(sv) == (IV)IoLINES(GvIOp(gv));
             }
             else {
                 GV * const gv = gv_fetchpvs(".", GV_ADD|GV_NOTQUAL, SVt_PV);

pp_hot.c

@@ -4062,7 +4062,7 @@ Perl_do_readline(pTHX)
     STRLEN tmplen = 0;
     STRLEN offset;
     PerlIO *fp;
-    IO * const io = GvIO(PL_last_in_gv);
+    IO * const io = PL_last_in_io = GvIO(PL_last_in_gv);
 
     /* process tied file handle if present */
 

pp_sys.c

@@ -507,7 +507,9 @@ PP(pp_glob)
 #endif /* !VMS */
 
     SAVESPTR(PL_last_in_gv);	/* We don't want this to be permanent. */
+    SAVESPTR(PL_last_in_io);
     PL_last_in_gv = gv;
+    PL_last_in_io = NULL;
 
     SAVESPTR(PL_rs);		/* This is not permanent, either. */
     PL_rs = newSVpvs_flags("\000", SVs_TEMP);
@@ -529,7 +531,7 @@ PP(pp_glob)
 
 PP(pp_rcatline)
 {
-    PL_last_in_gv = cGVOP_gv;
+    PL_last_in_gv = cGVOP_gv; /* do_readline() will set PL_last_in_io */
     return do_readline();
 }
 
@@ -2366,15 +2368,16 @@ PP_wrapped(pp_eof, MAXARG, 0)
             which = 2;
         }
         else {
-            gv = PL_last_in_gv;			/* eof */
+            gv = last_in_gv();			/* eof */
             which = 0;
         }
     }
+    PL_last_in_io = io = GvIO(gv);
 
     if (!gv)
         RETPUSHYES;
 
-    if ((io = GvIO(gv)) && (mg = SvTIED_mg((const SV *)io, PERL_MAGIC_tiedscalar))) {
+    if (io && (mg = SvTIED_mg((const SV *)io, PERL_MAGIC_tiedscalar))) {
         return tied_method1(SV_CONST(EOF), SP, MUTABLE_SV(io), mg, newSVuv(which));
     }
 
@@ -2409,13 +2412,15 @@ PP_wrapped(pp_tell, MAXARG, 0)
     GV *gv;
     IO *io;
 
-    if (MAXARG != 0 && (TOPs || POPs))
-        PL_last_in_gv = MUTABLE_GV(POPs);
-    else
+    if (MAXARG != 0 && (TOPs || POPs)) {
+        gv = PL_last_in_gv = MUTABLE_GV(POPs);
+    }
+    else {
         EXTEND(SP, 1);
-    gv = PL_last_in_gv;
+        gv = last_in_gv();
+    }
+    PL_last_in_io = io = GvIO(gv);
 
-    io = GvIO(gv);
     if (io) {
         const MAGIC * const mg = SvTIED_mg((const SV *)io, PERL_MAGIC_tiedscalar);
         if (mg) {
@@ -2451,7 +2456,7 @@ PP_wrapped(pp_sysseek, 3, 0)
 #endif
 
     GV * const gv = PL_last_in_gv = MUTABLE_GV(POPs);
-    IO *const io = GvIO(gv);
+    IO *const io = PL_last_in_io = GvIO(gv);
 
     if (io) {
         const MAGIC * const mg = SvTIED_mg((const SV *)io, PERL_MAGIC_tiedscalar);

sv.c

@@ -6808,6 +6808,8 @@ Perl_sv_clear(pTHX_ SV *const orig_sv)
             Safefree(IoBOTTOM_NAME(sv));
             if ((const GV *)sv == PL_statgv)
                 PL_statgv = NULL;
+            else if ((const IO *)sv == PL_last_in_io)
+                PL_last_in_io = NULL;
             goto freescalar;
         case SVt_REGEXP:
             /* FIXME for plugins */
@@ -16383,6 +16385,7 @@ perl_clone_using(PerlInterpreter *proto_perl, UV flags,
 
     PL_rs		= sv_dup_inc(proto_perl->Irs, param);
     PL_last_in_gv	= gv_dup(proto_perl->Ilast_in_gv, param);
+    PL_last_in_io	= io_dup(proto_perl->Ilast_in_io, param);
     PL_defoutgv		= gv_dup_inc(proto_perl->Idefoutgv, param);
     PL_toptarget	= sv_dup_inc(proto_perl->Itoptarget, param);
     PL_bodytarget	= sv_dup_inc(proto_perl->Ibodytarget, param);

t/op/magic.t

@@ -5,7 +5,7 @@ BEGIN {
     chdir 't' if -d 't';
     require './test.pl';
     set_up_inc( '../lib' );
-    plan (tests => 208); # some tests are run in BEGIN block
+    plan (tests => 212); # some tests are run in BEGIN block
 }
 
 # Test that defined() returns true for magic variables created on the fly,
@@ -743,6 +743,11 @@ is ${^LAST_FH}, \*STDIN, '${^LAST_FH} after another tell';
     () = tell $fh;
     is ${^LAST_FH}, \$fh, '${^LAST_FH} referencing lexical coercible glob';
 }
+{
+    # set PL_last_in_gv, and clear it, and clears PL_last_in_io
+    open my $fh, "<", "test.pl" or die;
+    () = tell $fh;
+}
 # 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';
 
@@ -755,6 +760,22 @@ for my $code ('tell $0', 'sysseek $0, 0, 0', 'seek $0, 0, 0', 'eof $0') {
                   undef, "check $code doesn't define \${^LAST_FH}");
 }
 
+# check each case where only the IO is still available that we still
+# have a working handle
+for my $code ('tell $fh', 'sysseek $fh, 0, 0', 'seek $fh, 0, 0', 'eof $fh') {
+    fresh_perl_is(<<EOS, "ok\n", undef, "check $code populates PL_last_in_io");
+my \$fh;
+{
+    open my \$fhg, "<", "test.pl" or die;
+    # only preserve the IO
+    my \$fh = *{\$fhg}{IO};
+    $code;
+    \$. = 42;
+}
+print \$. == 42 ? "ok\n" : "not ok \$.\n";
+EOS
+}
+
 # $|
 fresh_perl_is 'print $| = ~$|', "1\n", {switches => ['-l']},
  '[perl #4760] print $| = ~$|';