[PATCH] 6788196: Array bounds checks in io_util.c rely on undefined behaviour

[David Holmes - Sun Microsystems]

 >> In C, the result of an overflowing add of two signed integers is
 >> undefined.

Strewth! That's a surprise to me. I always thought that C defined 
integer arithmetic to always wrap. Checking for a negative to detect 
overflow is a common pattern - heck it's THE pattern for detecting 
overflow according to "Hacker's Delight"! But it isn't valid C ! ???

Even in Java this pattern is used a lot - I guess it's lucky the VM goes 
straight to machine code not C otherwise we'd be in trouble!

David Holmes

Martin Buchholz said the following on 12/24/08 06:42:
> Hi Gary,
> 
> Does this actually change the behavior with recent gccs?
> 
> It seems like the introduction of uint32_t is trading one
> non-portability for another, namely relying on C99 features.
> I have been waiting patiently for C99 compilers to emerge,
> but gcc for example is still not there yet.
> http://gcc.gnu.org/c99status.html
> 
> If you are going to use types like uint32_t, you should
> be including the standard header that defines them - <stdint.h>
> 
> More immediate and obvious improvements to the code would
> be to change the type of datalen to "jsize" and the type of nread
> to "jint".
> 
> I suggest, instead of using unsigned types, is to do what
> java code would do in a case like this, and cast to jlong
> instead of uint32_t to avoid overflow.  I approve this patch
> if you make that change.
> 
> I see you've eliminated one of the checks, which was unnecessary.
> Thanks for that.
> 
> Martin
> 
> On Tue, Dec 23, 2008 at 02:21, Gary Benson <gbenson at redhat.com> wrote:
>> Hi all,
>>
>> In C, the result of an overflowing add of two signed integers is
>> undefined.  The array bounds checks in readBytes and writeBytes
>> in jdk/src/share/native/java/io/io_util.c, however, rely on the
>> assumption that the result of the overflowing add will be negative.
>> The attached patch fixes.
>>
>> Cheers,
>> Gary
>>
>> --
>> http://gbenson.net/
>>