Skip to content

Commit

Permalink
S_newONCEOP: correctly copy array/hash at subroutine exit
Browse files Browse the repository at this point in the history
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
  • Loading branch information
richardleach committed Nov 21, 2024
1 parent ff0ce7d commit ccd591c
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 13 deletions.
8 changes: 4 additions & 4 deletions ext/B/t/optree_varinit.t
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ checkOptree ( name => 'void context state',
# 4 <$> const[IV 1] s
# 5 <1> padsv_store[$x:2,3] vKS/LVINTRO,STATE
# goto 6
# 8 <0> padsv[$x:2,3] vRM*/STATE
# 8 <0> padsv[$x:2,3] v/STATE
# 6 <;> nextstate(main 3 -e:1) v:%,{,fea=15
# 7 <@> leave[1 ref] vKP/REFC
EOT_EOT
Expand All @@ -416,7 +416,7 @@ EOT_EOT
# 4 <$> const(IV 1) s
# 5 <1> padsv_store[$x:2,3] vKS/LVINTRO,STATE
# goto 6
# 8 <0> padsv[$x:2,3] vRM*/STATE
# 8 <0> padsv[$x:2,3] v/STATE
# 6 <;> nextstate(main 3 -e:1) v:%,{,fea=15
# 7 <@> leave[1 ref] vKP/REFC
EONT_EONT
Expand All @@ -433,7 +433,7 @@ checkOptree ( name => 'scalar context state',
# 5 <0> padsv[$x:2,3] sRM*/LVINTRO,STATE
# 6 <2> sassign sKS/2
# goto 7
# a <0> padsv[$x:2,3] sRM*/STATE
# a <0> padsv[$x:2,3] s/STATE
# 7 <1> padsv_store[$y:2,3] vKS/LVINTRO
# 8 <;> nextstate(main 3 -e:1) v:%,{,fea=15
# 9 <@> leave[1 ref] vKP/REFC
Expand All @@ -445,7 +445,7 @@ EOT_EOT
# 5 <0> padsv[$x:2,3] sRM*/LVINTRO,STATE
# 6 <2> sassign sKS/2
# goto 7
# a <0> padsv[$x:2,3] sRM*/STATE
# a <0> padsv[$x:2,3] s/STATE
# 7 <1> padsv_store[$y:2,3] vKS/LVINTRO
# 8 <;> nextstate(main 3 -e:1) v:%,{,fea=15
# 9 <@> leave[1 ref] vKP/REFC
Expand Down
10 changes: 5 additions & 5 deletions op.c
Original file line number Diff line number Diff line change
Expand Up @@ -8596,18 +8596,18 @@ static OP *
S_newONCEOP(pTHX_ OP *initop, OP *padop)
{
const PADOFFSET target = padop->op_targ;
OP *const other = newOP(OP_PADSV,
padop->op_flags
OP *const nexop = newOP(padop->op_type,
(padop->op_flags & ~(OPf_REF|OPf_MOD|OPf_SPECIAL))
| ((padop->op_private & ~OPpLVAL_INTRO) << 8));
OP *const first = newOP(OP_NULL, 0);
OP *const nullop = newCONDOP(0, first, initop, other);
OP *const nullop = newCONDOP(0, first, initop, nexop);
/* XXX targlex disabled for now; see ticket #124160
newCONDOP(0, first, S_maybe_targlex(aTHX_ initop), other);
newCONDOP(0, first, S_maybe_targlex(aTHX_ initop), nexop);
*/
OP *const condop = first->op_next;

OpTYPE_set(condop, OP_ONCE);
other->op_targ = target;
nexop->op_targ = target;

/* Store the initializedness of state vars in a separate
pad entry. */
Expand Down
6 changes: 3 additions & 3 deletions pod/perldelta.pod
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,9 @@ with a misaligned access early in the build. [GH #22577]

=item *

On threaded builds on POSIX-like systems, if the perl signal handler
receives we now resend the signal to the main perl thread. Previously
this would crash. [GH #22487]
Declaring a lexically scoped array or hash using C<state> within a subroutine
and then immediately returning no longer triggers a "Bizarre copy of HASH/ARRAY
in subroutine exit" error. [GH #18630]

=back

Expand Down
11 changes: 10 additions & 1 deletion t/op/state.t
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ BEGIN {

use strict;

plan tests => 162;
plan tests => 164;

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

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

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

__DATA__
(state $a) = 1;
Expand Down

0 comments on commit ccd591c

Please sign in to comment.