8359344: C2: Malformed control flow after intrinsic bailout

src/hotspot/share/opto/library_call.cpp

@@ -1725,16 +1725,15 @@ bool LibraryCallKit::inline_string_char_access(bool is_store) {
   }
 
   // Save state and restore on bailout
-  SavedState old_state = clone_map_and_save_state();
+  SavedState old_state(this);
 
   value = must_be_not_null(value, true);
 
   Node* adr = array_element_address(value, index, T_CHAR);
   if (adr->is_top()) {
-    restore_state(old_state);
     return false;
   }
-  destruct_map_clone(old_state);
+  old_state.discard();
   if (is_store) {
     access_store_at(value, adr, TypeAryPtr::BYTES, ch, TypeInt::CHAR, T_CHAR, IN_HEAP | MO_UNORDERED | C2_MISMATCHED);
   } else {
@@ -2372,39 +2371,43 @@ DecoratorSet LibraryCallKit::mo_decorator_for_access_kind(AccessKind kind) {
   }
 }
 
-LibraryCallKit::SavedState LibraryCallKit::clone_map_and_save_state() {
-  SavedState state;
-  state.sp = sp();
-  state.jvms = jvms();
-  state.map = clone_map();
-  for (DUIterator_Fast imax, i = control()->fast_outs(imax); i < imax; i++) {
-    Node* out = control()->fast_out(i);
+LibraryCallKit::SavedState::SavedState(LibraryCallKit* kit) :
+  _kit(kit),
+  _sp(kit->sp()),
+  _jvms(kit->jvms()),
+  _map(kit->clone_map())
+{
+  for (DUIterator_Fast imax, i = kit->control()->fast_outs(imax); i < imax; i++) {
+    Node* out = kit->control()->fast_out(i);
     if (out->is_CFG()) {
-      state.ctrl_succ.push(out);
+      _ctrl_succ.push(out);
     }
   }
-  return state;
 }
 
-void LibraryCallKit::restore_state(const SavedState& state) {
-  jvms()->set_map(state.map);
-  jvms()->set_sp(state.sp);
-  state.map->set_jvms(jvms());
-  set_map(state.map);
-  set_sp(state.sp);
-  for (DUIterator_Fast imax, i = control()->fast_outs(imax); i < imax; i++) {
-    Node* out = control()->fast_out(i);
-    if (out->is_CFG() && out->in(0) == control() && out != map() && !state.ctrl_succ.member(out)) {
-      _gvn.hash_delete(out);
-      out->set_req(0, C->top());
-      C->record_for_igvn(out);
+LibraryCallKit::SavedState::~SavedState() {
+  if (discarded) {
+    return;
+  }
+  _kit->jvms()->set_map(_map);
+  _kit->jvms()->set_sp(_sp);
+  _map->set_jvms(_kit->jvms());
+  _kit->set_map(_map);
+  _kit->set_sp(_sp);
+  for (DUIterator_Fast imax, i = _kit->control()->fast_outs(imax); i < imax; i++) {
+    Node* out = _kit->control()->fast_out(i);
+    if (out->is_CFG() && out->in(0) == _kit->control() && out != _kit->map() && !_ctrl_succ.member(out)) {
+      _kit->_gvn.hash_delete(out);
+      out->set_req(0, _kit->C->top());
+      _kit->C->record_for_igvn(out);
       --i; --imax;
+      _kit->_gvn.hash_find_insert(out);
     }
   }
 }
 
-void LibraryCallKit::destruct_map_clone(const SavedState& state) {
-  GraphKit::destruct_map_clone(state.map);
+void LibraryCallKit::SavedState::discard() {
+  discarded = true;
 }
 
 bool LibraryCallKit::inline_unsafe_access(bool is_store, const BasicType type, const AccessKind kind, const bool unaligned) {
@@ -2468,7 +2471,7 @@ bool LibraryCallKit::inline_unsafe_access(bool is_store, const BasicType type, c
   offset = ConvL2X(offset);
 
   // Save state and restore on bailout
-  SavedState old_state = clone_map_and_save_state();
+  SavedState old_state(this);
 
   Node* adr = make_unsafe_address(base, offset, type, kind == Relaxed);
   assert(!stopped(), "Inlining of unsafe access failed: address construction stopped unexpectedly");
@@ -2477,7 +2480,6 @@ bool LibraryCallKit::inline_unsafe_access(bool is_store, const BasicType type, c
     if (type != T_OBJECT) {
       decorators |= IN_NATIVE; // off-heap primitive access
     } else {
-      restore_state(old_state);
       return false; // off-heap oop accesses are not supported
     }
   } else {
@@ -2495,7 +2497,6 @@ bool LibraryCallKit::inline_unsafe_access(bool is_store, const BasicType type, c
 
   const TypePtr* adr_type = _gvn.type(adr)->isa_ptr();
   if (adr_type == TypePtr::NULL_PTR) {
-    restore_state(old_state);
     return false; // off-heap access with zero address
   }
 
@@ -2504,8 +2505,7 @@ bool LibraryCallKit::inline_unsafe_access(bool is_store, const BasicType type, c
   assert(alias_type->index() != Compile::AliasIdxBot, "no bare pointers here");
 
   if (alias_type->adr_type() == TypeInstPtr::KLASS ||
-  alias_type->adr_type() == TypeAryPtr::RANGE) {
-    restore_state(old_state);
+      alias_type->adr_type() == TypeAryPtr::RANGE) {
     return false; // not supported
   }
 
@@ -2524,15 +2524,14 @@ bool LibraryCallKit::inline_unsafe_access(bool is_store, const BasicType type, c
     }
     if ((bt == T_OBJECT) != (type == T_OBJECT)) {
       // Don't intrinsify mismatched object accesses
-      restore_state(old_state);
       return false;
     }
     mismatched = (bt != type);
   } else if (alias_type->adr_type()->isa_oopptr()) {
     mismatched = true; // conservatively mark all "wide" on-heap accesses as mismatched
   }
 
-  destruct_map_clone(old_state);
+  old_state.discard();
   assert(!mismatched || alias_type->adr_type()->is_oopptr(), "off-heap access can't be mismatched");
 
   if (mismatched) {
@@ -2768,7 +2767,7 @@ bool LibraryCallKit::inline_unsafe_load_store(const BasicType type, const LoadSt
   // 32-bit machines ignore the high half of long offsets
   offset = ConvL2X(offset);
   // Save state and restore on bailout
-  SavedState old_state = clone_map_and_save_state();
+  SavedState old_state(this);
   Node* adr = make_unsafe_address(base, offset,type, false);
   const TypePtr *adr_type = _gvn.type(adr)->isa_ptr();
 
@@ -2777,11 +2776,10 @@ bool LibraryCallKit::inline_unsafe_load_store(const BasicType type, const LoadSt
   if (bt != T_ILLEGAL &&
       (is_reference_type(bt) != (type == T_OBJECT))) {
     // Don't intrinsify mismatched object accesses.
-    restore_state(old_state);
     return false;
   }
 
-  destruct_map_clone(old_state);
+  old_state.discard();
 
   // For CAS, unlike inline_unsafe_access, there seems no point in
   // trying to refine types. Just use the coarse types here.

src/hotspot/share/opto/library_call.hpp

@@ -135,18 +135,22 @@ class LibraryCallKit : public GraphKit {
    * (their types, for instance).
    *
    * So, if an intrinsic might cause this situation, one must start by saving the state in a
-   * SavedState using clone_map_and_save_state, and before returning in case of bailing out,
-   * fix the graph using restore_state.
+   * SavedState by constructing it, and the state will be restored on destruction. If the
+   * intrinsic is not bailing out, one need to call discard to prevent restoring the old state.
    */
-  struct SavedState {
-    uint sp;
-    JVMState* jvms;
-    SafePointNode* map;
-    Unique_Node_List ctrl_succ;
+  class SavedState {
+    LibraryCallKit* _kit;
+    uint _sp;
+    JVMState* _jvms;
+    SafePointNode* _map;
+    Unique_Node_List _ctrl_succ;
+    bool discarded = false;
+
+  public:
+    SavedState(LibraryCallKit*);
+    ~SavedState();
+    void discard();
   };
-  SavedState clone_map_and_save_state();
-  void restore_state(const SavedState&);
-  void destruct_map_clone(const SavedState& sfp);
 
   // Helper functions to inline natives
   Node* generate_guard(Node* test, RegionNode* region, float true_prob);

src/hotspot/share/opto/vectorIntrinsics.cpp

@@ -822,7 +822,7 @@ bool LibraryCallKit::inline_vector_mem_operation(bool is_store) {
   Node* offset = ConvL2X(argument(4));
 
   // Save state and restore on bailout
-  SavedState old_state = clone_map_and_save_state();
+  SavedState old_state(this);
 
   Node* addr = make_unsafe_address(base, offset, (is_mask ? T_BOOLEAN : elem_bt), true);
 
@@ -859,7 +859,6 @@ bool LibraryCallKit::inline_vector_mem_operation(bool is_store) {
     log_if_needed("  ** not supported: arity=%d op=%s vlen=%d etype=%s atype=%s ismask=no",
                     is_store, is_store ? "store" : "load",
                     num_elem, type2name(elem_bt), type2name(arr_type->elem()->array_element_basic_type()));
-    restore_state(old_state);
     return false;
   }
 
@@ -871,7 +870,6 @@ bool LibraryCallKit::inline_vector_mem_operation(bool is_store) {
         log_if_needed("  ** not supported: arity=%d op=%s vlen=%d*8 etype=%s/8 ismask=no",
                         is_store, "store",
                         num_elem, type2name(elem_bt));
-        restore_state(old_state);
         return false; // not supported
       }
     } else {
@@ -880,20 +878,17 @@ bool LibraryCallKit::inline_vector_mem_operation(bool is_store) {
         log_if_needed("  ** not supported: arity=%d op=%s vlen=%d*8 etype=%s/8 ismask=no",
                         is_store, "load",
                         mem_num_elem, type2name(mem_elem_bt));
-        restore_state(old_state);
         return false; // not supported
       }
     }
   }
   if (is_mask) {
     if (!is_store) {
       if (!arch_supports_vector(Op_LoadVector, num_elem, elem_bt, VecMaskUseLoad)) {
-        restore_state(old_state);
         return false; // not supported
       }
     } else {
       if (!arch_supports_vector(Op_StoreVector, num_elem, elem_bt, VecMaskUseStore)) {
-        restore_state(old_state);
         return false; // not supported
       }
     }
@@ -908,7 +903,6 @@ bool LibraryCallKit::inline_vector_mem_operation(bool is_store) {
   if (is_store) {
     Node* val = unbox_vector(argument(7), vbox_type, elem_bt, num_elem);
     if (val == nullptr) {
-      restore_state(old_state);
       return false; // operand unboxing failed
     }
     set_all_memory(reset_memory());
@@ -945,7 +939,7 @@ bool LibraryCallKit::inline_vector_mem_operation(bool is_store) {
     set_result(box);
   }
 
-  destruct_map_clone(old_state);
+  old_state.discard();
 
   if (needs_cpu_membar) {
     insert_mem_bar(Op_MemBarCPUOrder);
@@ -1022,7 +1016,7 @@ bool LibraryCallKit::inline_vector_mem_masked_operation(bool is_store) {
   Node* offset = ConvL2X(argument(5));
 
   // Save state and restore on bailout
-  SavedState old_state = clone_map_and_save_state();
+  SavedState old_state(this);
 
   Node* addr = make_unsafe_address(base, offset, elem_bt, true);
   const TypePtr *addr_type = gvn().type(addr)->isa_ptr();
@@ -1035,7 +1029,6 @@ bool LibraryCallKit::inline_vector_mem_masked_operation(bool is_store) {
     log_if_needed("  ** not supported: arity=%d op=%s vlen=%d etype=%s atype=%s",
                     is_store, is_store ? "storeMasked" : "loadMasked",
                     num_elem, type2name(elem_bt), type2name(arr_type->elem()->array_element_basic_type()));
-    restore_state(old_state);
     return false;
   }
 
@@ -1057,7 +1050,6 @@ bool LibraryCallKit::inline_vector_mem_masked_operation(bool is_store) {
       if (!offset_in_range->is_con()) {
         log_if_needed("  ** missing constant: offsetInRange=%s",
                         NodeClassNames[argument(8)->Opcode()]);
-        restore_state(old_state);
         return false;
       }
       needs_predicate = (offset_in_range->get_con() == 0);
@@ -1067,7 +1059,6 @@ bool LibraryCallKit::inline_vector_mem_masked_operation(bool is_store) {
       log_if_needed("  ** not supported: op=%s vlen=%d etype=%s mismatched_ms=%d",
                       is_store ? "storeMasked" : "loadMasked",
                       num_elem, type2name(elem_bt), mismatched_ms ? 1 : 0);
-      restore_state(old_state);
       return false;
     }
   }
@@ -1078,7 +1069,6 @@ bool LibraryCallKit::inline_vector_mem_masked_operation(bool is_store) {
       !arch_supports_vector(Op_VectorBlend, mem_num_elem, mem_elem_bt, VecMaskUseLoad))) {
     log_if_needed("  ** not supported: op=loadMasked vlen=%d etype=%s mismatched_ms=%d",
                     num_elem, type2name(elem_bt), mismatched_ms ? 1 : 0);
-    restore_state(old_state);
     return false;
   }
 
@@ -1089,7 +1079,6 @@ bool LibraryCallKit::inline_vector_mem_masked_operation(bool is_store) {
       log_if_needed("  ** not supported: arity=%d op=%s vlen=%d etype=%s mismatched_ms=1",
                       is_store, is_store ? "storeMasked" : "loadMasked",
                       num_elem, type2name(elem_bt));
-      restore_state(old_state);
       return false;
     }
   }
@@ -1100,7 +1089,6 @@ bool LibraryCallKit::inline_vector_mem_masked_operation(bool is_store) {
     log_if_needed("  ** not supported: arity=%d op=%s vlen=%d etype=%s",
                       is_store, is_store ? "storeMasked" : "loadMasked",
                       num_elem, type2name(elem_bt));
-    restore_state(old_state);
     return false;
   }
 
@@ -1121,7 +1109,6 @@ bool LibraryCallKit::inline_vector_mem_masked_operation(bool is_store) {
     log_if_needed("  ** unbox failed mask=%s",
                     is_store ? NodeClassNames[argument(9)->Opcode()]
                              : NodeClassNames[argument(8)->Opcode()]);
-    restore_state(old_state);
     return false;
   }
 
@@ -1130,7 +1117,6 @@ bool LibraryCallKit::inline_vector_mem_masked_operation(bool is_store) {
     if (val == nullptr) {
       log_if_needed("  ** unbox failed vector=%s",
                       NodeClassNames[argument(8)->Opcode()]);
-      restore_state(old_state);
       return false; // operand unboxing failed
     }
     set_all_memory(reset_memory());
@@ -1177,7 +1163,7 @@ bool LibraryCallKit::inline_vector_mem_masked_operation(bool is_store) {
     set_result(box);
   }
 
-  destruct_map_clone(old_state);
+  old_state.discard();
 
   if (can_access_non_heap) {
     insert_mem_bar(Op_MemBarCPUOrder);
@@ -1300,7 +1286,7 @@ bool LibraryCallKit::inline_vector_gather_scatter(bool is_scatter) {
   Node* offset = ConvL2X(argument(7));
 
   // Save state and restore on bailout
-  SavedState old_state = clone_map_and_save_state();
+  SavedState old_state(this);
 
   Node* addr = nullptr;
   if (!is_subword_type(elem_bt)) {
@@ -1322,15 +1308,13 @@ bool LibraryCallKit::inline_vector_gather_scatter(bool is_scatter) {
     log_if_needed("  ** not supported: arity=%d op=%s vlen=%d etype=%s atype=%s ismask=no",
                     is_scatter, is_scatter ? "scatter" : "gather",
                     num_elem, type2name(elem_bt), type2name(arr_type->elem()->array_element_basic_type()));
-    restore_state(old_state);
     return false;
   }
 
   ciKlass* vbox_klass = vector_klass->const_oop()->as_instance()->java_lang_Class_klass();
   const TypeInstPtr* vbox_type = TypeInstPtr::make_exact(TypePtr::NotNull, vbox_klass);
   ciKlass* vbox_idx_klass = vector_idx_klass->const_oop()->as_instance()->java_lang_Class_klass();
   if (vbox_idx_klass == nullptr) {
-    restore_state(old_state);
     return false;
   }
 
@@ -1345,7 +1329,6 @@ bool LibraryCallKit::inline_vector_gather_scatter(bool is_scatter) {
     // Get the first index vector.
     indexes = unbox_vector(argument(9), vbox_idx_type, T_INT, idx_num_elem);
     if (indexes == nullptr) {
-      restore_state(old_state);
       return false;
     }
   }
@@ -1358,7 +1341,6 @@ bool LibraryCallKit::inline_vector_gather_scatter(bool is_scatter) {
     mask = unbox_vector(m, mbox_type, elem_bt, num_elem);
     if (mask == nullptr) {
       log_if_needed("  ** unbox failed mask=%s", NodeClassNames[m->Opcode()]);
-      restore_state(old_state);
       return false;
     }
   }
@@ -1367,7 +1349,6 @@ bool LibraryCallKit::inline_vector_gather_scatter(bool is_scatter) {
   if (is_scatter) {
     Node* val = unbox_vector(argument(10), vbox_type, elem_bt, num_elem);
     if (val == nullptr) {
-      restore_state(old_state);
       return false; // operand unboxing failed
     }
     set_all_memory(reset_memory());
@@ -1390,7 +1371,7 @@ bool LibraryCallKit::inline_vector_gather_scatter(bool is_scatter) {
     set_result(box);
   }
 
-  destruct_map_clone(old_state);
+  old_state.discard();
   C->set_max_vector_size(MAX2(C->max_vector_size(), (uint)(num_elem * type2aelembytes(elem_bt))));
   return true;
 }