Re: [PATCH] Add check for spurious PUSHMARKs by introducing POPBLOCK_normal.

After chatting with rgs, I've bounced a copy of your message to perlbug
for application after blead reopens for 5.13


On Thu, Nov 19, 2009 at 12:33:47PM +0100, Gerard Goossen wrote:
> Add POPBLOCK_normal, which is similar to POPBLOCK, but instead of just
> resetting the stack marker, it checks that the current stack marker is identical
> to the saved stack marker, as should be the case when a block is normally
> exited (without any flow control statement).
> 
> This patch detects the spurious PUSHMARKs in Devel::PPPort::Perl_ppaddr_t and
> Time::HiRes::stat, and causes assertion failures without the just submitted patches.
> 
> Gerard Goossen

> From 5858494a3c96d45bc6510dc9001d22feb0a6e571 Mon Sep 17 00:00:00 2001
> From: Gerard Goossen <gerard@ggoossen.net>
> Date: Wed, 18 Nov 2009 20:31:06 +0100
> Subject: [PATCH 3/4] Add POPBLOCK_normal, which is similar to POPBLOCK, but instead of just
>  resetting the stack marker, it checks that the current stack marker is identical
>  to the saved stack marker, as should be the case when a block is normally
>  exited (without any flow control statement).
> 
> ---
>  cop.h    |   12 ++++++++++++
>  pp_ctl.c |   10 +++++-----
>  pp_hot.c |    4 ++--
>  3 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/cop.h b/cop.h
> index 0348324..5d5da29 100644
> --- a/cop.h
> +++ b/cop.h
> @@ -562,6 +562,18 @@ struct block {
>  	DEBUG_l( PerlIO_printf(Perl_debug_log, "Entering block %ld, type %s\n",	\
>  		    (long)cxstack_ix, PL_block_type[CxTYPE(cx)]); )
>  
> +/* Exit a block at the end of its normal lifetime */
> +#define POPBLOCK_normal(cx,pm) cx = &cxstack[cxstack_ix--],			\
> +	newsp		 = PL_stack_base + cx->blk_oldsp,		\
> +	PL_curcop	 = cx->blk_oldcop,				\
> +	assert(PL_markstack_ptr == PL_markstack + cx->blk_oldmarksp),	\
> +	PL_scopestack_ix = cx->blk_oldscopesp,				\
> +	pm		 = cx->blk_oldpm,				\
> +	gimme		 = cx->blk_gimme;				\
> +	DEBUG_SCOPE("POPBLOCK");					\
> +	DEBUG_l( PerlIO_printf(Perl_debug_log, "Leaving block %ld, type %s\n",		\
> +		    (long)cxstack_ix+1,PL_block_type[CxTYPE(cx)]); )
> +
>  /* Exit a block (RETURN and LAST). */
>  #define POPBLOCK(cx,pm) cx = &cxstack[cxstack_ix--],			\
>  	newsp		 = PL_stack_base + cx->blk_oldsp,		\
> diff --git a/pp_ctl.c b/pp_ctl.c
> index a629887..34b17bb 100644
> --- a/pp_ctl.c
> +++ b/pp_ctl.c
> @@ -2046,7 +2046,7 @@ PP(pp_leaveloop)
>      PMOP *newpm;
>      SV **mark;
>  
> -    POPBLOCK(cx,newpm);
> +    POPBLOCK_normal(cx,newpm);
>      assert(CxTYPE_is_LOOP(cx));
>      mark = newsp;
>      newsp = PL_stack_base + cx->blk_loop.resetsp;
> @@ -3770,7 +3770,7 @@ PP(pp_leaveeval)
>      const U8 save_flags = PL_op -> op_flags;
>      I32 optype;
>  
> -    POPBLOCK(cx,newpm);
> +    POPBLOCK_normal(cx,newpm);
>      POPEVAL(cx);
>      retop = cx->blk_eval.retop;
>  
> @@ -3889,7 +3889,7 @@ PP(pp_leavetry)
>      register PERL_CONTEXT *cx;
>      I32 optype;
>  
> -    POPBLOCK(cx,newpm);
> +    POPBLOCK_normal(cx,newpm);
>      POPEVAL(cx);
>      PERL_UNUSED_VAR(optype);
>  
> @@ -3954,7 +3954,7 @@ PP(pp_leavegiven)
>      PMOP *newpm;
>      PERL_UNUSED_CONTEXT;
>  
> -    POPBLOCK(cx,newpm);
> +    POPBLOCK_normal(cx,newpm);
>      assert(CxTYPE(cx) == CXt_GIVEN);
>  
>      SP = newsp;
> @@ -4524,7 +4524,7 @@ PP(pp_leavewhen)
>      SV **newsp;
>      PMOP *newpm;
>  
> -    POPBLOCK(cx,newpm);
> +    POPBLOCK_normal(cx,newpm);
>      assert(CxTYPE(cx) == CXt_WHEN);
>  
>      SP = newsp;
> diff --git a/pp_hot.c b/pp_hot.c
> index 48b57d6..9c563e7 100644
> --- a/pp_hot.c
> +++ b/pp_hot.c
> @@ -1858,7 +1858,7 @@ PP(pp_leave)
>  	cx->blk_oldpm = PL_curpm;	/* fake block should preserve $1 et al */
>      }
>  
> -    POPBLOCK(cx,newpm);
> +    POPBLOCK_normal(cx,newpm);
>  
>      gimme = OP_GIMME(PL_op, (cxstack_ix >= 0) ? gimme : G_SCALAR);
>  
> @@ -2498,7 +2498,7 @@ PP(pp_leavesublv)
>      if (CxMULTICALL(&cxstack[cxstack_ix]))
>  	return 0;
>  
> -    POPBLOCK(cx,newpm);
> +    POPBLOCK_normal(cx,newpm);
>      cxstack_ix++; /* temporarily protect top context */
>  
>      TAINT_NOT;
> -- 
> 1.6.5
> 


-- 

• [PATCH] Add check for spurious PUSHMARKs by introducingPOPBLOCK_normal. by Gerard Goossen
• Re: [PATCH] Add check for spurious PUSHMARKs by introducing POPBLOCK_normal. by jesse
• Re: [PATCH] Add check for spurious PUSHMARKs by introducingPOPBLOCK_normal. by Gerard Goossen