Skip to content

Commit 2efb6d3

Browse files
committed
S_newONCEOP: correctly copy array/hash at subroutine exit
Should a _state_ array or hash be declared immediately prior to returning, the error "Bizarre copy of HASH in subroutine exit" error would be triggered. Now, the array or hash is returned without error. Specifically: * Instead of always creating a PADSV, the appropriate padop is created. * The OPf_REF flag is switched off, allowing the return of the container variable's contents rather than the container SV itself. * The OPf_MOD & OPf_SPECIAL flags are switches off, as they are not relevant to this version of the padop. Note: The op created has been renamed from "other" to "nextop", since it is linked to the ONCE op via op_next, not op_other. Fixes GH#18630
1 parent 883bf45 commit 2efb6d3

File tree

4 files changed

+22
-11
lines changed

4 files changed

+22
-11
lines changed

ext/B/t/optree_varinit.t

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ checkOptree ( name => 'void context state',
406406
# 4 <$> const[IV 1] s
407407
# 5 <1> padsv_store[$x:2,3] vKS/LVINTRO,STATE
408408
# goto 6
409-
# 8 <0> padsv[$x:2,3] vRM*/STATE
409+
# 8 <0> padsv[$x:2,3] v/STATE
410410
# 6 <;> nextstate(main 3 -e:1) v:%,{,fea=15
411411
# 7 <@> leave[1 ref] vKP/REFC
412412
EOT_EOT
@@ -416,7 +416,7 @@ EOT_EOT
416416
# 4 <$> const(IV 1) s
417417
# 5 <1> padsv_store[$x:2,3] vKS/LVINTRO,STATE
418418
# goto 6
419-
# 8 <0> padsv[$x:2,3] vRM*/STATE
419+
# 8 <0> padsv[$x:2,3] v/STATE
420420
# 6 <;> nextstate(main 3 -e:1) v:%,{,fea=15
421421
# 7 <@> leave[1 ref] vKP/REFC
422422
EONT_EONT
@@ -433,7 +433,7 @@ checkOptree ( name => 'scalar context state',
433433
# 5 <0> padsv[$x:2,3] sRM*/LVINTRO,STATE
434434
# 6 <2> sassign sKS/2
435435
# goto 7
436-
# a <0> padsv[$x:2,3] sRM*/STATE
436+
# a <0> padsv[$x:2,3] s/STATE
437437
# 7 <1> padsv_store[$y:2,3] vKS/LVINTRO
438438
# 8 <;> nextstate(main 3 -e:1) v:%,{,fea=15
439439
# 9 <@> leave[1 ref] vKP/REFC
@@ -445,7 +445,7 @@ EOT_EOT
445445
# 5 <0> padsv[$x:2,3] sRM*/LVINTRO,STATE
446446
# 6 <2> sassign sKS/2
447447
# goto 7
448-
# a <0> padsv[$x:2,3] sRM*/STATE
448+
# a <0> padsv[$x:2,3] s/STATE
449449
# 7 <1> padsv_store[$y:2,3] vKS/LVINTRO
450450
# 8 <;> nextstate(main 3 -e:1) v:%,{,fea=15
451451
# 9 <@> leave[1 ref] vKP/REFC

op.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8596,18 +8596,18 @@ static OP *
85968596
S_newONCEOP(pTHX_ OP *initop, OP *padop)
85978597
{
85988598
const PADOFFSET target = padop->op_targ;
8599-
OP *const other = newOP(OP_PADSV,
8600-
padop->op_flags
8599+
OP *const nexop = newOP(padop->op_type,
8600+
(padop->op_flags & ~(OPf_REF|OPf_MOD|OPf_SPECIAL))
86018601
| ((padop->op_private & ~OPpLVAL_INTRO) << 8));
86028602
OP *const first = newOP(OP_NULL, 0);
8603-
OP *const nullop = newCONDOP(0, first, initop, other);
8603+
OP *const nullop = newCONDOP(0, first, initop, nexop);
86048604
/* XXX targlex disabled for now; see ticket #124160
8605-
newCONDOP(0, first, S_maybe_targlex(aTHX_ initop), other);
8605+
newCONDOP(0, first, S_maybe_targlex(aTHX_ initop), nexop);
86068606
*/
86078607
OP *const condop = first->op_next;
86088608

86098609
OpTYPE_set(condop, OP_ONCE);
8610-
other->op_targ = target;
8610+
nexop->op_targ = target;
86118611

86128612
/* Store the initializedness of state vars in a separate
86138613
pad entry. */

pod/perldelta.pod

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,9 @@ manager will later use a regex to expand these into links.
401401

402402
=item *
403403

404-
XXX
404+
Declaring a lexically scoped array or hash using C<state> within a subroutine
405+
and then immediately returning no longer triggers a "Bizarre copy of HASH/ARRAY
406+
in subroutine exit" error. [GH #18630]
405407

406408
=back
407409

t/op/state.t

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ BEGIN {
99

1010
use strict;
1111

12-
plan tests => 162;
12+
plan tests => 164;
1313

1414
# Before loading feature.pm, test it with CORE::
1515
ok eval 'CORE::state $x = 1;', 'CORE::state outside of feature.pm scope';
@@ -504,7 +504,16 @@ for (1,2) {
504504
or diag "got these warnings:\n@warnings";
505505
}
506506

507+
# [GH #18630] Returning state hash-assign risks "Bizarre copy of HASH in subroutine exit"
508+
{
509+
sub gh_18630H {state %h=(a=>1)}
510+
my $res = join '', gh_18630H, gh_18630H;
511+
is($res, "a1a1", 'HASH copied successfully in subroutine exit');
507512

513+
sub gh_18630A {state @a = qw(b 2)}
514+
$res = join '', gh_18630A , gh_18630A;
515+
is($res, "b2b2", 'ARRAY copied successfully in subroutine exit');
516+
}
508517

509518
__DATA__
510519
(state $a) = 1;

0 commit comments

Comments
 (0)