[PATCH] Coverity: flop max range

[p5pRT]

Migrated from rt.perl.org#121896 (status was 'resolved')

Searchable as RT121896$

[p5pRT]

From @jhi

Firstly\, rename {max\,j} -> {j\,n} since j as the count is very confusing.

The j > SSize_t_MAX cannot (usually) fire.

If the IV and SSize_t are the same (very likely) the count (an IV) cannot be larger than the max of SSize_t.

If IV is larger than SSize_t (e.g. long longs for IVs but only 32-bit pointers available) the count could conceivably be larger than the max of SSize_t\, but we still need to correctly dance around the max​: the signed maxima are tricky since we should not depend on any particular overflow/wraparound behavior.

If IV is smaller than SSize_t (I32 for IV but 64-bit pointers available​: sounds unlikely\, but possible)\, the count again cannot be larger than the max.

NOTE​: this logic only protects against the wraparound\, not against an OOM​: we will run out of memory long before we create SSize_t_MAX SVs. There is nothing magical about SSize_t as such\, except that it is a likely wraparound spot.

[p5pRT]

From @jhi

0001-The-max-size-of-the-flop-operator-range.patch

```diff From cb3c1befd1124a89c81116bf87197374fdf170d4 Mon Sep 17 00:00:00 2001 From: Jarkko Hietaniemi Date: Tue, 6 May 2014 17:30:11 -0400 Subject: [PATCH] The max size of the flop operator range. Firstly, rename {max,j} -> {j,n} since j as the count is very confusing. The j > SSize_t_MAX cannot (usually) fire. If the IV and SSize_t are the same (very likely) the count (an IV) cannot be larger than the max of SSize_t. If IV is larger than SSize_t (e.g. long longs for IVs but only 32-bit pointers available) the count could conceivably be larger than the max of SSize_t, but we still need to correctly dance around the max: the signed maxima are tricky since we should not depend on any particular overflow/wraparound behavior. If IV is smaller than SSize_t (I32 for IV but 64-bit pointers available: sounds unlikely, but possible), the count again cannot be larger than the max. NOTE: this logic only protects against the wraparound, not against an OOM: we will run out of memory long before we create SSize_t_MAX SVs. There is nothing magical about SSize_t as such, except that it is a likely wraparound spot. Fix for Coverity perl5 CID 28933. --- pp_ctl.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/pp_ctl.c b/pp_ctl.c index 380a7fe..527dd76 100644 --- a/pp_ctl.c +++ b/pp_ctl.c @@ -1184,25 +1184,33 @@ PP(pp_flop) SvGETMAGIC(right); if (RANGE_IS_NUMERIC(left,right)) { - IV i, j; - IV max; + IV i, j, n; if ((SvOK(left) && !SvIOK(left) && SvNV_nomg(left) < IV_MIN) || (SvOK(right) && (SvIOK(right) ? SvIsUV(right) && SvUV(right) > IV_MAX : SvNV_nomg(right) > IV_MAX))) DIE(aTHX_ "Range iterator outside integer range"); i = SvIV_nomg(left); - max = SvIV_nomg(right); - if (max >= i) { - j = max - i + 1; - if (j > SSize_t_MAX) - Perl_croak(aTHX_ "Out of memory during list extend"); - EXTEND_MORTAL(j); - EXTEND(SP, j); + j = SvIV_nomg(right); + if (j >= i) { + /* Dance carefully around signed max. */ + bool overflow = (i <= 0 && j > SSize_t_MAX + i - 1); + if (!overflow) { + n = j - i + 1; + /* The wraparound of signed integers is undefined + * behavior, but here we aim for count >=1, and + * negative count is just wrong. */ + if (n < 1) + overflow = TRUE; + } + if (overflow) + Perl_croak(aTHX_ "Out of memory during list extend"); + EXTEND_MORTAL(n); + EXTEND(SP, n); } else - j = 0; - while (j--) { + n = 0; + while (n--) { SV * const sv = sv_2mortal(newSViv(i++)); PUSHs(sv); } -- 1.8.5.2 (Apple Git-48) ```

[p5pRT]

From @bulk88

On Fri May 16 08​:53​:53 2014\, jhi wrote​:

the signed maxima are tricky since we should not depend on any particular overflow/wraparound behavior.

Before we go adding complexity to fix Perl to run on ANSI C Virtual Machine\, does Perl run on any 1s complement CPUs?

-- bulk88 ~ bulk88 at hotmail.com

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From @jhi

On Friday-201405-16\, 15​:12\, bulk88 via RT wrote​:

Before we go adding complexity to fix Perl to run on ANSI C Virtual Machine\, does Perl run on any 1s complement CPUs?

You may curse the gods of ANSI but they are not hearing our pleas​: the signed behavior of is not a defect​: http​://blog.regehr.org/archives/1149

I'm pretty certain Perl's code is beyond redemption for one's complement machines. In early eighties CDC Cybers were still around (though their prime time was 1970s)\, and they were one's complement machines\, or so I hear (I was busy programming in Z80 assembler\, so cannot vouch for it).

[p5pRT]

From @khwilliamson

On 05/16/2014 07​:41 PM\, Jarkko Hietaniemi wrote​:

CDC Cybers were still around (though their prime time was 1970s)\, and they were one's complement machines\, or so I hear

I can confirm that CDC machines were 1's complement; I imagine Crays as well\, but can't remember for sure.

[p5pRT]

From @tonycoz

On Fri\, May 16\, 2014 at 08​:53​:53AM -0700\, Jarkko Hietaniemi wrote​:

If IV is smaller than SSize_t (I32 for IV but 64-bit pointers available​: sounds unlikely\, but possible)\, the count again cannot be larger than the max.

