[PATCH] 6788196: Array bounds checks in io_util.c rely on undefined behaviour
gbenson at redhat.com
Wed Dec 24 10:25:43 UTC 2008
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.
> 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
> More immediate and obvious improvements to the code would be to
> change the type of datalen to "jsize" and the type of nread to
> 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 :)
More information about the core-libs-dev