mirror of
https://github.com/Perl/perl5.git
synced 2026-01-27 01:44:43 +00:00
fix Perl_rpp_is_lone() for immortals.
Perl_rpp_is_lone() checks whether an SV is kept alive only by the args
and/or temps stack, and thus can in various circumstances be used
directly rather than copying - such as a return value from a function, or
assigning a value to an array or hash.
However, this assumption fails on immortal SVs like PL_sv_undef. Their
refcount can reach 1 but they should still be copied rather than stolen.
This showed itself as an extremely intermittent failure in
cpan/Test-Harness/t/compat/test-harness-compat.t
It only showed up when both these conditions were met:
- on DEBUGGING builds, because that sets the initial ref count of the
immortals to 1000 rather than I32_MAX, making such problems easier to
reproduce; *and*
- under PERL_RC_STACK, because only on that build do you have SVs
on the args stack (but not temps stack) with RC==1 which are stealable.
Also under the RC build, as an optimisation, &PL_sv_undef is often
pushed on the stack without incrementing its RC. Thus it is more liklely
to reach RC==1.
I don't know why the failure on that test was intermittent, even after
setting PERL_HASH_SEED=0. Perhaps it was timing related. Certainly
anything which made it slower, such as strace, valgrind or ASAN, made
the problem go away or fail only after days rather than minutes.
This commit also decreases the value of SvREFCNT_IMMORTAL on
debugging builds from 1000 to 10 under PERL_RC_STACK, to make such
issues/bugs more likely to be triggered.
This commit is contained in:
parent
f54903ed18
commit
efc99d519e
7
inline.h
7
inline.h
@ -1040,9 +1040,9 @@ Indicates whether the stacked SV C<sv> (assumed to be not yet popped off
|
||||
the stack) is only kept alive due to a single reference from the argument
|
||||
stack and/or and the temps stack.
|
||||
|
||||
This can used for example to decide whether the copying of return values in rvalue
|
||||
context can be skipped, or whether it shouldn't be assigned to in lvalue
|
||||
context.
|
||||
This can used for example to decide whether the copying of return values
|
||||
in rvalue context can be skipped, or whether it shouldn't be assigned to
|
||||
in lvalue context.
|
||||
|
||||
=cut
|
||||
*/
|
||||
@ -1063,6 +1063,7 @@ Perl_rpp_is_lone(pTHX_ SV *sv)
|
||||
return SvREFCNT(sv) <= cBOOL(SvTEMP(sv))
|
||||
#ifdef PERL_RC_STACK
|
||||
+ 1
|
||||
&& !SvIMMORTAL(sv) /* PL_sv_undef etc are never stealable */
|
||||
#endif
|
||||
;
|
||||
}
|
||||
|
||||
12
sv.h
12
sv.h
@ -2415,7 +2415,17 @@ that already have a PV buffer allocated, but no SvTHINKFIRST.
|
||||
|
||||
#ifdef DEBUGGING
|
||||
/* exercise the immortal resurrection code in sv_free2() */
|
||||
# define SvREFCNT_IMMORTAL 1000
|
||||
# ifdef PERL_RC_STACK
|
||||
/* When the stack is ref-counted, the code tends to take a lot of
|
||||
* short cuts with immortals, such as skipping the bump of the ref
|
||||
* count of PL_sv_undef when pushing it on the stack. Exercise that
|
||||
* this doesn't cause problems, especially on code which
|
||||
* special-cases RC==1 etc.
|
||||
*/
|
||||
# define SvREFCNT_IMMORTAL 10
|
||||
# else
|
||||
# define SvREFCNT_IMMORTAL 1000
|
||||
# endif
|
||||
#else
|
||||
# define SvREFCNT_IMMORTAL ((~(U32)0)/2)
|
||||
#endif
|
||||
|
||||
21
t/op/hash.t
21
t/op/hash.t
@ -259,6 +259,27 @@ package Magic {
|
||||
::is(join( ':', %inner), "x:y", "magic keys");
|
||||
}
|
||||
|
||||
# Make sure PL_sv_undef is copied, and not stored directly, when assigning
|
||||
# to a hash. This failed on DEBUGGING + PERL_RC_STACK builds because the
|
||||
# test for a lone SV assumed that an SV on the stack with a ref count of 1
|
||||
# could be used directly rather than copied. However, PL_sv_undef and
|
||||
# friends could reach a ref count of 1 but still not be stealable.
|
||||
#
|
||||
# A DEBUGGING build sets the initial ref count of the immortals to 1000
|
||||
# rather than I32_MAX, making such problems easier to reproduce.
|
||||
|
||||
{
|
||||
my $bad = 0;
|
||||
for (1..1001) {
|
||||
# Each iteration may leave the RC of PL_sv_undef one lower.
|
||||
my %h = ('a', undef);
|
||||
# this could fail with
|
||||
# "Modification of non-creatable hash value attempted ..."
|
||||
eval { $h{a} = 1; };
|
||||
$bad = 1 if $@ ne "";
|
||||
}
|
||||
ok(!$bad, "PL_sv_undef RC 1");
|
||||
}
|
||||
|
||||
|
||||
done_testing();
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user