Check for overflow in Blob::Reshape

[longjon]

The check on count_ is more succinct than checking the individual dimensions, and more importantly includes the previously ignored case where bad values for the dimensions cause an overflow for count_, resulting in a silent failure to allocate. Printing the dimension values ensures that no useful debugging information is lost in the change.

[jeffdonahue]

I'd still keep the old checks...even number of negative dimensions?

[longjon]

Right, I had a vague sense I was forgetting something... those checks are back now.

[jeffdonahue]

cool, LGTM

[longjon]

By the way, overflow can still occur undetected if it happens twice, so that the result is positive. On a related note, we need to change types if we want to be able to allocate blobs larger than 2GB.

It seems like a good idea to change this function so that the allocation either succeeds correctly or fails immediately; currently if you ask for a blob of the wrong (large) size, you get a smaller blob and a headache instead (even with this PR).

[jeffdonahue]

But Jon, users will never need blobs larger than 2 GB!

Would checking for overflow after each multiply fix it? I can do that in #1486 when I'll have to loop over dimensions anyway if so.

[longjon]

Not the way it's done here; (1<<31) - 1 is positive; so is ((1<<31) - 1) * 3. However, if one checks the arithmetic with long, it'll work.

[jeffdonahue]

Oh right, makes sense. Checking next_dim <= INT_MAX / current_count seems like it could work too?

[longjon]

Replaced by #2426.