From c947bc1dc2208798835a2ed509163cd71f20b218 Mon Sep 17 00:00:00 2001 From: Father Chrysostomos Date: Sat, 21 Sep 2013 07:43:12 -0700 Subject: [PATCH] Remove bad assertion in gv.c:newGP MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 | 7 ++++--- t/op/blocks.t | 5 ++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/gv.c b/gv.c index fc4393e..9f0b57e 100644 --- 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 diff --git a/t/op/blocks.t b/t/op/blocks.t index d07e844..fb15eee 100644 --- a/t/op/blocks.t +++ b/t/op/blocks.t @@ -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'); -- 2.7.4