8057038: Speculative traps not robust when compilation and class unloading are concurrent

Speculative traps can be removed from MDO while being copied by compiler

Reviewed-by: kvn, iveresov

hotspot/src/share/vm/ci/ciMethodData.cpp

@@ -81,19 +81,38 @@ ciMethodData::ciMethodData() : ciMetadata(NULL) {
 void ciMethodData::load_extra_data() {
   MethodData* mdo = get_MethodData();
 
+  MutexLocker(mdo->extra_data_lock());
+
   // speculative trap entries also hold a pointer to a Method so need to be translated
   DataLayout* dp_src  = mdo->extra_data_base();
-  DataLayout* end_src = mdo->extra_data_limit();
+  DataLayout* end_src = mdo->args_data_limit();
   DataLayout* dp_dst  = extra_data_base();
   for (;; dp_src = MethodData::next_extra(dp_src), dp_dst = MethodData::next_extra(dp_dst)) {
     assert(dp_src < end_src, "moved past end of extra data");
-    // New traps in the MDO can be added as we translate the copy so
+    assert(((intptr_t)dp_dst) - ((intptr_t)extra_data_base()) == ((intptr_t)dp_src) - ((intptr_t)mdo->extra_data_base()), "source and destination don't match");
-    // look at the entries in the copy.
+
-    switch(dp_dst->tag()) {
+    // New traps in the MDO may have been added since we copied the
+    // data (concurrent deoptimizations before we acquired
+    // extra_data_lock above) or can be removed (a safepoint may occur
+    // in the translate_from call below) as we translate the copy:
+    // update the copy as we go.
+    int tag = dp_src->tag();
+    if (tag != DataLayout::arg_info_data_tag) {
+      memcpy(dp_dst, dp_src, ((intptr_t)MethodData::next_extra(dp_src)) - ((intptr_t)dp_src));
+    }
+
+    switch(tag) {
     case DataLayout::speculative_trap_data_tag: {
       ciSpeculativeTrapData* data_dst = new ciSpeculativeTrapData(dp_dst);
       SpeculativeTrapData* data_src = new SpeculativeTrapData(dp_src);
+
       data_dst->translate_from(data_src);
+
+#ifdef ASSERT
+      SpeculativeTrapData* data_src2 = new SpeculativeTrapData(dp_src);
+      assert(data_src2->method() == data_src->method() && data_src2->bci() == data_src->bci(), "entries changed while translating");
+#endif
+
       break;
     }
     case DataLayout::bit_data_tag:
@@ -244,8 +263,8 @@ ciProfileData* ciMethodData::next_data(ciProfileData* current) {
 }
 
 ciProfileData* ciMethodData::bci_to_extra_data(int bci, ciMethod* m, bool& two_free_slots) {
-  DataLayout* dp  = data_layout_at(data_size());
+  DataLayout* dp  = extra_data_base();
-  DataLayout* end = data_layout_at(data_size() + extra_data_size());
+  DataLayout* end = args_data_limit();
   two_free_slots = false;
   for (;dp < end; dp = MethodData::next_extra(dp)) {
     switch(dp->tag()) {
@@ -492,8 +511,8 @@ ByteSize ciMethodData::offset_of_slot(ciProfileData* data, ByteSize slot_offset_
 
 ciArgInfoData *ciMethodData::arg_info() const {
   // Should be last, have to skip all traps.
-  DataLayout* dp  = data_layout_at(data_size());
+  DataLayout* dp  = extra_data_base();
-  DataLayout* end = data_layout_at(data_size() + extra_data_size());
+  DataLayout* end = args_data_limit();
   for (; dp < end; dp = MethodData::next_extra(dp)) {
     if (dp->tag() == DataLayout::arg_info_data_tag)
       return new ciArgInfoData(dp);
@@ -535,8 +554,8 @@ template<class T> void ciMethodData::dump_replay_data_call_type_helper(outputStr
 }
 
 void ciMethodData::dump_replay_data_extra_data_helper(outputStream* out, int round, int& count) {
-  DataLayout* dp  = data_layout_at(data_size());
+  DataLayout* dp  = extra_data_base();
-  DataLayout* end = data_layout_at(data_size() + extra_data_size());
+  DataLayout* end = args_data_limit();
 
   for (;dp < end; dp = MethodData::next_extra(dp)) {
     switch(dp->tag()) {
@@ -653,8 +672,8 @@ void ciMethodData::print_data_on(outputStream* st) {
     data->print_data_on(st);
   }
   st->print_cr("--- Extra data:");
-  DataLayout* dp  = data_layout_at(data_size());
+  DataLayout* dp  = extra_data_base();
-  DataLayout* end = data_layout_at(data_size() + extra_data_size());
+  DataLayout* end = args_data_limit();
   for (;; dp = MethodData::next_extra(dp)) {
     assert(dp < end, "moved past end of extra data");
     switch (dp->tag()) {

hotspot/src/share/vm/ci/ciMethodData.hpp

@@ -410,6 +410,9 @@ private:
   // Area dedicated to parameters. NULL if no parameter profiling for
   // this method.
   DataLayout* _parameters;
+  int parameters_size() const {
+    return _parameters == NULL ? 0 : parameters_type_data()->size_in_bytes();
+  }
 
   ciMethodData(MethodData* md);
   ciMethodData();
@@ -461,9 +464,6 @@ private:
   address data_base() const {
     return (address) _data;
   }
-  DataLayout* limit_data_position() const {
-    return (DataLayout*)((address)data_base() + _data_size);
-  }
 
   void load_extra_data();
   ciProfileData* bci_to_extra_data(int bci, ciMethod* m, bool& two_free_slots);
@@ -524,7 +524,9 @@ public:
   ciProfileData* next_data(ciProfileData* current);
   bool is_valid(ciProfileData* current) { return current != NULL; }
 
-  DataLayout* extra_data_base() const { return limit_data_position(); }
+  DataLayout* extra_data_base() const  { return data_layout_at(data_size()); }
+  DataLayout* args_data_limit() const  { return data_layout_at(data_size() + extra_data_size() -
+                                                               parameters_size()); }
 
   // Get the data at an arbitrary bci, or NULL if there is none. If m
   // is not NULL look for a SpeculativeTrapData if any first.

hotspot/src/share/vm/oops/methodData.cpp

@@ -86,7 +86,7 @@ ProfileData::ProfileData() {
 
 char* ProfileData::print_data_on_helper(const MethodData* md) const {
   DataLayout* dp  = md->extra_data_base();
-  DataLayout* end = md->extra_data_limit();
+  DataLayout* end = md->args_data_limit();
   stringStream ss;
   for (;; dp = MethodData::next_extra(dp)) {
     assert(dp < end, "moved past end of extra data");
@@ -1048,14 +1048,15 @@ void MethodData::post_initialize(BytecodeStream* stream) {
     stream->next();
     data->post_initialize(stream, this);
   }
-  if (_parameters_type_data_di != -1) {
+  if (_parameters_type_data_di != no_parameters) {
     parameters_type_data()->post_initialize(NULL, this);
   }
 }
 
 // Initialize the MethodData* corresponding to a given method.
 MethodData::MethodData(methodHandle method, int size, TRAPS)
-  : _extra_data_lock(Monitor::leaf, "MDO extra data lock") {
+  : _extra_data_lock(Monitor::leaf, "MDO extra data lock"),
+    _parameters_type_data_di(parameters_uninitialized) {
   No_Safepoint_Verifier no_safepoint;  // init function atomic wrt GC
   ResourceMark rm;
   // Set the method back-pointer.
@@ -1111,7 +1112,7 @@ MethodData::MethodData(methodHandle method, int size, TRAPS)
     DataLayout *dp = data_layout_at(data_size + extra_size + arg_data_size);
     dp->initialize(DataLayout::parameters_type_data_tag, 0, parms_cell);
   } else {
-    _parameters_type_data_di = -1;
+    _parameters_type_data_di = no_parameters;
   }
 
   // Set an initial hint. Don't use set_hint_di() because
@@ -1236,7 +1237,7 @@ DataLayout* MethodData::next_extra(DataLayout* dp) {
 }
 
 ProfileData* MethodData::bci_to_extra_data_helper(int bci, Method* m, DataLayout*& dp, bool concurrent) {
-  DataLayout* end = extra_data_limit();
+  DataLayout* end = args_data_limit();
 
   for (;; dp = next_extra(dp)) {
     assert(dp < end, "moved past end of extra data");
@@ -1285,7 +1286,7 @@ ProfileData* MethodData::bci_to_extra_data(int bci, Method* m, bool create_if_mi
          "code needs to be adjusted");
 
   DataLayout* dp  = extra_data_base();
-  DataLayout* end = extra_data_limit();
+  DataLayout* end = args_data_limit();
 
   // Allocation in the extra data space has to be atomic because not
   // all entries have the same size and non atomic concurrent
@@ -1330,7 +1331,7 @@ ProfileData* MethodData::bci_to_extra_data(int bci, Method* m, bool create_if_mi
 
 ArgInfoData *MethodData::arg_info() {
   DataLayout* dp    = extra_data_base();
-  DataLayout* end   = extra_data_limit();
+  DataLayout* end   = args_data_limit();
   for (; dp < end; dp = next_extra(dp)) {
     if (dp->tag() == DataLayout::arg_info_data_tag)
       return new ArgInfoData(dp);
@@ -1357,7 +1358,7 @@ void MethodData::print_value_on(outputStream* st) const {
 void MethodData::print_data_on(outputStream* st) const {
   ResourceMark rm;
   ProfileData* data = first_data();
-  if (_parameters_type_data_di != -1) {
+  if (_parameters_type_data_di != no_parameters) {
     parameters_type_data()->print_data_on(st);
   }
   for ( ; is_valid(data); data = next_data(data)) {
@@ -1367,7 +1368,7 @@ void MethodData::print_data_on(outputStream* st) const {
   }
   st->print_cr("--- Extra data:");
   DataLayout* dp    = extra_data_base();
-  DataLayout* end   = extra_data_limit();
+  DataLayout* end   = args_data_limit();
   for (;; dp = next_extra(dp)) {
     assert(dp < end, "moved past end of extra data");
     // No need for "OrderAccess::load_acquire" ops,
@@ -1565,7 +1566,7 @@ public:
 // redefined method
 void MethodData::clean_extra_data(CleanExtraDataClosure* cl) {
   DataLayout* dp  = extra_data_base();
-  DataLayout* end = extra_data_limit();
+  DataLayout* end = args_data_limit();
 
   int shift = 0;
   for (; dp < end; dp = next_extra(dp)) {
@@ -1610,7 +1611,7 @@ void MethodData::clean_extra_data(CleanExtraDataClosure* cl) {
 void MethodData::verify_extra_data_clean(CleanExtraDataClosure* cl) {
 #ifdef ASSERT
   DataLayout* dp  = extra_data_base();
-  DataLayout* end = extra_data_limit();
+  DataLayout* end = args_data_limit();
 
   for (; dp < end; dp = next_extra(dp)) {
     switch(dp->tag()) {

hotspot/src/share/vm/oops/methodData.hpp

@@ -2107,7 +2107,12 @@ private:
 
   // data index for the area dedicated to parameters. -1 if no
   // parameter profiling.
+  enum { no_parameters = -2, parameters_uninitialized = -1 };
   int _parameters_type_data_di;
+  int parameters_size_in_bytes() const {
+    ParametersTypeData* param = parameters_type_data();
+    return param == NULL ? 0 : param->size_in_bytes();
+  }
 
   // Beginning of the data entries
   intptr_t _data[1];
@@ -2130,7 +2135,7 @@ private:
 
   // Helper for data_at
   DataLayout* limit_data_position() const {
-    return (DataLayout*)((address)data_base() + _data_size);
+    return data_layout_at(_data_size);
   }
   bool out_of_bounds(int data_index) const {
     return data_index >= data_size();
@@ -2371,10 +2376,11 @@ public:
   }
 
   // Add a handful of extra data records, for trap tracking.
-  DataLayout* extra_data_base() const { return limit_data_position(); }
+  DataLayout* extra_data_base() const  { return limit_data_position(); }
   DataLayout* extra_data_limit() const { return (DataLayout*)((address)this + size_in_bytes()); }
-  int extra_data_size() const { return (address)extra_data_limit()
+  DataLayout* args_data_limit() const  { return (DataLayout*)((address)this + size_in_bytes() -
-                               - (address)extra_data_base(); }
+                                                              parameters_size_in_bytes()); }
+  int extra_data_size() const          { return (address)extra_data_limit() - (address)extra_data_base(); }
   static DataLayout* next_extra(DataLayout* dp);
 
   // Return (uint)-1 for overflow.
@@ -2429,11 +2435,12 @@ public:
 
   // Return pointer to area dedicated to parameters in MDO
   ParametersTypeData* parameters_type_data() const {
-    return _parameters_type_data_di != -1 ? data_layout_at(_parameters_type_data_di)->data_in()->as_ParametersTypeData() : NULL;
+    assert(_parameters_type_data_di != parameters_uninitialized, "called too early");
+    return _parameters_type_data_di != no_parameters ? data_layout_at(_parameters_type_data_di)->data_in()->as_ParametersTypeData() : NULL;
   }
 
   int parameters_type_data_di() const {
-    assert(_parameters_type_data_di != -1, "no args type data");
+    assert(_parameters_type_data_di != parameters_uninitialized && _parameters_type_data_di != no_parameters, "no args type data");
     return _parameters_type_data_di;
   }
 
@@ -2480,8 +2487,8 @@ public:
   static bool profile_return_jsr292_only();
 
   void clean_method_data(BoolObjectClosure* is_alive);
-
   void clean_weak_method_links();
+  Mutex* extra_data_lock() { return &_extra_data_lock; }
 };
 
 #endif // SHARE_VM_OOPS_METHODDATAOOP_HPP