RFR (XXS): 8075288: malloc without free in VM_PopulateDumpSharedSpace::doit()

[Ioi Lam]

There's this comment right above the code:

   // dunno what this is for.

I also don't understand why we need to allocate a saved_vtbl, clear 
vtbl_list, and then vtbl_list later.  I think I tried removing these 
lines in the past and got into problems. So I'd suggest leaving them 
alone until you understand what really is going on.

Thanks
- Ioi

On 4/10/15 8:00 AM, Mikael Gerdin wrote:
> PopulateDumpSharedSpace is CDS, redirecting to hotspot-runtime-dev at ...
>
> On 2015-04-10 16:49, Carsten Varming wrote:
>> Dear Jungwoo,
>>
>> Why is this array not resource allocated?
>
> Actually, none of this really matters since the VM terminates after 
> executing the CDS dump. This has probably been done to satisfy some 
> static analysis tool that Google runs on the sources.
>
> /Mikael
>
>>
>> Carsten
>>
>> On Fri, Apr 10, 2015 at 9:31 AM, Jungwoo Ha <jwha at google.com
>> <mailto:jwha at google.com>> wrote:
>>
>>     What's the conclusion? Should I fix it or discuss it separately?
>>
>>     On Thu, Apr 9, 2015 at 2:43 PM, Kim Barrett <kim.barrett at oracle.com
>>     <mailto:kim.barrett at oracle.com>> wrote:
>>
>>         On Apr 9, 2015, at 1:25 PM, Jungwoo Ha <jwha at google.com
>>         <mailto:jwha at google.com>> wrote:
>>          >
>>          > Can someone sponsor this change?
>>          >
>>          > https://bugs.openjdk.java.net/browse/JDK-8075288
>>          > http://cr.openjdk.java.net/~jwha/8075288/webrev.00
>>
>>         I think the change is correct.  Good find!
>>
>>         However, it’s not clear to me why this is using malloc (and now
>>         free), rather than stack allocation.
>>         The size is a declared variable at the allocation point, but
>>         it’s initialized from a constant with a
>>         not big value (17) and never modified, so could be changed to be
>>         a constant.
>>
>>         That is, instead of
>>
>>           592   char* saved_vtbl = (char*)os::malloc(vtbl_list_size *
>>         sizeof(void*), mtClass);
>>           593   memmove(saved_vtbl, vtbl_list, vtbl_list_size *
>>         sizeof(void*));
>>
>>         use
>>
>>            void* saved_vtbl[vtbl_list_size];
>>            memmove(saved_vtbl, vtbl_list, ARRAY_SIZE(saved_vtbl));
>>
>>         Maybe the worry is that vtbl_list_size might not always be a
>>         small constant?
>>
>>         Probably this question of whether to use malloc/free here at all
>>         should be a separate CR.
>>
>>
>>