Skip to content

Commit

Permalink
${LAST_FH}: keep a reference count when saving on the save stack
Browse files Browse the repository at this point in the history
PL_last_in_gv is a weak pointer to the most recent input handle, if
this was localized by localizing $., if the GV it referenced was
freed, the popping of the new value from the save stack could set
PL_last_in_gv to a now freed, and possible re-used SV slot.

To avoid that, keep a reference count to the saved GV when saving it
on the save stack.

This could possibly delay closing a file but I expect localizing $. is
rare, so I don't expect it to be a problem in practice.

Fixes Perl#19124
  • Loading branch information
tonycoz committed Aug 1, 2024
1 parent d716ef9 commit 3b22d26
Show file tree
Hide file tree
Showing 8 changed files with 72 additions and 39 deletions.
3 changes: 1 addition & 2 deletions mg.c
Original file line number Diff line number Diff line change
Expand Up @@ -3208,8 +3208,7 @@ Perl_magic_set(pTHX_ SV *sv, MAGIC *mg)
case '.':
if (PL_localizing) {
if (PL_localizing == 1) {
SAVESPTR(PL_last_in_gv);
SAVESPTR(PL_last_in_io);
SAVE_LAST_IN();
}
}
else if (SvOK(sv) && (gv = last_in_gv()) && GvIO(gv))
Expand Down
3 changes: 1 addition & 2 deletions pp_sys.c
Original file line number Diff line number Diff line change
Expand Up @@ -506,8 +506,7 @@ PP(pp_glob)
}
#endif /* !VMS */

SAVESPTR(PL_last_in_gv); /* We don't want this to be permanent. */
SAVESPTR(PL_last_in_io);
SAVE_LAST_IN();
PL_last_in_gv = gv;
PL_last_in_io = NULL;

Expand Down
1 change: 1 addition & 0 deletions regen/scope_types.pl
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ BEGIN
SAVEt_FREEPADNAME
SAVEt_STRLEN_SMALL
SAVEt_FREERCPV
SAVEt_LAST_IN
/* two args */
Expand Down
13 changes: 13 additions & 0 deletions scope.c
Original file line number Diff line number Diff line change
Expand Up @@ -1391,6 +1391,19 @@ Perl_leave_scope(pTHX_ I32 base)
Safefree(a0.any_ptr);
break;

case SAVEt_LAST_IN:
a0 = ap[0];
PL_last_in_gv = (GV*)a0.any_gv;
/* the various functions that set PL_last_in_gv are
* incautious, so don't check PL_last_in_gv is a GV with GP.
*/
PL_last_in_io = GvIO(PL_last_in_gv);
/* this will set PL_last_in_gv to NULL if it is freed,
* similarly for PL_last_in_io
*/
SvREFCNT_dec(PL_last_in_gv);
break;

