7028374: race in fix_oop_relocations for scavengeable nmethods

Reviewed-by: kvn

hotspot/src/share/vm/code/codeCache.cpp

@@ -337,7 +337,6 @@ void CodeCache::scavenge_root_nmethods_do(CodeBlobClosure* f) {
     if (is_live) {
       // Perform cur->oops_do(f), maybe just once per nmethod.
       f->do_code_blob(cur);
-      cur->fix_oop_relocations();
     }
   }
 
@@ -552,6 +551,19 @@ void CodeCache::gc_epilogue() {
 }
 
 
+void CodeCache::verify_oops() {
+  MutexLockerEx mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
+  VerifyOopClosure voc;
+  FOR_ALL_ALIVE_BLOBS(cb) {
+    if (cb->is_nmethod()) {
+      nmethod *nm = (nmethod*)cb;
+      nm->oops_do(&voc);
+      nm->verify_oop_relocations();
+    }
+  }
+}
+
+
 address CodeCache::first_address() {
   assert_locked_or_safepoint(CodeCache_lock);
   return (address)_heap->begin();

hotspot/src/share/vm/code/codeCache.hpp

@@ -122,6 +122,7 @@ class CodeCache : AllStatic {
   // GC support
   static void gc_epilogue();
   static void gc_prologue();
+  static void verify_oops();
   // If "unloading_occurred" is true, then unloads (i.e., breaks root links
   // to) any unmarked codeBlobs in the cache.  Sets "marked_for_unloading"
   // to "true" iff some code got unloaded.

hotspot/src/share/vm/code/nmethod.cpp

@@ -1105,6 +1105,20 @@ void nmethod::fix_oop_relocations(address begin, address end, bool initialize_im
 }
 
 
+void nmethod::verify_oop_relocations() {
+  // Ensure sure that the code matches the current oop values
+  RelocIterator iter(this, NULL, NULL);
+  while (iter.next()) {
+    if (iter.type() == relocInfo::oop_type) {
+      oop_Relocation* reloc = iter.oop_reloc();
+      if (!reloc->oop_is_immediate()) {
+        reloc->verify_oop_relocation();
+      }
+    }
+  }
+}
+
+
 ScopeDesc* nmethod::scope_desc_at(address pc) {
   PcDesc* pd = pc_desc_at(pc);
   guarantee(pd != NULL, "scope must be present");
@@ -1823,6 +1837,7 @@ void nmethod::oops_do_marking_epilogue() {
     assert(cur != NULL, "not NULL-terminated");
     nmethod* next = cur->_oops_do_mark_link;
     cur->_oops_do_mark_link = NULL;
+    cur->fix_oop_relocations();
     NOT_PRODUCT(if (TraceScavenge)  cur->print_on(tty, "oops_do, unmark\n"));
     cur = next;
   }

hotspot/src/share/vm/code/nmethod.hpp

@@ -459,6 +459,7 @@ private:
 public:
   void fix_oop_relocations(address begin, address end) { fix_oop_relocations(begin, end, false); }
   void fix_oop_relocations()                           { fix_oop_relocations(NULL, NULL, false); }
+  void verify_oop_relocations();
 
   bool is_at_poll_return(address pc);
   bool is_at_poll_or_poll_return(address pc);

hotspot/src/share/vm/code/relocInfo.cpp

@@ -798,6 +798,14 @@ void oop_Relocation::fix_oop_relocation() {
 }
 
 
+void oop_Relocation::verify_oop_relocation() {
+  if (!oop_is_immediate()) {
+    // get the oop from the pool, and re-insert it into the instruction:
+    verify_value(value());
+  }
+}
+
+
 RelocIterator virtual_call_Relocation::parse_ic(nmethod* &nm, address &ic_call, address &first_oop,
                                                 oop* &oop_addr, bool *is_optimized) {
   assert(ic_call != NULL, "ic_call address must be set");

hotspot/src/share/vm/code/relocInfo.hpp

@@ -765,7 +765,8 @@ class Relocation VALUE_OBJ_CLASS_SPEC {
 
  protected:
   // platform-dependent utilities for decoding and patching instructions
-  void       pd_set_data_value       (address x, intptr_t off); // a set or mem-ref
+  void       pd_set_data_value       (address x, intptr_t off, bool verify_only = false); // a set or mem-ref
+  void       pd_verify_data_value    (address x, intptr_t off) { pd_set_data_value(x, off, true); }
   address    pd_call_destination     (address orig_addr = NULL);
   void       pd_set_call_destination (address x);
   void       pd_swap_in_breakpoint   (address x, short* instrs, int instrlen);
@@ -880,6 +881,12 @@ class DataRelocation : public Relocation {
     else
       pd_set_data_value(x, o);
   }
+  void        verify_value(address x) {
+    if (addr_in_const())
+      assert(*(address*)addr() == x, "must agree");
+    else
+      pd_verify_data_value(x, offset());
+  }
 
   // The "o" (displacement) argument is relevant only to split relocations
   // on RISC machines.  In some CPUs (SPARC), the set-hi and set-lo ins'ns
@@ -950,6 +957,8 @@ class oop_Relocation : public DataRelocation {
 
   void fix_oop_relocation();        // reasserts oop value
 
+  void verify_oop_relocation();
+
   address value()  { return (address) *oop_addr(); }
 
   bool oop_is_immediate()  { return oop_index() == 0; }