Use of freed comppad array during clear_yystack() #9745
Comments
From @nwc10MHX mailed p5p in 20081026231720.34258457@r2d2 with a patch to resolve the use of freed comppad array during clear_yystack() |
From p5p@spam.wizbit.beNote that the patch is already applied since 8 Apr 2009 |
|
p5p@spam.wizbit.be - Status changed from 'new' to 'open' |
From p5p@spam.wizbit.beNote2: I also tried to do a binary search on it but I can not reproduce Tested perl @24131, @26975, @31562, @34169, @34595, @35088 |
From @iabynI've pulled now pulled this fix into maint, but as marcus says, this is Prr-5.10.0 I did a lot of work on freeing up ops during error recovery, |
From gerard@ggoossen.netWith the attached patch PL_compcv instead of PL_comppad is stored in the There is an additional patch, which ads a test for "eval" that (with the previous Gerard Goossen |
From gerard@ggoossen.net0001-Store-the-PL_compcv-instead-of-the-the-PL_comppad-in.patchFrom 048dd1c49d9881c69baaa96a5909ca455118d676 Mon Sep 17 00:00:00 2001
From: Gerard Goossen <gerard@ggoossen.net>
Date: Tue, 8 Dec 2009 20:41:28 +0100
Subject: [PATCH 1/2] Store the PL_compcv instead of the the PL_comppad in parser stack,
and make it reference counted. Properly solves [perl #66094]
---
pad.h | 3 ++-
parser.h | 2 +-
perly.c | 42 ++++++++++++++++++++++++++++--------------
3 files changed, 31 insertions(+), 16 deletions(-)
diff --git a/pad.h b/pad.h
index 986f7c2..8602eda 100644
--- a/pad.h
+++ b/pad.h
@@ -249,7 +249,8 @@ Restore the old pad saved into the local variable opad by PAD_SAVE_LOCAL()
PTR2UV(PL_comppad), PTR2UV(PL_curpad)));
#define PAD_RESTORE_LOCAL(opad) \
- PL_comppad = opad && SvIS_FREED(opad) ? NULL : opad; \
+ assert(!opad || !SvIS_FREED(opad)); \
+ PL_comppad = opad; \
PL_curpad = PL_comppad ? AvARRAY(PL_comppad) : NULL; \
DEBUG_Xv(PerlIO_printf(Perl_debug_log, \
"Pad 0x%"UVxf"[0x%"UVxf"] restore_local\n", \
diff --git a/parser.h b/parser.h
index 462dcfd..4ef4608 100644
--- a/parser.h
+++ b/parser.h
@@ -15,7 +15,7 @@ typedef struct {
YYSTYPE val; /* semantic value */
short state;
I32 savestack_ix; /* size of savestack at this state */
- AV *comppad; /* value of PL_comppad when this value was created */
+ CV *compcv; /* value of PL_compcv when this value was created */
#ifdef DEBUGGING
const char *name; /* token/rule name for -Dpv */
#endif
diff --git a/perly.c b/perly.c
index eff36d1..ac87b8e 100644
--- a/perly.c
+++ b/perly.c
@@ -266,8 +266,10 @@ S_clear_yystack(pTHX_ const yy_parser *parser)
#ifdef DISABLE_STACK_FREE
+ for (i=0; i< parser->yylen; i++) {
+ SvREFCNT_dec(ps[-i].compcv);
+ }
ps -= parser->yylen;
- PERL_UNUSED_VAR(i);
#else
/* clear any reducing ops (1st pass) */
@@ -278,8 +280,9 @@ S_clear_yystack(pTHX_ const yy_parser *parser)
if ( ! (ps[-i].val.opval->op_attached
&& !ps[-i].val.opval->op_latefreed))
{
- if (ps[-i].comppad != PL_comppad) {
- PAD_RESTORE_LOCAL(ps[-i].comppad);
+ if (ps[-i].compcv != PL_compcv) {
+ PL_compcv = ps[-i].compcv;
+ PAD_SET_CUR_NOSAVE(CvPADLIST(PL_compcv), 1);
}
op_free(ps[-i].val.opval);
}
@@ -294,8 +297,9 @@ S_clear_yystack(pTHX_ const yy_parser *parser)
if (yy_type_tab[yystos[ps->state]] == toketype_opval
&& ps->val.opval)
{
- if (ps->comppad != PL_comppad) {
- PAD_RESTORE_LOCAL(ps->comppad);
+ if (ps->compcv != PL_compcv) {
+ PL_compcv = ps->compcv;
+ PAD_SET_CUR_NOSAVE(CvPADLIST(PL_compcv), 1);
}
YYDPRINTF ((Perl_debug_log, "(freeing op)\n"));
#ifndef DISABLE_STACK_FREE
@@ -304,6 +308,7 @@ S_clear_yystack(pTHX_ const yy_parser *parser)
#endif
op_free(ps->val.opval);
}
+ SvREFCNT_dec(ps->compcv);
ps--;
}
}
@@ -451,7 +456,7 @@ Perl_yyparse (pTHX)
YYPUSHSTACK;
ps->state = yyn;
ps->val = parser->yylval;
- ps->comppad = PL_comppad;
+ ps->compcv = SvREFCNT_inc(PL_compcv);
ps->savestack_ix = PL_savestack_ix;
#ifdef DEBUGGING
ps->name = (const char *)(yytname[yytoken]);
@@ -525,12 +530,12 @@ Perl_yyparse (pTHX)
}
-#ifndef DISABLE_STACK_FREE
/* any just-reduced ops with the op_latefreed flag cleared need to be
* freed; the rest need the flag resetting */
{
int i;
for (i=0; i< parser->yylen; i++) {
+#ifndef DISABLE_STACK_FREE
if (yy_type_tab[yystos[ps[-i].state]] == toketype_opval
&& ps[-i].val.opval)
{
@@ -538,9 +543,10 @@ Perl_yyparse (pTHX)
if (ps[-i].val.opval->op_latefreed)
op_free(ps[-i].val.opval);
}
+#endif
+ SvREFCNT_dec(ps[-i].compcv);
}
}
-#endif
parser->ps = ps -= (parser->yylen-1);
@@ -549,7 +555,7 @@ Perl_yyparse (pTHX)
number reduced by. */
ps->val = yyval;
- ps->comppad = PL_comppad;
+ ps->compcv = SvREFCNT_inc(PL_compcv);
ps->savestack_ix = PL_savestack_ix;
#ifdef DEBUGGING
ps->name = (const char *)(yytname [yyr1[yyn]]);
@@ -584,6 +590,7 @@ Perl_yyparse (pTHX)
/* Return failure if at end of input. */
if (parser->yychar == YYEOF) {
/* Pop the error token. */
+ SvREFCNT_dec(ps->compcv);
YYPOPSTACK;
/* Pop the rest of the stack. */
while (ps > parser->stack) {
@@ -593,12 +600,14 @@ Perl_yyparse (pTHX)
&& ps->val.opval)
{
YYDPRINTF ((Perl_debug_log, "(freeing op)\n"));
- if (ps->comppad != PL_comppad) {
- PAD_RESTORE_LOCAL(ps->comppad);
+ if (ps->compcv != PL_compcv) {
+ PL_compcv = ps->compcv;
+ PAD_SET_CUR_NOSAVE(CvPADLIST(PL_compcv), 1);
}
ps->val.opval->op_latefree = 0;
op_free(ps->val.opval);
}
+ SvREFCNT_dec(ps->compcv);
YYPOPSTACK;
}
YYABORT;
@@ -639,12 +648,14 @@ Perl_yyparse (pTHX)
LEAVE_SCOPE(ps->savestack_ix);
if (yy_type_tab[yystos[ps->state]] == toketype_opval && ps->val.opval) {
YYDPRINTF ((Perl_debug_log, "(freeing op)\n"));
- if (ps->comppad != PL_comppad) {
- PAD_RESTORE_LOCAL(ps->comppad);
+ if (ps->compcv != PL_compcv) {
+ PL_compcv = ps->compcv;
+ PAD_SET_CUR_NOSAVE(CvPADLIST(PL_compcv), 1);
}
ps->val.opval->op_latefree = 0;
op_free(ps->val.opval);
}
+ SvREFCNT_dec(ps->compcv);
YYPOPSTACK;
yystate = ps->state;
@@ -659,7 +670,7 @@ Perl_yyparse (pTHX)
YYPUSHSTACK;
ps->state = yyn;
ps->val = parser->yylval;
- ps->comppad = PL_comppad;
+ ps->compcv = SvREFCNT_inc(PL_compcv);
ps->savestack_ix = PL_savestack_ix;
#ifdef DEBUGGING
ps->name ="<err>";
@@ -673,6 +684,9 @@ Perl_yyparse (pTHX)
`-------------------------------------*/
yyacceptlab:
yyresult = 0;
+ for (ps=parser->ps; ps > parser->stack; ps--) {
+ SvREFCNT_dec(ps->compcv);
+ }
parser->ps = parser->stack; /* disable cleanup */
goto yyreturn;
--
1.6.5
|
From gerard@ggoossen.net0002-Add-a-test-that-eval-does-not-create-additional-refe.patchFrom e875e8dc9780564bcaecb4dc9c109a2d3c98d7c9 Mon Sep 17 00:00:00 2001
From: Gerard Goossen <gerard@ggoossen.net>
Date: Wed, 9 Dec 2009 17:23:00 +0100
Subject: [PATCH 2/2] Add a test that "eval" does not create additional reference to ouside
variables.
---
t/op/eval.t | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/t/op/eval.t b/t/op/eval.t
index 58a6334..305d7f3 100644
--- a/t/op/eval.t
+++ b/t/op/eval.t
@@ -6,7 +6,7 @@ BEGIN {
require './test.pl';
}
-print "1..103\n";
+print "1..105\n";
eval 'print "ok 1\n";';
@@ -560,6 +560,15 @@ $test++;
curr_test($test);
+{
+ # test that the CV compiled for the eval is freed by checking that no additional
+ # reference to outside lexicals are made.
+ my $x;
+ is(Internals::SvREFCNT($x), 1, "originally only 1 referece");
+ eval '$x';
+ is(Internals::SvREFCNT($x), 1, "execution eval doesn't create new references");
+}
+
fresh_perl_is(<<'EOP', "ok\n", undef, 'RT #70862');
$::{'@'}='';
eval {};
--
1.6.5
|
From @rgarcia2009/12/9 Gerard Goossen <gerard@ggoossen.net>:
Thanks, all applied (plus one more to remove compilation warnings). |
|
@rgs - Status changed from 'open' to 'resolved' |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Migrated from rt.perl.org#66094 (status was 'resolved')
Searchable as RT66094$
The text was updated successfully, but these errors were encountered: