RFR(xxxs) (jdk10): 8168542: os::realloc should return a valid pointer for input size=0

[Robbin Ehn]

Hi Thomas,

realloc with size 0 is equivalent to free, not an error generally speaking.
But in jvm context I think we should not do realloc with size 0.

So instead of getting an OOM it's much better to crash so we can find out who tried realloc 0.

  ALWAYSINLINE char* ReallocateHeap(char *old, size_t size, MEMFLAGS flag,
      AllocFailType alloc_failmode = AllocFailStrategy::EXIT_OOM) {
+  guarantee(size != 0, "Reallocation size must be non-zero");

Or if we (unlikely) think realloc size 0, is okay:

-  if (p == NULL && alloc_failmode == AllocFailStrategy::EXIT_OOM) {
+  if (size != 0 && p == NULL && alloc_failmode == AllocFailStrategy::EXIT_OOM) {

And let the caller handle the NULL pointer.

As David put it:
"I just don't like the allocate-1-byte approach"

/Robbin


On 02/04/2017 01:09 PM, Thomas Stüfe wrote:
> Hi guys,
>
> picking up this trivial issue which got pushed to jdk10, could I please
> have a re-review? Oh, also a sponsor.
>
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8168542
>
> webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8168542-os_realloc_size_0/jdk10-webrev.01/webrev/
>
> For reference, the old discussion back in october 16:
> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2016-October/021675.html
>
> The webrev is unchanged from the proposal for jdk9, just rebased to
> jdk10/hs.
>
> @Chris, I decided to not follow your suggestion to move the comparison into
> the #ifndef assert. You are right, this is redundant with the check inside
> os::malloc(), but as you said, checking at the entry of os::realloc() makes
> it clearer.
>
> Thank you!
>
> Kind Regards, Thomas
>