This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch, fortran, RFC] PR28105 Overflow check for ALLOCATE statement

On 11/22/2010 11:37 PM, Janne Blomqvist wrote:
the attached patch implements overflow checking for the ALLOCATE
statement. It also fixes the minor regression introduced a few days
ago when the old "<  0" overflow check was removed as a side-effect of
fixing the unsignedness of size_t.

Frankly, I do not like the way the overflow check is done - namely by requiring an overflow:

D.1506 = MAX_EXPR <my_size, 0>;
D.1507 = D.1506 != 0 ? 2147483647 / D.1506 <= 0 ? 1 : 0 : 0;
D.1508 = NON_LVALUE_EXPR <D.1506>;
D.1509 = ((<unnamed-unsigned:32>) D.1508 > -1 ? 1 : 0) + D.1507;
overflow.1 = D.1509;
if (overflow.1 != 0)
_gfortran_runtime_error (&"Integer overflow when calculating the amount of memory to allocate"[1]{lb: 1 sz: 1});

Namely, doing the "MAXSIZE/my_size <= 0" check where my_size is the signed requested size of the array. Explicitly asking for an overflow seems to be wrong. Additionally, I have no idea what the supposed result is; MAXSIZE (= "TYPE_MAX_VALUE (sizetype)") is unsigned and does not fit into a signed variable, however, the calculation is supposed to be done signed as both the D.1506 and the D.1507 is signed.

Additionally, you completely ignore one important ingredient: The size of the actual object which is to be allocated; using derived types those can be large. Example:

  use iso_c_binding, only: c_intptr_t
  integer(1), allocatable :: a(:)
  integer(4), allocatable :: b(:)
  allocate(a(huge(0_c_intptr_t)*i))  ! OK
  allocate(b(huge(0_c_intptr_t)*i))  ! Too big

That's the reason I was thinking of:

if (my_size > MAXSIZE/sizeof(type)) error()

if one wants to include the multiplications for allocate(array(sizeA,sizeB)) then one can do:
if (sizeB > 0 && sizeA > MAXSIZE/sizeB)

That way one always avoids an overflow. I am not sure whether one rejects a value close to MAXSIZE, but at least one avoids signed/unsigned trickery.

For the conditions, one should consider to use __builtin_expect to give the compiler some additional optimization potential.

Wrt whether these checks should be done always, or only when
-fcheck=mem is enabled, IMHO some benchmarks should be run and if no
difference can be seen they can be left enabled. So, does anybody know
of suitable benchmarks that are influenced by ALLOCATE performance?
Polyhedron, maybe not so much?

I don't. The issue is presumably largest for calls to small functions called in loops, which get later inlined. The less conditions and other trickery is done, the more likely the compiler can move the malloc out of the loop. I think it is trivial to construct such a program but it is hard to know how representative such a program would be for the real world. The probability has to be weighted against using a too large value for ALLOCATE without -check=all/-check=mem. I am inclined to make it conditional to -check=mem; but on the other hand, I live in a 64bit world where the chance of an overflow is rather low (for somewhat reasonable programs).


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]