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

[Gary Benson]

Martin Buchholz wrote:
> Does this actually change the behavior with recent gccs?

I don't think anything changed recently, not on Intel or SPARC,
but I develop on PowerPC, and GCC on 32-bit PowerPC seems to
overflow to 1, -1 or 0... sometimes.  But that's not the point;
the behaviour is undefined, so the result could be anything,
on any platform.

> 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>

I didn't realise there was a header required.  I remembered I'd
seen something similar in HotSpot so I just copied that piece of
code.

> 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.

Would it not simply be better to cast to unsigned int?  I don't
know about other platforms, but on 32-bit PowerPC casting to
jlong would use three more registers and add a load of extra
instructions whereas casting to unsigned int adds none.  The
impact on performance and memory usage would be negligable of
course, but using 64-bit types where I don't need to makes me
a little uneasy...

> I see you've eliminated one of the checks, which was unnecessary.
> Thanks for that.

No problem :)

Cheers,
Gary

-- 
http://gbenson.net/