Stack overflow when local'izing readonly arrays #16954
Comments
From @dur-randirCreated by @dur-randirWhile fuzzing perl v5.29.9-63-g2496d8f3f7 built with afl and run local@-[0..7000] to cause stack overflow. GDB stack trace is following #1 0x00005555557d0110 in Perl_sv_vsetpvfn (sv=0x555555d70a78, Perl Info |
From @tonycozOn Sun, 14 Apr 2019 03:03:11 -0700, randir wrote:
Similarly for local@-{0..7000} This is caused by the av_delete() for SAVEt_ADELETE croaking due to @- being readonly. So it tries to "restore" element 7000 calling av_delete(), which croaks, and then tries to unwind the scope, trying to delete element 6999, which croaks and so on until we run out of stack. We could fail earlier for the test case by throwing errors in aslice and hslice if we're localising a readonly array/slice, but this won't help for the more general case since the array/hash might be set readonly after the local. This would need to be done in many ops that localise for a more comprehensive fix, though most of those won't localise in bulk like aslice/hslice. ( git grep LVAL_INTRO pp*.c ) Tony |
|
The RT System itself - Status changed from 'new' to 'open' |
From @tonycozOn Mon, 22 Apr 2019 18:19:56 -0700, tonyc wrote:
The aslice case turned out to be simple. The hslice case is not so simple, since at least one test re-ties %- for testing, which doesn't work on a read-only hash. Tony |
From @tonycoz0001-perl-134028-disallow-localising-slices-of-a-RO-hash-.patchFrom f27364453c2e45a51516c6705bcc61277c6faa78 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Tue, 23 Apr 2019 20:15:42 +1000
Subject: (perl #134028) disallow localising slices of a RO hash/array
This patch shouldn't be applied.
Currently it causes re/subst.t to fail since it marks %-/%+ read-only
which disallows the tie for the RT #23624 test.
It also needs its own tests.
---
ext/Tie-Hash-NamedCapture/NamedCapture.xs | 1 +
pp.c | 12 ++++++++++++
2 files changed, 13 insertions(+)
diff --git a/ext/Tie-Hash-NamedCapture/NamedCapture.xs b/ext/Tie-Hash-NamedCapture/NamedCapture.xs
index 7eaae5614d..9d24321a55 100644
--- a/ext/Tie-Hash-NamedCapture/NamedCapture.xs
+++ b/ext/Tie-Hash-NamedCapture/NamedCapture.xs
@@ -32,6 +32,7 @@ _tie_it(SV *sv)
sv_unmagic((SV *)hv, PERL_MAGIC_tied);
sv_magic((SV *)hv, rv, PERL_MAGIC_tied, NULL, 0);
+ SvREADONLY_on((SV *)hv);
SvREFCNT_dec(rv); /* As sv_magic increased it by one. */
SV *
diff --git a/pp.c b/pp.c
index babf34843e..d57a622936 100644
--- a/pp.c
+++ b/pp.c
@@ -4870,6 +4870,9 @@ PP(pp_aslice)
MAGIC *mg;
HV *stash;
+ if (SvREADONLY(av) && MARK < SP)
+ Perl_croak_no_modify();
+
can_preserve = SvCANEXISTDELETE(av);
}
@@ -4903,6 +4906,9 @@ PP(pp_aslice)
if (!svp || !*svp)
DIE(aTHX_ PL_no_aelem, elem);
if (localizing) {
+ if (SvREADONLY(*svp))
+ Perl_croak_no_modify();
+
if (preeminent)
save_aelem(av, elem, svp);
else
@@ -5299,6 +5305,9 @@ PP(pp_hslice)
MAGIC *mg;
HV *stash;
+ if (SvREADONLY(hv) && MARK < SP)
+ Perl_croak_no_modify();
+
if (SvCANEXISTDELETE(hv))
can_preserve = TRUE;
}
@@ -5325,6 +5334,9 @@ PP(pp_hslice)
DIE(aTHX_ PL_no_helem_sv, SVfARG(keysv));
}
if (localizing) {
+ if (SvREADONLY(*svp))
+ Perl_croak_no_modify();
+
if (HvNAME_get(hv) && isGV_or_RVCV(*svp))
save_gp(MUTABLE_GV(*svp), !(PL_op->op_flags & OPf_SPECIAL));
else if (preeminent)
--
2.11.0
|
Migrated from rt.perl.org#134028 (status was 'open')
Searchable as RT134028$
The text was updated successfully, but these errors were encountered: