Re: When is PL_curcop NULL?

On Tue, Aug 06, 2013 at 01:11:56PM -0000, Father Chrysostomos wrote:
> Nicholas Clark wrote:
> > On Mon, Aug 05, 2013 at 09:07:53AM -0000, Father Chrysostomos wrote:
> > > Do we still need these [null] checks?
> > 
> > I think so, but I can't prove it. Moreover, I think that S_cop_free() should
> > have this:
> > 
> > +    if (PL_curcop == cop)
> > +       PL_curcop = NULL;
> >  }
> > 
> > 
> > because without that, we have an interpreter-global variable pointing to
> > freed memory. And that sort of dangling pointer has been the cause of
> > several different previous bugs.
> 
> I tried adding that block locally and *also* adding assert(PL_curcop)
> to newGP, and got no failures.  I think that both are a good idea.  We
> have many months before the next stable release and plenty of time to
> smoke out places where that assertion is wrong.  (And we will leave
> the null checks in newGP for now anyway, so non-debugging builds are
> not sabotaged.)

I even tried this, to make a flag that the tokenizer would check, so that it
would abort if it ever re-instated a freed COP to curcop, and all tests pass.
(Yes, I realise that this is attempting a read from freed memory, so it's not
perfect, and I didn't run it under ASAN, as the flat is too hot already,
without turning that machine on and giving it a serious workout)

diff --git a/op.c b/op.c
index d87396b..08c4908 100644
--- a/op.c
+++ b/op.c
@@ -217,6 +217,7 @@ Perl_Slab_Alloc(pTHX_ size_t sz)
            slot->opslot_next = slab2->opslab_first;    \
            slab2->opslab_first = slot;                 \
            o = &slot->opslot_op;                       \
+           o->op_freed = 0; \
            o->op_slabbed = 1
 
     /* The partially-filled slab is next in the chain. */
@@ -771,6 +772,7 @@ Perl_op_free(pTHX_ OP *o)
     }
 
     op_clear(o);
+    o->op_freed = 1;
     FreeOp(o);
 #ifdef DEBUG_LEAKING_SCALARS
     if (PL_op == o)
@@ -958,6 +960,9 @@ S_cop_free(pTHX_ COP* cop)
     if (! specialWARN(cop->cop_warnings))
        PerlMemShared_free(cop->cop_warnings);
     cophh_free(CopHINTHASH_get(cop));
+    if (PL_curcop == cop) {
+       PL_curcop = NULL;
+    }
 }
 
 STATIC void
diff --git a/op.h b/op.h
index dcfd5be..f85bca9 100644
--- a/op.h
+++ b/op.h
@@ -58,7 +58,8 @@ typedef PERL_BITFIELD16 Optype;
     PERL_BITFIELD16 op_savefree:1;     \
     PERL_BITFIELD16 op_static:1;       \
     PERL_BITFIELD16 op_folded:1;       \
-    PERL_BITFIELD16 op_spare:2;                \
+    PERL_BITFIELD16 op_freed:1;                \
+    PERL_BITFIELD16 op_spare:1;                \
     U8         op_flags;               \
     U8         op_private;
 #endif
diff --git a/toke.c b/toke.c
index 911fb94..c8f0d62 100644
--- a/toke.c
+++ b/toke.c
@@ -778,6 +778,7 @@ Perl_parser_free(pTHX_  const yy_parser *parser)
     PERL_ARGS_ASSERT_PARSER_FREE;
 
     PL_curcop = parser->saved_curcop;
+    assert(!PL_curcop->op_freed);
     SvREFCNT_dec(parser->linestr);
 
     if (PL_parser->lex_flags & LEX_DONT_CLOSE_RSFP)




Nicholas Clark

• When is PL_curcop NULL? by Father Chrysostomos
• Re: When is PL_curcop NULL? by Nicholas Clark
• Re: When is PL_curcop NULL? by Father Chrysostomos
• Re: When is PL_curcop NULL? by Nicholas Clark
• Re: When is PL_curcop NULL? by Father Chrysostomos
• Re: When is PL_curcop NULL? by Nicholas Clark