op.c: Stop copying constants under ithreads

This fixes bugs #21979, #89188, #109746, #114838 and #115388 and
mostly fixes #109744 and #105906 (other issues still remain in those
two tickets).

Because the PADTMP flag was doing double duty, indicating that a
pad slot was in use in addition to indicating a target, constants
could not be shared between pad slots, as freeing one const op (and
turning of its PADTMP flag in pad_free) would mark some other pad
slot as free.

I believe this may have been fixed already by change 3b1c21fabed,
which made const ops use pad_swipe (which removes the SV from the
pad) instead of pad_free (which marks it as available for reuse).  But
the copying still happens.

In any case, as of the previous commit, whether a pad slot for a con-
stant is in use is now stored in the pad name.  Slots in use for const
ops now have &PL_sv_no names.

So there is no longer any reason to copy the constants.

The difference can be observed thus:

Before:

$ ./perl -lIlib -MDevel::Peek -e 'sub foo(){42} Dump foo; Dump foo'
SV = IV(0x7f94ea02ef10) at 0x7f94ea02ef20
  REFCNT = 2
  FLAGS = (PADTMP,IOK,READONLY,pIOK)
  IV = 42
SV = IV(0x7f94ea02eeb0) at 0x7f94ea02eec0
  REFCNT = 1
  FLAGS = (PADTMP,IOK,READONLY,pIOK)
  IV = 42

After:

$ ./perl -lIlib -MDevel::Peek -e 'sub foo(){42} Dump foo; Dump foo'
SV = IV(0x7f894882ef10) at 0x7f894882ef20
  REFCNT = 3
  FLAGS = (IOK,READONLY,pIOK)
  IV = 42
SV = IV(0x7f894882ef10) at 0x7f894882ef20
  REFCNT = 3
  FLAGS = (IOK,READONLY,pIOK)
  IV = 42

Notice the different addresses.

There are still special cases for copying &PL_sv_undef, which I need
to tackle.

Since most constants created by ‘use constant’ have the PADMY flag on
(since they reside in lexical variables inside constant.pm) and PADMY
and PADTMP are exclusive, I have stop turning on PADTMP for constants.
It is no longer necessary now, since before its purpose was to mark
pad entries as being in use.  That means many to-do tests pass.

1 files changed, 1 insertions, 11 deletions

diff --git a/op.c b/op.c
index bc62048944..f7cfe39018 100644
--- a/op.c
+++ b/op.c
@@ -1752,16 +1752,7 @@ S_finalize_op(pTHX_ OP* o)
 	 * for reference counts, sv_upgrade() etc. */
 	if (cSVOPo->op_sv) {
 	    const PADOFFSET ix = pad_alloc(OP_CONST, SVf_READONLY);
-	    if (o->op_type != OP_METHOD_NAMED &&
-		(SvPADTMP(cSVOPo->op_sv) || SvPADMY(cSVOPo->op_sv)))
-	    {
-		/* If op_sv is already a PADTMP/MY then it is being used by
-		 * some pad, so make a copy. */
-		sv_setsv(PAD_SVl(ix),cSVOPo->op_sv);
-		if (!SvIsCOW(PAD_SVl(ix))) SvREADONLY_on(PAD_SVl(ix));
-		SvREFCNT_dec(cSVOPo->op_sv);
-	    }
-	    else if (o->op_type != OP_METHOD_NAMED
+	    if (o->op_type != OP_METHOD_NAMED
 		&& cSVOPo->op_sv == &PL_sv_undef) {
 		/* PL_sv_undef is hack - it's unsafe to store it in the
 		   AV that is the pad, because av_fetch treats values of
@@ -1775,7 +1766,6 @@ S_finalize_op(pTHX_ OP* o)
 	    }
 	    else {
 		SvREFCNT_dec(PAD_SVl(ix));
-		SvPADTMP_on(cSVOPo->op_sv);
 		PAD_SETSV(ix, cSVOPo->op_sv);
 		/* XXX I don't know how this isn't readonly already. */
 		if (!SvIsCOW(PAD_SVl(ix))) SvREADONLY_on(PAD_SVl(ix));