This is a live mirror of the Perl 5 development currently hosted at https://github.com/perl/perl5
pad.c: More clearly separate targ/constant allocation
authorFather Chrysostomos <sprout@cpan.org>
Tue, 26 Aug 2014 20:06:10 +0000 (13:06 -0700)
committerFather Chrysostomos <sprout@cpan.org>
Thu, 28 Aug 2014 20:04:16 +0000 (13:04 -0700)
pad_reset was turning off PADTMP on targets to make them ‘available’
again.  But constants allocated later could then end up using those
slots.  We can’t share pad slots between constants and targets,
because operators allocated earlier will trigger ‘Modification of
a read-only value’ errors when trying to assign return values to
their targets.

Under non-threaded builds, in which constants are not generally stored
in the pad, there are still ‘constants’ that are allocated as such
(with &PL_sv_no names) but are not actually read-only.  Filehandle
names associated with ‘open my $fh’ are one such example.  If that
filehandle name’s pad slot is shared with a target, then the file-
handle will be named using some random string from a previous
statement.

The solution here is to change the way we use the PADTMP flag when
allocating pad slots.  pad_reset should *not* turn off the flag,
because we need to know that these entries are used as targets by pre-
vious statements.  pad_alloc should not tread PADTMP entries as being
in use when allocating targets, but only when allocating constants.

This gets all tests passing under -Accflags=-DUSE_BROKEN_PAD_RESET
on non-threaded builds.

pad.c

diff --git a/pad.c b/pad.c
index 3a79206..7f896c4 100644 (file)
--- a/pad.c
+++ b/pad.c
@@ -745,7 +745,13 @@ Perl_pad_alloc(pTHX_ I32 optype, U32 tmptype)
                   (sv = names[PL_padix]) && sv != &PL_sv_undef)
                continue;
            sv = *av_fetch(PL_comppad, PL_padix, TRUE);
                   (sv = names[PL_padix]) && sv != &PL_sv_undef)
                continue;
            sv = *av_fetch(PL_comppad, PL_padix, TRUE);
-           if (!(SvFLAGS(sv) & (SVs_PADTMP | SVs_PADMY)) &&
+           if (!(SvFLAGS(sv) &
+#ifdef USE_BROKEN_PAD_RESET
+                   (SVs_PADMY|(tmptype & SVf_READONLY ? SVs_PADTMP : 0))
+#else
+                   (SVs_PADMY|SVs_PADTMP)
+#endif
+                ) &&
                !IS_PADGV(sv))
                break;
        }
                !IS_PADGV(sv))
                break;
        }
@@ -1658,15 +1664,6 @@ S_pad_reset(pTHX)
     );
 
     if (!TAINTING_get) {       /* Can't mix tainted and non-tainted temporaries. */
     );
 
     if (!TAINTING_get) {       /* Can't mix tainted and non-tainted temporaries. */
-        I32 po;
-       for (po = AvMAX(PL_comppad); po > PL_padix_floor; po--) {
-           if (PL_curpad[po] && !SvIMMORTAL(PL_curpad[po])
-            && !SvPADMY(PL_curpad[po])
-            && (  PadnamelistMAX(PL_comppad_name) < po
-               || !PadnamelistARRAY(PL_comppad_name)[po]
-               || !PadnameLEN(PadnamelistARRAY(PL_comppad_name)[po]) ))
-               SvPADTMP_off(PL_curpad[po]);
-       }
        PL_padix = PL_padix_floor;
     }
 #endif
        PL_padix = PL_padix_floor;
     }
 #endif