Set the ppaddr member of padsv ops at peep time

There was a race in the "PL_ppaddr[OP_PADSV] save/restore dance" in that
perl might be compiling ops inside and outside the scope of the pragma at
the same time. We solve this by setting the op_ppaddr member of padsv ops
to a_pp_deref during the peephole optimization phase.

diff --git a/Types.xs b/Types.xs
index 8cf501cc631aec3b350d6baa35c57879ed373b4e..bb6c6a629eb071e7984acd3d860b70fb6e701cbd 100644 (file)
--- a/Types.xs
+++ b/Types.xs
@@ -27,6 +27,10 @@
 # define LT_WORKAROUND_REQUIRE_PROPAGATION !LT_HAS_PERL(5, 10, 1)
 #endif
 
+#ifndef LT_HAS_RPEEP
+# define LT_HAS_RPEEP LT_HAS_PERL(5, 13, 5)
+#endif
+
 #ifndef HvNAME_get
 # define HvNAME_get(H) HvNAME(H)
 #endif
@@ -48,7 +52,8 @@
 #  define LT_MULTIPLICITY 0
 # endif
 #endif
-#if LT_MULTIPLICITY && !defined(tTHX)
+
+#ifndef tTHX
 # define tTHX PerlInterpreter*
 #endif
 
@@ -133,6 +138,17 @@ typedef SV lt_hint_t;
 
 #endif /* LT_THREADSAFE */
 
+/* ... "Seen" pointer table ................................................ */
+
+#if !LT_HAS_RPEEP
+
+#define PTABLE_NAME        ptable_seen
+#define PTABLE_VAL_FREE(V) NOOP
+
+#include "ptable.h"
+
+#endif /* !LT_HAS_RPEEP */
+
 /* ... Global data ......................................................... */
 
 #define MY_CXT_KEY __PACKAGE__ "::_guts" XS_VERSION
@@ -141,9 +157,11 @@ typedef struct {
 #if LT_THREADSAFE
  ptable *tbl; /* It really is a ptable_hints */
  tTHX    owner;
+#endif
+#if !LT_HAS_RPEEP
+ ptable *seen; /* It really is a ptable_seen */
 #endif
  SV     *default_meth;
- OP *  (*pp_padsv_saved)(pTHX);
 } my_cxt_t;
 
 START_MY_CXT
@@ -205,6 +223,9 @@ STATIC void lt_thread_cleanup(pTHX_ void *ud) {
  dMY_CXT;
 
  ptable_hints_free(MY_CXT.tbl);
+#if !LT_HAS_RPEEP
+ ptable_seen_free(MY_CXT.seen);
+#endif /* !LT_HAS_RPEEP */
 }
 
 #endif /* LT_THREADSAFE */
