summaryrefslogtreecommitdiff
path: root/pp.h
diff options
context:
space:
mode:
authorFather Chrysostomos <sprout@cpan.org>2012-06-23 13:59:34 -0700
committerFather Chrysostomos <sprout@cpan.org>2012-06-23 13:59:34 -0700
commit2c5f48c251e2743cd9eea14146c6844e4ee5a55c (patch)
tree0aa592e615933cbce7ffd3400bbf99140da0b014 /pp.h
parent585d73c3f7e6f6ad6480ed2d56f83c15e5936eb2 (diff)
downloadperl-2c5f48c251e2743cd9eea14146c6844e4ee5a55c.tar.gz
[perl #112966] Crash on delete local; other local bugs
Commit bee7c5743fa appears to have fixed this. But what it does is barely significant: diff --git a/sv.c b/sv.c index b96f7c1..a4994f5 100644 --- a/sv.c +++ b/sv.c @@ -9525,6 +9525,11 @@ Perl_sv_bless(pTHX_ SV *const sv, HV *const stash) SvUPGRADE(tmpRef, SVt_PVMG); SvSTASH_set(tmpRef, MUTABLE_HV(SvREFCNT_inc_simple(stash))); + if (Gv_AMG(stash)) + SvAMAGIC_on(sv); + else + (void)SvAMAGIC_off(sv); + if(SvSMAGICAL(tmpRef)) if(mg_find(tmpRef, PERL_MAGIC_ext) || mg_find(tmpRef, PERL_MAGIC_uvar)) mg_set(tmpRef); The crash can still be triggered another way. Instead of a blessing, we need to modify a method (to turn on the potentially-overloaded flag) and then use an operator that respects overloading. This exam- ple crashes before and after bee7c5743fa: eval 'sub Sample::foo {}'; "".bless {},'Sample'; delete local $Sample::{ '()' }; It is the recalculation of overload caches before a localised deletion that causes the crash. And it only happens when the '()' key does not exist. Actually, it turns out that S_delete_local doesn’t behave correctly for rmagical aggregates, except for %ENV: $ ./perl -Ilib -MDevel::Peek -e 'delete local $ISA[0]' Bus error $ ./perl -XIlib -MDevel::Peek -e '??; delete local $::{foo}' Bus error It’s this line, which occurs twice in pp.c:S_do_delete_local, which is at fault: const bool can_preserve = SvCANEXISTDELETE(osv) || mg_find((const SV *)osv, PERL_MAGIC_env); When can_preserve is true, the ‘preeminent’ variable is set based on whether the element exists. Otherwise it is set to true. Why the term ‘preeminent’ was chosen I don’t know, but in this case it means that the element already exists, so it has to be restored after- wards. We can’t just do save_delete. The code for saving a hash element assumes it is non-null, and crashes otherwise. The logic for setting can_preserve is wrong. SvCANEXISTDELETE returns true for non-magical variables and for variables with those tie meth- ods implemented. For magical variables that are not tied, it returns the wrong answer. PERL_MAGIC_env seems to have been added as an exception, to keep it working. But other magical aggregates were not accounted for. This logic was copied from other functions (aslice, hslice, etc.), which are similarly buggy, but they don’t crash: $ ./perl -Ilib -le ' { local $::{foo} } print exists $::{foo}' $ ./perl -Ilib -le 'm??; { local $::{foo} } print exists $::{foo}' 1 In all these cases, it is SvCANEXISTDELETE that is buggy. So this commit fixes it and adds tests for all the code paths that use it. Now no exception needs to be made for PERL_MAGIC_env.
Diffstat (limited to 'pp.h')
-rw-r--r--pp.h4
1 files changed, 2 insertions, 2 deletions
diff --git a/pp.h b/pp.h
index deafed36b8..93aeb914f0 100644
--- a/pp.h
+++ b/pp.h
@@ -475,8 +475,8 @@ True if this op will be the return value of an lvalue subroutine
#define SvCANEXISTDELETE(sv) \
(!SvRMAGICAL(sv) \
- || ((mg = mg_find((const SV *) sv, PERL_MAGIC_tied)) \
- && (stash = SvSTASH(SvRV(SvTIED_obj(MUTABLE_SV(sv), mg)))) \
+ || !(mg = mg_find((const SV *) sv, PERL_MAGIC_tied)) \
+ || ( (stash = SvSTASH(SvRV(SvTIED_obj(MUTABLE_SV(sv), mg)))) \
&& gv_fetchmethod_autoload(stash, "EXISTS", TRUE) \
&& gv_fetchmethod_autoload(stash, "DELETE", TRUE) \
) \