diff options
author | Father Chrysostomos <sprout@cpan.org> | 2012-06-23 13:59:34 -0700 |
---|---|---|
committer | Father Chrysostomos <sprout@cpan.org> | 2012-06-23 13:59:34 -0700 |
commit | 2c5f48c251e2743cd9eea14146c6844e4ee5a55c (patch) | |
tree | 0aa592e615933cbce7ffd3400bbf99140da0b014 /pp.h | |
parent | 585d73c3f7e6f6ad6480ed2d56f83c15e5936eb2 (diff) | |
download | perl-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.h | 4 |
1 files changed, 2 insertions, 2 deletions
@@ -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) \ ) \ |