Remove bad assertion in gv.c:newGP
authorFather Chrysostomos <sprout@cpan.org>
Sat, 21 Sep 2013 14:43:12 +0000 (07:43 -0700)
committerFather Chrysostomos <sprout@cpan.org>
Sat, 21 Sep 2013 14:44:06 +0000 (07:44 -0700)
See the thread starting at
<20130805090753.12203.qmail@lists-nntp.develooper.com>.

Under the assumption that PL_curcop could never be null in newGP (even
when set to null by S_cop_free in op.c), I added an assertion to
newGP, which still leaving the null checks in place.  The idea was
that, if PL_curcop is ever null there, we want to know about it, since
it is probably a bug.

It turns out this code (reduced from DBIx::Class’s test suite), can
fail that assertion:

INIT {
  bless {} and exit;
}

exit() calls leave_scope, which frees the INIT block.  Since PL_curcop
(the statement inside INIT) would end up pointing to free memory, it
is set to null.  When it exits, call_sv does FREETMPS:

case 2:
    /* my_exit() was called */
    SET_CURSTASH(PL_defstash);
    FREETMPS;
    JMPENV_POP;
    my_exit_jump();
    assert(0); /* NOTREACHED */

FREETMPS ends up freeing the object (bless {}), which results
in a DESTROY method lookup.  For the sake of caching methods,
*main::DESTROY is autovivified.  GV creation trips on the assertion in
newGP that makes sure PL_curcop is null.

So this proves that we really do need to check for a null
PL_curcop in newGP.

While we could avoid having DESTROY lookup vivify *main::DESTROY
(since the cache there would only be used for explicit ->DESTROY
calls, the usual DESTROY cache being stuffed into SvSTASH(stash)),
that would complicate things for no gain.

gv.c
t/op/blocks.t

diff --git a/gv.c b/gv.c
index fc4393e..9f0b57e 100644 (file)
--- a/gv.c
+++ b/gv.c
@@ -176,9 +176,10 @@ Perl_newGP(pTHX_ GV *const gv)
     gp->gp_sv = newSV(0);
 #endif
 
-    /* PL_curcop should never be null here. */
-    assert(PL_curcop);
-    /* But for non-debugging builds play it safe */
+    /* PL_curcop may be null here.  E.g.,
+       INIT { bless {} and exit }
+       frees INIT before looking up DESTROY (and creating *DESTROY)
+    */
     if (PL_curcop) {
        gp->gp_line = CopLINE(PL_curcop); /* 0 otherwise Newxz */
 #ifdef USE_ITHREADS
index d07e844..fb15eee 100644 (file)
@@ -6,7 +6,7 @@ BEGIN {
     require './test.pl';
 }
 
-plan tests => 6;
+plan tests => 7;
 
 my @expect = qw(
 b1
@@ -142,3 +142,6 @@ void
 void
 void
 expEct
+
+fresh_perl_is('END { print "ok\n" } INIT { bless {} and exit }', "ok\n",
+              {}, 'null PL_curcop in newGP');