RFR(S): 8216426: Usage of array placement new may lead to memory corruption

[Erik Österlund]

Hi Martin,

Unfortunately, according to the C++03 standard, that field needs to be 
public for the class to be a POD. And if it is not a POD, we will have 
undefined behaviour. Yay for C++.

Thanks,
/Erik

On 2019-01-15 11:51, Doerr, Martin wrote:
> Hi Robbin,
>
> that's fine. I like it simple.
>   
> Version which uses memset and removed constructor:
> http://cr.openjdk.java.net/~mdoerr/8216426_array_placement_new/webrev.01/
>
> I don't like making the field public because it can easily get used incorrectly because there are bits encoded into the pointer.
> So I think we should better not provide direct access to it.
>
> Best regards,
> Martin
>
>
> -----Original Message-----
> From: Robbin Ehn <robbin.ehn at oracle.com>
> Sent: Dienstag, 15. Januar 2019 10:26
> To: Erik Österlund <erik.osterlund at oracle.com>; Doerr, Martin <martin.doerr at sap.com>; hotspot-runtime-dev at openjdk.java.net; Kim Barrett <kim.barrett at oracle.com>; David Holmes (david.holmes at oracle.com) <david.holmes at oracle.com>
> Subject: Re: RFR(S): 8216426: Usage of array placement new may lead to memory corruption
>
> On 2019-01-15 10:15, Erik Österlund wrote:
>> Hi,
>>
>> Perhaps an alternative approach is to make the Bucket a POD (make the private member public, and remove the
>> constructor), and then simply memset the memory to zero.
> Sorry, we have NMT tracking via NEW_C_HEAP_ARRAY(Bucket, _size, F);.
> So yes, make it a POD and use memset.
>
> /Robbin
>
>> Thanks,
>> /Erik
>>
>> On 2019-01-15 09:03, Robbin Ehn wrote:
>>> Hi Martin,
>>>
>>> The Bucket should extend CHeapObj<F>, now that it could.
>>> Otherwise looks good, thanks.
>>>
>>> Originally Bucket did this and I used memset, but gdb was very upset by this.
>>> So I changed to array placement new, which meant I had to remove CHeapObj<F> if I remember correctly :)
>>>
>>> /Robbin
>>>
>>> On 2019-01-14 17:39, Doerr, Martin wrote:
>>>> Hi,
>>>>
>>>> thanks for looking at this issue and especially for adding comments.
>>>> Sounds like this issue should better get fixed although there are no known problems.
>>>>
>>>> Should we fix it by replacing the array placement new by normal placement new for each element?
>>>> http://cr.openjdk.java.net/~mdoerr/8216426_array_placement_new/webrev.00/
>>>>
>>>> Best regards,
>>>> Martin
>>>>