RFR(M) 8176100: [REDO][REDO] G1 Needs pre barrier on dereference of weak JNI handles

[coleen.phillimore at oracle.com]

This looks good still.   For 
https://bugs.openjdk.java.net/browse/JDK-8176363

why are the two locks "special"?  Is that because there is no leaf-2?   
Just curious.

Thanks,
Coleen

On 3/14/17 9:11 AM, Mikael Gerdin wrote:
> Here we go again...
> This time around the fix is exactly the same but since I changed the 
> lock ranks in 8176363 we shouldn't be hitting any lock ordering 
> assertions.
>
>
> Hi all,
> Please review this revised change to jweak to support G1.
>
> I've copied Kim's original description of the fix below for reference:
>
> >
> > The problem being addressed is that, when using G1, dereferencing a
> > jweak must ensure the obtained referent is ensured live, just as for
> > other weak reference types.  This has always been a requirement, and
> > failure to do so appears to be a since-day1 G1 bug.
> >
> > There are two categories of places that need to address this issue:
> > JNIHandles::resolve and friends, and returning a jobject from native
> > code to Java.  In both of these places the jobject whose contained oop
> > is being extracted might really be a jweak.  jweak is
> > representationally indistinguishable from jobject; jweak is just a
> > typedef for jobject.  The documentation says a jweak can be used
> > anywhere a jobject is permitted, making that type equivalence
> > necessary for a C API.  (A C++ API might be able to have distinct
> > types via inheritance, though would still need a method for
> > distinguishing them at runtime.  But since JNI is a C API...).
> >
> > The basic approach being taken is to "tag" jweaks by setting the low
> > order bit (recall that jweak == jobject == _jobject*), in order to
> > distinguish them from non-jweak jobjects.  This tagging is only being
> > done when the selected GC needs the distinction, e.g. G1. This gives
> > us a representational difference between jobject and jweak on which to
> > base the different behavior, hiding that distinction behind the
> > existing API.
> >
> > JNIHandles::resolve and friends are changed to unconditionally strip
> > off any potential weak tag when translating from jobject to to oop*.
> > That's cheaper than testing for the conditional use of tagging and
> > then stripping.  Also stripped when deleting JNI handles.
> >
> > TemplateInterpreterGenerator::generate_native_entry and
> > SharedRuntime::generate_native_wrapper are changed to conditionally
> > emit code to apply the G1 pre-barrier to the value obtained from a
> > jobject result, when the result is tagged as a jweak, which only
> > occurs when using G1.
> >
> > For arm32/arm64, this required moving the g1_write_barrier_pre
> > definitions from InterpreterMacroAssembler to MacroAssembler.  Also
> > moved g1_write_barrier_post, for consistency.
> >
>
> In addition to Kim's final version of the change (which was backed 
> out) this updated version adds code to mask out the weak tag bit in 
> the fast JNI field getters on all platforms where fast JNI getters are 
> used.
>
> Since the fast JNI getters only read primitive values from objects me 
> and Kim decided that there was no need to invoke the G1 pre-barrier if 
> a jweak was used as the "receiver" for the getters, this simplifies 
> the change to the fast getters and I was able to come up with a 
> seemingly efficient instruction encoding on all platforms.
>
> A couple of comments about the implementations:
> My goal was to encode clearing of the weak_tag_mask (which at the 
> moment is 1).
> On SPARC and 32 bit arm there is an instruction that can be used 
> straight off to perform an and with a value that is first negated, 
> ANDN on SPARC and BTC on ARM.
> On 64 bit ARM the encoding for immediates allows encoding of the 
> bitwise inverse of 1 (0xfffffffffffffffe) but if weak_tag_mask is 
> changed the encoding may fail.
> On x86 I pass the mask as a signed 32 bit immediate but note that the 
> assembler encodes it as an 8 bit immediate and sets a sign-extension 
> bit in the opcode.
>
>
> Testing:
> Standard JPRT and tier2-3 HotSpot tests.
> I've modified the CallWithJNIWeak test to call all the primitive 
> getters and some other interesting cases I came up with.
> I've also run the CallWithJNIWeak test on all platforms on both 
> product and fastdebug builds (since fast JNI getters are implicitly 
> disabled in debug builds due to VerifyJNIFields being enabled by 
> default in debug builds.
>
> The aarch64 change has been tested in the previous round of this change.
>
> Full webrev:
> http://cr.openjdk.java.net/~mgerdin/8176100/webrev.0/
>
> Bugs:
> https://bugs.openjdk.java.net/browse/JDK-8176100
> https://bugs.openjdk.java.net/browse/JDK-8175085
> https://bugs.openjdk.java.net/browse/JDK-8166188
>
> Thanks
> /Mikael