This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
fix leak in /[(?{]/
authorDavid Mitchell <davem@iabyn.com>
Wed, 6 Mar 2019 10:36:23 +0000 (10:36 +0000)
committerDavid Mitchell <davem@iabyn.com>
Thu, 7 Mar 2019 11:21:26 +0000 (11:21 +0000)
This pattern is correctly interpreted by the parser as not containing
any code blocks, e.g. (?{...}). It's then passed to the regex compiler,
which thinks it may after all contain a code block not seen before (e.g.
interpolated in at runtime). So it evals the code qr'[(?{]' to compile
any code blocks.  Again the parser doesn't see any code blocks, so the
regex compiler realises it was wrong, and attempts to free the hidden
anon CV associated with compiling a qr// (this CV would take ownership
of any found code blocks, but is empty apart from a single OP_CONST
containing the text of regex).

This freeing of the CV was going wrong, resulting in the op slab(s)
associated with the anon CV leaking.

This was because cv_forget_slab(PL_compcv) was being called, which
converts a compiling CV into a compiled  CV, where CvSTART() no longer
points to the op slab, and instead the slab can only be accessed
indirectly via the ops in CvROOT().

Then when the CV is freed, because it is no longer marked as SvSLABBED,
the freeing code assumes that any associated ops are attached via
SvROOT() - but they haven't been yet - they're still sitting on the
yyparse stack. So they leak.

The solution seems to be a simple as removing the call to
cv_forget_slab().

I sort of understood this as I wrote this commit message, but it's
fading already. Don't ask me to explain this in a week's time, let alone
next year.

op.c
t/re/pat.t

diff --git a/op.c b/op.c
index 19634f9..d24208e 100644 (file)
--- a/op.c
+++ b/op.c
@@ -7112,8 +7112,13 @@ Perl_pmruntime(pTHX_ OP *o, OP *expr, OP *repl, UV flags, I32 floor)
 #  endif
                }
 #endif
-               /* But we know that one op is using this CV's slab. */
-               cv_forget_slab(PL_compcv);
+                /* This LEAVE_SCOPE will restore PL_compcv to point to the
+                 * outer CV (the one whose slab holds the pm op). The
+                 * inner CV (which holds expr) will be freed later, once
+                 * all the entries on the parse stack have been popped on
+                 * return from this function. Which is why its safe to
+                 * call op_free(expr) below.
+                 */
                LEAVE_SCOPE(floor);
                pm->op_pmflags &= ~PMf_HAS_CV;
            }
index 8e9ad81..141b283 100644 (file)
@@ -1928,6 +1928,10 @@ EOP
     }
     {
         # buffer overflow
+
+        # This test also used to leak - fixed by the commit which added
+        # this line.
+
         fresh_perl_is("BEGIN{\$^H=0x200000}\ns/[(?{//xx",
                       "Unmatched [ in regex; marked by <-- HERE in m/[ <-- HERE (?{/ at (eval 1) line 1.\n",
                       {}, "buffer overflow for regexp component");