RFR: 8221149: os::malloc checks MallocCatchPtr outside of ifdef ASSERT block

[Stefan Karlsson]

Hi Thomas,

On 2019-03-28 09:21, Thomas Stüfe wrote:
> Hi Stefan,
> 
> On Thu, Mar 28, 2019 at 8:47 AM Stefan Karlsson 
> <stefan.karlsson at oracle.com <mailto:stefan.karlsson at oracle.com>> wrote:
> 
>     Hi all,
> 
>     Please review this patch to move the MallocCatchPtr check into the
>     ifdef
>     ASSERT block, just like the other usages of it.
> 
>     http://cr.openjdk.java.net/~stefank/8221149/webrev.01/
>     https://bugs.openjdk.java.net/browse/JDK-8221149
> 
> 
> Looks good. Note that you also could remove the test completely from 
> os::realloc for the oldptr!=NULL case (lines 764ff) since we allocated 
> using os::malloc(), which already does the test.

Thanks for reviewing.

I've incorporated your proposal:
  http://cr.openjdk.java.net/~stefank/8221149/webrev.02.delta
  http://cr.openjdk.java.net/~stefank/8221149/webrev.02

StefanK

> 
>     A side note: Is the intention that MallocCatchPtr should find pointers
>     to the memory address returned from ::malloc, or the memory address we
>     hand out from os::malloc? Currently it's the latter and it's not
>     obvious
>     from the the code if this was the intention from the beginning.
> 
>        704   // Wrap memory with guard
>        705   GuardedMemory guarded(ptr, size + nmt_header_size);
>        706   ptr = guarded.get_user_ptr();
>        707
>        708   if ((intptr_t)ptr == (intptr_t)MallocCatchPtr) {
> 
> 
> I find the test as it is is more useful since usually one wants to 
> follow pointers one sees in the hotspot.
> 
> However, we may just test both pointers, yes? So, break if 
> MallocCatchPtr is either the outside or the inside pointer.
> 
> I am not really emotionally invested though :)
> 
> Cheers Thomas
> 
>     Thanks,
>     StefanK
>