Skip to content

Commit d002288

Browse files
committed
pp_sort: avoid potential I32 overflow from the comparator
Coverity says: CID 584092: Integer handling issues (INTEGER_OVERFLOW) Expression "result", where "Perl_SvIV(my_perl, *my_perl->Istack_sp)" is known to be equal to 0, overflows the type of "result", which is type "I32". The sort comparison function returns IV (a Perl integer), but all the sorting routines in this file want to work with I32. Instead of converting (and possibly truncating) the value directly, normalize the result to -1/0/1, which fits in any integer type.
1 parent cefd2df commit d002288

File tree

1 file changed

+15
-6
lines changed

1 file changed

+15
-6
lines changed

pp_sort.c

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,7 +1167,6 @@ static I32
11671167
S_sortcv(pTHX_ SV *const a, SV *const b)
11681168
{
11691169
const I32 oldsaveix = PL_savestack_ix;
1170-
I32 result;
11711170
PMOP * const pm = PL_curpm;
11721171
COP * const cop = PL_curcop;
11731172
SV *olda, *oldb;
@@ -1191,7 +1190,11 @@ S_sortcv(pTHX_ SV *const a, SV *const b)
11911190
/* entry zero of a stack is always PL_sv_undef, which
11921191
* simplifies converting a '()' return into undef in scalar context */
11931192
assert(PL_stack_sp > PL_stack_base || *PL_stack_base == &PL_sv_undef);
1194-
result = SvIV(*PL_stack_sp);
1193+
const IV iv = SvIV(*PL_stack_sp);
1194+
const I32 result =
1195+
iv > 0 ? 1 :
1196+
iv < 0 ? -1 :
1197+
0;
11951198
rpp_popfree_to_NN(PL_stack_base);
11961199

11971200
LEAVE_SCOPE(oldsaveix);
@@ -1206,7 +1209,6 @@ static I32
12061209
S_sortcv_stacked(pTHX_ SV *const a, SV *const b)
12071210
{
12081211
const I32 oldsaveix = PL_savestack_ix;
1209-
I32 result;
12101212
AV * const av = GvAV(PL_defgv);
12111213
PMOP * const pm = PL_curpm;
12121214
COP * const cop = PL_curcop;
@@ -1256,7 +1258,11 @@ S_sortcv_stacked(pTHX_ SV *const a, SV *const b)
12561258
/* entry zero of a stack is always PL_sv_undef, which
12571259
* simplifies converting a '()' return into undef in scalar context */
12581260
assert(PL_stack_sp > PL_stack_base || *PL_stack_base == &PL_sv_undef);
1259-
result = SvIV(*PL_stack_sp);
1261+
const IV iv = SvIV(*PL_stack_sp);
1262+
const I32 result =
1263+
iv > 0 ? 1 :
1264+
iv < 0 ? -1 :
1265+
0;
12601266
rpp_popfree_to_NN(PL_stack_base);
12611267

12621268
LEAVE_SCOPE(oldsaveix);
@@ -1273,7 +1279,6 @@ S_sortcv_xsub(pTHX_ SV *const a, SV *const b)
12731279
{
12741280
const I32 oldsaveix = PL_savestack_ix;
12751281
CV * const cv=MUTABLE_CV(PL_sortcop);
1276-
I32 result;
12771282
PMOP * const pm = PL_curpm;
12781283

12791284
PERL_ARGS_ASSERT_SORTCV_XSUB;
@@ -1291,7 +1296,11 @@ S_sortcv_xsub(pTHX_ SV *const a, SV *const b)
12911296
/* entry zero of a stack is always PL_sv_undef, which
12921297
* simplifies converting a '()' return into undef in scalar context */
12931298
assert(PL_stack_sp > PL_stack_base || *PL_stack_base == &PL_sv_undef);
1294-
result = SvIV(*PL_stack_sp);
1299+
const IV iv = SvIV(*PL_stack_sp);
1300+
const I32 result =
1301+
iv > 0 ? 1 :
1302+
iv < 0 ? -1 :
1303+
0;
12951304
rpp_popfree_to_NN(PL_stack_base);
12961305

12971306
LEAVE_SCOPE(oldsaveix);

0 commit comments

Comments
 (0)