@@ -212,6 +233,7 @@ STATIC void lt_thread_cleanup(pTHX_ void *ud) {
 /* ... Hint tags ........................................................... */
 
 #if LT_WORKAROUND_REQUIRE_PROPAGATION
+
 STATIC IV lt_require_tag(pTHX) {
 #define lt_require_tag() lt_require_tag(aTHX)
  const CV *cv, *outside;
@@ -255,13 +277,16 @@ get_enclosing_cv:
 
  return PTR2IV(cv);
 }
+
 #endif /* LT_WORKAROUND_REQUIRE_PROPAGATION */
 
 STATIC SV *lt_tag(pTHX_ SV *value) {
 #define lt_tag(V) lt_tag(aTHX_ (V))
  lt_hint_t *h;
  SV *code = NULL;
+#if LT_THREADSAFE
  dMY_CXT;
+#endif
 
  if (SvROK(value)) {
   value = SvRV(value);
@@ -294,7 +319,9 @@ STATIC SV *lt_tag(pTHX_ SV *value) {
 STATIC SV *lt_detag(pTHX_ const SV *hint) {
 #define lt_detag(H) lt_detag(aTHX_ (H))
  lt_hint_t *h;
+#if LT_THREADSAFE
  dMY_CXT;
+#endif
 
  if (!(hint && SvIOK(hint)))
   return NULL;
@@ -461,7 +488,7 @@ STATIC void lt_map_delete(pTHX_ const OP *o) {
 STATIC OP *lt_pp_padsv(pTHX) {
  lt_op_info oi;
 
- if ((PL_op->op_private & OPpLVAL_INTRO) && lt_map_fetch(PL_op, &oi)) {
+ if (lt_map_fetch(PL_op, &oi)) {
   PADOFFSET targ = PL_op->op_targ;
   SV *sv         = PAD_SVl(targ);
 
@@ -520,41 +547,15 @@ STATIC OP *lt_pp_padsv(pTHX) {
   return CALL_FPTR(oi.old_pp_padsv)(aTHX);
  }
 
- return CALL_FPTR(PL_ppaddr[OP_PADSV])(aTHX);
-}
-
-STATIC void lt_pp_padsv_save(pMY_CXT) {
-#define lt_pp_padsv_save() lt_pp_padsv_save(aMY_CXT)
- if (MY_CXT.pp_padsv_saved)
-  return;
-
- MY_CXT.pp_padsv_saved = PL_ppaddr[OP_PADSV];
- PL_ppaddr[OP_PADSV]   = lt_pp_padsv;
-}
-
-STATIC void lt_pp_padsv_restore(pMY_CXT_ OP *o) {
-#define lt_pp_padsv_restore(O) lt_pp_padsv_restore(aMY_CXT_ (O))
- OP *(*saved)(pTHX) = MY_CXT.pp_padsv_saved;
-
- if (!saved)
-  return;
-
- if (o->op_ppaddr == lt_pp_padsv)
-  o->op_ppaddr = saved;
-
- PL_ppaddr[OP_PADSV]   = saved;
- MY_CXT.pp_padsv_saved = 0;
+ return CALL_FPTR(PL_op->op_ppaddr)(aTHX);
 }
 
 /* ... Our ck_pad{any,sv} .................................................. */
 
-/* Sadly, the PADSV OPs we are interested in don't trigger the padsv check
- * function, but are instead manually mutated from a PADANY. This is why we set
- * PL_ppaddr[OP_PADSV] in the padany check function so that PADSV OPs will have
- * their pp_ppaddr set to our pp_padsv. PL_ppaddr[OP_PADSV] is then reset at the
- * beginning of every ck_pad{any,sv}. Some unwanted OPs can still call our
- * pp_padsv, but much less than if we would have set PL_ppaddr[OP_PADSV]
- * globally. */
+/* Sadly, the padsv OPs we are interested in don't trigger the padsv check
+ * function, but are instead manually mutated from a padany. So we store
+ * the op entry in the op map in the padany check function, and we set their
+ * op_ppaddr member in our peephole optimizer replacement below. */
 
 STATIC OP *(*lt_old_ck_padany)(pTHX_ OP *) = 0;
 
@@ -563,8 +564,6 @@ STATIC OP *lt_ck_padany(pTHX_ OP *o) {
  SV *code;
  dMY_CXT;
 
- lt_pp_padsv_restore(o);
-
  o = CALL_FPTR(lt_old_ck_padany)(aTHX_ o);
 
  stash = PL_in_my_stash;
@@ -628,9 +627,7 @@ STATIC OP *lt_ck_padany(pTHX_ OP *o) {
    SvREFCNT_inc(orig_meth);
   }
 
-  lt_pp_padsv_save();
-
-  lt_map_store(o, orig_pkg, type_pkg, type_meth, MY_CXT.pp_padsv_saved);
+  lt_map_store(o, orig_pkg, type_pkg, type_meth, o->op_ppaddr);
  } else {
 skip:
   lt_map_delete(o);
@@ -644,13 +641,111 @@ STATIC OP *(*lt_old_ck_padsv)(pTHX_ OP *) = 0;
 STATIC OP *lt_ck_padsv(pTHX_ OP *o) {
  dMY_CXT;
 
- lt_pp_padsv_restore(o);
-
  lt_map_delete(o);
 
  return CALL_FPTR(lt_old_ck_padsv)(aTHX_ o);
 }
 
+/* ... Our peephole optimizer .............................................. */
+
+STATIC peep_t lt_old_peep = 0; /* This is actually the rpeep past 5.13.5 */
+
+#if !LT_HAS_RPEEP
+# define LT_PEEP_REC_PROTO STATIC void lt_peep_rec(pTHX_ OP *o, ptable *seen)
+#else /* !LT_HAS_RPEEP */
+# define LT_PEEP_REC_PROTO STATIC void lt_peep_rec(pTHX_ OP *o)
+#endif /* LT_HAS_RPEEP */
+
+LT_PEEP_REC_PROTO;
+LT_PEEP_REC_PROTO {
+#if !LT_HAS_RPEEP
+# define lt_peep_rec(O) lt_peep_rec(aTHX_ (O), seen)
+#else /* !LT_HAS_RPEEP */
+# define lt_peep_rec(O) lt_peep_rec(aTHX_ (O))
+#endif /* LT_HAS_RPEEP */
+
+#if !LT_HAS_RPEEP
+ if (ptable_fetch(seen, o))
+  return;
+#endif
+
+ for (; o; o = o->op_next) {
+  lt_op_info *oi = NULL;
+
+#if !LT_HAS_RPEEP
+  ptable_seen_store(seen, o, o);
+#endif
+  switch (o->op_type) {
+   case OP_PADSV:
+    if (o->op_ppaddr != lt_pp_padsv && o->op_private & OPpLVAL_INTRO) {
+#ifdef USE_ITHREADS
+     MUTEX_LOCK(&lt_op_map_mutex);
+#endif
+     oi = ptable_fetch(lt_op_map, o);
+     if (oi) {
+      oi->old_pp_padsv = o->op_ppaddr;
+      o->op_ppaddr     = lt_pp_padsv;
+     }
+#ifdef USE_ITHREADS
+     MUTEX_UNLOCK(&lt_op_map_mutex);
+#endif
+    }
+    break;
+#if !LT_HAS_RPEEP
+   case OP_MAPWHILE:
+   case OP_GREPWHILE:
+   case OP_AND:
+   case OP_OR:
+   case OP_ANDASSIGN:
+   case OP_ORASSIGN:
+   case OP_COND_EXPR:
+   case OP_RANGE:
+# if LT_HAS_PERL(5, 10, 0)
+   case OP_ONCE:
+   case OP_DOR:
+   case OP_DORASSIGN:
+# endif
+    lt_peep_rec(cLOGOPo->op_other);
+    break;
+   case OP_ENTERLOOP:
+   case OP_ENTERITER:
+    lt_peep_rec(cLOOPo->op_redoop);
+    lt_peep_rec(cLOOPo->op_nextop);
+    lt_peep_rec(cLOOPo->op_lastop);
+    break;
+# if LT_HAS_PERL(5, 9, 5)
+   case OP_SUBST:
+    lt_peep_rec(cPMOPo->op_pmstashstartu.op_pmreplstart);
+    break;
+# else
+   case OP_QR:
+   case OP_MATCH:
+   case OP_SUBST:
+    lt_peep_rec(cPMOPo->op_pmreplstart);
+    break;
+# endif
+#endif /* !LT_HAS_RPEEP */
+   default:
+    break;
+  }
+ }
+}
+
+STATIC void lt_peep(pTHX_ OP *o) {
+#if !LT_HAS_RPEEP
+ dMY_CXT;
+ ptable *seen = MY_CXT.seen;
+
+ ptable_seen_clear(seen);
+#endif /* !LT_HAS_RPEEP */
+
+ lt_old_peep(aTHX_ o);
+ lt_peep_rec(o);
+}
+
+/* --- Interpreter setup/teardown ------------------------------------------ */
+
+
 STATIC U32 lt_initialized = 0;
 
 STATIC void lt_teardown(pTHX_ void *root) {
@@ -666,6 +761,9 @@ STATIC void lt_teardown(pTHX_ void *root) {
 
 #if LT_THREADSAFE
  ptable_hints_free(MY_CXT.tbl);
+#endif
+#if !LT_HAS_RPEEP
+ ptable_seen_free(MY_CXT.seen);
 #endif
  SvREFCNT_dec(MY_CXT.default_meth);
 
@@ -674,6 +772,13 @@ STATIC void lt_teardown(pTHX_ void *root) {
  PL_check[OP_PADSV]  = MEMBER_TO_FPTR(lt_old_ck_padsv);
  lt_old_ck_padsv     = 0;
 
+#if LT_HAS_RPEEP
+ PL_rpeepp   = lt_old_peep;
+#else
+ PL_peepp    = lt_old_peep;
+#endif
+ lt_old_peep = 0;
+
  lt_initialized = 0;
 }
 
@@ -685,11 +790,13 @@ STATIC void lt_setup(pTHX) {
  {
   MY_CXT_INIT;
 #if LT_THREADSAFE
-  MY_CXT.tbl            = ptable_new();
-  MY_CXT.owner          = aTHX;
+  MY_CXT.tbl          = ptable_new();
+  MY_CXT.owner        = aTHX;
 #endif
-  MY_CXT.pp_padsv_saved = 0;
-  MY_CXT.default_meth   = newSVpvn("TYPEDSCALAR", 11);
+#if !LT_HAS_RPEEP
+  MY_CXT.seen         = ptable_new();
+#endif
+  MY_CXT.default_meth = newSVpvn("TYPEDSCALAR", 11);
   SvREADONLY_on(MY_CXT.default_meth);
  }
 
@@ -698,6 +805,14 @@ STATIC void lt_setup(pTHX) {
  lt_old_ck_padsv     = PL_check[OP_PADSV];
  PL_check[OP_PADSV]  = MEMBER_TO_FPTR(lt_ck_padsv);
 
+#if LT_HAS_RPEEP
+ lt_old_peep = PL_rpeepp;
+ PL_rpeepp   = lt_peep;
+#else
+ lt_old_peep = PL_peepp;
+ PL_peepp    = lt_peep;
+#endif
+
 #if LT_MULTIPLICITY
  call_atexit(lt_teardown, aTHX);
 #else
@@ -741,24 +856,34 @@ CLONE(...)
 PROTOTYPE: DISABLE
 PREINIT:
  ptable *t;
+#if !LT_HAS_RPEEP
+ ptable *s;
+#endif
  SV     *cloned_default_meth;
 PPCODE:
  {
-  lt_ptable_clone_ud ud;
-  dMY_CXT;
-
-  t = ptable_new();
-  lt_ptable_clone_ud_init(ud, t, MY_CXT.owner);
-  ptable_walk(MY_CXT.tbl, lt_ptable_clone, &ud);
-  cloned_default_meth = lt_dup_inc(MY_CXT.default_meth, &ud);
-  lt_ptable_clone_ud_deinit(ud);
+  {
+   lt_ptable_clone_ud ud;
+   dMY_CXT;
+
+   t = ptable_new();
+   lt_ptable_clone_ud_init(ud, t, MY_CXT.owner);
+   ptable_walk(MY_CXT.tbl, lt_ptable_clone, &ud);
+   cloned_default_meth = lt_dup_inc(MY_CXT.default_meth, &ud);
+   lt_ptable_clone_ud_deinit(ud);
+  }
+#if !LT_HAS_RPEEP
+  s = ptable_new();
+#endif
  }
  {
   MY_CXT_CLONE;
-  MY_CXT.tbl            = t;
-  MY_CXT.owner          = aTHX;
-  MY_CXT.pp_padsv_saved = 0;
-  MY_CXT.default_meth   = cloned_default_meth;
+  MY_CXT.tbl          = t;
+  MY_CXT.owner        = aTHX;
+#if !LT_HAS_RPEEP
+  MY_CXT.seen         = s;
+#endif
+  MY_CXT.default_meth = cloned_default_meth;
  }
  reap(3, lt_thread_cleanup, NULL);
  XSRETURN(0);