Skip to content

Commit 4419193

Browse files
committed
Perl_sv_setsv_flags: extract croaks to a cold helper function
Perl_sv_setsv_flags has a number of fail-safe checks which will croak if triggered. However, these code paths are *really* cold - they aren't even hit by the test harness. Since they are so cold and always result in an immediate croak, they can be pulled out into an unoptimized helper function. This leaves Perl_sv_setsv_flags smaller and therefore more cache friendly.
1 parent 5a659e4 commit 4419193

File tree

4 files changed

+54
-25
lines changed

4 files changed

+54
-25
lines changed

embed.fnc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5815,6 +5815,9 @@ S |void |assert_uft8_cache_coherent \
58155815
|STRLEN from_cache \
58165816
|STRLEN real \
58175817
|NN SV * const sv
5818+
S |void |croak_sv_setsv_flags \
5819+
|NN SV * const dsv \
5820+
|NN SV * const ssv
58185821
S |bool |curse |NN SV * const sv \
58195822
|const bool check_refcnt
58205823
RS |STRLEN |expect_number |NN const char ** const pattern

embed.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2158,6 +2158,7 @@
21582158
# define F0convert S_F0convert
21592159
# define anonymise_cv_maybe(a,b) S_anonymise_cv_maybe(aTHX_ a,b)
21602160
# define assert_uft8_cache_coherent(a,b,c,d) S_assert_uft8_cache_coherent(aTHX_ a,b,c,d)
2161+
# define croak_sv_setsv_flags(a,b) S_croak_sv_setsv_flags(aTHX_ a,b)
21612162
# define curse(a,b) S_curse(aTHX_ a,b)
21622163
# define expect_number(a) S_expect_number(aTHX_ a)
21632164
# define find_array_subscript(a,b) S_find_array_subscript(aTHX_ a,b)

proto.h

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

sv.c

Lines changed: 45 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4275,24 +4275,17 @@ Perl_sv_setsv_flags(pTHX_ SV *dsv, SV* ssv, const I32 flags)
42754275
SvREFCNT_dec(old_rv);
42764276
return;
42774277
}
4278-
4278+
/*
42794279
#if NVSIZE <= IVSIZE
42804280
both_type = (stype | dtype);
42814281
#endif
4282-
4282+
*/
42834283
if (UNLIKELY(both_type == SVTYPEMASK)) {
4284-
if (SvIS_FREED(dsv)) {
4285-
Perl_croak(aTHX_ "panic: attempt to copy value %" SVf
4286-
" to a freed scalar %p", SVfARG(ssv), (void *)dsv);
4287-
}
4288-
if (SvIS_FREED(ssv)) {
4289-
Perl_croak(aTHX_ "panic: attempt to copy freed scalar %p to %p",
4290-
(void*)ssv, (void*)dsv);
4291-
}
4284+
croak_sv_setsv_flags(dsv, ssv);
4285+
NOT_REACHED;
42924286
}
42934287

42944288

4295-
42964289
SV_CHECK_THINKFIRST_COW_DROP(dsv);
42974290
dtype = SvTYPE(dsv); /* THINKFIRST may have changed type */
42984291

@@ -4390,14 +4383,7 @@ Perl_sv_setsv_flags(pTHX_ SV *dsv, SV* ssv, const I32 flags)
43904383
invlist_clone(ssv, dsv);
43914384
return;
43924385
default:
4393-
{
4394-
const char * const type = sv_reftype(ssv,0);
4395-
if (PL_op)
4396-
/* diag_listed_as: Bizarre copy of %s */
4397-
Perl_croak(aTHX_ "Bizarre copy of %s in %s", type, OP_DESC(PL_op));
4398-
else
4399-
Perl_croak(aTHX_ "Bizarre copy of %s", type);
4400-
}
4386+
croak_sv_setsv_flags(dsv, ssv);
44014387
NOT_REACHED; /* NOTREACHED */
44024388

44034389
case SVt_REGEXP:
@@ -4451,12 +4437,8 @@ Perl_sv_setsv_flags(pTHX_ SV *dsv, SV* ssv, const I32 flags)
44514437
else if (UNLIKELY(dtype == SVt_PVAV || dtype == SVt_PVHV
44524438
|| dtype == SVt_PVFM))
44534439
{
4454-
const char * const type = sv_reftype(dsv,0);
4455-
if (PL_op)
4456-
/* diag_listed_as: Cannot copy to %s */
4457-
Perl_croak(aTHX_ "Cannot copy to %s in %s", type, OP_DESC(PL_op));
4458-
else
4459-
Perl_croak(aTHX_ "Cannot copy to %s", type);
4440+
croak_sv_setsv_flags(dsv, ssv);
4441+
NOT_REACHED;
44604442
} else if (sflags & SVf_ROK) {
44614443
if (isGV_with_GP(dsv)
44624444
&& SvTYPE(SvRV(ssv)) == SVt_PVGV && isGV_with_GP(SvRV(ssv))) {
@@ -17736,6 +17718,44 @@ Perl_report_uninit(pTHX_ const SV *uninit_sv)
1773617718
GCC_DIAG_RESTORE_STMT;
1773717719
}
1773817720

17721+
/* This helper function for Perl_sv_setsv_flags is as cold as they come.
17722+
* We should almost never call it and it will definitely croak when we do.
17723+
* Therefore it should not matter that it is not close to the main function
17724+
* or that we make it redo work that the caller already did.
17725+
* The main aim is to keep Perl_sv_setsv_flags as slim as possible and this
17726+
* includes keeping the call sites for this function small.
17727+
*/
17728+
void S_croak_sv_setsv_flags(pTHX_ SV * const dsv, SV * const ssv)
17729+
{
17730+
OP *op = PL_op;
17731+
if (SvIS_FREED(dsv)) {
17732+
Perl_croak(aTHX_ "panic: attempt to copy value %" SVf
17733+
" to a freed scalar %p", SVfARG(ssv), (void *)dsv);
17734+
}
17735+
if (SvIS_FREED(ssv)) {
17736+
Perl_croak(aTHX_ "panic: attempt to copy freed scalar %p to %p",
17737+
(void*)ssv, (void*)dsv);
17738+
}
17739+
17740+
if (SvTYPE(ssv) > SVt_PVLV)
17741+
{
17742+
const char * const type = sv_reftype(ssv,0);
17743+
if (op)
17744+
/* diag_listed_as: Bizarre copy of %s */
17745+
Perl_croak(aTHX_ "Bizarre copy of %s in %s", type, OP_DESC(op));
17746+
else
17747+
Perl_croak(aTHX_ "Bizarre copy of %s", type);
17748+
}
17749+
17750+
const char * const type = sv_reftype(dsv,0);
17751+
if (op)
17752+
/* diag_listed_as: Cannot copy to %s */
17753+
Perl_croak(aTHX_ "Cannot copy to %s in %s", type, OP_DESC(op));
17754+
else
17755+
Perl_croak(aTHX_ "Cannot copy to %s", type);
17756+
17757+
}
17758+
1773917759
/*
1774017760
* ex: set ts=8 sts=4 sw=4 et:
1774117761
*/

0 commit comments

Comments
 (0)