Skip to content

Commit 91dde38

Browse files
richardleachjkeenan
authored andcommitted
Perl_newSVsv_flags_NN_PVxx: do not copy the SVprv_WEAKREF flag
When copying source SV flags to the new destination SV, this function failed to account for SVprv_WEAKREF and SVf_IVisUV flags having the same numerical value - 0x80000000. The SVprv_WEAKREF flag was consequently erroneously propagated when copying weakened references. This didn't trip existing tests because SVt_IVs (the predominant SV type for RVs) are copied using different code paths. This commit: * Always drops the SVprv_WEAKREF flag in the affected code path * Adds additional tests for copying weakened SVs
1 parent a47e682 commit 91dde38

File tree

2 files changed

+35
-1
lines changed

2 files changed

+35
-1
lines changed

sv.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5046,6 +5046,10 @@ S_newSVsv_flags_NN_PVxx(pTHX_ SV* dsv, SV* ssv, const I32 flags)
50465046
SvNV_set(dsv, SvNVX(ssv));
50475047
break;
50485048
case SVf_ROK: /* [ 3% ]*/
5049+
/* Another corner case here. SVf_IVisUV and SVprv_WEAKREF
5050+
* have the same underlying value. We do not want to
5051+
* propagate the latter. */
5052+
SvFLAGS(dsv) &= ~SVprv_WEAKREF;
50495053
SvRV_set(dsv, SvREFCNT_inc(SvRV(ssv)));
50505054
return dsv;
50515055
default: /* [ 2% ]*/

t/op/svflags.t

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ BEGIN {
1010
# Tests the new documented mechanism for determining the original type
1111
# of an SV.
1212

13-
plan tests => 16;
13+
plan tests => 22;
1414
use strict;
1515
use B qw(svref_2object SVf_IOK SVf_NOK SVf_POK);
1616

@@ -83,3 +83,33 @@ is($xobj->FLAGS & (SVf_IOK | SVf_POK), SVf_POK, "correct base flags on PV");
8383
$y = $x + 10;
8484

8585
is($xobj->FLAGS & (SVf_IOK | SVf_POK), (SVf_IOK | SVf_POK), "POK still set on PV used as number");
86+
87+
88+
# GH #23637, GH #23646 - newSVsv_flags_NN erroneously copied WEAKREF in *some* code paths
89+
90+
my $ref = [];
91+
my ($wref, $cref);
92+
93+
# Weakened reference SV is an SVt_IV
94+
$wref = $ref;
95+
builtin::weaken($wref);
96+
ok(builtin::is_weak($wref), 'a weakened SVt_IV ref has WEAKREF set');
97+
$cref = [ $wref ];
98+
ok(!builtin::is_weak( $cref->[0] ), 'SVt_IV copies do NOT have WEAKREF set');
99+
100+
# Weakened reference SV is an SVt_PV
101+
$wref = 'blip';
102+
$wref = $ref;
103+
builtin::weaken($wref);
104+
ok(builtin::is_weak($wref), 'a weakened SVt_PV ref has WEAKREF set');
105+
$cref = [ $wref ];
106+
ok(!builtin::is_weak( $cref->[0] ), 'SVt_PV copies do NOT have WEAKREF set');
107+
108+
# Weakened reference SV is an SVt_PVIV
109+
$wref = 1;
110+
$wref = $ref;
111+
builtin::weaken($wref);
112+
ok(builtin::is_weak($wref), 'a weakened SVt_PVIV ref has WEAKREF set');
113+
$cref = [ $wref ];
114+
ok(!builtin::is_weak( $cref->[0] ), 'SVt_PVIV copies do NOT have WEAKREF set');
115+

0 commit comments

Comments
 (0)