summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Mitchell <davem@iabyn.com>2015-12-11 12:06:39 +0000
committerDavid Mitchell <davem@iabyn.com>2015-12-11 12:14:57 +0000
commit2baebb988343167d06d3d536e2de40b4427dc37e (patch)
tree8085a6070450a904261fd858ad443d206081a722
parentfacc1dc2646135cef09100080f18ecafbf1df900 (diff)
downloadperl-2baebb988343167d06d3d536e2de40b4427dc37e.tar.gz
avoid leaks when calling mg_set() in leave_scope()
In leave_scope() in places like SAVEt_SV, it does stuff like if (SvSMAGICAL(...)) mg_set(...) SvREFCNT_dec_NN(ARG0_SV) If mg_set() dies (e.g. it calls STORE() and STORE() dies), then ARG0_SV would leak. Fix this by putting ARG0_SV back in the save stack in this case. A similar thing applies to SAVEt_AV and SAVEt_HV, but I couldn't think of a simple test for those, as tied array and hashes don't have set magic (just RMG). Also, SAVEt_AV and SAVEt_HV share a lot of common code, so I made SAVEt_HV goto into the SAVEt_AV code block for the common part.
-rw-r--r--scope.c25
-rw-r--r--t/op/svleak.t26
2 files changed, 43 insertions, 8 deletions
diff --git a/scope.c b/scope.c
index 7df465f9f0..e5687f4ac2 100644
--- a/scope.c
+++ b/scope.c
@@ -841,9 +841,18 @@ Perl_leave_scope(pTHX_ I32 base)
*svp = ARG0_SV;
SvREFCNT_dec(sv);
if (UNLIKELY(SvSMAGICAL(ARG0_SV))) {
+ /* mg_set could die, skipping the freeing of ARG0_SV and
+ * refsv; Ensure that they're always freed in that case */
+ dSS_ADD;
+ SS_ADD_PTR(ARG0_SV);
+ SS_ADD_UV(SAVEt_FREESV);
+ SS_ADD_PTR(refsv);
+ SS_ADD_UV(SAVEt_FREESV);
+ SS_ADD_END(4);
PL_localizing = 2;
mg_set(ARG0_SV);
PL_localizing = 0;
+ break;
}
SvREFCNT_dec_NN(ARG0_SV);
SvREFCNT_dec(refsv);
@@ -898,23 +907,25 @@ Perl_leave_scope(pTHX_ I32 base)
case SAVEt_AV: /* array reference */
SvREFCNT_dec(GvAV(ARG1_GV));
GvAV(ARG1_GV) = ARG0_AV;
+ avhv_common:
if (UNLIKELY(SvSMAGICAL(ARG0_SV))) {
+ /* mg_set might die, so make sure ARG1 isn't leaked */
+ dSS_ADD;
+ SS_ADD_PTR(ARG1_SV);
+ SS_ADD_UV(SAVEt_FREESV);
+ SS_ADD_END(2);
PL_localizing = 2;
mg_set(ARG0_SV);
PL_localizing = 0;
+ break;
}
SvREFCNT_dec_NN(ARG1_GV);
break;
case SAVEt_HV: /* hash reference */
SvREFCNT_dec(GvHV(ARG1_GV));
GvHV(ARG1_GV) = ARG0_HV;
- if (UNLIKELY(SvSMAGICAL(ARG0_SV))) {
- PL_localizing = 2;
- mg_set(ARG0_SV);
- PL_localizing = 0;
- }
- SvREFCNT_dec_NN(ARG1_GV);
- break;
+ goto avhv_common;
+
case SAVEt_INT_SMALL:
*(int*)ARG0_PTR = (int)(uv >> SAVE_TIGHT_SHIFT);
break;
diff --git a/t/op/svleak.t b/t/op/svleak.t
index 076f2bfdaf..4c7a493d52 100644
--- a/t/op/svleak.t
+++ b/t/op/svleak.t
@@ -15,7 +15,7 @@ BEGIN {
use Config;
-plan tests => 129;
+plan tests => 130;
# run some code N times. If the number of SVs at the end of loop N is
# greater than (N-1)*delta at the end of loop 1, we've got a leak
@@ -493,3 +493,27 @@ $x = $mdr::a[0]{foo}{$mdr::k}{$mdr::i};
$x = $mdr::h[0]{foo}{$mdr::k}{$mdr::i};
$x = $mdr::r->[0]{foo}{$mdr::k}{$mdr::i};
EOF
+
+# un-localizing a tied (or generally magic) item could leak if the things
+# called by mg_set() died
+
+{
+ package MG_SET;
+
+ sub TIESCALAR { bless [] }
+ sub FETCH { 1; }
+ my $do_die = 0;
+ sub STORE { die if $do_die; }
+
+ sub f {
+ local $s;
+ tie $s, 'MG_SET';
+ local $s;
+ $do_die = 1;
+ }
+ sub g {
+ eval { my $x = f(); };
+ }
+
+ ::leak(5,0, \&g, "MG_SET");
+}