case SAVEt_CLEARPADRANGE:
{
I32 i;
Expand Down
7 changes: 7 additions & 0 deletions scope.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,13 @@ scope has the given name. C<name> must be a literal string.

#define SAVECOPLINE(c) SAVEI32(CopLINE(c))

#ifdef PERL_CORE

#define SAVE_LAST_IN() \
save_pushptr(SvREFCNT_inc(PL_last_in_gv), SAVEt_LAST_IN)

#endif

/*
=for apidoc_section $stack
=for apidoc Am|SSize_t|SSNEW |Size_t size
Expand Down
70 changes: 36 additions & 34 deletions scope_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,45 +44,46 @@
#define SAVEt_FREEPADNAME 23
#define SAVEt_STRLEN_SMALL 24
#define SAVEt_FREERCPV 25
#define SAVEt_LAST_IN 26

/* two args */

#define SAVEt_AV 26
#define SAVEt_DESTRUCTOR 27
#define SAVEt_DESTRUCTOR_X 28
#define SAVEt_GENERIC_PVREF 29
#define SAVEt_GENERIC_SVREF 30
#define SAVEt_GP 31
#define SAVEt_GVSV 32
#define SAVEt_HINTS 33
#define SAVEt_HPTR 34
#define SAVEt_HV 35
#define SAVEt_I32 36
#define SAVEt_INT 37
#define SAVEt_ITEM 38
#define SAVEt_IV 39
#define SAVEt_LONG 40
#define SAVEt_PPTR 41
#define SAVEt_SAVESWITCHSTACK 42
#define SAVEt_SHARED_PVREF 43
#define SAVEt_SPTR 44
#define SAVEt_STRLEN 45
#define SAVEt_SV 46
#define SAVEt_SVREF 47
#define SAVEt_VPTR 48
#define SAVEt_ADELETE 49
#define SAVEt_APTR 50
#define SAVEt_RCPV 51
#define SAVEt_AV 27
#define SAVEt_DESTRUCTOR 28
#define SAVEt_DESTRUCTOR_X 29
#define SAVEt_GENERIC_PVREF 30
#define SAVEt_GENERIC_SVREF 31
#define SAVEt_GP 32
#define SAVEt_GVSV 33
#define SAVEt_HINTS 34
#define SAVEt_HPTR 35
#define SAVEt_HV 36
#define SAVEt_I32 37
#define SAVEt_INT 38
#define SAVEt_ITEM 39
#define SAVEt_IV 40
#define SAVEt_LONG 41
#define SAVEt_PPTR 42
#define SAVEt_SAVESWITCHSTACK 43
#define SAVEt_SHARED_PVREF 44
#define SAVEt_SPTR 45
#define SAVEt_STRLEN 46
#define SAVEt_SV 47
#define SAVEt_SVREF 48
#define SAVEt_VPTR 49
#define SAVEt_ADELETE 50
#define SAVEt_APTR 51
#define SAVEt_RCPV 52

/* three args */

#define SAVEt_HELEM 52
#define SAVEt_PADSV_AND_MORTALIZE 53
#define SAVEt_SET_SVFLAGS 54
#define SAVEt_GVSLOT 55
#define SAVEt_AELEM 56
#define SAVEt_DELETE 57
#define SAVEt_HINTS_HH 58
#define SAVEt_HELEM 53
#define SAVEt_PADSV_AND_MORTALIZE 54
#define SAVEt_SET_SVFLAGS 55
#define SAVEt_GVSLOT 56
#define SAVEt_AELEM 57
#define SAVEt_DELETE 58
#define SAVEt_HINTS_HH 59

static const U8 leave_scope_arg_counts[] = {
0, /* SAVEt_ALLOC */
Expand Down Expand Up @@ -111,6 +112,7 @@ static const U8 leave_scope_arg_counts[] = {
1, /* SAVEt_FREEPADNAME */
1, /* SAVEt_STRLEN_SMALL */
1, /* SAVEt_FREERCPV */
1, /* SAVEt_LAST_IN */
2, /* SAVEt_AV */
2, /* SAVEt_DESTRUCTOR */
2, /* SAVEt_DESTRUCTOR_X */
Expand Down Expand Up @@ -146,6 +148,6 @@ static const U8 leave_scope_arg_counts[] = {
3 /* SAVEt_HINTS_HH */
};

#define MAX_SAVEt 58
#define MAX_SAVEt 59

/* ex: set ro ft=c: */
1 change: 1 addition & 0 deletions sv.c
Original file line number Diff line number Diff line change
Expand Up @@ -15331,6 +15331,7 @@ Perl_ss_dup(pTHX_ PerlInterpreter *proto_perl, CLONE_PARAMS* param)
case SAVEt_FREESV:
case SAVEt_MORTALIZESV:
case SAVEt_READONLY_OFF:
case SAVEt_LAST_IN:
sv = (const SV *)POPPTR(ss,ix);
TOPPTR(nss,ix) = sv_dup_inc(sv, param);
break;
Expand Down
13 changes: 12 additions & 1 deletion t/op/magic.t
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ BEGIN {
chdir 't' if -d 't';
require './test.pl';
set_up_inc( '../lib' );
plan (tests => 212); # some tests are run in BEGIN block
plan (tests => 213); # some tests are run in BEGIN block
}

# Test that defined() returns true for magic variables created on the fly,
Expand Down Expand Up @@ -776,6 +776,17 @@ print \$. == 42 ? "ok\n" : "not ok \$.\n";
EOS
}

# check a saved (localized) ${^LAST_FH} isn't freed out from
# under us, this code would assert
# [github #19124]
fresh_perl_is(<<'EOS', "ok\n", undef, "last_fh on save stack not orphaned");
open my $x, "<", "op/magic.t" or die;
<$x>;
{ local $.; undef $x; }
seek ${^LAST_FH}, 0, 0;
print "ok\n";
EOS

# $|
fresh_perl_is 'print $| = ~$|', "1\n", {switches => ['-l']},
'[perl #4760] print $| = ~$|';
Expand Down

0 comments on commit 3b22d26

Please sign in to comment.