RFR: JDK-8220714: C2 Compilation failure when accessing off-heap memory using Unsafe

[Vladimir Kozlov]

Hi Roman,

I would suggest to wait when Vladimir I. look on this - he made most changes in this code.

Looking on changes and it seems you can modify it a little:

   // Can base be NULL? Otherwise, always on-heap access.
   bool can_access_non_heap = TypePtr::NULL_PTR->higher_equal(_gvn.type(base));
   if (!can_access_non_heap) {
     heap_base_oop = base;
     decorators |= IN_HEAP;
   } else if (type == T_OBJECT) {
     return false; // off-heap oop accesses are not supported
   }

Thanks,
Vladimir K

On 3/15/19 4:39 AM, Roman Kennke wrote:
> A user reported misbehaving off-heap access. It looks like a C2
> compilation failure that seems to only trigger with Shenandoah:
> 
> https://mail.openjdk.java.net/pipermail/shenandoah-dev/2019-March/009060.html
> 
> Eg with G1 generates this assembly for swapping two array elements:
> mov (%r8),%r9d
> mov (%r10),%r11d
> mov %r9d,(%r10)
> mov %r11d,(%r8)
> 
> While with Shenandoah we get this:
> mov (%r9),%ecx
> mov %ecx,(%r10,%r11,1)
> mov %ecx,(%r9)
> 
> I.e. the two loads seem to have been wrongly coalesced into one.
> 
> Even though that is only triggered by Shenandoah, it seems to be a legit
> and generic C2 problem.
> 
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8220714
> Webrev:
> http://cr.openjdk.java.net/~rkennke/JDK-8220714/webrev.00/
> 
> The issue seems to be that off-heap accesses are supposed to use
> MO_RELAXED mem-ordering instead of MO_UNORDERED, as implemented in
> C2Access::needs_cpu_membar(). However, it seems we wrongly set this here
> (library_call.cpp around l2410:
> 
>    // Can base be NULL? Otherwise, always on-heap access.
>    bool can_access_non_heap =
> TypePtr::NULL_PTR->higher_equal(_gvn.type(heap_base_oop));
>    if (!can_access_non_heap) {
>      decorators |= IN_HEAP;
>    }
> 
> However, heap_base_oop is initialized to top() a few lines up, and then
> never updated, at least not for the off-heap-access case. And top
> doesn't match NULL_PTR afaik.
> 
> The proposed fix uses base instead of heap_base_oop for that check, this
> should be updated correcly through make_unsafe_addr() and
> classify_unsafe_addr().
> 
> For some reason, this bug only seems to be exposed when running
> Shenandoah, and then only when actually compiling with barriers. We
> could not reproduce this with any other GC. It totally eludes me why
> that is so. For this reason, the testcase goes under the
> gc/shenandoah/compiler directory.
> 
> Roman
>