This can't be true.

IVs are used to hold pointers and are always at least as large as a pointer.

(I haven't looked at the patch yet.)

Tony

[p5pRT]

From @jhi

On Saturday-201405-17\, 0​:08\, Tony Cook wrote​:

On Fri\, May 16\, 2014 at 08​:53​:53AM -0700\, Jarkko Hietaniemi wrote​:

If IV is smaller than SSize_t (I32 for IV but 64-bit pointers available​: sounds unlikely\, but possible)\, the count again cannot be larger than the max.

This can't be true.

IVs are used to hold pointers and are always at least as large as a pointer.

Right. So doubly cannot be true.

[p5pRT]

From @jhi

On Friday-201405-16\, 23​:46\, Karl Williamson wrote​:

On 05/16/2014 07​:41 PM\, Jarkko Hietaniemi wrote​:

CDC Cybers were still around (though their prime time was 1970s)\, and they were one's complement machines\, or so I hear

I can confirm that CDC machines were 1's complement; I imagine Crays as well\, but can't remember for sure. .

Early Crays\, possibly\, they were contemporaries with Cyber and could have had "compatibility". But already C90 and T3D were firmly two's complement. Cray over their history has been through many wildly different implementations.

[p5pRT]

From @jhi

On Saturday-201405-17\, 0​:09\, Tony Cook via RT wrote​:

On Fri\, May 16\, 2014 at 08​:53​:53AM -0700\, Jarkko Hietaniemi wrote​:

If IV is smaller than SSize_t (I32 for IV but 64-bit pointers available​: sounds unlikely\, but possible)\, the count again cannot be larger than the max.

This can't be true.

IVs are used to hold pointers and are always at least as large as a pointer.

(I haven't looked at the patch yet.)

Updated patch attached. (Only diddled with the commit message.)

Tony

[p5pRT]

From @jhi

0001-The-max-size-of-the-flop-operator-range.patch

```diff From 24dd6e99023c0c5750a66b3f78e813e40602cbe8 Mon Sep 17 00:00:00 2001 From: Jarkko Hietaniemi Date: Mon, 19 May 2014 17:57:06 -0400 Subject: [PATCH] The max size of the flop operator range. Firstly, rename {max,j} -> {j,n} since j as the count is very confusing. The j > SSize_t_MAX cannot (usually) fire. (1) If the IV and SSize_t are the same (very likely) the count (an IV) simply cannot be larger than the max of SSize_t. (2) If IV is larger than SSize_t (e.g. long longs for IVs but only 32-bit pointers available) the count could conceivably be larger than the max of SSize_t, but we still need to correctly dance around the max: the signed maxima are tricky since the behavior of overflow is undefined. (3) The IV cannot be smaller than than SSize_t (e.g. I32 for IV but 64-bit pointers available): IV is supposed to be large enough to contain pointers). NOTE: this logic only protects against the wraparound, not against an OOM: we will run out of memory long before we create (even) SSize_t_MAX SVs. There is nothing magical about SSize_t as such, except that it is a likely wraparound spot. Fix for Coverity perl5 CID 28933. --- pp_ctl.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/pp_ctl.c b/pp_ctl.c index 380a7fe..527dd76 100644 --- a/pp_ctl.c +++ b/pp_ctl.c @@ -1184,25 +1184,33 @@ PP(pp_flop) SvGETMAGIC(right); if (RANGE_IS_NUMERIC(left,right)) { - IV i, j; - IV max; + IV i, j, n; if ((SvOK(left) && !SvIOK(left) && SvNV_nomg(left) < IV_MIN) || (SvOK(right) && (SvIOK(right) ? SvIsUV(right) && SvUV(right) > IV_MAX : SvNV_nomg(right) > IV_MAX))) DIE(aTHX_ "Range iterator outside integer range"); i = SvIV_nomg(left); - max = SvIV_nomg(right); - if (max >= i) { - j = max - i + 1; - if (j > SSize_t_MAX) - Perl_croak(aTHX_ "Out of memory during list extend"); - EXTEND_MORTAL(j); - EXTEND(SP, j); + j = SvIV_nomg(right); + if (j >= i) { + /* Dance carefully around signed max. */ + bool overflow = (i <= 0 && j > SSize_t_MAX + i - 1); + if (!overflow) { + n = j - i + 1; + /* The wraparound of signed integers is undefined + * behavior, but here we aim for count >=1, and + * negative count is just wrong. */ + if (n < 1) + overflow = TRUE; + } + if (overflow) + Perl_croak(aTHX_ "Out of memory during list extend"); + EXTEND_MORTAL(n); + EXTEND(SP, n); } else - j = 0; - while (j--) { + n = 0; + while (n--) { SV * const sv = sv_2mortal(newSViv(i++)); PUSHs(sv); } -- 1.8.5.2 (Apple Git-48) ```

[p5pRT]

From @tonycoz

On Mon May 19 15​:01​:05 2014\, jhi wrote​:

On Saturday-201405-17\, 0​:09\, Tony Cook via RT wrote​:

On Fri\, May 16\, 2014 at 08​:53​:53AM -0700\, Jarkko Hietaniemi wrote​:

If IV is smaller than SSize_t (I32 for IV but 64-bit pointers available​: sounds unlikely\, but possible)\, the count again cannot be larger than the max.

This can't be true.

IVs are used to hold pointers and are always at least as large as a pointer.

(I haven't looked at the patch yet.)

Updated patch attached. (Only diddled with the commit message.)

Applied as b262c4c91294fc9179a7343473b880ca445c4e43 by Jarkko.

Tony

[p5pRT]

@tonycoz - Status changed from 'open' to 'resolved'