[perl #105906] Bleadperl d164302a breaks Lexical::Var under threads+mad

On Sun Dec 11 13:05:22 2011, sprout wrote:
> d164302a58430157957e90a71e7a08de7eabbc94 is the first bad commit
> commit d164302a58430157957e90a71e7a08de7eabbc94
> Author: Gerard Goossen <gerard@ggoossen.net>
> Date:   Wed Jun 15 11:32:53 2011 +0200
> 
>    Add finalize_optree function which can take over all the compile
>    time checking/finalization now being done by the peephole
> optimizer.
> 
>    This function takes the optree after it is finished building. It
>    takes over some of the checking and final conversions which are
> currently being
>    done by the peephole optimizer.
> 
>    Add the moment this is an unnecessary extra step after the peephole
> optimizer, but with
>    a separate code generation step, the current peephole optimizer
> can't exists and
>    this function will take over all its essential compile time
> functions.

‘use Lexical::Var '$foo' => \753;’ overrides PL_check[OP_RV2SV] so that
$foo compiles down to a constant (753).

‘no Lexical::Var '$foo' => \$foo’ disassociates the name $foo from the
reference passed to it, if it is already associated with that reference.

As of commit d164302a58, the constant gets copied under threads+mad, so
the ‘no Lexical::Var’ doesn’t work, as \$foo is no longer returning a
reference to the same scalar that Lexical::Var tried to make it return.

Under threads, constants are relocated to the pad for thread-safety. 
Commit 8ae73bce (back in April) extended this to ops stored in MADPROPs
to fix test failures, if I understand it correctly.

But MADPROPs’ ops were treated differently.  The SV would always be
copied to the pad.

For normal ops, the SV would be copied to the pad if it were already
marked PADTMP, but the SV itself would be placed in the pad (and marked
PADTMP) otherwise.

Now that, as of commit d164302a58, MADPROPs follow the same code path, a
MADPROP’s op can take ownership of the SV (placing it in its own pad
slot), leaving the ‘real’ op to copy the SV.

Why does the SV need to be copied to begin with?  Why can’t two pad
entries share the same SV?

Then again, why does it need to be marked PADTMP?  If it’s marked
PADTMP, it means that taking a reference to it will copy it.  Isn’t the
readonliness sufficient?

Why cannot XS modules create ops that return specific scalars?

I’m probably misunderstanding things, but I don’t understand why
• SVOPs created by Perl itself can’t have PADTMP set to begin with,
• the same SV can’t be stored in multiple pad slots, and
• why PADTMP needs to be turned on when an SV is stored in a pad slot.

All this makes it hard for XS modules to do useful things.

There is also this interesting bit:

	    else if (o->op_type != OP_METHOD_NAMED
		&& cSVOPo->op_sv == &PL_sv_undef) {
		/* PL_sv_undef is hack - it's unsafe to store it in the
		   AV that is the pad, because av_fetch treats values of
		   PL_sv_undef as a "free" AV entry and will merrily
		   replace them with a new SV, causing pad_alloc to think
		   that this pad slot is free. (When, clearly, it is not)
		*/
		SvOK_off(PAD_SVl(ix));
		SvPADTMP_on(PAD_SVl(ix));
		SvREADONLY_on(PAD_SVl(ix));

I don’t believe &PL_sv_placeholder is ever accessible to Perl, so using
that instead of &PL_sv_undef in pad.c would allow XS modules to provide
an OP_CONST that actually returns &PL_sv_undef.

Another interesting thing about the else immediately following is that
READONLY is turned on without regard for whether it might be making a
corrupt cow:

	    }
	    else {
		SvREFCNT_dec(PAD_SVl(ix));
		SvPADTMP_on(cSVOPo->op_sv);
		PAD_SETSV(ix, cSVOPo->op_sv);
		/* XXX I don't know how this isn't readonly already. */
		SvREADONLY_on(PAD_SVl(ix));
	    }

$ ./perl -Ilib -le 'BEGIN { $::{foo} = qr// } print foo'
(?^:)
Assertion failed: (he->shared_he_he.hent_hek == hek), function
S_unshare_hek_or_pvn, file hv.c, line 2597.
Abort trap

So in trying to research one tiny regression, I’ve ended up opening
*another* whole can of worms.  (Is there any end in sight? :-)

-- 

Father Chrysostomos


---
via perlbug:  queue: perl5 status: new
https://rt.perl.org:443/rt3/Ticket/Display.html?id=105906

• [perl #105906] Bleadperl d164302a breaks Lexical::Var under threads+mad by Father Chrysostomos via RT
• Re: [perl #109726] PL_sv_undef loses identity by Nicholas Clark
• [perl #109726] PL_sv_undef loses identity by Father Chrysostomos via RT
• Re: [perl #105906] Bleadperl d164302a breaks Lexical::Var underthreads+mad by Zefram
• [perl #105906] Bleadperl d164302a breaks Lexical::Var under threads+mad by Father Chrysostomos via RT
• Re: [perl #105906] Bleadperl d164302a breaks Lexical::Var underthreads+mad by Zefram