From cbacc9aa064469dbad90cdce51a3e7abbdf202be Mon Sep 17 00:00:00 2001
From: Father Chrysostomos <sprout@cpan.org>
Date: Fri, 14 Sep 2012 22:08:19 -0700
Subject: [PATCH] [perl #114888] Localise PL_comppad_name in cv_clone

In 9ef8d56 I made closures share their pad name lists, and not just
the names themselves, for speed (no need to SvREFCNT_inc each name and
copy the list).

To make that work, I had to set PL_comppad_name in cv_clone, before
the pad_new call.  But I failed to move the PL_comppad_name localisa-
tion from pad_new to cv_clone.

So cv_clone would merrily clobber the previous value of
PL_comppad_name *before* localising it.

This only manifested itself in source filters.  Most of the time,
pp_anoncode is called at run time when either no code is being com-
piled (PL_comppad_name is only used at compile time) or inside a
BEGIN block which itself localises PL_comppad_name.  But inside a
Filter::Util::Call source filter there was no buffer like that to
protect it.

This meant that pad name creation (my $x) would create the name in the
PL_comppad_name belonging to the last-cloned sub.  A subsequent name
lookup ($x) would look in the correct place, as it uses the moral
equivalent of PadlistNAMES(CvPADLIST(PL_compcv)), not PL_comppad_name.
So it would not find it, resulting in a global variable or a stricture
violation.
---
 pad.c          |  3 ++-
 t/op/closure.t | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/pad.c b/pad.c
index fd75d42..711fd21 100644
--- a/pad.c
+++ b/pad.c
@@ -247,8 +247,8 @@ Perl_pad_new(pTHX_ int flags)
 
     if (flags & padnew_SAVE) {
 	SAVECOMPPAD();
-	SAVESPTR(PL_comppad_name);
 	if (! (flags & padnew_CLONE)) {
+	    SAVESPTR(PL_comppad_name);
 	    SAVEI32(PL_padix);
 	    SAVEI32(PL_comppad_name_fill);
 	    SAVEI32(PL_min_intro_pending);
@@ -2004,6 +2004,7 @@ Perl_cv_clone(pTHX_ CV *proto)
     if (SvMAGIC(proto))
 	mg_copy((SV *)proto, (SV *)cv, 0, 0);
 
+    SAVESPTR(PL_comppad_name);
     PL_comppad_name = protopad_name;
     CvPADLIST(cv) = pad_new(padnew_CLONE|padnew_SAVE);
     CvPADLIST(cv)->xpadl_id = protopadlist->xpadl_id;
diff --git a/t/op/closure.t b/t/op/closure.t
index 756ad04..089ceb5 100644
--- a/t/op/closure.t
+++ b/t/op/closure.t
@@ -789,4 +789,30 @@ sub staleval {
 staleval 1;
 staleval;
 
+# [perl #114888]
+# Test that closure creation localises PL_comppad_name properly.  Usually
+# at compile time a BEGIN block will localise PL_comppad_name for use, so
+# pp_anoncode can mess with it without any visible effects.
+# But inside a source filter, it affects the directly enclosing compila-
+# tion scope.
+SKIP: {
+    skip_if_miniperl("no XS on miniperl (for source filters)");
+    fresh_perl_is <<'    [perl #114888]', "ok\n", {stderr=>1},
+	use strict;
+	BEGIN {
+	    package Foo;
+	    use Filter::Util::Call;
+	    sub import { filter_add( sub {
+		my $status = filter_read();
+		sub { $status };
+		$status;
+	    })}
+	    Foo->import
+	}
+	my $x = "ok\n";	# stores $x in the wrong padnamelist
+	print $x;	# cannot find it - strict violation
+    [perl #114888]
+        'closures in source filters do not interfere with pad names';
+}
+
 done_testing();
-- 
2.7.4