RFR: 8256732: Zero: broken +ZeroTLAB exposes badly initialized memory [v4]

[David Holmes]

On Fri, 5 Mar 2021 16:04:52 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Looks like memory is badly initialized when `-XX:+ZeroTLAB` is specified.
>> 
>> Manifests like this:
>> 
>> $ CONF=linux-x86_64-zero-fastdebug make exploded-test TEST=compiler/memoryinitialization/ZeroTLABTest.java
>> 
>> command: main -Xcomp -XX:+UseTLAB -XX:+ZeroTLAB compiler.memoryinitialization.ZeroTLABTest
>> reason: User specified action: run main/othervm -Xcomp -XX:+UseTLAB -XX:+ZeroTLAB compiler.memoryinitialization.ZeroTLABTest
>> Mode: othervm [/othervm specified]
>> elapsed time (seconds): 0.098
>> configuration:
>> STDOUT:
>> Error occurred during initialization of VM
>> java.lang.NullPointerException
>> at java.lang.System.getProperty(java.base/System.java:836)
>> 
>> The cause is simple: Zero calls `ThreadLocalAllocBuffer::allocate`:
>> 
>> if (UseTLAB) {
>>   result = (oop) THREAD->tlab().allocate(obj_size);
>> }
>> 
>> ...which actually does mangle the object space in debug builds:
>> 
>> inline HeapWord* ThreadLocalAllocBuffer::allocate(size_t size) {
>>   invariants();
>>   HeapWord* obj = top();
>>   if (pointer_delta(end(), obj) >= size) {
>>     // successful thread-local allocation
>> #ifdef ASSERT
>>     // Skip mangling the space corresponding to the object header to
>>     // ensure that the returned space is not considered parsable by
>>     // any concurrent GC thread.
>>     size_t hdr_size = oopDesc::header_size();
>>     Copy::fill_to_words(obj + hdr_size, size - hdr_size, badHeapWordVal);
>> #endif // ASSERT
>>     // This addition is safe because we know that top is
>>     // at least size below end, so the add can't wrap.
>>     set_top(obj + size);
>> 
>>     invariants();
>>     return obj;
>>   }
>>   return NULL;
>> }
>> 
>> So if we do `+ZeroTLAB` in debug builds, Zero skips initializing the object body, and gets scrambled memory for newly allocated object. Then everything breaks.
>
> Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits:
> 
>  - Refactor TLAB allocation code a bit
>  - Adjust for the fact ThreadLocalAllocBuffer can mangle object space
>  - Merge branch 'master' into JDK-8256732-zero-zerotlab-broken
>  - Merge branch 'master' into JDK-8256732-zero-zerotlab-broken
>  - Maintain the body/header initialization order
>  - 8256732: Zero: broken +ZeroTLAB exposes badly initialized memory

Marked as reviewed by dholmes (Reviewer).

src/hotspot/share/interpreter/zero/bytecodeInterpreter.cpp line 1817:

> 1815:               //   - if TLAB is pre-zeroed, we can skip this path
> 1816:               //   - in debug mode, ThreadLocalAllocBuffer::allocate mangles
> 1817:               //     this area, and we still need to initialize it

That still sounds like a bug to me.

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

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