RFR: Deoptimization bug

[Roman Kennke]

Let's turn this into a proper fix and RFR. I think a better and
simpler/less intrusive fix is the one below (JDK12, JDK11 and JDK8),
e.g. WB all oops from deoptimization before entering slow_enter(). All
other paths to slow_enter() should have the WB already (that only
happens when coming from interpreter/C1/C2).

Tests: tier3_gc_shenandoah

Aleksey, can you verify that it helps our failing tests too? So far I
failed to setup those lucene tests...

JDK11/12:

diff --git a/src/hotspot/share/runtime/deoptimization.cpp
b/src/hotspot/share/runtime/deoptimization.cpp
--- a/src/hotspot/share/runtime/deoptimization.cpp
+++ b/src/hotspot/share/runtime/deoptimization.cpp
@@ -1098,7 +1098,7 @@
     if (mon_info->eliminated()) {
       assert(!mon_info->owner_is_scalar_replaced() || realloc_failures,
"reallocation was missed");
       if (!mon_info->owner_is_scalar_replaced()) {
-        Handle obj(thread, mon_info->owner());
+        Handle obj(thread, Access<>::resolve(mon_info->owner()));
         markOop mark = obj->mark();
         if (UseBiasedLocking && mark->has_bias_pattern()) {
           // New allocated objects may have the mark set to anonymously
biased.


JDK8:
diff --git a/src/share/vm/runtime/deoptimization.cpp
b/src/share/vm/runtime/deoptimization.cpp
--- a/src/share/vm/runtime/deoptimization.cpp
+++ b/src/share/vm/runtime/deoptimization.cpp
@@ -990,7 +990,7 @@
     if (mon_info->eliminated()) {
       assert(!mon_info->owner_is_scalar_replaced() || realloc_failures,
"reallocation was missed");
       if (!mon_info->owner_is_scalar_replaced()) {
-        Handle obj = Handle(mon_info->owner());
+        Handle obj =
Handle(oopDesc::bs()->write_barrier(mon_info->owner()));
         markOop mark = obj->mark();
         if (UseBiasedLocking && mark->has_bias_pattern()) {
           // New allocated objects may have the mark set to anonymously
biased.

Roman


Am 04.10.18 um 21:44 schrieb Roman Kennke:
> We've observed a rather nasty bug in some tests in jdk8u lately. From
> what I can tell, this affects all versions though. I am not 100% sure
> that what I'm thinking is correct, so let me describe it for further
> discussion.
> 
> Deoptimization happens at safepoints. It may happen at a safepoint in
> the middle of concurrent evac. In fact (but this is not really relevant
> for the fix I think), evac threads might continue to run concurrently
> with the deopt-safepoint. (We do have an option to make GC workers
> synchronize at safepoints too, we might consider this.)
> 
> Deoptimization generates valid interpreter frames out of compiled
> frames. One of the funny things it needs to do is recreate locks that
> the compiler has eliminated (via escape analysis). In order to do this,
> it creates a bunch of compiledVFrame objects that get populated with
> on-stack-objects (or sometimes new objects, because it also needs to
> recreate objects that have been eliminated because of escape analysis).
> Those on-stack-objects may be from-space-copies. If such a
> from-space-copy is subsequently passed to slow_enter() (to re-create the
> lock), it will take the from-space-mark and write that into the
> displaced header. This blows up later, of course.
> 
> A possible quick fix for this may be:
> http://cr.openjdk.java.net/~rkennke/fix-locking.patch
> 
> But I'm not really happy with it. We probably should comb through
> deoptimization.cpp and find the right places where we need to apply WB.
> 
> Roman
>