RFR: 8269537: memset() is called after operator new [v4]

[Leo Korinth]

On Wed, 20 Oct 2021 09:36:38 GMT, Leo Korinth <lkorinth at openjdk.org> wrote:

>> The basic problem is that we are relying on undefined behaviour, as documented in the code:
>> 
>> // This whole business of passing information from ResourceObj::operator new
>> // to the ResourceObj constructor via fields in the "object" is technically UB.
>> // But it seems to work within the limitations of HotSpot usage (such as no
>> // multiple inheritance) with the compilers and compiler options we're using.
>> // And it gives some possibly useful checking for misuse of ResourceObj.
>> 
>> 
>> I am removing the undefined behaviour by passing the type of allocation through a thread local variable.
>> 
>> This solution has some advantages:
>> 1) it is not UB
>> 2) it is simpler and easier to understand
>> 3) it uses less memory (I could make it use even less if I made the enum `allocation_type` a u8)
>> 4) in the *very* unlikely situation that stack memory (or embedded) already equals the data calculated from the address of the object, the code will also work. 
>> 
>> When doing the change, I also updated  `allocated_on_stack()` to the new name `allocated_on_stack_or_embedded()` which is much harder to misinterpret.
>> 
>> I also disallow to "fake" the memory type by explicitly calling `ResourceObj::set_allocation_type`.
>> 
>> This forced me to change two places that is faking the allocation type of an embedded `GrowableArray` from  `STACK_OR_EMBEDDED` to `C_HEAP`. The faking of the type is hard to understand as a `STACK_OR_EMBEDDED` `GrowableArray` can allocate any type of object. My guess is that `GrowableArray` has changed behaviour, or maybe that it was hard to understand because the old naming of `allocated_on_stack()`. 
>> 
>> I have also tried to update the comments. In doing that I not only changed the comments for this change, but also for the *incorrect* advice to always delete object you allocate with new.
>> 
>> Testing on debug build tier1-3
>> Testing on release build tier1
>
> Leo Korinth has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review updates

This comment will keep this pull request alive a bit longer.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5387