Hi Nick,
I've cc'd Kim as our C++ expert.
Kim I have a query below if we're entering UB territory ...
On 23/01/2020 6:02 pm, Nick Gasson wrote:
Hi David,
On 01/23/20 15:46 pm, David Holmes wrote:
So on 32-bit size_t is 32-bit and so may have ~half the capacity allowed
for by the jlong size variable. On 64-bit overflow is not possible
because jlong is signed and size_t is not.
So we only need an overflow check on 32-bit.
OK. So wrap the check in #ifndef _LP64?
Yes please.
But your checks don't look correct to me. If size is already aligned to
HeapWordSize then "sz < (size_t)size" won't be true but you already have
a completely bogus value for sz when you truncated size to 32-bits.
On a 32-bit system we already know the upper 32-bits of `size' are zero
because Unsafe.checkSize() checks this prior to calling the native
function, so I think the cast is OK.
That precondition needs to be documented in this method then. Hmmm so
you have a guaranteed 32-bit value, but it might act like an unsigned
32-bit value - is that the case? Any reason not to have the Java code
ensure the size acts like a 32-bit signed int? Then no overflow is possible.
That aside IIRC the overflow check is not ideal here because we already
enter undefined behaviour territory inside align_up if we actually
overflow. I'm not sure what the officially sanctioned mechanism for
avoiding overflow would be here given the addition is hidden inside
align_up. Hopefully Kim can clarify this aspect.
Thanks,
David
Thanks,
Nick
