8172990: [AOT] Missing GC scan of _metaspace_got array containing Klass*
authorkvn
Wed, 18 Jan 2017 14:36:54 -0800
changeset 43459 80727d67b5e2
parent 43458 61ab1daf0fc8
child 43460 591b3e506e67
8172990: [AOT] Missing GC scan of _metaspace_got array containing Klass* Summary: added back _metaspace_got array scan. Reviewed-by: dlong
hotspot/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/CallSiteRelocationSymbol.java
hotspot/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/JavaCallSiteRelocationSymbol.java
hotspot/src/share/vm/aot/aotCodeHeap.cpp
hotspot/src/share/vm/aot/aotCompiledMethod.cpp
hotspot/src/share/vm/aot/aotCompiledMethod.hpp
hotspot/src/share/vm/runtime/deoptimization.cpp
--- a/hotspot/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/CallSiteRelocationSymbol.java	Wed Jan 18 10:30:56 2017 -0500
+++ b/hotspot/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/CallSiteRelocationSymbol.java	Wed Jan 18 14:36:54 2017 -0800
@@ -59,12 +59,6 @@
         addExternalPltToGotRelocation(binaryContainer, symbol, relocationOffset);
     }
 
-    protected static void addMetaspaceGotRelocation(BinaryContainer binaryContainer, String symbolName, int symbolOffset, int relocationOffset) {
-        ByteContainer container = binaryContainer.getMetaspaceGotContainer();
-        Symbol symbol = container.createGotSymbol(symbolOffset, symbolName);
-        addExternalPltToGotRelocation(binaryContainer, symbol, relocationOffset);
-    }
-
     /**
      * Add an {@link RelocType#EXTERNAL_GOT_TO_PLT} relocation to the
      * {@link BinaryContainer#getExtLinkageGOTContainer()}.
--- a/hotspot/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/JavaCallSiteRelocationSymbol.java	Wed Jan 18 10:30:56 2017 -0500
+++ b/hotspot/src/jdk.aot/share/classes/jdk.tools.jaotc/src/jdk/tools/jaotc/JavaCallSiteRelocationSymbol.java	Wed Jan 18 14:36:54 2017 -0800
@@ -37,6 +37,7 @@
 final class JavaCallSiteRelocationSymbol extends CallSiteRelocationSymbol {
 
     private static final byte[] zeroSlot = new byte[8];
+    // -1 represents Universe::non_oop_word() value
     private static final byte[] minusOneSlot = {-1, -1, -1, -1, -1, -1, -1, -1};
 
     public JavaCallSiteRelocationSymbol(CompiledMethodInfo mi, Call call, CallSiteRelocationInfo callSiteRelocation, BinaryContainer binaryContainer) {
@@ -79,30 +80,39 @@
         }
 
         // Add relocation to GOT cell for call resolution jump.
+        // This GOT cell will be initialized during JVM startup with address
+        // of JVM runtime call resolution function.
         String gotSymbolName = "got." + getResolveSymbolName(binaryContainer, mi, call);
         Symbol gotSymbol = binaryContainer.getGotSymbol(gotSymbolName);
         addExternalPltToGotRelocation(binaryContainer, gotSymbol, stub.getResolveJumpOffset());
 
         // Add relocation to resolve call jump instruction address for GOT cell.
+        // This GOT cell will be initialized with address of resolution jump instruction and
+        // will be updated with call destination address by JVM runtime call resolution code.
         String pltJmpSymbolName = relocationSymbolName("plt.jmp", mi, call, callSiteRelocation);
         addCodeContainerRelocation(binaryContainer, pltJmpSymbolName, stub.getResolveJumpStart(), gotStartOffset);
 
         // Add relocation to GOT cell for dispatch jump.
+        // The dispatch jump loads destination address from this GOT cell.
         String gotEntrySymbolName = relocationSymbolName("got.entry", mi, call, callSiteRelocation);
         addExtLinkageGotContainerRelocation(binaryContainer, gotEntrySymbolName, gotStartOffset, stub.getDispatchJumpOffset());
 
-        // Virtual call needs initial -1 value.
+        // Virtual call needs initial -1 value for Klass pointer.
+        // Non virtual call needs initial 0 value for Method pointer to call c2i adapter.
         byte[] slot = isVirtualCall ? minusOneSlot : zeroSlot;
-        final int gotMetaOffset = binaryContainer.appendMetaspaceGotBytes(slot, 0, slot.length);
+        final int gotMetaOffset = binaryContainer.appendExtLinkageGotBytes(slot, 0, slot.length);
 
         // Add relocation to GOT cell for move instruction (Klass* for virtual, Method* otherwise).
         String gotMoveSymbolName = relocationSymbolName("got.move", mi, call, callSiteRelocation);
-        addMetaspaceGotRelocation(binaryContainer, gotMoveSymbolName, gotMetaOffset, stub.getMovOffset());
+        addExtLinkageGotContainerRelocation(binaryContainer, gotMoveSymbolName, gotMetaOffset, stub.getMovOffset());
 
         if (isVirtualCall) {
             // Nothing.
         } else {
             // Add relocation to GOT cell for c2i adapter jump.
+            // The c2i jump instruction loads destination address from this GOT cell.
+            // This GOT cell is initialized with -1 and will be updated
+            // by JVM runtime call resolution code.
             String gotC2ISymbolName = relocationSymbolName("got.c2i", mi, call, callSiteRelocation);
             addExtLinkageGotContainerRelocation(binaryContainer, gotC2ISymbolName, gotStartOffset + 8, stub.getC2IJumpOffset());
         }
--- a/hotspot/src/share/vm/aot/aotCodeHeap.cpp	Wed Jan 18 10:30:56 2017 -0500
+++ b/hotspot/src/share/vm/aot/aotCodeHeap.cpp	Wed Jan 18 14:36:54 2017 -0800
@@ -830,38 +830,19 @@
   }
 }
 
-// Yes, this is faster than going through the relocations,
-// but there are two problems:
-// 1) GOT slots are sometimes patched with non-Metadata values
-// 2) We don't want to scan metadata for dead methods
-// Unfortunately we don't know if the metadata belongs to
-// live aot methods or not, so process them all.  If this
-// is for mark_on_stack, some old methods may stick around
-// forever instead of getting cleaned up.
+// Scan only metaspace_got cells which should have only Klass*,
+// metadata_got cells are scanned only for alive AOT methods
+// by AOTCompiledMethod::metadata_do().
 void AOTCodeHeap::got_metadata_do(void f(Metadata*)) {
   for (int i = 1; i < _metaspace_got_size; i++) {
     Metadata** p = &_metaspace_got[i];
     Metadata* md = *p;
     if (md == NULL)  continue;  // skip non-oops
-    intptr_t meta = (intptr_t)md;
-    if (meta == -1)  continue;  // skip non-oops
     if (Metaspace::contains(md)) {
       f(md);
-    }
-  }
-  for (int i = 1; i < _metadata_got_size; i++) {
-    Metadata** p = &_metadata_got[i];
-    Metadata* md = *p;
-    intptr_t meta = (intptr_t)md;
-    if ((meta & 1) == 1) {
-      // already resolved
-      md = (Metadata*)(meta & ~1);
     } else {
-      continue;
-    }
-    if (md == NULL)  continue;  // skip non-oops
-    if (Metaspace::contains(md)) {
-      f(md);
+      intptr_t meta = (intptr_t)md;
+      fatal("Invalid value in _metaspace_got[%d] = " INTPTR_FORMAT, i, meta);
     }
   }
 }
@@ -910,8 +891,6 @@
       aot->metadata_do(f);
     }
   }
-#if 0
-  // With the marking above, this call doesn't seem to be needed
+  // Scan metaspace_got cells.
   got_metadata_do(f);
-#endif
 }
--- a/hotspot/src/share/vm/aot/aotCompiledMethod.cpp	Wed Jan 18 10:30:56 2017 -0500
+++ b/hotspot/src/share/vm/aot/aotCompiledMethod.cpp	Wed Jan 18 14:36:54 2017 -0800
@@ -71,15 +71,6 @@
 }
 #endif
 
-void AOTCompiledMethod::oops_do(OopClosure* f) {
-  if (_oop != NULL) {
-    f->do_oop(&_oop);
-  }
-#if 0
-  metadata_oops_do(metadata_begin(), metadata_end(), f);
-#endif
-}
-
 bool AOTCompiledMethod::do_unloading_oops(address low_boundary, BoolObjectClosure* is_alive, bool unloading_occurred) {
   return false;
 }
@@ -161,9 +152,6 @@
       *entry = (Metadata*)meta; // Should be atomic on x64
       return (Metadata*)m;
     }
-    // need to resolve it here..., patching of GOT need to be CAS or atomic operation.
-    // FIXIT: need methods for debuginfo.
-    // return _method;
   }
   ShouldNotReachHere(); return NULL;
 }
@@ -288,11 +276,19 @@
           f(cichk->holder_method());
           f(cichk->holder_klass());
         } else {
+          // Get Klass* or NULL (if value is -1) from GOT cell of virtual call PLT stub.
           Metadata* ic_oop = ic->cached_metadata();
           if (ic_oop != NULL) {
             f(ic_oop);
           }
         }
+      } else if (iter.type() == relocInfo::static_call_type ||
+                 iter.type() == relocInfo::opt_virtual_call_type){
+        // Check Method* in AOT c2i stub for other calls.
+        Metadata* meta = (Metadata*)nativeLoadGot_at(nativePltCall_at(iter.addr())->plt_c2i_stub())->data();
+        if (meta != NULL) {
+          f(meta);
+        }
       }
     }
   }
@@ -332,7 +328,12 @@
     st->print("%4d ", _aot_id);    // print compilation number
     st->print("    aot[%2d]", _heap->dso_id());
     // Stubs have _method == NULL
-    st->print("   %s", (_method == NULL ? _name : _method->name_and_sig_as_C_string()));
+    if (_method == NULL) {
+      st->print("   %s", _name);
+    } else {
+      ResourceMark m;
+      st->print("   %s", _method->name_and_sig_as_C_string());
+    }
     if (Verbose) {
       st->print(" entry at " INTPTR_FORMAT, p2i(_code));
     }
--- a/hotspot/src/share/vm/aot/aotCompiledMethod.hpp	Wed Jan 18 10:30:56 2017 -0500
+++ b/hotspot/src/share/vm/aot/aotCompiledMethod.hpp	Wed Jan 18 14:36:54 2017 -0800
@@ -257,8 +257,6 @@
     return (int) (*_state_adr);
   }
 
-  virtual void oops_do(OopClosure* f);
-
   // inlined and non-virtual for AOTCodeHeap::oops_do
   void do_oops(OopClosure* f) {
     assert(_is_alive(), "");
--- a/hotspot/src/share/vm/runtime/deoptimization.cpp	Wed Jan 18 10:30:56 2017 -0500
+++ b/hotspot/src/share/vm/runtime/deoptimization.cpp	Wed Jan 18 14:36:54 2017 -0800
@@ -1596,9 +1596,9 @@
       get_method_data(thread, profiled_method, create_if_missing);
 
     // Log a message
-    Events::log_deopt_message(thread, "Uncommon trap: reason=%s action=%s pc=" INTPTR_FORMAT " method=%s @ %d",
+    Events::log_deopt_message(thread, "Uncommon trap: reason=%s action=%s pc=" INTPTR_FORMAT " method=%s @ %d %s",
                               trap_reason_name(reason), trap_action_name(action), p2i(fr.pc()),
-                              trap_method->name_and_sig_as_C_string(), trap_bci);
+                              trap_method->name_and_sig_as_C_string(), trap_bci, nm->compiler_name());
 
     // Print a bunch of diagnostics, if requested.
     if (TraceDeoptimization || LogCompilation) {