summaryrefslogtreecommitdiff
path: root/pp_hot.c
diff options
context:
space:
mode:
authorFather Chrysostomos <sprout@cpan.org>2011-12-26 13:25:31 -0800
committerFather Chrysostomos <sprout@cpan.org>2011-12-26 13:25:45 -0800
commitda1dff9483c6c62608e52ee5f466381813d929ff (patch)
treee204b621616bbacd34ef973a8daa6d2faf6ecb84 /pp_hot.c
parent9675db7270c7502ec2ae7845f5c13d83e0526a19 (diff)
downloadperl-da1dff9483c6c62608e52ee5f466381813d929ff.tar.gz
Fix two (er, four) sub:lvalue { &$x } bugs
The lvalue context that the last statement of an lvalue subroutine provides, when applied to entersub, causes the ops below the entersub to be complied oddly. Compare regular subs and lvalue subs: $ ./perl -Ilib -MO=Concise,bar,foo -e 'sub bar { &$x } sub foo:lvalue { &$x }' main::bar: 5 <1> leavesub[1 ref] K/REFC,1 ->(end) - <@> lineseq KP ->5 1 <;> nextstate(main 1 -e:1) v ->2 4 <1> entersub[t2] K/TARG ->5 - <1> ex-list K ->4 2 <0> pushmark s ->3 - <1> ex-rv2cv vK ->- - <1> ex-rv2sv sK/1 ->- 3 <#> gvsv[*x] s ->4 main::foo: b <1> leavesublv[1 ref] K/REFC,1 ->(end) - <@> lineseq KP ->b 6 <;> nextstate(main 2 -e:1) v ->7 a <1> entersub[t2] K/LVINTRO,TARG,INARGS ->b - <1> ex-list K ->a 7 <0> pushmark s ->8 9 <1> rv2cv vK/NO() ->a - <1> ex-rv2sv sK/1 ->9 8 <#> gvsv[*x] s ->9 -e syntax OK Notice that, in the second case, the rv2cv is not being optimised away. Under strict mode, this allows a sub call on a string, since rv2cv is not subject to strict refs. It’s this code in op.c:op_lvalue_flags that is to blame: if (kid->op_type != OP_GV) { /* Restore RV2CV to check lvalueness */ restore_2cv: if (kid->op_next && kid->op_next != kid) { /* Happens? */ okid->op_next = kid->op_next; kid->op_next = okid; } else okid->op_next = NULL; okid->op_type = OP_RV2CV; okid->op_targ = 0; okid->op_ppaddr = PL_ppaddr[OP_RV2CV]; okid->op_private |= OPpLVAL_INTRO; okid->op_private &= ~1; break; } This code is a little strange. Using rv2cv to check lvalueness causes the problem with strict refs. The lvalue check could just as well go in entersub. The way this is currently written (and this is something I missed when supposedly fixing lvalue subs), the rv2cv op will reject a non-lvalue subroutine even when the caller is not called in lvalue context. So we actually have two bugs. Presumably the check was done in rv2cv to keep entersub fast. But the code I quoted above is only part of it. There is also a special block to create an rv2cv op anew to deal with method calls. This commit fixes both issues by moving the run-time lvalueness check to entersub. I put it after PUSHSUB for speed in the most common case (when there is no error). PUSHSUB already calls a function (was_lvalue_sub) to determine whether the current sub call is happen- ing in lvalue context. So the check I am adding after it only has to check a couple of flags, instead of calling was_lvalue_sub itself. This also fixes a bug I introduced earlier in the 5.15.x series. This is supposed to die (in fact, I made the mistake earlier of changing tests that were checking for this, but so many tests were wrong back then it was an easy mistake to make): $ ./perl -Ilib -e 'sub bar {$x} sub foo:lvalue { bar}; foo=3' And a fourth bug I discovered when writing tests: sub AUTOLOAD :lvalue { warn autoloading; $x } sub _102486 { warn "called" } &{'_102486'} = 72; warn $x __END__ autoloading at - line 1. 72 at - line 4. And it happens even if there is an lvalue sub defined under that name: sub AUTOLOAD :lvalue { warn autoloading; $x } sub _102486 :lvalue { warn "called" } &{'_102486'} = 72; warn $x __END__ autoloading at - line 1. 72 at - line 4. Since the sub cannot be seen at compile time, the lvalue check in rv2cv, as mentioned above. The autoloading is happening in rv2cv, too, instead of entersub (the code is repeated), but the sub is not checked for definition first. It was put in rv2cv because it had to come before the lvalue check. Putting the latter in entersub lets us delete that repeated autoload code, which is completely wrong anyway.
Diffstat (limited to 'pp_hot.c')
-rw-r--r--pp_hot.c3
1 files changed, 3 insertions, 0 deletions
diff --git a/pp_hot.c b/pp_hot.c
index 99cd2e199a..a4e171c44d 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -2714,6 +2714,9 @@ try_autoload:
MARK++;
}
}
+ if ((cx->blk_u16 & OPpENTERSUB_LVAL_MASK) == OPpLVAL_INTRO &&
+ !CvLVALUE(cv))
+ DIE(aTHX_ "Can't modify non-lvalue subroutine call");
/* warning must come *after* we fully set up the context
* stuff so that __WARN__ handlers can safely dounwind()
* if they want to